Fix attached sounds stopping if objects are removed serverside (#14436)

Restores backwards compatibility for death sounds or other sounds that are not supposed to be "cut off" abruptly.

---------

Co-authored-by: sfan5 <sfan5@live.de>
Co-authored-by: grorp <gregor.parzefall@posteo.de>

doc/lua_api.md

@@ -1095,6 +1095,7 @@ Table used to specify how a sound is played:
     -- its end in `-start_time` seconds.
     -- It is unspecified what happens if `loop` is false and `start_time` is
     -- smaller than minus the sound's length.
+
     -- Available since feature `sound_params_start_time`.
 
     loop = false,
@@ -1108,6 +1109,21 @@ Table used to specify how a sound is played:
     -- Attach the sound to an object.
     -- Can't be used together with `pos`.
 
+    -- For backward compatibility, sounds continue playing at the last location
+    -- of the object if an object is removed (for example if an entity dies).
+    -- It is not recommended to rely on this.
+    -- For death sounds, prefer playing a positional sound instead.
+
+    -- If you want to stop a sound when an entity dies or is deactivated,
+    -- store the handle and call `minetest.sound_stop` in `on_die` / `on_deactivate`.
+
+    -- Ephemeral sounds are entirely unaffected by the object being removed
+    -- or leaving the active object range.
+
+    -- Non-ephemeral sounds stop playing on clients if objects leave
+    -- the active object range; they should start playing again if objects
+    --- come back into range (but due to a known bug, they don't yet).
+
     to_player = name,
     -- Only play for this player.
     -- Can't be used together with `exclude_player`.

src/client/client.cpp

@@ -2123,11 +2123,3 @@ const std::string &Client::getFormspecPrepend() const
 {
 	return m_env.getLocalPlayer()->formspec_prepend;
 }
-
-void Client::removeActiveObjectSounds(u16 id)
-{
-	for (auto it : m_sounds_to_objects) {
-		if (it.second == id)
-			m_sound->stopSound(it.first);
-	}
-}

src/client/client.h

@@ -471,9 +471,6 @@ private:
 
 	bool canSendChatMessage() const;
 
-	// remove sounds attached to object
-	void removeActiveObjectSounds(u16 id);
-
 	float m_packetcounter_timer = 0.0f;
 	float m_connection_reinit_timer = 0.1f;
 	float m_avg_rtt_timer = 0.0f;

src/network/clientpackethandler.cpp

@@ -471,7 +471,6 @@ void Client::handleCommand_ActiveObjectRemoveAdd(NetworkPacket* pkt)
 		for (u16 i = 0; i < removed_count; i++) {
 			*pkt >> id;
 			m_env.removeActiveObject(id);
-			removeActiveObjectSounds(id);
 		}
 
 		// Read added objects

src/server.cpp

@@ -2041,7 +2041,8 @@ void Server::SendActiveObjectRemoveAdd(RemoteClient *client, PlayerSAO *playersa
 	if (my_radius <= 0)
 		my_radius = radius;
 
-	std::queue<u16> removed_objects, added_objects;
+	std::queue<std::pair<bool, u16>> removed_objects;
+	std::queue<u16> added_objects;
 	m_env->getRemovedActiveObjects(playersao, my_radius, player_radius,
 		client->m_known_objects, removed_objects);
 	m_env->getAddedActiveObjects(playersao, my_radius, player_radius,
@@ -2057,13 +2058,21 @@ void Server::SendActiveObjectRemoveAdd(RemoteClient *client, PlayerSAO *playersa
 	std::string data;
 
 	// Handle removed objects
+
 	writeU16((u8*)buf, removed_objects.size());
 	data.append(buf, 2);
 	while (!removed_objects.empty()) {
 		// Get object
-		u16 id = removed_objects.front();
+		const auto [gone, id] = removed_objects.front();
 		ServerActiveObject* obj = m_env->getActiveObject(id);
 
+		// Stop sounds if objects go out of range.
+		// This fixes https://github.com/minetest/minetest/issues/8094.
+		// We may not remove sounds if an entity was removed on the server.
+		// See https://github.com/minetest/minetest/issues/14422.
+		if (!gone) // just out of range for client, not gone on server?
+			stopAttachedSounds(client->peer_id, id);
+
 		// Add to data buffer for sending
 		writeU16((u8*)buf, id);
 		data.append(buf, 2);
@@ -2278,19 +2287,30 @@ void Server::fadeSound(s32 handle, float step, float gain)
 		m_playing_sounds.erase(it);
 }
 
-void Server::stopAttachedSounds(u16 id)
+void Server::stopAttachedSounds(session_t peer_id, u16 object_id)
 {
-	assert(id);
+	assert(peer_id != PEER_ID_INEXISTENT);
+	assert(object_id);
 
-	for (auto it = m_playing_sounds.begin(); it != m_playing_sounds.end(); ) {
+	for (auto it = m_playing_sounds.begin(); it != m_playing_sounds.end();) {
-		const ServerPlayingSound &sound = it->second;
+		ServerPlayingSound &sound = it->second;
 
-		if (sound.object == id) {
+		if (sound.object != object_id)
-			// Remove sound reference
+			continue;
+
+		auto clients_it = sound.clients.find(peer_id);
+		if (clients_it == sound.clients.end())
+			continue;
+
+		NetworkPacket pkt(TOCLIENT_STOP_SOUND, 4);
+		pkt << it->first;
+		Send(peer_id, &pkt);
+
+		sound.clients.erase(clients_it);
+		if (sound.clients.empty())
 			it = m_playing_sounds.erase(it);
-		}
 		else
-			it++;
+			++it;
 	}
 }
 

src/server.h

@@ -239,7 +239,8 @@ public:
 	s32 playSound(ServerPlayingSound &params, bool ephemeral=false);
 	void stopSound(s32 handle);
 	void fadeSound(s32 handle, float step, float gain);
-	void stopAttachedSounds(u16 id);
+	// Stop all sounds attached to an object for a certain client
+	void stopAttachedSounds(session_t peer_id, u16 object_id);
 
 	// Envlock
 	std::set<std::string> getPlayerEffectivePrivs(const std::string &name);

src/serverenvironment.cpp

@@ -19,6 +19,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 
 #include <algorithm>
 #include <stack>
+#include <utility>
 #include "serverenvironment.h"
 #include "settings.h"
 #include "log.h"
@@ -1253,7 +1254,7 @@ void ServerEnvironment::clearObjects(ClearObjectsMode mode)
 			return false;
 		}
 
-		processActiveObjectRemove(obj, id);
+		processActiveObjectRemove(obj);
 
 		// Delete active object
 		return true;
@@ -1742,7 +1743,7 @@ void ServerEnvironment::getAddedActiveObjects(PlayerSAO *playersao, s16 radius,
 void ServerEnvironment::getRemovedActiveObjects(PlayerSAO *playersao, s16 radius,
 	s16 player_radius,
 	std::set<u16> &current_objects,
-	std::queue<u16> &removed_objects)
+	std::queue<std::pair<bool /* gone? */, u16>> &removed_objects)
 {
 	f32 radius_f = radius * BS;
 	f32 player_radius_f = player_radius * BS;
@@ -1763,12 +1764,12 @@ void ServerEnvironment::getRemovedActiveObjects(PlayerSAO *playersao, s16 radius
 		if (object == NULL) {
 			infostream << "ServerEnvironment::getRemovedActiveObjects():"
 				<< " object in current_objects is NULL" << std::endl;
-			removed_objects.push(id);
+			removed_objects.emplace(true, id);
 			continue;
 		}
 
 		if (object->isGone()) {
-			removed_objects.push(id);
+			removed_objects.emplace(true, id);
 			continue;
 		}
 
@@ -1780,7 +1781,7 @@ void ServerEnvironment::getRemovedActiveObjects(PlayerSAO *playersao, s16 radius
 			continue;
 
 		// Object is no longer visible
-		removed_objects.push(id);
+		removed_objects.emplace(false, id);
 	}
 }
 
@@ -1973,7 +1974,7 @@ void ServerEnvironment::removeRemovedObjects()
 			}
 		}
 
-		processActiveObjectRemove(obj, id);
+		processActiveObjectRemove(obj);
 
 		// Delete
 		return true;
@@ -2210,7 +2211,7 @@ void ServerEnvironment::deactivateFarObjects(bool _force_delete)
 			return false;
 		}
 
-		processActiveObjectRemove(obj, id);
+		processActiveObjectRemove(obj);
 
 		// Delete active object
 		return true;
@@ -2273,14 +2274,12 @@ bool ServerEnvironment::saveStaticToBlock(
 	return true;
 }
 
-void ServerEnvironment::processActiveObjectRemove(ServerActiveObject *obj, u16 id)
+void ServerEnvironment::processActiveObjectRemove(ServerActiveObject *obj)
 {
 	// Tell the object about removal
 	obj->removingFromEnvironment();
 	// Deregister in scripting api
 	m_script->removeObjectReference(obj);
-	// stop attached sounds
-	m_server->stopAttachedSounds(id);
 }
 
 PlayerDatabase *ServerEnvironment::openPlayerDatabase(const std::string &name,

src/serverenvironment.h

@@ -19,6 +19,10 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 
 #pragma once
 
+#include <set>
+#include <random>
+#include <utility>
+
 #include "activeobject.h"
 #include "environment.h"
 #include "map.h"
@@ -26,8 +30,6 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "server/activeobjectmgr.h"
 #include "util/numeric.h"
 #include "util/metricsbackend.h"
-#include <set>
-#include <random>
 
 class IGameDef;
 struct GameParams;
@@ -303,7 +305,7 @@ public:
 	void getRemovedActiveObjects(PlayerSAO *playersao, s16 radius,
 		s16 player_radius,
 		std::set<u16> &current_objects,
-		std::queue<u16> &removed_objects);
+		std::queue<std::pair<bool /* gone? */, u16>> &removed_objects);
 
 	/*
 		Get the next message emitted by some active object.
@@ -454,7 +456,7 @@ private:
 	bool saveStaticToBlock(v3s16 blockpos, u16 store_id,
 			ServerActiveObject *obj, const StaticObject &s_obj, u32 mod_reason);
 
-	void processActiveObjectRemove(ServerActiveObject *obj, u16 id);
+	void processActiveObjectRemove(ServerActiveObject *obj);
 
 	/*
 		Member variables