On 15.09.24 10:55, Will Godfrey wrote:
On Sat, 14 Sep 2024 13:05:07 +0100 Will Godfrey
<willgodf...@musically.me.uk> wrote:
There is a change in the behaviour of the the System effects On/Off switch
On 15.09.24 15:12, ichthyo wrote:
On 15.09.24 10:55, Will Godfrey wrote:
On Sat, 14 Sep 2024 13:05:07 +0100 Will Godfrey
<willgodf...@musically.me.uk> wrote:
There is a change in the behaviour of the the System effects On/Off
switch -
I've found the commit where this changed. It is 72a4ef2
Hi Will,
...commit 72a4ef2 was in the first feature, which we landed already this
spring. The changeset does "remove UI state regarding effect number and
type", so this is at the point where we have already integrated the new data
connection for the EffectUI and now all widgets in that part are switched
over to the data values sent from core.
...and it's easy to see why the behaviour changed: simply because there
was no "behaviour" for this switch in the core at all before the introduction
of GuiDataExchange. Thus I formulated a logic that seems correct for me:
void SynthEngine::pushEffectUpdate(uchar partNum)
{
...
EffectDTO dto;
dto.effNum = effnum;
dto.effType = effInstance[effnum]->geteffect();
dto.isInsert = isInsert;
dto.enabled = (0 != dto.effType && ((isPart && !currPart.Pefxbypass[effnum])
||(isInsert && Pinsparts[effnum] != -1)
||(!isInsert && syseffEnable[effnum])));
dto.changed = effInstance[effnum]->geteffectpar(-1);
dto.currPreset = effInstance[effnum]->getpreset();
dto.insertFxRouting = isPart || !isInsert? -1 : Pinsparts[effnum];
dto.partFxRouting = !isPart? +1 : currPart.Pefxroute[effnum];
dto.partFxBypass = !isPart? false : currPart.Pefxbypass[effnum];
Now it may seem easy just to "amend" this behaviour, but this would define
something logically incorrect, and I am very reluctant to send an effect as
"enabled" into the GUI which in fact is *not* eabled and has otherwise no valid
parameters, other than type = 0
Which brings me to the conclusion that /actually/ what we want is specific
/additional/ behaviour to the end that when an previously unoccupied Sys-Effect
slot is switched to a type (and thus becomes occupied), then the enabled switch
should actually also be set.
Now it seems this is the case already, just the user can not change the type
because I have tied the enabled-state of the type selector box to the enabled
state of the System-Effect slot.
I have just pushed out a draft fix on my "guiconnect" branch, which now
does no longer disable the type select drop-down, and also immediately
sets the checkbox once the user selects an effect type. On later push-updates,
the enabled-checkbox will then be set from the actual enabled-state of the
corresponding slot in the core, which IMHO is the correct behaviour, but
also would give the user the additional convenience.
Does this look OK for you?
Another note: Yesterday I have reworked the build switches on the CMake build
to properly handle the assertions on Release builds. See the corresponding new
changeset. Basically I have now added a new toggle as convenience for us
developers. When selecting BuildForDiagnostic, then even on release builds
the debug info is included and also the assertions remain activated. By default
this new toggle is OFF, meaning that from now on, Release builds will no longer
include the assertion checking code, which could give use a small-ish
performance gain.
-- Hermann
_______________________________________________
Yoshimi-devel mailing list
Yoshimi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/yoshimi-devel
