surge-synthesizer / surge

Synthesizer plug-in (previously released as Vember Audio Surge)
https://surge-synthesizer.github.io/
GNU General Public License v3.0
3.16k stars 399 forks source link

Problems on switch from Formula to other LFO shapes #6705

Open Andreya-Autumn opened 1 year ago

Andreya-Autumn commented 1 year ago

Assign Formula output >1 to modulate something. Switch to Sine.

If you modulated from 2nd or 3rd Formula output, those assignments are now coming from the Raw or EG-only outputs. Everything is fine.

If you modulated from the 4th output or higher, the mod assignment sticks around in the UI even though it won't do anything. When right clicking the modded param it displays as "LFO 3.8" for example.

What's more. If you were viewing the 8th formula output before switching, the display will clamp down to LFO output three (EG output), but if you arm and assign from there, the assignment will actually come from the first LFO output. See picture. That's a bit borked.

Slightly Borked

baconpaul commented 1 year ago

I think for 1.2 I’m going to try and chicken box this as follows: if you are on formula and switch to non formula and have a mapped modulator other than .1 pop up a chicken box before you switch with a don’t ask again option. That won’t close this issue but it will make the behavior less grody

mkruselj commented 1 year ago

Hm I am not really sure we need a dialog for this... Would much rather we bump the issue up a milestone and do it right. It's an edge case scenario...

baconpaul commented 1 year ago

Ok

baconpaul commented 1 year ago

looking at this issue, @mkruselj you have some concept of "do it right" you allude to above. Care to expand on that? :)

like what should the behavior be here?

mkruselj commented 1 year ago

The right behavior IMO would be clearing the more-than-third output mod assignments when going from formula mod back to regular LFO shapes and reverting the UI back to first output. Pretty much the same thing which happens to some mod routings when switching osc or FX types.

We have undo so no biggie.

Andreya-Autumn commented 1 year ago

Agreed.

baconpaul commented 1 year ago

so the formula with extra routings switch to sine blow away the routings issue. The problem with that is right now the only thing we store in the undo stack is the type; and if we blow away the routings we also need to store all the modulations. We don't have an undo gesture for that (the easiest is our 'ahh screw it undo the entire patch' gesture but that's super heavy for changing mod types). So that issue is a bit tricky.

baconpaul commented 1 year ago

so SGE::lfoShapeChanged already has some handling for this but it doesn't actually remove the modulations. But this may be easier as a result, namely

1: in the undo at top if prior == formula do a patch push undo (and a param push undo for the rest)

  1. if prior == formula and outbound mods from me are at index higher than 1 nuke them

but we also have to deal with all that cache stuff there which is painful, and i think may be slightly wrong now we have undo anyway. So not a 'knock it out in 30 minutes tonight' issue alas, but that's where to look