Skip to content

Conversation

@Rossmaxx
Copy link
Contributor

@Rossmaxx Rossmaxx commented Sep 3, 2024

Gate knob is one of the most broken "features" and per @LostRobotMusic, It shouldn't even exist.

</key>
</effect>
<effect name="dualfilter" autoquit_denominator="4" on="1" autoquit_numerator="4" wet="1" autoquit_syncmode="0" gate="0.02" autoquit="1">
<effect name="dualfilter" autoquit_denominator="4" on="1" autoquit_numerator="4" wet="1" autoquit_syncmode="0" autoquit="1">
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I do this for rest of the presets?

@bratpeki
Copy link
Member

bratpeki commented Sep 3, 2024

How do effects now look in the UI? Could you drop a screenshot?

@Rossmaxx
Copy link
Contributor Author

Rossmaxx commented Sep 3, 2024

I haven't compiled yet tbh. I'll update soon

@Rossmaxx
Copy link
Contributor Author

Rossmaxx commented Sep 3, 2024

image

@bratpeki here you go

Copy link
Contributor

@LostRobotMusic LostRobotMusic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In all of the Effects, you're removing the checkGate checks without also removing the unused outSum variables.

@Rossmaxx
Copy link
Contributor Author

Rossmaxx commented Sep 3, 2024

Good catch, I'll check

@messmerd
Copy link
Member

messmerd commented Sep 3, 2024

Wouldn't it be easier if I just remove the gate in #7484? I'm already touching that code

@DomClark
Copy link
Member

DomClark commented Sep 3, 2024

The way the "stop processing effects without input" feature turns off effects is by checking whether the output level falls below the gate value. Removing gate checking entirely will break this feature. The idea is to remove the gate knob and force a gate value of zero, not to remove all code relating to the gate.

@Rossmaxx
Copy link
Contributor Author

Rossmaxx commented Sep 4, 2024

I looked at @messmerd 's PR again and the diff is similar so it would be better to consolidate.

Since I'm closing, I'm leaving dom's point to messmerd

@Rossmaxx Rossmaxx closed this Sep 4, 2024
@messmerd
Copy link
Member

messmerd commented Sep 4, 2024

It looks like #7438 removes the Gate knob, so hopefully we can merge that soon and I won't have to do it in my PR.

@Rossmaxx Rossmaxx deleted the remove-gate branch September 28, 2024 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants