[MP] Fix nested spawning during "ready".

We want our spawns to be notified after ready, but we need to notify
them in the order they entered tree, so that nested spawners can be used
during "ready" (instead of having to await a frame).

4 files changed, 50 insertions, 15 deletions

diff --git a/modules/multiplayer/multiplayer_spawner.cpp b/modules/multiplayer/multiplayer_spawner.cpp
index 7ed69a84d0..0aa54b69f9 100644
--- a/modules/multiplayer/multiplayer_spawner.cpp
+++ b/modules/multiplayer/multiplayer_spawner.cpp
@@ -199,10 +199,6 @@ void MultiplayerSpawner::_notification(int p_what) {
 				Node *node = Object::cast_to<Node>(ObjectDB::get_instance(E.key));
 				ERR_CONTINUE(!node);
 				node->disconnect(SceneStringNames::get_singleton()->tree_exiting, callable_mp(this, &MultiplayerSpawner::_node_exit));
-				// This is unlikely, but might still crash the engine.
-				if (node->is_connected(SceneStringNames::get_singleton()->ready, callable_mp(this, &MultiplayerSpawner::_node_ready))) {
-					node->disconnect(SceneStringNames::get_singleton()->ready, callable_mp(this, &MultiplayerSpawner::_node_ready));
-				}
 				get_multiplayer()->object_configuration_remove(node, this);
 			}
 			tracked_nodes.clear();
@@ -244,11 +240,11 @@ void MultiplayerSpawner::_track(Node *p_node, const Variant &p_argument, int p_s
 	if (!tracked_nodes.has(oid)) {
 		tracked_nodes[oid] = SpawnInfo(p_argument.duplicate(true), p_scene_id);
 		p_node->connect(SceneStringNames::get_singleton()->tree_exiting, callable_mp(this, &MultiplayerSpawner::_node_exit).bind(p_node->get_instance_id()), CONNECT_ONE_SHOT);
-		p_node->connect(SceneStringNames::get_singleton()->ready, callable_mp(this, &MultiplayerSpawner::_node_ready).bind(p_node->get_instance_id()), CONNECT_ONE_SHOT);
+		_spawn_notify(p_node->get_instance_id());
 	}
 }
 
-void MultiplayerSpawner::_node_ready(ObjectID p_id) {
+void MultiplayerSpawner::_spawn_notify(ObjectID p_id) {
 	get_multiplayer()->object_configuration_add(ObjectDB::get_instance(p_id), this);
 }
 
diff --git a/modules/multiplayer/multiplayer_spawner.h b/modules/multiplayer/multiplayer_spawner.h
index 8d401a6818..8a54140e32 100644
--- a/modules/multiplayer/multiplayer_spawner.h
+++ b/modules/multiplayer/multiplayer_spawner.h
@@ -77,7 +77,7 @@ private:
 	void _track(Node *p_node, const Variant &p_argument, int p_scene_id = INVALID_ID);
 	void _node_added(Node *p_node);
 	void _node_exit(ObjectID p_id);
-	void _node_ready(ObjectID p_id);
+	void _spawn_notify(ObjectID p_id);
 
 	Vector<String> _get_spawnable_scenes() const;
 	void _set_spawnable_scenes(const Vector<String> &p_scenes);
diff --git a/modules/multiplayer/scene_replication_interface.cpp b/modules/multiplayer/scene_replication_interface.cpp
index e1b7b0c346..233ff76c7d 100644
--- a/modules/multiplayer/scene_replication_interface.cpp
+++ b/modules/multiplayer/scene_replication_interface.cpp
@@ -125,6 +125,20 @@ void SceneReplicationInterface::on_reset() {
 }
 
 void SceneReplicationInterface::on_network_process() {
+	// Prevent endless stalling in case of unforseen spawn errors.
+	if (spawn_queue.size()) {
+		ERR_PRINT("An error happened during last spawn, this usually means the 'ready' signal was not emitted by the spawned node.");
+		for (const ObjectID &oid : spawn_queue) {
+			Node *node = get_id_as<Node>(oid);
+			ERR_CONTINUE(!node);
+			if (node->is_connected(SceneStringNames::get_singleton()->ready, callable_mp(this, &SceneReplicationInterface::_node_ready))) {
+				node->disconnect(SceneStringNames::get_singleton()->ready, callable_mp(this, &SceneReplicationInterface::_node_ready));
+			}
+		}
+		spawn_queue.clear();
+	}
+
+	// Process timed syncs.
 	uint64_t msec = OS::get_singleton()->get_ticks_msec();
 	for (KeyValue<int, PeerInfo> &E : peers_info) {
 		const HashSet<ObjectID> to_sync = E.value.sync_nodes;
@@ -144,17 +158,39 @@ Error SceneReplicationInterface::on_spawn(Object *p_obj, Variant p_config) {
 	// Track node.
 	const ObjectID oid = node->get_instance_id();
 	TrackedNode &tobj = _track(oid);
+
+	// Spawn state needs to be callected after "ready", but the spawn order follows "enter_tree".
 	ERR_FAIL_COND_V(tobj.spawner != ObjectID(), ERR_ALREADY_IN_USE);
 	tobj.spawner = spawner->get_instance_id();
-	spawned_nodes.insert(oid);
+	spawn_queue.insert(oid);
+	node->connect(SceneStringNames::get_singleton()->ready, callable_mp(this, &SceneReplicationInterface::_node_ready).bind(oid), Node::CONNECT_ONE_SHOT);
+	return OK;
+}
+
+void SceneReplicationInterface::_node_ready(const ObjectID &p_oid) {
+	ERR_FAIL_COND(!spawn_queue.has(p_oid)); // Bug.
 
-	if (multiplayer->has_multiplayer_peer() && spawner->is_multiplayer_authority()) {
-		if (tobj.net_id == 0) {
-			tobj.net_id = ++last_net_id;
+	// If we are a nested spawn, we need to wait until the parent is ready.
+	if (p_oid != *(spawn_queue.begin())) {
+		return;
+	}
+
+	for (const ObjectID &oid : spawn_queue) {
+		ERR_CONTINUE(!tracked_nodes.has(oid));
+
+		TrackedNode &tobj = tracked_nodes[oid];
+		MultiplayerSpawner *spawner = get_id_as<MultiplayerSpawner>(tobj.spawner);
+		ERR_CONTINUE(!spawner);
+
+		spawned_nodes.insert(oid);
+		if (multiplayer->has_multiplayer_peer() && spawner->is_multiplayer_authority()) {
+			if (tobj.net_id == 0) {
+				tobj.net_id = ++last_net_id;
+			}
+			_update_spawn_visibility(0, oid);
 		}
-		_update_spawn_visibility(0, oid);
 	}
-	return OK;
+	spawn_queue.clear();
 }
 
 Error SceneReplicationInterface::on_despawn(Object *p_obj, Variant p_config) {
diff --git a/modules/multiplayer/scene_replication_interface.h b/modules/multiplayer/scene_replication_interface.h
index a5e610cff6..cf45db2138 100644
--- a/modules/multiplayer/scene_replication_interface.h
+++ b/modules/multiplayer/scene_replication_interface.h
@@ -51,7 +51,6 @@ private:
 
 		bool operator==(const ObjectID &p_other) { return id == p_other; }
 
-		_FORCE_INLINE_ MultiplayerSpawner *get_spawner() const { return spawner.is_valid() ? Object::cast_to<MultiplayerSpawner>(ObjectDB::get_instance(spawner)) : nullptr; }
 		TrackedNode() {}
 		TrackedNode(const ObjectID &p_id) { id = p_id; }
 		TrackedNode(const ObjectID &p_id, uint32_t p_net_id) {
@@ -75,7 +74,10 @@ private:
 	HashSet<ObjectID> spawned_nodes;
 	HashSet<ObjectID> sync_nodes;
 
-	// Pending spawn information.
+	// Pending local spawn information (handles spawning nested nodes during ready).
+	HashSet<ObjectID> spawn_queue;
+
+	// Pending remote spawn information.
 	ObjectID pending_spawn;
 	int pending_spawn_remote = 0;
 	const uint8_t *pending_buffer = nullptr;
@@ -89,6 +91,7 @@ private:
 
 	TrackedNode &_track(const ObjectID &p_id);
 	void _untrack(const ObjectID &p_id);
+	void _node_ready(const ObjectID &p_oid);
 
 	void _send_sync(int p_peer, const HashSet<ObjectID> p_synchronizers, uint16_t p_sync_net_time, uint64_t p_msec);
 	Error _make_spawn_packet(Node *p_node, MultiplayerSpawner *p_spawner, int &r_len);