Add opt-in blackout while no music is playing#124
Conversation
Adds a global "Blackout on Silence" toggle to the main control window, below the Next Effect button. When enabled, the stage fades to black within one second after the input level stayed below the minimum level threshold for a 1.5s grace period, and resumes as soon as music is audible again. The toggle is off by default, so existing behaviour is unchanged, and it is persisted through the regular settings save/load. The effect list and its per-effect RND/settings columns move down to make room for the toggle, and the window grows slightly so all entries still fit.
codingjoe
left a comment
codingjoe
left a comment
There was a problem hiding this comment.
Nice, it looks promising. It's a really cool idea! I had a few comments, but nothing major. Ping me when you are doing and I will give it a final review. Cheerse! Joe
| float fade = ctrl.silenceFade(); | ||
| if (fade <= 0) |
There was a problem hiding this comment.
I am not a big fan of early returns. Please consider inverting the logic and only executing the subsequent code if the conditions are met.
| if (fade < 1) { | ||
| // additive blending cannot darken, so switch to normal blending | ||
| // for a translucent black overlay while fading out | ||
| blendMode(BLEND); | ||
| noStroke(); | ||
| fill(0, 0, 0, (1-fade)*100); | ||
| rect(-width/2, -height/2, width, height); | ||
| blendMode(ADD); | ||
| } |
There was a problem hiding this comment.
Currently the time-based blackout is separate from the blackout effect. Maybe those should be be unified. It would also be a nice indication in the interface if the effect is toggled on silence.
There was a problem hiding this comment.
I am unsure about this one. I dont see the sense in the Blackout Effect. Blackout is already the case if no Effect is selected, so why does it exist in the first place? Integrating that thing I dont quite understand deeper into this feature feels wrong. Also it feels wrong to interact with the Blackout Effect from the stage, I think they should stay rather decoupled.
If we really need to make it more clear to the user that "Blackout on silence" is currently kicking in, I would rather suggest to add some kind of indication to the button activating the feature itself. I don't see the necessity though.
| if (!blackoutOnSilenceToggle.getState()) | ||
| return 1; | ||
| int quiet = millis()-lastAudibleMillis; | ||
| if (quiet <= 1500) | ||
| return 1; |
There was a problem hiding this comment.
Again,not a fan of early returns. return 1 should be the last statement if all other blocks were not executed and didn't return.
97f0cfd to
81faf41
Compare
|
@codingjoe consider pinged :) I adressed the early returns. Regarding the blackout effect topic i need further clarification. |
|
Nice, LGTM! |
2 participants
Adds a global "Blackout on Silence" toggle to the main control window, below the Next Effect button. When enabled, the stage fades to black within one second after the input level stayed below the minimum level threshold for a 1.5s grace period, and resumes as soon as music is audible again. The toggle is off by default, so existing behaviour is unchanged, and it is persisted through the regular settings save/load.
The effect list and its per-effect RND/settings columns move down to make room for the toggle, and the window grows slightly so all entries still fit.
Disclosure: This PR was created with help of AI coding assistance, but thoughtfully reviewed by me