Improve ServerEnvironment::getRemovedActiveObjects() in many ways

src/server.cpp

@@ -2047,91 +2047,72 @@ void Server::SendActiveObjectRemoveAdd(RemoteClient *client, PlayerSAO *playersa
 	if (my_radius <= 0)
 		my_radius = radius;
 
-	std::queue<std::pair<bool, u16>> removed_objects;
+	std::vector<std::pair<bool, u16>> removed_objects;
-	std::queue<u16> added_objects;
+	std::vector<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,
 		client->m_known_objects, added_objects);
 
-	int removed_count = removed_objects.size();
-	int added_count   = added_objects.size();
-
 	if (removed_objects.empty() && added_objects.empty())
 		return;
 
-	char buf[4];
+	NetworkPacket pkt(TOCLIENT_ACTIVE_OBJECT_REMOVE_ADD,
-	std::string data;
+		2 * removed_objects.size() + 32 * added_objects.size(), client->peer_id);
 
-	// Handle removed objects
+	// Removed objects
+	pkt << static_cast<u16>(removed_objects.size());
 
-	writeU16((u8*)buf, removed_objects.size());
+	std::vector<u16> sounds_to_stop;
-	data.append(buf, 2);
+
-	while (!removed_objects.empty()) {
+	for (auto &it : removed_objects) {
-		// Get object
+		const auto [gone, id] = it;
-		const auto [gone, id] = removed_objects.front();
+		ServerActiveObject *obj = m_env->getActiveObject(id);
-		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);
+			sounds_to_stop.push_back(id);
 
-		// Add to data buffer for sending
+		pkt << id;
-		writeU16((u8*)buf, id);
-		data.append(buf, 2);
 
 		// Remove from known objects
 		client->m_known_objects.erase(id);
-
 		if (obj && obj->m_known_by_count > 0)
 			obj->m_known_by_count--;
-
-		removed_objects.pop();
 	}
 
-	// Handle added objects
+	if (!sounds_to_stop.empty())
-	writeU16((u8*)buf, added_objects.size());
+		stopAttachedSounds(client->peer_id, sounds_to_stop);
-	data.append(buf, 2);
-	while (!added_objects.empty()) {
-		// Get object
-		u16 id = added_objects.front();
-		ServerActiveObject *obj = m_env->getActiveObject(id);
-		added_objects.pop();
 
+	// Added objects
+	pkt << static_cast<u16>(added_objects.size());
+
+	for (u16 id : added_objects) {
+		ServerActiveObject *obj = m_env->getActiveObject(id);
 		if (!obj) {
-			warningstream << FUNCTION_NAME << ": NULL object id="
+			warningstream << FUNCTION_NAME << ": found NULL object id="
 				<< (int)id << std::endl;
 			continue;
 		}
 
-		// Get object type
 		u8 type = obj->getSendType();
 
-		// Add to data buffer for sending
+		pkt << id << type;
-		writeU16((u8*)buf, id);
+		pkt.putLongString(obj->getClientInitializationData(client->net_proto_version));
-		data.append(buf, 2);
-		writeU8((u8*)buf, type);
-		data.append(buf, 1);
-
-		data.append(serializeString32(
-			obj->getClientInitializationData(client->net_proto_version)));
 
 		// Add to known objects
 		client->m_known_objects.insert(id);
-
 		obj->m_known_by_count++;
 	}
 
-	NetworkPacket pkt(TOCLIENT_ACTIVE_OBJECT_REMOVE_ADD, data.size(), client->peer_id);
-	pkt.putRawString(data.c_str(), data.size());
 	Send(&pkt);
 
-	verbosestream << "Server::SendActiveObjectRemoveAdd: "
+	verbosestream << "Server::SendActiveObjectRemoveAdd(): "
-		<< removed_count << " removed, " << added_count << " added, "
+		<< removed_objects.size() << " removed, " << added_objects.size()
-		<< "packet size is " << pkt.getSize() << std::endl;
+		<< " added, packet size is " << pkt.getSize() << std::endl;
 }
 
 void Server::SendActiveObjectMessages(session_t peer_id, const std::string &datas,
@@ -2288,13 +2269,14 @@ void Server::fadeSound(s32 handle, float step, float gain)
 		m_playing_sounds.erase(it);
 }
 
-void Server::stopAttachedSounds(session_t peer_id, u16 object_id)
+void Server::stopAttachedSounds(session_t peer_id,
+	const std::vector<u16> &object_ids)
 {
 	assert(peer_id != PEER_ID_INEXISTENT);
-	assert(object_id);
+	assert(!object_ids.empty());
 
 	auto cb = [&] (const s32 id, ServerPlayingSound &sound) -> bool {
-		if (sound.object != object_id)
+		if (!CONTAINS(object_ids, sound.object))
 			return false;
 
 		auto clients_it = sound.clients.find(peer_id);

src/server.h

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

src/server/activeobjectmgr.cpp

@@ -153,9 +153,9 @@ void ActiveObjectMgr::getObjectsInArea(const aabb3f &box,
 	}
 }
 
-void ActiveObjectMgr::getAddedActiveObjectsAroundPos(const v3f &player_pos, f32 radius,
+void ActiveObjectMgr::getAddedActiveObjectsAroundPos(v3f player_pos, f32 radius,
-		f32 player_radius, std::set<u16> &current_objects,
+		f32 player_radius, const std::set<u16> &current_objects,
-		std::queue<u16> &added_objects)
+		std::vector<u16> &added_objects)
 {
 	/*
 		Go through the object list,
@@ -188,7 +188,7 @@ void ActiveObjectMgr::getAddedActiveObjectsAroundPos(const v3f &player_pos, f32
 		if (n != current_objects.end())
 			continue;
 		// Add to added_objects
-		added_objects.push(id);
+		added_objects.push_back(id);
 	}
 }
 

src/server/activeobjectmgr.h

@@ -45,8 +45,8 @@ public:
 			std::vector<ServerActiveObject *> &result,
 			std::function<bool(ServerActiveObject *obj)> include_obj_cb);
 
-	void getAddedActiveObjectsAroundPos(const v3f &player_pos, f32 radius,
+	void getAddedActiveObjectsAroundPos(v3f player_pos, f32 radius,
-			f32 player_radius, std::set<u16> &current_objects,
+			f32 player_radius, const std::set<u16> &current_objects,
-			std::queue<u16> &added_objects);
+			std::vector<u16> &added_objects);
 };
 } // namespace server

src/serverenvironment.cpp

@@ -1727,8 +1727,8 @@ u16 ServerEnvironment::addActiveObject(std::unique_ptr<ServerActiveObject> objec
 */
 void ServerEnvironment::getAddedActiveObjects(PlayerSAO *playersao, s16 radius,
 	s16 player_radius,
-	std::set<u16> &current_objects,
+	const std::set<u16> &current_objects,
-	std::queue<u16> &added_objects)
+	std::vector<u16> &added_objects)
 {
 	f32 radius_f = radius * BS;
 	f32 player_radius_f = player_radius * BS;
@@ -1746,8 +1746,8 @@ void ServerEnvironment::getAddedActiveObjects(PlayerSAO *playersao, s16 radius,
 */
 void ServerEnvironment::getRemovedActiveObjects(PlayerSAO *playersao, s16 radius,
 	s16 player_radius,
-	std::set<u16> &current_objects,
+	const std::set<u16> &current_objects,
-	std::queue<std::pair<bool /* gone? */, u16>> &removed_objects)
+	std::vector<std::pair<bool /* gone? */, u16>> &removed_objects)
 {
 	f32 radius_f = radius * BS;
 	f32 player_radius_f = player_radius * BS;
@@ -1765,15 +1765,15 @@ void ServerEnvironment::getRemovedActiveObjects(PlayerSAO *playersao, s16 radius
 	for (u16 id : current_objects) {
 		ServerActiveObject *object = getActiveObject(id);
 
-		if (object == NULL) {
+		if (!object) {
-			infostream << "ServerEnvironment::getRemovedActiveObjects():"
+			warningstream << FUNCTION_NAME << ": found NULL object id="
-				<< " object in current_objects is NULL" << std::endl;
+				<< (int)id << std::endl;
-			removed_objects.emplace(true, id);
+			removed_objects.emplace_back(true, id);
 			continue;
 		}
 
 		if (object->isGone()) {
-			removed_objects.emplace(true, id);
+			removed_objects.emplace_back(true, id);
 			continue;
 		}
 
@@ -1785,7 +1785,7 @@ void ServerEnvironment::getRemovedActiveObjects(PlayerSAO *playersao, s16 radius
 			continue;
 
 		// Object is no longer visible
-		removed_objects.emplace(false, id);
+		removed_objects.emplace_back(false, id);
 	}
 }
 

src/serverenvironment.h

@@ -286,8 +286,8 @@ public:
 	*/
 	void getAddedActiveObjects(PlayerSAO *playersao, s16 radius,
 		s16 player_radius,
-		std::set<u16> &current_objects,
+		const std::set<u16> &current_objects,
-		std::queue<u16> &added_objects);
+		std::vector<u16> &added_objects);
 
 	/*
 		Find out what new objects have been removed from
@@ -295,8 +295,8 @@ public:
 	*/
 	void getRemovedActiveObjects(PlayerSAO *playersao, s16 radius,
 		s16 player_radius,
-		std::set<u16> &current_objects,
+		const std::set<u16> &current_objects,
-		std::queue<std::pair<bool /* gone? */, u16>> &removed_objects);
+		std::vector<std::pair<bool /* gone? */, u16>> &removed_objects);
 
 	/*
 		Get the next message emitted by some active object.

src/unittest/test_serveractiveobjectmgr.cpp

@@ -173,12 +173,12 @@ void TestServerActiveObjectMgr::testGetAddedActiveObjectsAroundPos()
 		saomgr.registerObject(std::make_unique<MockServerActiveObject>(nullptr, p));
 	}
 
-	std::queue<u16> result;
+	std::vector<u16> result;
 	std::set<u16> cur_objects;
 	saomgr.getAddedActiveObjectsAroundPos(v3f(), 100, 50, cur_objects, result);
 	UASSERTCMP(int, ==, result.size(), 1);
 
-	result = std::queue<u16>();
+	result.clear();
 	cur_objects.clear();
 	saomgr.getAddedActiveObjectsAroundPos(v3f(), 740, 50, cur_objects, result);
 	UASSERTCMP(int, ==, result.size(), 2);