Clean up sound_fade (#10119)

hecktest · web-flow · commit 5bda36143f9b · 2020-08-19T19:26:37.000+02:00
Add proper documentation and correct gain reduction calculations.

Co-authored-by: hecktest &lt;&gt;
diff --git a/doc/lua_api.txt b/doc/lua_api.txt
@@ -5238,9 +5238,12 @@ Sounds
 * `minetest.sound_fade(handle, step, gain)`
     * `handle` is a handle returned by `minetest.sound_play`
     * `step` determines how fast a sound will fade.
-      Negative step will lower the sound volume, positive step will increase
-      the sound volume.
+      The gain will change by this much per second,
+      until it reaches the target gain.
+      Note: Older versions used a signed step. This is deprecated, but old
+      code will still work. (the client uses abs(step) to correct it)
     * `gain` the target gain for the fade.
+      Fading to zero will delete the sound.
 
 Timing
 ------
diff --git a/src/client/sound_openal.cpp b/src/client/sound_openal.cpp
@@ -337,14 +337,12 @@ class OpenALSoundManager: public ISoundManager
 	};
 
 	std::unordered_map<int, FadeState> m_sounds_fading;
-	float m_fade_delay;
 public:
 	OpenALSoundManager(SoundManagerSingleton *smg, OnDemandSoundFetcher *fetcher):
 		m_fetcher(fetcher),
 		m_device(smg->m_device.get()),
 		m_context(smg->m_context.get()),
-		m_next_id(1),
-		m_fade_delay(0)
+		m_next_id(1)
 	{
 		infostream << "Audio: Initialized: OpenAL " << std::endl;
 	}
@@ -616,38 +614,45 @@ class OpenALSoundManager: public ISoundManager
 
 	void fadeSound(int soundid, float step, float gain)
 	{
-		m_sounds_fading[soundid] = FadeState(step, getSoundGain(soundid), gain);
+		// Ignore the command if step isn't valid.
+		if (step == 0)
+			return;
+		float current_gain = getSoundGain(soundid);
+		step = gain - current_gain > 0 ? abs(step) : -abs(step);
+		if (m_sounds_fading.find(soundid) != m_sounds_fading.end()) {
+			auto current_fade = m_sounds_fading[soundid];
+			// Do not replace the fade if it's equivalent.
+			if (current_fade.target_gain == gain && current_fade.step == step)
+				return;
+			m_sounds_fading.erase(soundid);
+		}
+		gain = rangelim(gain, 0, 1);
+		m_sounds_fading[soundid] = FadeState(step, current_gain, gain);
 	}
 
 	void doFades(float dtime)
 	{
-		m_fade_delay += dtime;
-
-		if (m_fade_delay < 0.1f)
-			return;
+		for (auto i = m_sounds_fading.begin(); i != m_sounds_fading.end();) {
+			FadeState& fade = i->second;
+			assert(fade.step != 0);
+			fade.current_gain += (fade.step * dtime);
 
-		float chkGain = 0;
-		for (auto i = m_sounds_fading.begin();
-				i != m_sounds_fading.end();) {
-			if (i->second.step < 0.f)
-				chkGain = -(i->second.current_gain);
+			if (fade.step < 0.f)
+				fade.current_gain = std::max(fade.current_gain, fade.target_gain);
 			else
-				chkGain = i->second.current_gain;
+				fade.current_gain = std::min(fade.current_gain, fade.target_gain);
 
-			if (chkGain < i->second.target_gain) {
-				i->second.current_gain += (i->second.step * m_fade_delay);
-				i->second.current_gain = rangelim(i->second.current_gain, 0, 1);
-
-				updateSoundGain(i->first, i->second.current_gain);
-				++i;
-			} else {
-				if (i->second.target_gain <= 0.f)
-					stopSound(i->first);
+			if (fade.current_gain <= 0.f)
+				stopSound(i->first);
+			else
+				updateSoundGain(i->first, fade.current_gain);
 
+			// The increment must happen during the erase call, or else it'll segfault.
+			if (fade.current_gain == fade.target_gain)
 				m_sounds_fading.erase(i++);
-			}
+			else
+				i++;
 		}
-		m_fade_delay = 0;
 	}
 
 	bool soundExists(int sound)
