Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modulation not cleared when changing osc type #2224

Closed
mkruselj opened this issue Jun 28, 2020 · 6 comments · Fixed by #3935
Closed

Modulation not cleared when changing osc type #2224

mkruselj opened this issue Jun 28, 2020 · 6 comments · Fixed by #3935
Labels
Bug Report Item submitted using the Bug Report template Modulation Modulation related issues UX Issues related to user experience (UX) - mouse, touch, keyboard, MIDI inputs, etc.
Milestone

Comments

@mkruselj
Copy link
Collaborator

I noticed that modulation is not cleared when you change osc type. You can end up with stuff that doesn't make sense, like slider background gets blue for integer sliders in sine osc which aren't modulatable.

Maybe in cases when the parameter name is the same when switching from osc type A to osc type B, it should be retained (WT/window morph, skew horizontal or vertical, not sure off-hand if there are any others).

@mkruselj mkruselj added Modulation Modulation related issues UX Issues related to user experience (UX) - mouse, touch, keyboard, MIDI inputs, etc. labels Jun 28, 2020
@mkruselj mkruselj added this to the 1.7 0 milestone Jun 28, 2020
@ghost
Copy link

ghost commented Jun 29, 2020

Confirmed during beta-testing with:

Surge-NIGHTLY-2020-06-29-9c42bd3 | Win-10, VST3i

LICEcap

@baconpaul
Copy link
Collaborator

Oh man this has been there forever, and the fix is deep in a scary bit of code. I may push it out to 1.7.1.

@baconpaul
Copy link
Collaborator

OK so this is tricky because of one thing. Notes to self.

the switch happens in SurgeSynthesizer::loadOscAlgos and only if queue_type is != -1
queue_type is not set on patch load so we could in theory clobber modulation there

it is set by parameter automation - unclear if we want to blow away then
it is set by the snapshot menu - char we do want to blow away then
but also sneakily it set every time you drag and drop a wavetable in.
The first time you drop a wavetable in (so you are in sine and you go to wavetable) you want to reset
but the second time you drag one in it also re-spawns.
So that's where we need to be careful.

@baconpaul
Copy link
Collaborator

Here's the logic which would mean param automation and menu resets, and wave drop does the right thing

bool SurgeSynthesizer::loadOscalgos()
{
   for (int s = 0; s < 2; s++)
   {
      for (int i = 0; i < n_oscs; i++)
      {
         bool resend = false;
         if (storage.getPatch().scene[s].osc[i].queue_type > -1)
         {
            bool reloadMod = true;
            if( storage.getPatch().scene[s].osc[i].type.val.i ==
                storage.getPatch().scene[s].osc[i].queue_type )
            {
               reloadMod = false;
            }

@baconpaul
Copy link
Collaborator

Group decision was to not clear when you daw automate, to avoid breaking saved tracks, even though you can daw automate into a weird state.

@baconpaul
Copy link
Collaborator

Pushing this to 1.7.n since it is a rare-ish case and a bit tricky to get this late in.

@baconpaul baconpaul modified the milestones: 1.7 0, 1.7.n Jun 30, 2020
@mkruselj mkruselj modified the milestones: 1.7.n, 1.7.1 Jul 17, 2020
@baconpaul baconpaul modified the milestones: 1.7.1, Currently Unscheduled Aug 2, 2020
@mkruselj mkruselj modified the milestones: Currently Unscheduled, 1.7.n Aug 15, 2020
@baconpaul baconpaul modified the milestones: 1.7.n, 1.7.2 Sep 1, 2020
@mkruselj mkruselj modified the milestones: 1.8.0, 1.8.n, 1.9.0 Oct 7, 2020
@mkruselj mkruselj added the Bug Report Item submitted using the Bug Report template label Nov 9, 2020
mkruselj added a commit that referenced this issue Feb 19, 2021
Closes #2224

Also comment out currently unimplemented osc types from SurgeStorage and update extra content repo hash
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Report Item submitted using the Bug Report template Modulation Modulation related issues UX Issues related to user experience (UX) - mouse, touch, keyboard, MIDI inputs, etc.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants