From 69bb7e5b1199e6af3256fe948dbfa3102c1ccbc8 Mon Sep 17 00:00:00 2001
From: Adam Scott <ascott.ca@gmail.com>
Date: Wed, 23 Nov 2022 18:13:13 -0500
Subject: [PATCH] Fix singleton scene cyclic loading

---
 editor/editor_autoload_settings.cpp           | 41 ++++++++++++-------
 main/main.cpp                                 | 41 ++++++++++++-------
 modules/gdscript/gdscript_analyzer.cpp        | 21 ++++++----
 modules/gdscript/gdscript_cache.cpp           | 25 -----------
 modules/gdscript/gdscript_cache.h             |  1 -
 .../gdscript/tests/gdscript_test_runner.cpp   | 41 ++++++++++++-------
 6 files changed, 91 insertions(+), 79 deletions(-)

diff --git a/editor/editor_autoload_settings.cpp b/editor/editor_autoload_settings.cpp
index 598bcdc6390..8cb1dfd24e8 100644
--- a/editor/editor_autoload_settings.cpp
+++ b/editor/editor_autoload_settings.cpp
@@ -400,27 +400,38 @@ void EditorAutoloadSettings::_autoload_text_changed(const String p_name) {
 }
 
 Node *EditorAutoloadSettings::_create_autoload(const String &p_path) {
-	Ref<Resource> res = ResourceLoader::load(p_path);
-	ERR_FAIL_COND_V_MSG(res.is_null(), nullptr, "Can't autoload: " + p_path + ".");
 	Node *n = nullptr;
-	Ref<PackedScene> scn = res;
-	Ref<Script> scr = res;
-	if (scn.is_valid()) {
-		n = scn->instantiate();
-	} else if (scr.is_valid()) {
-		StringName ibt = scr->get_instance_base_type();
-		bool valid_type = ClassDB::is_parent_class(ibt, "Node");
-		ERR_FAIL_COND_V_MSG(!valid_type, nullptr, "Script does not inherit from Node: " + p_path + ".");
+	if (ResourceLoader::get_resource_type(p_path) == "PackedScene") {
+		// Cache the scene reference before loading it (for cyclic references)
+		Ref<PackedScene> scn;
+		scn.instantiate();
+		scn->set_path(p_path);
+		scn->reload_from_file();
+		ERR_FAIL_COND_V_MSG(!scn.is_valid(), nullptr, vformat("Can't autoload: %s.", p_path));
 
-		Object *obj = ClassDB::instantiate(ibt);
+		if (scn.is_valid()) {
+			n = scn->instantiate();
+		}
+	} else {
+		Ref<Resource> res = ResourceLoader::load(p_path);
+		ERR_FAIL_COND_V_MSG(res.is_null(), nullptr, vformat("Can't autoload: %s.", p_path));
 
-		ERR_FAIL_COND_V_MSG(!obj, nullptr, "Cannot instance script for Autoload, expected 'Node' inheritance, got: " + String(ibt) + ".");
+		Ref<Script> scr = res;
+		if (scr.is_valid()) {
+			StringName ibt = scr->get_instance_base_type();
+			bool valid_type = ClassDB::is_parent_class(ibt, "Node");
+			ERR_FAIL_COND_V_MSG(!valid_type, nullptr, vformat("Script does not inherit from Node: %s.", p_path));
 
-		n = Object::cast_to<Node>(obj);
-		n->set_script(scr);
+			Object *obj = ClassDB::instantiate(ibt);
+
+			ERR_FAIL_COND_V_MSG(!obj, nullptr, vformat("Cannot instance script for Autoload, expected 'Node' inheritance, got: %s.", ibt));
+
+			n = Object::cast_to<Node>(obj);
+			n->set_script(scr);
+		}
 	}
 
-	ERR_FAIL_COND_V_MSG(!n, nullptr, "Path in Autoload not a node or script: " + p_path + ".");
+	ERR_FAIL_COND_V_MSG(!n, nullptr, vformat("Path in Autoload not a node or script: %s.", p_path));
 
 	return n;
 }
diff --git a/main/main.cpp b/main/main.cpp
index 55f71772586..91dd2082bbc 100644
--- a/main/main.cpp
+++ b/main/main.cpp
@@ -2742,27 +2742,38 @@ bool Main::start() {
 				for (const KeyValue<StringName, ProjectSettings::AutoloadInfo> &E : autoloads) {
 					const ProjectSettings::AutoloadInfo &info = E.value;
 
-					Ref<Resource> res = ResourceLoader::load(info.path);
-					ERR_CONTINUE_MSG(res.is_null(), "Can't autoload: " + info.path);
 					Node *n = nullptr;
-					Ref<PackedScene> scn = res;
-					Ref<Script> script_res = res;
-					if (scn.is_valid()) {
-						n = scn->instantiate();
-					} else if (script_res.is_valid()) {
-						StringName ibt = script_res->get_instance_base_type();
-						bool valid_type = ClassDB::is_parent_class(ibt, "Node");
-						ERR_CONTINUE_MSG(!valid_type, "Script does not inherit from Node: " + info.path);
+					if (ResourceLoader::get_resource_type(info.path) == "PackedScene") {
+						// Cache the scene reference before loading it (for cyclic references)
+						Ref<PackedScene> scn;
+						scn.instantiate();
+						scn->set_path(info.path);
+						scn->reload_from_file();
+						ERR_CONTINUE_MSG(!scn.is_valid(), vformat("Can't autoload: %s.", info.path));
 
-						Object *obj = ClassDB::instantiate(ibt);
+						if (scn.is_valid()) {
+							n = scn->instantiate();
+						}
+					} else {
+						Ref<Resource> res = ResourceLoader::load(info.path);
+						ERR_CONTINUE_MSG(res.is_null(), vformat("Can't autoload: %s.", info.path));
 
-						ERR_CONTINUE_MSG(!obj, "Cannot instance script for autoload, expected 'Node' inheritance, got: " + String(ibt) + ".");
+						Ref<Script> script_res = res;
+						if (script_res.is_valid()) {
+							StringName ibt = script_res->get_instance_base_type();
+							bool valid_type = ClassDB::is_parent_class(ibt, "Node");
+							ERR_CONTINUE_MSG(!valid_type, vformat("Script does not inherit from Node: %s.", info.path));
 
-						n = Object::cast_to<Node>(obj);
-						n->set_script(script_res);
+							Object *obj = ClassDB::instantiate(ibt);
+
+							ERR_CONTINUE_MSG(!obj, vformat("Cannot instance script for autoload, expected 'Node' inheritance, got: %s."));
+
+							n = Object::cast_to<Node>(obj);
+							n->set_script(script_res);
+						}
 					}
 
-					ERR_CONTINUE_MSG(!n, "Path in autoload not a node or script: " + info.path);
+					ERR_CONTINUE_MSG(!n, vformat("Path in autoload not a node or script: %s.", info.path));
 					n->set_name(info.name);
 
 					//defer so references are all valid on _ready()
diff --git a/modules/gdscript/gdscript_analyzer.cpp b/modules/gdscript/gdscript_analyzer.cpp
index 9b0dc9577b8..13e44dab482 100644
--- a/modules/gdscript/gdscript_analyzer.cpp
+++ b/modules/gdscript/gdscript_analyzer.cpp
@@ -3125,14 +3125,19 @@ void GDScriptAnalyzer::reduce_identifier(GDScriptParser::IdentifierNode *p_ident
 					}
 				}
 			} else if (ResourceLoader::get_resource_type(autoload.path) == "PackedScene") {
-				Error err = OK;
-				Ref<GDScript> scr = GDScriptCache::get_packed_scene_script(autoload.path, err);
-				if (err == OK && scr.is_valid()) {
-					Ref<GDScriptParserRef> singl_parser = get_parser_for(scr->get_path());
-					if (singl_parser.is_valid()) {
-						err = singl_parser->raise_status(GDScriptParserRef::INTERFACE_SOLVED);
-						if (err == OK) {
-							result = type_from_metatype(singl_parser->get_parser()->head->get_datatype());
+				if (GDScriptLanguage::get_singleton()->get_named_globals_map().has(name)) {
+					Variant constant = GDScriptLanguage::get_singleton()->get_named_globals_map()[name];
+					Node *node = Object::cast_to<Node>(constant);
+					if (node != nullptr) {
+						Ref<Script> scr = node->get_script();
+						if (scr.is_valid()) {
+							Ref<GDScriptParserRef> singl_parser = get_parser_for(scr->get_path());
+							if (singl_parser.is_valid()) {
+								Error err = singl_parser->raise_status(GDScriptParserRef::INTERFACE_SOLVED);
+								if (err == OK) {
+									result = type_from_metatype(singl_parser->get_parser()->head->get_datatype());
+								}
+							}
 						}
 					}
 				}
diff --git a/modules/gdscript/gdscript_cache.cpp b/modules/gdscript/gdscript_cache.cpp
index f35318e4c69..2e7263b6521 100644
--- a/modules/gdscript/gdscript_cache.cpp
+++ b/modules/gdscript/gdscript_cache.cpp
@@ -365,31 +365,6 @@ Ref<PackedScene> GDScriptCache::get_packed_scene(const String &p_path, Error &r_
 	return scene;
 }
 
-Ref<GDScript> GDScriptCache::get_packed_scene_script(const String &p_path, Error &r_error) {
-	r_error = OK;
-	Ref<PackedScene> scene = get_packed_scene(p_path, r_error);
-
-	if (r_error != OK) {
-		return Ref<GDScript>();
-	}
-
-	int node_count = scene->get_state()->get_node_count();
-	if (node_count == 0) {
-		return Ref<GDScript>();
-	}
-
-	const int ROOT_NODE = 0;
-	for (int i = 0; i < scene->get_state()->get_node_property_count(ROOT_NODE); i++) {
-		if (scene->get_state()->get_node_property_name(ROOT_NODE, i) != SNAME("script")) {
-			continue;
-		}
-
-		return scene->get_state()->get_node_property_value(ROOT_NODE, i);
-	}
-
-	return Ref<GDScript>();
-}
-
 void GDScriptCache::clear_unreferenced_packed_scenes() {
 	if (singleton == nullptr) {
 		return;
diff --git a/modules/gdscript/gdscript_cache.h b/modules/gdscript/gdscript_cache.h
index 0f9d87aa67e..2195932aa3e 100644
--- a/modules/gdscript/gdscript_cache.h
+++ b/modules/gdscript/gdscript_cache.h
@@ -102,7 +102,6 @@ public:
 	static Error finish_compiling(const String &p_owner);
 
 	static Ref<PackedScene> get_packed_scene(const String &p_path, Error &r_error, const String &p_owner = "");
-	static Ref<GDScript> get_packed_scene_script(const String &p_path, Error &r_error);
 	static void clear_unreferenced_packed_scenes();
 
 	static bool is_destructing() {
diff --git a/modules/gdscript/tests/gdscript_test_runner.cpp b/modules/gdscript/tests/gdscript_test_runner.cpp
index 7f42643c8f7..f59983ca90d 100644
--- a/modules/gdscript/tests/gdscript_test_runner.cpp
+++ b/modules/gdscript/tests/gdscript_test_runner.cpp
@@ -71,27 +71,38 @@ void init_autoloads() {
 			continue;
 		}
 
-		Ref<Resource> res = ResourceLoader::load(info.path);
-		ERR_CONTINUE_MSG(res.is_null(), "Can't autoload: " + info.path);
 		Node *n = nullptr;
-		Ref<PackedScene> scn = res;
-		Ref<Script> script = res;
-		if (scn.is_valid()) {
-			n = scn->instantiate();
-		} else if (script.is_valid()) {
-			StringName ibt = script->get_instance_base_type();
-			bool valid_type = ClassDB::is_parent_class(ibt, "Node");
-			ERR_CONTINUE_MSG(!valid_type, "Script does not inherit from Node: " + info.path);
+		if (ResourceLoader::get_resource_type(info.path) == "PackedScene") {
+			// Cache the scene reference before loading it (for cyclic references)
+			Ref<PackedScene> scn;
+			scn.instantiate();
+			scn->set_path(info.path);
+			scn->reload_from_file();
+			ERR_CONTINUE_MSG(!scn.is_valid(), vformat("Can't autoload: %s.", info.path));
 
-			Object *obj = ClassDB::instantiate(ibt);
+			if (scn.is_valid()) {
+				n = scn->instantiate();
+			}
+		} else {
+			Ref<Resource> res = ResourceLoader::load(info.path);
+			ERR_CONTINUE_MSG(res.is_null(), vformat("Can't autoload: %s.", info.path));
 
-			ERR_CONTINUE_MSG(!obj, "Cannot instance script for autoload, expected 'Node' inheritance, got: " + String(ibt) + ".");
+			Ref<Script> scr = res;
+			if (scr.is_valid()) {
+				StringName ibt = scr->get_instance_base_type();
+				bool valid_type = ClassDB::is_parent_class(ibt, "Node");
+				ERR_CONTINUE_MSG(!valid_type, vformat("Script does not inherit from Node: %s.", info.path));
 
-			n = Object::cast_to<Node>(obj);
-			n->set_script(script);
+				Object *obj = ClassDB::instantiate(ibt);
+
+				ERR_CONTINUE_MSG(!obj, vformat("Cannot instance script for Autoload, expected 'Node' inheritance, got: %s.", ibt));
+
+				n = Object::cast_to<Node>(obj);
+				n->set_script(scr);
+			}
 		}
 
-		ERR_CONTINUE_MSG(!n, "Path in autoload not a node or script: " + info.path);
+		ERR_CONTINUE_MSG(!n, vformat("Path in autoload not a node or script: %s.", info.path));
 		n->set_name(info.name);
 
 		for (int i = 0; i < ScriptServer::get_language_count(); i++) {