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

Nimbus "Freeze" parameter can't be modulated #6814

Closed
mx opened this issue Jan 21, 2023 · 4 comments · Fixed by #6815
Closed

Nimbus "Freeze" parameter can't be modulated #6814

mx opened this issue Jan 21, 2023 · 4 comments · Fixed by #6815
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

@mx
Copy link
Collaborator

mx commented Jan 21, 2023

Bug Description:
The "Freeze" parameter in the Nimbus effect cannot be modulated by a scene envelope. Confusingly, ED was able to actually make it work and even send me a patch where it is modulated, which I could load! However, myself and at least one other person confirmed that we weren't able to do it on our systems!

Surge XT Version
Latest nightly

  • Version: Latest nightly
  • Plugin Type: standalone
  • Bitness: 64-bit

Reproduction Steps:
Steps to reproduce the behavior:

  1. Load template "Init Saw"
  2. Add Nimbus effect to first FX on chain A.
  3. Click on "S-LFO 1". Change it to the template "Basic ADSR".
  4. Set D to 1 second, S to 100%, every other slider to 0.
  5. Try to drag and drop it onto the "Freeze" slider. Does not work. (It can successfully be dropped on any other parameter.)

Expected Behavior:
Freeze parameter is modulatable.

Screenshots:
N/A

Computer Information (please complete the following!):

  • OS: Windows 10
  • Host: Standalone

Additional Information:
N/A

@mx mx added the Bug Report Item submitted using the Bug Report template label Jan 21, 2023
@baconpaul baconpaul added this to the Surge XT 1.2 milestone Jan 22, 2023
@mx
Copy link
Collaborator Author

mx commented Jan 22, 2023

The following two methods of adding modulation do work:

  1. Going through the modulation list view, and manually adding the source and target.
  2. Going through routing mode (Click to enable routing mode on the envelope, then click and drag the "Freeze" slider to 100%).

I can also confirm another method does not work: right-click on the freeze slider, add modulation from, select the envelope.

@baconpaul
Copy link
Collaborator

Oh I mis understood so it’s just drag and drop which fails? I didn’t try that

@baconpaul
Copy link
Collaborator

so here's the problem

That is a toggle (the string values are 'on and off') but it is a float so it can be modulated

The type (ct_float_toggle) returns false for can_setvalue_from_string which supresses typeins (understandably)

So the fix is

1: make setvalue from string return true for ct_float
2: set up the thing as a 'special' type with a custom handler.
3: implement the value from string for on and off to 0 1
4. implement the modulation from string

@baconpaul
Copy link
Collaborator

baconpaul commented Jan 22, 2023

basically this parameter advertises that you can't type in values for it. so the paths which let you then just silently dont

a better ui would be to pop a small error in SurgeGUIEditor::promptForUserValueEntry inside this return

if (p)
    {
        if (!p->can_setvalue_from_string())
        {
            return; // this is what bails out silently
        }
    }

to avoid this problem again in the future I guess

I haven't fixed any of these just diagnosed them but i did confirm that if I do step 1 above then it pops up the dialog. Just the dialog doesn't work because of steps 2-4

@mkruselj mkruselj added Modulation Modulation related issues UX Issues related to user experience (UX) - mouse, touch, keyboard, MIDI inputs, etc. labels Jan 22, 2023
mkruselj added a commit to mkruselj/surge that referenced this issue Jan 22, 2023
Closes surge-synthesizer#6814

Also rename calculate_modulation_value_from_string() to set_modulation_value_from_string() to match set_value_from_string() API.
mkruselj added a commit that referenced this issue Jan 24, 2023
* Make typein for ct_float_toggle work

Closes #6814

Co-authored-by: Paul Walker <[email protected]>
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.

3 participants