feat(audio): mute on pause

[mwerle]

DRAFT PR

TODO:

• add option whether or not audio should be muted on pause
• investigate if muting is possible when the game window loses focus
• determine best place to send out the new LUA events "onFocusGained/Lost"
• mute only SFX or at least add an option whether or not to mute the music

---

Mute all audio when the game is paused, either by opening the settings window, or by clicking on the Pause icon, and resume when the game is resumed.

NOTE: This PR conflicts directly with #5644 ; so as it appears this is very much personal preference, it may be necessary to add a settings toggle which would make this a slightly less trivial PR.

[DamagedEngine]

The toggle could have a third option added which only mutes the game when the game window is out-of-focus.

The setting could be called "Mute on pause:" with the following options:

1. Always
2. When out-of-focus
3. Never

This way the music would not disturb players who pause the game to do something else, without interrupting the music for players who only paused the game to interact with menus and look around.

[impaktor]

I would expect music to keep playing in paused state.

[DamagedEngine]

Perhaps it would be better to just add a mute-on-defocus option without tying it to the pause function? I do think the game needs a way to stop it from playing sound in the background when another window such as a web browser is in focus. That desire is presumably why some users might expect the game to become muted on pause.

[mwerle]

I would expect music to keep playing in paused state.

I would not - hence this PR. Especially things like engine noise are incredibly annoying (to me) when I pause the game (I almost always have the music muted already anyway) to work on tweaking some code, but the audio engine does not allow just pausing and resuming the FX (intended or bug?)

However, point taken, I will create an option. Not sure if it's (easily) possible to detect the out-of-focus state, but will check if that can be supported.

Thank you for the feedback.

[mwerle]

Perhaps it would be better to just add a mute-on-defocus option without tying it to the pause function? I do think the game needs a way to stop it from playing sound in the background when another window such as a web browser is in focus. That desire is presumably why some users might expect the game to become muted on pause.

Conversely I personally do NOT want the audio to become muted just because I switch to a note-taking app or browser in the middle of a gaming session when I am not pausing the game to do so (eg, during a long transfer burn). I am still playing, just using out-of-game tooling to support my game experience.

As I said, this is highly personal. On the one hand having a million game option toggles should be avoided (difficult to test, if nothing else), otoh, something as personal as this I think warrants it.

[mwerle]

I pushed an update which also (un)mutes on focus. This additional setting works in conjunction with the (un)mute-on-pause.

While my testing so far appears to show it working as intended, there are two (possibly more) issues:

• doesn't work in the main menu (not sure if there's a place where the handlers can be implemented where it always works?)
• the place where I implemented sending out the new LUA "onFocusGained/Lost" events (Pi.cpp:GameLoop::Update()) is probably not where this logic should be. I'd like suggestions on where to implement this instead (please see the commit message for more details)

[impaktor]

We probably want the opinions of @bszlrd and @sturnclaw, and why not @fluffyfreak who works as a game dev: what is the sane / expected behaviour for music & FX when paused, and/or paused and/or unfocused window. FX be muted feels natural, but not sure about the other.

I assume FX and music can be handled separately since we can set their volume independently)

[sturnclaw]

Quick feedback here before I go to bed:

IMO, the focus-related events should be pushed to the UI event queue rather than the main game event queue. IIRC, the main event queue is (intentionally) not emitted while the game is paused, and so all events which exist at a higher level than the game-world context should be sent to the UI queue instead. There is an existing example of how to do so in the codebase; you can find it by looking up callsites of the appropriate LuaEvent overloads which take a LuaRef as their first argument.

I will fully review the comment in question tomorrow regarding where to hang the event generation code off of.

On the whole I like the idea here; mute-on-minimize/focus loss and mute-on-pause are definitely orthogonal states; personally I'd like the ambient FX to stop playing but the music to continue while paused (as a baseline) regardless of the above setting states (ambient FX is game-layer audio, music is application-layer/interface-layer audio), but we can look at that in more depth with a deeper change to the sound system.

Side complication: how does this PR interact with interface sounds (i.e. clicks)? They should not be muted while the pause menu is open. We don't currently have a lot of them, but audible interface feedback is something I'd like to normalize across the UI long-term.

[mwerle]

So possibly it's a bug, but muting SFX and then unmuting them (using the LUA functions) does NOT restore the SFX volume. This is different to how muting/unmuting the Master Volume works. This is primarily why I went for Master Volume in the current implementation..

Personal preference is for it to behave as I have implemented it - ie,both music and SFX are muted. I guess this will require another setting.. but then again it probably won't impact me much since, as I mentioned, I almost always have the music turned off anyway so assuming the above is agreed to be a bug, I can fix that bug and then change my implementation to only (un)mute the SFX..

Fully agree that interface clicks should not be muted, so will need to modify this PR anyway. Thanks for all the feedback so far.

[mwerle]

IMO, the focus-related events should be pushed to the UI event queue rather than the main game event queue. ... There is an existing example of how to do so in the codebase; you can find it by looking up callsites of the appropriate LuaEvent overloads which take a LuaRef as their first argument.

Based on the only example I could find ("onGamePaused" and "onGameResumed"), I changed the event emitting to:

				LuaEvent::Queue(PiGui::GetEventQueue(), hasFocus? "onFocusGained" : "onFocusLost");

but haven't figured out how to register and receive these events on the LUA side, as "onGamePaused" is also emitted to the main game event queue and registered in the normal way on the LUA side.

[sturnclaw]

Hopefully these answer your questions. I've left a few tangentially-related comments, as well as one nitpick on the wording of the UI string for the focus-lost option.

[sturnclaw]

These four event handlers should become ui.Events:Register(..., function() ... end). onGamePaused et. al. is pushed to the UI event queue and there is no guarantee that events on the main queue will be processed until the pause ends.

[mwerle]

It turns out it needed to be ui.Events.Register(...) but thank you, finally managed to get the UI events working.

This is now also the only example in the entire codebase which registers a UI event in the LUA side (as far as my grep skills can tell).

[mwerle]

Thank you; implemented in b5e8818 (to be squashed)

[sturnclaw]

The proper place to handle this would be to listen for SDL_WINDOWEVENT_FOCUS_GAINED et. al. in core/GuiApplication.cpp, specifically the PollEvents function. That should dispatch to a virtual member method:

virtual void OnWindowFocusChanged(bool newFocus) {}

That method would then be implemented by Pi::App, and an event dispatched to the UI event queue like so:

void Pi::App::OnWindowFocusChanged(bool newFocus)
{
    if (!PiGui::GetEventQueue().IsValid())
        return;
    LuaEvent::Queue(PiGui::GetEventQueue(), newFocus ? "onFocusGained" : "onFocusLost");
}

[mwerle]

Thank you; implemented in b5e8818 (to be squashed)

[sturnclaw]

Nitpick: the codebase's stylistic conventions require a space on both sides of the equals-sign here. We used to have a clang-format nag running on PRs to catch this stuff automatically, but it got far too opinionated without config-file levers to disable its opinions.

[mwerle]

Thanks; typo.

[mwerle]

Fixed in 9240816 (to be squashed)

[sturnclaw]

On a tangent: I hate boilerplate like this and would much prefer we just expose an Engine.GetConfigOption(key, defaultValue), Engine.SetConfigOption(key, value) API with consistent (and searchable!) string keys used directly.

This is not actionable feedback within the scope of this PR, but is something you could potentially implement in a separate, later PR if you so desired.

[mwerle]

Not a LUA programmer so will take your lead on this. Personally I'm not so fond of string parameterisation like that since compilers/analysers cannot detect errors, which can lead to some very subtle and difficult to trace bugs.

[sturnclaw]

Suggested change

	"message": "Mute on Focus"
	"message": "Mute on Window Focus Lost"

[mwerle]

Fixed in 3bd058e (to be squashed)

[mwerle]

Thank you for the detailed information.

Addressed all issues, but still need to implement muting only the SFX. That will require a bugfix to the way the SFX are muted/unmuted (SFX volume is not restored when SFX are unmuted).

[mwerle]

Ok, basically, the way the music system is implemented does not really allow for muting/unmuting of music and effects.

Music: when muted, stops the music player, when unmuted, restarts playing the music from the beginning. (ie, it neither pauses the music, nor continues playing it muted. It's more a stop/start than a mute/unmute function)

Effects: literally does nothing. Once a sample has been queued for playing, it is played at its current volume. Muting it does nothing. Changing volume does nothing. Furthermore, UI effects and "background noise" appear to both come under "Effects" so it is not possible to mute the background noise but leave UI clicks enabled.

TEST: Start a new game and listen to the background noise effects. Open settings window and try to mute or change the volume of the effects. It will affect interface clicks (since each one is a new effect) but will not change the background noise effects.
Toggling through the external views (F1) will restart the effects; volume changes/muting take effect for the next time an effect is played.

---

Since this does not work as desired by the community, I will retract this PR and maintain this in my personal branch. Muting "Master Volume" does what I want it to do. Fixing the entire audio system so muting can work as desired by the community is beyond my interest and skill level.

That said, some of the commits may be useful to merge anyway to enable some other functionality, such as events for lost/gained focus. I also found a couple of minor bugs which should be merged. I will create a new "minor-fixes" branch for those.