Fixed SyncGroup bug and adds UnitTest that verifies the implementation.

Author: AndreaCatania

core/core.cpp

@@ -32,14 +32,14 @@ const char *get_process_phase_name(ProcessPhase pp) {
 
 std::string get_log_level_txt(NS::PrintMessageType p_level) {
 	std::string log_level_str = "";
-	if (NS::PrintMessageType::INFO == p_level) {
+	if (NS::PrintMessageType::VERBOSE == p_level) {
+		log_level_str = "[VERBOSE] ";
+	} else if (NS::PrintMessageType::INFO == p_level) {
 		log_level_str = "[INFO] ";
 	} else if (NS::PrintMessageType::WARNING == p_level) {
 		log_level_str = "[WARNING] ";
 	} else if (NS::PrintMessageType::ERROR == p_level) {
 		log_level_str = "[ERROR] ";
-	} else if (NS::PrintMessageType::__INTERNAL == p_level) {
-		log_level_str = "";
 	}
 	return log_level_str;
 }

core/core.h

@@ -71,7 +71,7 @@ const char *get_process_phase_name(ProcessPhase pp);
 NS_NAMESPACE_BEGIN
 
 enum PrintMessageType : std::uint8_t {
-	__INTERNAL = 0,
+	VERBOSE = 0,
 	INFO = 1,
 	WARNING = 2,
 	ERROR = 3,

godot4/gd_scene_synchronizer.cpp

@@ -649,15 +649,15 @@ const NS::SyncGroup *GdSceneSynchronizer::sync_group_get(uint32_t p_group_id) co
 }
 
 void GdSceneSynchronizer::sync_group_add_node_by_id(uint32_t p_net_id, uint32_t p_group_id, bool p_realtime) {
-	scene_synchronizer.sync_group_add_object_by_id({ p_net_id }, NS::SyncGroupId{ p_group_id }, p_realtime);
+	scene_synchronizer.sync_group_add_object(NS::ObjectNetId{ p_net_id }, NS::SyncGroupId{ p_group_id }, p_realtime);
 }
 
 void GdSceneSynchronizer::sync_group_add_node(NS::ObjectData *p_object_data, uint32_t p_group_id, bool p_realtime) {
 	scene_synchronizer.sync_group_add_object(p_object_data, NS::SyncGroupId{ p_group_id }, p_realtime);
 }
 
 void GdSceneSynchronizer::sync_group_remove_node_by_id(uint32_t p_net_id, uint32_t p_group_id) {
-	scene_synchronizer.sync_group_remove_object_by_id({ p_net_id }, NS::SyncGroupId{ p_group_id });
+	scene_synchronizer.sync_group_remove_object(NS::ObjectNetId{ p_net_id }, NS::SyncGroupId{ p_group_id });
 }
 
 void GdSceneSynchronizer::sync_group_remove_node(NS::ObjectData *p_object_data, uint32_t p_group_id) {

net_utilities.cpp

@@ -63,33 +63,12 @@ void NS::SyncGroup::mark_changes_as_notified() {
 
 void NS::SyncGroup::add_listening_peer(int p_peer) {
 	NS::VecFunc::insert_unique(listening_peers, p_peer);
-	if (NS::VecFunc::insert_unique(networked_peers, p_peer)) {
-		NS::VecFunc::insert_unique(peers_with_newly_calculated_latency, p_peer);
-	}
-
-	// Make all the controlled objects as simulated.
-	const std::vector<ObjectData *> *controlled_objects_ptr = scene_sync->get_peer_controlled_objects_data(p_peer);
-	if (controlled_objects_ptr) {
-		for (ObjectData *od : (*controlled_objects_ptr)) {
-			add_new_sync_object(od, true);
-		}
-	}
-
-	notify_controllers_about_simulating_peer(p_peer, true);
+	notify_simulating_peers_about_listener_status(p_peer, true);
 }
 
 void NS::SyncGroup::remove_listening_peer(int p_peer) {
 	NS::VecFunc::remove_unordered(listening_peers, p_peer);
-
-	// Remove all the controlled objects.
-	const std::vector<ObjectData *> *controlled_objects_ptr = scene_sync->get_peer_controlled_objects_data(p_peer);
-	if (controlled_objects_ptr) {
-		for (ObjectData *od : (*controlled_objects_ptr)) {
-			remove_sync_object(*od);
-		}
-	}
-
-	notify_controllers_about_simulating_peer(p_peer, false);
+	notify_simulating_peers_about_listener_status(p_peer, false);
 }
 
 uint32_t NS::SyncGroup::add_new_sync_object(ObjectData *p_object_data, bool p_is_simulated) {
@@ -100,13 +79,6 @@ uint32_t NS::SyncGroup::add_new_sync_object(ObjectData *p_object_data, bool p_is
 		if (NS::VecFunc::insert_unique(networked_peers, peer)) {
 			NS::VecFunc::insert_unique(peers_with_newly_calculated_latency, peer);
 		}
-
-		const bool is_listener_controlled = NS::VecFunc::has(listening_peers, peer);
-		if (is_listener_controlled && !p_is_simulated) {
-			// This object is controlled by a listener, so it's always simulated.
-			p_is_simulated = true;
-			SceneSynchronizerDebugger::singleton()->print(NS::WARNING, "The object `" + p_object_data->object_name + "` is being controlled by a peer listening to this sync group, so it can't be made non simulating.");
-		}
 	}
 
 	if (p_is_simulated) {
@@ -133,7 +105,8 @@ uint32_t NS::SyncGroup::add_new_sync_object(ObjectData *p_object_data, bool p_is
 			}
 
 			if (p_object_data->get_controlled_by_peer() > 0) {
-				notify_controller_about_simulating_peers(p_object_data->get_controlled_by_peer(), true);
+				VecFunc::insert_unique(simulating_peers, p_object_data->get_controlled_by_peer());
+				update_listeners_to_simulating_peer(p_object_data->get_controlled_by_peer(), true);
 			}
 		}
 
@@ -172,9 +145,6 @@ void NS::SyncGroup::remove_sync_object(std::size_t p_index, bool p_is_simulated)
 		}
 	}
 
-	const bool is_listener_controlled = NS::VecFunc::has(listening_peers, associted_peer);
-	ENSURE_MSG(!is_listener_controlled, "It's not possible to remove an object controlled by a listening peer from sync group.");
-
 	if (p_is_simulated) {
 		simulated_sync_objects.remove_at_unordered(p_index);
 		simulated_sync_objects_list_changed = true;
@@ -212,8 +182,9 @@ void NS::SyncGroup::remove_sync_object(std::size_t p_index, bool p_is_simulated)
 		}
 
 		if (!is_simulating) {
-			// No other objects associated to this peer are simulated, so notify the peer as non simulating.
-			notify_controller_about_simulating_peers(associted_peer, false);
+			// No other objects associated to this peer are simulated, remove from simulating peers.
+			VecFunc::remove_unordered(simulating_peers, associted_peer);
+			update_listeners_to_simulating_peer(associted_peer, false);
 		}
 
 		if (!is_networking) {
@@ -353,20 +324,17 @@ void NS::SyncGroup::notify_peer_has_newly_calculated_latency(int p_peer) {
 	}
 }
 
-void NS::SyncGroup::notify_controller_about_simulating_peers(int p_peer_controller, bool p_simulating) {
-	NS::NetworkedControllerBase *controller = scene_sync->get_controller_for_peer(p_peer_controller);
-	if (controller) {
-		// This is a controller, notify about simulating peers.
-		for (int peer : listening_peers) {
-			controller->server_set_peer_simulating_this_controller(peer, p_simulating);
-		}
+void NS::SyncGroup::notify_simulating_peers_about_listener_status(int p_peer_listener, bool p_simulating) {
+	for (int peer : simulating_peers) {
+		NetworkedControllerBase *controller = scene_sync->get_controller_for_peer(peer);
+		controller->server_set_peer_simulating_this_controller(p_peer_listener, p_simulating);
 	}
 }
 
-void NS::SyncGroup::notify_controllers_about_simulating_peer(int p_peer, bool p_simulating) {
-	for (const int listening_peer : listening_peers) {
-		NetworkedControllerBase *controller = scene_sync->get_controller_for_peer(listening_peer, false);
-		controller->server_set_peer_simulating_this_controller(p_peer, p_simulating);
+void NS::SyncGroup::update_listeners_to_simulating_peer(int p_simulating_peer, bool p_simulating) {
+	NetworkedControllerBase *controller = scene_sync->get_controller_for_peer(p_simulating_peer);
+	for (int peer : listening_peers) {
+		controller->server_set_peer_simulating_this_controller(peer, p_simulating);
 	}
 }
 

net_utilities.h

@@ -6,6 +6,7 @@
 #include "core/core.h"
 #include "core/processor.h"
 #include "core/var_data.h"
+#include "modules/network_synchronizer/core/core.h"
 #include <chrono>
 #include <map>
 #include <memory>
@@ -523,6 +524,7 @@ struct SyncGroup {
 	};
 
 public:
+	SyncGroupId group_id = SyncGroupId::NONE;
 	SceneSynchronizerBase *scene_sync = nullptr;
 
 private:
@@ -533,7 +535,13 @@ struct SyncGroup {
 	LocalVector<TrickledObjectInfo> trickled_sync_objects;
 
 	/// Contains the list of peers being networked by this sync group.
+	/// In other terms, if an object controller by peer 76 is contained into
+	/// the simulated or trickled list that peer is into this array.
 	std::vector<int> networked_peers;
+	/// The list of peers that have at least one objects into the simulated
+	/// vector.
+	std::vector<int> simulating_peers;
+	/// The peers for which the server measured the latency.
 	std::vector<int> peers_with_newly_calculated_latency;
 
 	std::vector<int> listening_peers;
@@ -558,6 +566,8 @@ struct SyncGroup {
 	void remove_listening_peer(int p_peer);
 	const std::vector<int> &get_listening_peers() const { return listening_peers; };
 
+	const std::vector<int> &get_simulating_peers() const { return simulating_peers; }
+
 	/// Returns the `index` or `UINT32_MAX` on error.
 	uint32_t add_new_sync_object(struct ObjectData *p_object_data, bool p_is_simulated);
 	void remove_sync_object(std::size_t p_index, bool p_is_simulated);
@@ -575,10 +585,13 @@ struct SyncGroup {
 
 	void notify_peer_has_newly_calculated_latency(int p_peer);
 
-private:
-	void notify_controller_about_simulating_peers(int p_peer, bool p_simulating);
-	void notify_controllers_about_simulating_peer(int p_peer, bool p_simulating);
+	// Used when a new listener is added or removed.
+	void notify_simulating_peers_about_listener_status(int p_peer_listener, bool p_simulating);
+	// Used when a new simulated object (which is controlled) is added, to notify
+	// such controller about the listeners.
+	void update_listeners_to_simulating_peer(int p_simulating_peer, bool p_simulating);
 
+private:
 	std::size_t find_simulated(const struct ObjectData &p_object_data) const;
 	std::size_t find_trickled(const struct ObjectData &p_object_data) const;
 };

scene_synchronizer.cpp

@@ -686,8 +686,13 @@ const NS::SyncGroup *SceneSynchronizerBase::sync_group_get(SyncGroupId p_group_i
 	return static_cast<ServerSynchronizer *>(synchronizer)->sync_group_get(p_group_id);
 }
 
-void SceneSynchronizerBase::sync_group_add_object_by_id(ObjectNetId p_node_id, SyncGroupId p_group_id, bool p_realtime) {
-	NS::ObjectData *nd = get_object_data(p_node_id);
+void SceneSynchronizerBase::sync_group_add_object(ObjectLocalId p_id, SyncGroupId p_group_id, bool p_realtime) {
+	NS::ObjectData *nd = get_object_data(p_id);
+	sync_group_add_object(nd, p_group_id, p_realtime);
+}
+
+void SceneSynchronizerBase::sync_group_add_object(ObjectNetId p_id, SyncGroupId p_group_id, bool p_realtime) {
+	NS::ObjectData *nd = get_object_data(p_id);
 	sync_group_add_object(nd, p_group_id, p_realtime);
 }
 
@@ -696,8 +701,13 @@ void SceneSynchronizerBase::sync_group_add_object(NS::ObjectData *p_object_data,
 	static_cast<ServerSynchronizer *>(synchronizer)->sync_group_add_object(p_object_data, p_group_id, p_realtime);
 }
 
-void SceneSynchronizerBase::sync_group_remove_object_by_id(ObjectNetId p_node_id, SyncGroupId p_group_id) {
-	NS::ObjectData *nd = get_object_data(p_node_id);
+void SceneSynchronizerBase::sync_group_remove_object(ObjectLocalId p_id, SyncGroupId p_group_id) {
+	NS::ObjectData *nd = get_object_data(p_id);
+	sync_group_remove_object(nd, p_group_id);
+}
+
+void SceneSynchronizerBase::sync_group_remove_object(ObjectNetId p_id, SyncGroupId p_group_id) {
+	NS::ObjectData *nd = get_object_data(p_id);
 	sync_group_remove_object(nd, p_group_id);
 }
 
@@ -706,6 +716,21 @@ void SceneSynchronizerBase::sync_group_remove_object(NS::ObjectData *p_object_da
 	static_cast<ServerSynchronizer *>(synchronizer)->sync_group_remove_object(p_object_data, p_group_id);
 }
 
+void SceneSynchronizerBase::sync_group_fetch_object_grups(NS::ObjectLocalId p_id, std::vector<SyncGroupId> &r_simulated_groups, std::vector<SyncGroupId> &r_trickled_groups) const {
+	const NS::ObjectData *object_data = get_object_data(p_id);
+	sync_group_fetch_object_grups(object_data, r_simulated_groups, r_trickled_groups);
+}
+
+void SceneSynchronizerBase::sync_group_fetch_object_grups(NS::ObjectNetId p_id, std::vector<SyncGroupId> &r_simulated_groups, std::vector<SyncGroupId> &r_trickled_groups) const {
+	const NS::ObjectData *object_data = get_object_data(p_id);
+	sync_group_fetch_object_grups(object_data, r_simulated_groups, r_trickled_groups);
+}
+
+void SceneSynchronizerBase::sync_group_fetch_object_grups(const NS::ObjectData *p_object_data, std::vector<SyncGroupId> &r_simulated_groups, std::vector<SyncGroupId> &r_trickled_groups) const {
+	ENSURE_MSG(is_server(), "This function CAN be used only on the server.");
+	static_cast<ServerSynchronizer *>(synchronizer)->sync_group_fetch_object_grups(p_object_data, r_simulated_groups, r_trickled_groups);
+}
+
 void SceneSynchronizerBase::sync_group_replace_objects(SyncGroupId p_group_id, LocalVector<NS::SyncGroup::SimulatedObjectInfo> &&p_new_realtime_nodes, LocalVector<NS::SyncGroup::TrickledObjectInfo> &&p_new_trickled_nodes) {
 	ENSURE_MSG(is_server(), "This function CAN be used only on the server.");
 	static_cast<ServerSynchronizer *>(synchronizer)->sync_group_replace_object(p_group_id, std::move(p_new_realtime_nodes), std::move(p_new_trickled_nodes));
@@ -748,6 +773,11 @@ const std::vector<int> *SceneSynchronizerBase::sync_group_get_listening_peers(Sy
 	return static_cast<ServerSynchronizer *>(synchronizer)->sync_group_get_listening_peers(p_group_id);
 }
 
+const std::vector<int> *SceneSynchronizerBase::sync_group_get_simulating_peers(SyncGroupId p_group_id) const {
+	ENSURE_V_MSG(is_server(), nullptr, "This function CAN be used only on the server.");
+	return static_cast<ServerSynchronizer *>(synchronizer)->sync_group_get_simulating_peers(p_group_id);
+}
+
 void SceneSynchronizerBase::sync_group_set_trickled_update_rate(ObjectLocalId p_node_id, SyncGroupId p_group_id, float p_update_rate) {
 	NS::ObjectData *od = get_object_data(p_node_id);
 	ENSURE_MSG(is_server(), "This function CAN be used only on the server.");
@@ -1769,6 +1799,7 @@ SyncGroupId ServerSynchronizer::sync_group_create() {
 	SyncGroupId id;
 	id.id = sync_groups.size();
 	sync_groups.resize(id.id + 1);
+	sync_groups[id.id].group_id = id;
 	sync_groups[id.id].scene_sync = scene_synchronizer;
 	return id;
 }
@@ -1779,19 +1810,39 @@ const NS::SyncGroup *ServerSynchronizer::sync_group_get(SyncGroupId p_group_id)
 }
 
 void ServerSynchronizer::sync_group_add_object(NS::ObjectData *p_object_data, SyncGroupId p_group_id, bool p_realtime) {
-	ERR_FAIL_COND(p_object_data == nullptr);
+	ENSURE(p_object_data);
 	ERR_FAIL_COND_MSG(p_group_id.id >= sync_groups.size(), "The group id `" + itos(p_group_id.id) + "` doesn't exist.");
 	ERR_FAIL_COND_MSG(p_group_id == SyncGroupId::GLOBAL, "You can't change this SyncGroup in any way. Create a new one.");
 	sync_groups[p_group_id.id].add_new_sync_object(p_object_data, p_realtime);
 }
 
 void ServerSynchronizer::sync_group_remove_object(NS::ObjectData *p_object_data, SyncGroupId p_group_id) {
-	ERR_FAIL_COND(p_object_data == nullptr);
 	ERR_FAIL_COND_MSG(p_group_id.id >= sync_groups.size(), "The group id `" + itos(p_group_id.id) + "` doesn't exist.");
+	ENSURE(p_object_data);
 	ERR_FAIL_COND_MSG(p_group_id == SyncGroupId::GLOBAL, "You can't change this SyncGroup in any way. Create a new one.");
 	sync_groups[p_group_id.id].remove_sync_object(*p_object_data);
 }
 
+void ServerSynchronizer::sync_group_fetch_object_grups(const ObjectData *p_object_data, std::vector<SyncGroupId> &r_simulated_groups, std::vector<SyncGroupId> &r_trickled_groups) const {
+	ENSURE(p_object_data);
+
+	r_simulated_groups.clear();
+	r_trickled_groups.clear();
+
+	SyncGroupId id = { 0 };
+	for (const SyncGroup &group : sync_groups) {
+		if (group.get_simulated_sync_objects().find(SyncGroup::SimulatedObjectInfo(const_cast<ObjectData *>(p_object_data))) != -1) {
+			r_simulated_groups.push_back(id);
+		}
+
+		if (group.get_trickled_sync_objects().find(SyncGroup::TrickledObjectInfo(const_cast<ObjectData *>(p_object_data))) != -1) {
+			r_trickled_groups.push_back(id);
+		}
+
+		id += 1;
+	}
+}
+
 void ServerSynchronizer::sync_group_replace_object(SyncGroupId p_group_id, LocalVector<NS::SyncGroup::SimulatedObjectInfo> &&p_new_realtime_nodes, LocalVector<NS::SyncGroup::TrickledObjectInfo> &&p_new_trickled_nodes) {
 	ERR_FAIL_COND_MSG(p_group_id.id >= sync_groups.size(), "The group id `" + itos(p_group_id.id) + "` doesn't exist.");
 	ERR_FAIL_COND_MSG(p_group_id == SyncGroupId::GLOBAL, "You can't change this SyncGroup in any way. Create a new one.");
@@ -1834,10 +1885,15 @@ void ServerSynchronizer::sync_group_update(int p_peer_id) {
 }
 
 const std::vector<int> *ServerSynchronizer::sync_group_get_listening_peers(SyncGroupId p_group_id) const {
-	ERR_FAIL_COND_V_MSG(p_group_id.id >= sync_groups.size(), nullptr, "The group id `" + itos(p_group_id.id) + "` doesn't exist.");
+	ENSURE_V_MSG(p_group_id.id < sync_groups.size(), nullptr, "The group id `" + std::string(p_group_id) + "` doesn't exist.");
 	return &sync_groups[p_group_id.id].get_listening_peers();
 }
 
+const std::vector<int> *ServerSynchronizer::sync_group_get_simulating_peers(SyncGroupId p_group_id) const {
+	ENSURE_V_MSG(p_group_id.id < sync_groups.size(), nullptr, "The group id `" + std::string(p_group_id) + "` doesn't exist.");
+	return &sync_groups[p_group_id.id].get_simulating_peers();
+}
+
 void ServerSynchronizer::set_peer_networking_enable(int p_peer, bool p_enable) {
 	PeerData *pd = MapFunc::get_or_null(scene_synchronizer->peer_data, p_peer);
 	ENSURE(pd);
@@ -2622,7 +2678,7 @@ void ClientSynchronizer::process_received_server_state() {
 	if (need_rewind) {
 		SceneSynchronizerDebugger::singleton()->notify_event(SceneSynchronizerDebugger::FrameEvent::CLIENT_DESYNC_DETECTED);
 		SceneSynchronizerDebugger::singleton()->print(
-				__INTERNAL,
+				VERBOSE,
 				std::string("Recover input: ") + std::string(last_checked_input) + " - Last input: " + std::string(inner_player_controller->get_stored_frame_index(-1)),
 				scene_synchronizer->get_network_interface().get_owner_name());