surge-synthesizer / shortcircuit-xt

Will be a sampler when its done!
GNU General Public License v3.0
243 stars 27 forks source link

VA Oscillator and ModMatrix woes #1027

Open baconpaul opened 1 month ago

baconpaul commented 1 month ago

When you change the type and associated controls in the VA oscillator they change on the screen but not in the mod matrix. There's always the useless targets there.

Gotta think about what to do about this. Hard. But not great as it stands now.. This is why, I think SC2 and SC1 had separate types.

So basically is the dynamic parameter set being advertised properly or is it just a UI thing. And if it's just a UI thing we gotta do some lifting. So write down the issue so I don't forget.

mkruselj commented 1 month ago

I think what is easiest is to mark the created mod routing but one that is not applicable disabled. And literally show it in the UI. Perhaps by wrapping the target name in [], or strike through the text, or 50% opaque that whole mod matrix slot. Because user might want to at some point return to the mode where this mod routing works, and so it would be uncool to destructively remove it when it's invalid.

baconpaul commented 1 month ago

Yeah just need to carefully write the code - we adjust correctly on processor type change but not in value change which is what’s happening here

Andreya-Autumn commented 1 month ago

I think what is easiest is to mark the created mod routing but one that is not applicable disabled. And literally show it in the UI. Perhaps by wrapping the target name in [], or strike through the text, or 50% opaque that whole mod matrix slot. Because user might want to at some point return to the mode where this mod routing works, and so it would be uncool to destructively remove it when it's invalid.

For things like the harmonic trem xover (the dial is still there but disabled). That makes sense, and it's almost there already. The target slot goes empty when the dial disables, and comes back on re-enable.

It would indeed feel more congruent if the mod matrix could also show a dial which changing identity. Presently the alternative oscillator params are different param indices. Which don't disable, they just go invisible making room for another param to draw its dial in the same spot. That's why this doesn't "just work".

Andreya-Autumn commented 1 month ago

@baconpaul How about this:

enum FloatParams
{
    fpOffset,
    fpLevel,
    fpOne,
    fpTwo
}
enum IntParams
{
    ipWaveform
};
basic_blocks::params::ParamMetaData paramAt(int idx) const
{
    using pmd = basic_blocks::params::ParamMetaData;
    int wave = this->getIntParam(ipWaveform);

    switch (idx)
    {
    // other params
    case fpOne:
        if (wave == 0)
            return pmd().sensibleChoices().withName("Feedback")
        else if (wave == 1)
            return pmd().sensibleChoices().withName("Sync")
    //same idea for fpTwo
    }
}

And then the matrix should get the updates via floatAttachments[]->getLabel(), same as for the other case right?

baconpaul commented 1 month ago

The problem is the disappearing knobs still appear in mod matrix. We handle renaming configured properly (I’ll check va to see if it is when I resolve this)

the ur-problem is that “unused” is a feature of the ui choices not the processor and that’s kinda what we need to fix

Andreya-Autumn commented 1 month ago

That was the point of the above proposal. With it there are no dissappearing knobs! They rename instead. Which we do indeed handle correctly. All the "Frequency L" params rename properly (including in the mod matrix), so no reason we can't solve this the same way, or?

Andreya-Autumn commented 1 month ago

So besides doing what I wrote above, we'd also go through and make each osc type process function use "fpOne" where it currently might use "fpSync" or something. That should work right?

Andreya-Autumn commented 1 month ago

I see what you're saying about the ur-problem though. In this particular case I think we can make it so that nothing needs to be "unused" by sharing the parameters between several functions. But of course that may not always be the case indeed.

baconpaul commented 1 month ago

Yeah I don't know the answer quite yet. I'm leaving it be at this juncture but didn't want to not remember it.

mkruselj commented 1 month ago

Basically if we can change the name and range and scaling of a parameter at runtime, this obviates the need for hiding knobs. You would have x generic (modulatable) params per proc and equally many mod targets, which would rename (or disable) themselves as needed.

baconpaul commented 1 month ago

right now the VA oscillator shows a different number of knobs based on the waveform choice.

You should just try it. You'll see the inconsistency right away

If it had a fixed knob count then yes this would work.

We already handle dynamic names and ranges properly in the mod matrix and ui. that works (if the processor is configured properly. I haven't tested that they all are but have tested that the ones which are work). This is a problem only if there's a change not in 'enablement' but in 'is there-ness'