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

Scale LFO/EG separate outputs by amplitude optionally #6674

Closed
baconpaul opened this issue Oct 31, 2022 · 8 comments · Fixed by #6820
Closed

Scale LFO/EG separate outputs by amplitude optionally #6674

baconpaul opened this issue Oct 31, 2022 · 8 comments · Fixed by #6820
Labels
Feature Request New feature request Modulation Modulation related issues
Milestone

Comments

@baconpaul
Copy link
Collaborator

The raw outputs don't take amplitude into account

They probably should but that will break old patches so

  1. Add a toggle and a menu
  2. Maybe update patch version and have the new default be true
@baconpaul baconpaul added the Feature Request New feature request label Oct 31, 2022
@baconpaul baconpaul added this to the Surge XT 1.1.3 milestone Oct 31, 2022
@Andreya-Autumn
Copy link
Collaborator

Agree, both seem good.

@baconpaul
Copy link
Collaborator Author

So I have this coded but @mkruselj and @Andreya-Autumn an opinion

Should this setting/option be per LFO or per patch.

I have coded it as per LFO but that makes it hard to change an entire patch.

Everything else (default, scale, extend, option) works without problem. But as I was going to write the streaming code I thought: do I want to stream this 12 times or 1?

I have my code stashed away in baconpaul/lfo-scale-6674 and once we decide above it is 30 minutes to close this issue

My gut is "per patch" is right.

@mkruselj
Copy link
Collaborator

mkruselj commented Jan 25, 2023

It should per LFO IMO. Maybe with an option to apply current setting to all LFOs in the scene (not the patch). Just like how we deal with temp sync on envs.

@mkruselj mkruselj added the Modulation Modulation related issues label Jan 25, 2023
@Andreya-Autumn
Copy link
Collaborator

Agreed. Default to on, turn-offable per LFO seems best to me.

@mkruselj
Copy link
Collaborator

mkruselj commented Jan 25, 2023

It should default to off to retain existing behavior.

@Andreya-Autumn
Copy link
Collaborator

Is that necessary you think? It should be off for saved patches obviously. But beyond that I'm not sure we need to stay consistent here.

@mkruselj
Copy link
Collaborator

We should for the XT1 cycle at least. We did so far for pretty much all other cases, always kept existing behavior as default.

@Andreya-Autumn
Copy link
Collaborator

We didn't for the mono retrig-from-current behavior, but admittedly the new behavior is more obviously "better" in that case.
I feel like this change is also for the better. But I'm not attached, if y'all disagree let's keep current default until XT2.

baconpaul added a commit to baconpaul/surge that referenced this issue Jan 25, 2023
Allow users to toggle raw outputs scaled by amplitude on an LFO-by-LFO
basis in the hamburger menu.

Closes surge-synthesizer#6674
baconpaul added a commit that referenced this issue Jan 25, 2023
Allow users to toggle raw outputs scaled by amplitude on an LFO-by-LFO
basis in the hamburger menu.

Closes #6674
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request New feature request Modulation Modulation related issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants