Add sound volume when unfocused setting

[srifqi]

Goal of the PR
This PR adds a new setting to set sound volume multiplier when Minetest window is unfocused/inactive (sound_volume_unfocused, located in Settings > Graphics and Audio > Audio > Volume when unfocused).

How does the PR work?
This PR checks whether the game window is focused. If the window is not focused, the sound volume will be multiplied by sound_volume_unfocused setting. The sound volume will be set back to sound_volume again when the window is focused.

Does it resolve any reported issue?
This PR tries to fix #14026.

Does this relate to a goal in the roadmap?
Yes, this PR tries to fix a UI/UX issue.

To do

This PR is Ready for Review.

How to test

1. Play inside a world with music.
2. Wait/trigger the music inside the world.
3. Note the sound volume.
4. Focus to another window. Make Minetest window unfocused.
5. Notice that the sound volume is lower (depends on the set value).

Additional comments

This PR changes the volume instantly. Is it fine or should a smooth transition be added for the volume change?

I am not sure whether this new setting should be in the in-game volume change dialog.

I also found that the main menu music (the game theme music) does not respect sound_volume setting.

[Desour]

I suggest to make sound_volume_unfocused a multiplier. Otherwise you could get into situations where you lower the volume in-game and unexpectedly get a higher volume once you unfocus the window.

I also found that the main menu music (the game theme music) does not respect sound_volume setting.

Good to know. The main menu sound is handled in GUIEngine, FYI, in case you want to fix this yourself while you're at it. :)

Edit: And you have to edit defaultsettings (or else it segfaults).

[srifqi]

I suggest to make sound_volume_unfocused a multiplier. Otherwise you could get into situations where you lower the volume in-game and unexpectedly get a higher volume once you unfocus the window.

I was going to do that, but I changed my mind at the last minute. Now that you mentioned it, I checked the code of my reference game and I found that it is indeed a multiplier.

private Func<double, double, double> getAggregateFunction(AdjustableProperty type)
{
    switch (type)
    {
        default:
            return (a, b) => a * b; // <- This is for Volume and other values.

        case AdjustableProperty.Balance:
            return (a, b) => a + b;
    }
}

[srifqi]

The main menu sound is handled in GUIEngine

Done. :)

---

I might reorder/squash some commits later.

[Desour]

Tested, works. Thanks! c:

Besides isWindowActive(), there's also isWindowFocused(). In SDL and X11 backends, active means focused and not minimized. I wasn't able to find out what it means in win32.
Sometimes we're using one function sometimes the other. I don't know how to get a minimized window with input focus, so both seem fine to me.

[grorp]

This code is now duplicated between game.cpp and guiEngine.cpp:

minetest/src/client/game.cpp

Lines 3212 to 3230 in 9ebac40

	bool mute_sound = g_settings->getBool("mute_sound");
	if (mute_sound) {
	sound_manager->setListenerGain(0.0f);
	} else {
	// Check if volume is in the proper range, else fix it.
	float old_volume = g_settings->getFloat("sound_volume");
	float new_volume = rangelim(old_volume, 0.0f, 1.0f);
	if (old_volume != new_volume) {
	g_settings->setFloat("sound_volume", new_volume);
	}
	if (!device->isWindowActive()) {
	new_volume *= g_settings->getFloat("sound_volume_unfocused");
	new_volume = rangelim(new_volume, 0.0f, 1.0f);
	}
	sound_manager->setListenerGain(new_volume);
	}

minetest/src/gui/guiEngine.cpp

Lines 314 to 332 in 9ebac40

	bool mute_sound = g_settings->getBool("mute_sound");
	if (mute_sound) {
	m_sound_manager->setListenerGain(0.0f);
	} else {
	// Check if volume is in the proper range, else fix it.
	float old_volume = g_settings->getFloat("sound_volume");
	float new_volume = rangelim(old_volume, 0.0f, 1.0f);
	if (old_volume != new_volume) {
	g_settings->setFloat("sound_volume", new_volume);
	}
	if (!device->isWindowActive()) {
	new_volume *= g_settings->getFloat("sound_volume_unfocused");
	new_volume = rangelim(new_volume, 0.0f, 1.0f);
	}
	m_sound_manager->setListenerGain(new_volume);
	}

Could this code please be moved to a common function?

[srifqi]

I wasn't able to find out what it means in win32.

Differences:

1. CIrrDeviceWin32::isWindowActive() calls GetActiveWindow() (active window attached to the calling thread's message queue).
2. CIrrDeviceWin32::isWindowFocused() calls GetFocus() (keyboard focus).

While the documentation says,

Use the GetForegroundWindow function to retrieve the handle to the window with which the user is currently working.

Irrlicht sets the Minetest window as active and foreground simultaneously (code):

// set this as active window
if (!ExternalWindow)
{
	SetActiveWindow(HWnd);
	SetForegroundWindow(HWnd);
}

Hence I chose to use isWindowActive(). CMIIW.

[srifqi]

This code is now duplicated between game.cpp and guiEngine.cpp:

I created a helper function sound_control_by_window() inside src/client/sound.h.

Regarding the ISoundManager * parameter, since std::unique_ptr would not allow passing itself to another function and the pointed data never becomes owned by the helper function (the helper function only modifies the pointed data), the raw pointer is passed instead.

[grorp]

This PR seems like a good solution to #14026.

I created a helper function sound_control_by_window() inside src/client/sound.h.

Now I'm getting lots of these warnings:

In file included from /mt/src/client/sound/sound_openal.h:22,
                 from /mt/src/client/sound/sound_openal.cpp:24:
/mt/src/client/sound.h:190:13: warning: 'void sound_control_by_window(Settings*, ISoundManager*, irr::IrrlichtDevice*)' defined but not used [-Wunused-function]
  190 | static void sound_control_by_window(Settings *settings, ISoundManager *sound_mgr, irr::IrrlichtDevice *device) {
      |             ^~~~~~~~~~~~~~~~~~~~~~~
[ 38%] Building CXX object src/CMakeFiles/minetest.dir/gui/guiEngine.cpp.o
In file included from /mt/src/client/sound.cpp:20:
/mt/src/client/sound.h:190:13: warning: 'void sound_control_by_window(Settings*, ISoundManager*, irr::IrrlichtDevice*)' defined but not used [-Wunused-function]
  190 | static void sound_control_by_window(Settings *settings, ISoundManager *sound_mgr, irr::IrrlichtDevice *device) {
      |             ^~~~~~~~~~~~~~~~~~~~~~~
[ 38%] Building CXX object src/CMakeFiles/minetest.dir/gui/guiFormSpecMenu.cpp.o
In file included from /mt/src/client/sound/sound_manager_messages.h:22,
                 from /mt/src/client/sound/sound_manager.h:29,
                 from /mt/src/client/sound/proxy_sound_manager.h:22,
                 from /mt/src/client/sound/proxy_sound_manager.cpp:20:
/mt/src/client/sound/../sound.h:190:13: warning: 'void sound_control_by_window(Settings*, ISoundManager*, irr::IrrlichtDevice*)' defined but not used [-Wunused-function]
  190 | static void sound_control_by_window(Settings *settings, ISoundManager *sound_mgr, irr::IrrlichtDevice *device) {
      |             ^~~~~~~~~~~~~~~~~~~~~~~
[ 38%] Building CXX object src/CMakeFiles/minetest.dir/network/clientpackethandler.cpp.o
In file included from /mt/src/client/sound/sound_manager_messages.h:22,
                 from /mt/src/client/sound/sound_manager.h:29,
                 from /mt/src/client/sound/sound_manager.cpp:25:
/mt/src/client/sound/../sound.h:190:13: warning: 'void sound_control_by_window(Settings*, ISoundManager*, irr::IrrlichtDevice*)' defined but not used [-Wunused-function]
  190 | static void sound_control_by_window(Settings *settings, ISoundManager *sound_mgr, irr::IrrlichtDevice *device) {
      |             ^~~~~~~~~~~~~~~~~~~~~~~

etc.

A solution would be to make sound_control_by_window non-static and move its implementation to sound.cpp.

[Desour]

Now I'm getting lots of these warnings:

AFAIK, one should mark inline functions in headers with inline, and inline functions in cpp files static (or use an unnamed namespace). static produces one function for each translation unit, while inline allows to surpass the ODR, by assuming that all definitions are the same, and makes the linker unify them.

Anyway, this function doesn't need to be inlined.
(Also, I wouldn't pass g_settings around.)

[srifqi]

Now I'm getting lots of these warnings:

Thank you for noticing it. I must have missed that.

---

Also, I wouldn't pass g_settings around.

Oh, that is true. I forgot that g_settings is a global variable.

[grorp]

LGTM

[srifqi]

I changed the helper function a little.

[srifqi]

Thank you for the merge.