From bf52d1e6242607440bfa35d6e81e5909e0c46977 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20M=C3=BCller?= <34514239+appgurueu@users.noreply.github.com> Date: Wed, 6 Mar 2024 20:36:02 +0100 Subject: [PATCH] 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 Co-authored-by: grorp --- doc/lua_api.md | 16 ++++++++++++ src/client/client.cpp | 8 ------ src/client/client.h | 3 --- src/network/clientpackethandler.cpp | 1 - src/server.cpp | 40 +++++++++++++++++++++-------- src/server.h | 3 ++- src/serverenvironment.cpp | 19 +++++++------- src/serverenvironment.h | 10 +++++--- 8 files changed, 63 insertions(+), 37 deletions(-) diff --git a/doc/lua_api.md b/doc/lua_api.md index 0b2c703f3..f01e3cd35 100644 --- a/doc/lua_api.md +++ b/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`. diff --git a/src/client/client.cpp b/src/client/client.cpp index 914b9ce35..d6781a965 100644 --- a/src/client/client.cpp +++ b/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); - } -} diff --git a/src/client/client.h b/src/client/client.h index ae003c011..9f898e78a 100644 --- a/src/client/client.h +++ b/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; diff --git a/src/network/clientpackethandler.cpp b/src/network/clientpackethandler.cpp index f62730d43..10ecdd34c 100644 --- a/src/network/clientpackethandler.cpp +++ b/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 diff --git a/src/server.cpp b/src/server.cpp index 5e828e00c..eebd3ba56 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -2041,7 +2041,8 @@ void Server::SendActiveObjectRemoveAdd(RemoteClient *client, PlayerSAO *playersa if (my_radius <= 0) my_radius = radius; - std::queue removed_objects, added_objects; + std::queue> removed_objects; + std::queue 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(); ) { - const ServerPlayingSound &sound = it->second; + for (auto it = m_playing_sounds.begin(); it != m_playing_sounds.end();) { + ServerPlayingSound &sound = it->second; - if (sound.object == id) { - // Remove sound reference + if (sound.object != object_id) + 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; } } diff --git a/src/server.h b/src/server.h index 1c0905935..9f9f08ac7 100644 --- a/src/server.h +++ b/src/server.h @@ -239,7 +239,8 @@ public: s32 playSound(ServerPlayingSound ¶ms, 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 getPlayerEffectivePrivs(const std::string &name); diff --git a/src/serverenvironment.cpp b/src/serverenvironment.cpp index f3fe2c731..6d394309c 100644 --- a/src/serverenvironment.cpp +++ b/src/serverenvironment.cpp @@ -19,6 +19,7 @@ with this program; if not, write to the Free Software Foundation, Inc., #include #include +#include #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 ¤t_objects, - std::queue &removed_objects) + std::queue> &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, diff --git a/src/serverenvironment.h b/src/serverenvironment.h index 01f232c8c..4a4872a8b 100644 --- a/src/serverenvironment.h +++ b/src/serverenvironment.h @@ -19,6 +19,10 @@ with this program; if not, write to the Free Software Foundation, Inc., #pragma once +#include +#include +#include + #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 -#include class IGameDef; struct GameParams; @@ -303,7 +305,7 @@ public: void getRemovedActiveObjects(PlayerSAO *playersao, s16 radius, s16 player_radius, std::set ¤t_objects, - std::queue &removed_objects); + std::queue> &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