Fixed disengaging manually enabled stops when a crescendo was in the Override=Off mode https://github.com/GrandOrgue/grandorgue/issues/1935

[oleg68]

Resolves #1935

This PR finally fixes the add-mode crescendo issues.

Earlier GOSetter used GOCombination::ExternalElementSet for maintaining the crescendo.
Now it uses the internal drawstop OR switch fith an internal state crescendo-X.

[larspalo]

Seems to be working as it should. I just wonder if it's really necessary to keep the code that you've commented out?

[oleg68]

Seems to be working as it should. I just wonder if it's really necessary to keep the code that you've commented out?

I removed the commented code

[larspalo]

@oleg68 I just found an issue/regression caused by this PR. In "Add mode" (or override off) I now cannot manually remove a stop that has been activated by the crescendo at any programmed step. A drawstop that is not active in a crescendo step can be switched on/off at will though, but as soon as a drawstop is activated by the crescendo I cannot switch it off manually - which I think should be possible! Override mode seems to be working as expected though.

[larspalo]

@oleg68 Even a GC doesn't work on a fully programmed crescendo in add mode with this PR.

[oleg68]

The new behavior is by design: the actual drawstop state is the result of the OR function of the user state and the crescendo state. It is similar to the crescendo behavior on a pneumatic-action organ.

[larspalo]

The new behavior is by design: the actual drawstop state is the result of the OR function of the user state and the crescendo state. It is similar to the crescendo behavior on a pneumatic-action organ.

I understand that, but even on a pneumatic-action organ the GC would usually (at least in all cases I've encountered) still cancel sound output and de-activate any visibly active drawstops/tabs. In this regard this PR doesn't behave well, I think.

Any way, this is quite a bit of a behaviour change so I'd expect some to be surprised. Perhaps better help explanation of this "Add mode" is warranted compared to the current. On the other hand, if used wisely and with separate crescendo stop activator switches from the normal "user visible" drawstops/tabs, I think that this PR does indeed introduce some benefits that can be exploited by a sample set producer.

However, I do think that some, or actually a lot of, confusion might arise if users program the normal visible drawstops/tabs into the crescendo like earlier and then use the "Add mode" and realise that they cannot any longer manually (or currently not even via GC) affect the state of stops added (or in any step included) by the crescendo. Many would think that they have encountered a bug!

[oleg68]

@larspalo @rousseldenis what may we do now? I see several options:

1. leave the new behavior and document it.
2. leave the new behavior and add capability of GC to reset crescendo position
3. Make the capability of switch stops off manually even they have been activated by crescendo (SetButtonState(false) will clear all internal states, not only the default one). But if the user change the position of crescendo, the stops will be engaged again.

What choice would be youth?

[hnb2907]

Hi,

Looking at this from a theatre organ perspective....

This request fixes the previous behaviour, where crescendo incorrectly changed state of stops over a user controlled pressing stops and/or piston change.

In a real theatre organ logic/wiring: The crescendo state is internally OR'ed with the output states of the stops, whether they are controlled manually, or by pistons, or GC.
When crescendo>0, pressing GC would cancel the stops on the GUI, or by manually changing stops, or by pistons; always the crescendo state remain applied.

Also, on a theatre organ, the crescendo doesn't change the actual stop state; this implies that the crescendo state is not shown on the stops on the GUI (or real stop tabs on a real organ console).

This is how I tried to show on the diagram in this post (maybe it wasn't clear)!? ;)
#1935 (comment)

Unfortunately, I don't know how different organ types implement crescendo. But it appears we are trying to reconcile many different conflicting behaviours, from differently designed logic on different organs.

Best wishes,
Chris.

[larspalo]

Also, on a theatre organ, the crescendo doesn't change the actual stop state; this implies that the crescendo state is not shown on the stops on the GUI (or real stop tabs on a real organ console).

Yes, I hinted at that in my post above, so that if this feature is used to program separate stop activating switches (and not the normal GUI visible drawstops/tabs, but separate ones only used for/by the crescendo) this PR indeed would do what could be expected as the "normal" drawstops/tabs state wouldn't be affected by the crescendo at all (I've worked with Mark Bugeja on a few sample sets that works in a similar fashion but it currently always requires quite a few tricks to achieve such a result!). The real problem with this PR, as I tried to explain, will occur if a user programs the "normal" GUI drawstops/tabs into the crescendo in the "Add mode".

With separate crescendo stop activating switches it's very easy to have another selector switch used for toggling between using the crescendo states and the manual drawstops/tabs which would make it clear what is actually currently used to activate the sound. As it's now, I'm mainly concerned that normal users could be very surprised by this new behaviour of the "Add mode" if they start programming the "normal" drawstops/tabs into it.

@oleg68 I'm personally not really sure what would be the absolutely best way to proceed from here! But I'm leaning towards your option 2 above, but I'll support your option 3 too. If others agree with option 2, and it's implemented, I think that the GC should cancel all the stops (that doesn't have GCState=-1) and thus in effect return "manual" control, but the crescendo still should remember what step it's at, and if the crescendo pedal is again changed it should immediately activate whatever combination comes next, which is pretty much how the normal "Override" mode works and "takes over" control, but of course still with the inner workings of the new "Add mode" - if it's active. I currently think that option could provide the best benefits. What thoughts/opinion does others have? @rousseldenis

[hnb2907]

For me, preferably #1, and 2nd choice is #2.
Neither fully replicate the theatre organ behaviour, but are closest.

Thanks, Chris

[larspalo]

For me, preferably #1,

Have you tested the build and seen what it does and how it behaves?

[hnb2907]

Personally, if I was able to program proficiently in C++, I would offer to implement a new additional feature mode for theatre organ behaviour. I could easily do this with labview source code, but not C++ 🙂

C.

[hnb2907]

For me, preferably #1,

Have you tested the build and seen what it does and how it behaves?

I've tried this one #1935 (comment)

[larspalo]

Personally, if I was able to program proficiently in C++, I would offer to implement a new additional feature mode for theatre organ behaviour. I could easily do this with labview source code, but not C++ 🙂

To get a theatre (or older style pneumatic) crescendo you'd still need "separate activators" for the "back end" stops in order to get the "normal GUI" drawstops/tabs OR the crescendo "activators" without the visible GUI stops being visibly activated by the crescendo - even though the back end stops of course would sound regardless of the visible GUI states. This current PR, when programming the "normal" visible drawstops into the crescendo, will not provide that behaviour.

[hnb2907]

Personally, if I was able to program proficiently in C++, I would offer to implement a new additional feature mode for theatre organ behaviour. I could easily do this with labview source code, but not C++ 🙂

To get a theatre (or older style pneumatic) crescendo you'd still need "separate activators" for the "back end" stops in order to get the "normal GUI" drawstops/tabs OR the crescendo "activators" without the visible GUI stops being visibly activated by the crescendo - even though the back end stops of course would sound regardless of the visible GUI states. This current PR, when programming the "normal" visible drawstops into the crescendo, will not provide that behaviour.

Hi Lars,

Even though I don't know the architecture of the GO code, I think I understand correctly what you are saying, and it makes sense. :)

You are correct, this PR doesn't provide that behaviour, however it does confirm that the original problem that you reported, is fixed, and is a big improvement. This PR makes GO more viable, even though the GUI behaviour is not as a theatre organ behaves.

Maybe I should create a new PR for a new "theatre organ" behaviour setting, which could be implemented as you described?

Best wishes,
Chris.

[oleg68]

@larspalo I implemented the option 2: now GC switches the stops engaged by a crescendo off.

[larspalo]

I implemented the option 2: now GC switches the stops engaged by a crescendo off.

@oleg68 Are you yourself happy with that solution? So far we haven't heard anything from @rousseldenis either.

[oleg68]

Are you yourself happy with that solution? So far we haven't heard anything from @rousseldenis either.

I think it is a good compromise.

An electric-action crescendo usually lights the stops that are currently engaged by it. I found it more convenient for the organist than one of a pneumatic action where the organist does not see what stops are affected. So I wouldn't implement the pure pneumatic crescendo.

I might implement (3), but @hnb2907 disagrees. So I would leave on (2).

[larspalo]

So I wouldn't implement the pure pneumatic crescendo.

I agree. It can be done by a sample set producer for a certain sample set if it's really desired.

I'm basically satisfied with this PR, everything I've tested with it works as expected. Some might be surprised by the change but I'd not expect many to be negative once they understand the purpose of it.

The only thing that still is a bit confusing even for me is when I:

• add stops manually while in "Add mode" (which works fine both up and down as long as the crescendo combination doesn't have them as active yet)
• then I increase the crescendo to/beyond a point where the stops appear
• click on the manually added stop again, nothing visually happens (as expected) yet...
• then decrease the crescendo, now the stop will be gone from manual adding (de-activated)

Thus the manual "on state" will disappear if the crescendo combination includes it and it's clicked again in the GUI. Clicking it twice in the higher crescendo step will not add it back as "manually on" though. This manual "removal when active in crescendo add mode" behaviour is perhaps a bit un-balanced when it's not possible to also manually "add when active" too. But I'm not going to insist on a change of the behaviour, I'm just pointing out my findings while testing it.

[oleg68]

@larspalo What behavior would be better? Not to disengage the stop manually at all if it has been engaged by the crescendo? But what IS about activating a combination without this stop?