Merge pull request #78988 from RandomShaper/fix_change_scene

Reimplement scene change

doc/classes/SceneTree.xml

@@ -42,7 +42,7 @@
 			<description>
 				Changes the running scene to the one at the given [param path], after loading it into a [PackedScene] and creating a new instance.
 				Returns [constant OK] on success, [constant ERR_CANT_OPEN] if the [param path] cannot be loaded into a [PackedScene], or [constant ERR_CANT_CREATE] if that scene cannot be instantiated.
-				[b]Note:[/b] The scene change is deferred, which means that the new scene node is added to the tree at the end of the frame. This ensures that both scenes aren't running at the same time, while still freeing the previous scene in a safe way similar to [method Node.queue_free]. As such, you won't be able to access the loaded scene immediately after the [method change_scene_to_file] call.
+				[b]Note:[/b] The new scene node is added to the tree at the end of the frame. This ensures that both scenes aren't running at the same time, while still freeing the previous scene in a safe way similar to [method Node.queue_free]. As such, you won't be able to access the loaded scene immediately after the [method change_scene_to_file] call.
 			</description>
 		</method>
 		<method name="change_scene_to_packed">
@@ -51,7 +51,7 @@
 			<description>
 				Changes the running scene to a new instance of the given [PackedScene] (which must be valid).
 				Returns [constant OK] on success, [constant ERR_CANT_CREATE] if the scene cannot be instantiated, or [constant ERR_INVALID_PARAMETER] if the scene is invalid.
-				[b]Note:[/b] The scene change is deferred, which means that the new scene node is added to the tree at the end of the frame. You won't be able to access it immediately after the [method change_scene_to_packed] call.
+				[b]Note:[/b] The new scene node is added to the tree at the end of the frame. You won't be able to access it immediately after the [method change_scene_to_packed] call.
 			</description>
 		</method>
 		<method name="create_timer">

scene/main/scene_tree.cpp

@@ -550,6 +550,10 @@ bool SceneTree::process(double p_time) {
 #endif // _3D_DISABLED
 #endif // TOOLS_ENABLED
 
+	if (unlikely(pending_new_scene)) {
+		_flush_scene_change();
+	}
+
 	return _quit;
 }
 
@@ -1376,27 +1380,16 @@ Node *SceneTree::get_current_scene() const {
 	return current_scene;
 }
 
-void SceneTree::_change_scene(Node *p_to) {
-	ERR_FAIL_COND_MSG(!Thread::is_main_thread(), "Changing scene can only be done from the main thread.");
-	if (current_scene) {
-		memdelete(current_scene);
-		current_scene = nullptr;
-	}
-
-	// If we're quitting, abort.
-	if (unlikely(_quit)) {
-		if (p_to) { // Prevent memory leak.
-			memdelete(p_to);
-		}
-		return;
-	}
-
-	if (p_to) {
-		current_scene = p_to;
-		root->add_child(p_to);
-		// Update display for cursor instantly.
-		root->update_mouse_cursor_state();
+void SceneTree::_flush_scene_change() {
+	if (prev_scene) {
+		memdelete(prev_scene);
+		prev_scene = nullptr;
 	}
+	current_scene = pending_new_scene;
+	root->add_child(pending_new_scene);
+	pending_new_scene = nullptr;
+	// Update display for cursor instantly.
+	root->update_mouse_cursor_state();
 }
 
 Error SceneTree::change_scene_to_file(const String &p_path) {
@@ -1415,7 +1408,22 @@ Error SceneTree::change_scene_to_packed(const Ref<PackedScene> &p_scene) {
 	Node *new_scene = p_scene->instantiate();
 	ERR_FAIL_NULL_V(new_scene, ERR_CANT_CREATE);
 
-	call_deferred(SNAME("_change_scene"), new_scene);
+	// If called again while a change is pending.
+	if (pending_new_scene) {
+		queue_delete(pending_new_scene);
+		pending_new_scene = nullptr;
+	}
+
+	prev_scene = current_scene;
+
+	if (current_scene) {
+		// Let as many side effects as possible happen or be queued now,
+		// so they are run before the scene is actually deleted.
+		root->remove_child(current_scene);
+	}
+	DEV_ASSERT(!current_scene);
+
+	pending_new_scene = new_scene;
 	return OK;
 }
 
@@ -1593,8 +1601,6 @@ void SceneTree::_bind_methods() {
 	ClassDB::bind_method(D_METHOD("reload_current_scene"), &SceneTree::reload_current_scene);
 	ClassDB::bind_method(D_METHOD("unload_current_scene"), &SceneTree::unload_current_scene);
 
-	ClassDB::bind_method(D_METHOD("_change_scene"), &SceneTree::_change_scene);
-
 	ClassDB::bind_method(D_METHOD("set_multiplayer", "multiplayer", "root_path"), &SceneTree::set_multiplayer, DEFVAL(NodePath()));
 	ClassDB::bind_method(D_METHOD("get_multiplayer", "for_path"), &SceneTree::get_multiplayer, DEFVAL(NodePath()));
 	ClassDB::bind_method(D_METHOD("set_multiplayer_poll_enabled", "enabled"), &SceneTree::set_multiplayer_poll_enabled);
@@ -1833,6 +1839,14 @@ SceneTree::SceneTree() {
 }
 
 SceneTree::~SceneTree() {
+	if (prev_scene) {
+		memdelete(prev_scene);
+		prev_scene = nullptr;
+	}
+	if (pending_new_scene) {
+		memdelete(pending_new_scene);
+		pending_new_scene = nullptr;
+	}
 	if (root) {
 		root->_set_tree(nullptr);
 		root->_propagate_after_exit_tree();

scene/main/scene_tree.h

@@ -178,6 +178,8 @@ class SceneTree : public MainLoop {
 	TypedArray<Node> _get_nodes_in_group(const StringName &p_group);
 
 	Node *current_scene = nullptr;
+	Node *prev_scene = nullptr;
+	Node *pending_new_scene = nullptr;
 
 	Color debug_collisions_color;
 	Color debug_collision_contact_color;
@@ -188,7 +190,7 @@ class SceneTree : public MainLoop {
 	Ref<Material> collision_material;
 	int collision_debug_contacts;
 
-	void _change_scene(Node *p_to);
+	void _flush_scene_change();
 
 	List<Ref<SceneTreeTimer>> timers;
 	List<Ref<Tween>> tweens;