Browse Source

GDScript: Enhance handling of cyclic dependencies

Pedro J. Estébanez 1 year ago
parent
commit
c1391489e3

+ 39 - 29
core/io/resource_format_binary.cpp

@@ -749,44 +749,54 @@ Error ResourceLoaderBinary::load() {
 		String t = get_unicode_string();
 		String t = get_unicode_string();
 
 
 		Ref<Resource> res;
 		Ref<Resource> res;
+		Resource *r = nullptr;
 
 
-		if (cache_mode == ResourceFormatLoader::CACHE_MODE_REPLACE && ResourceCache::has(path)) {
-			//use the existing one
-			Ref<Resource> cached = ResourceCache::get_ref(path);
-			if (cached->get_class() == t) {
-				cached->reset_state();
-				res = cached;
-			}
+		MissingResource *missing_resource = nullptr;
+
+		if (main) {
+			res = ResourceLoader::get_resource_ref_override(local_path);
+			r = res.ptr();
 		}
 		}
+		if (!r) {
+			if (cache_mode == ResourceFormatLoader::CACHE_MODE_REPLACE && ResourceCache::has(path)) {
+				//use the existing one
+				Ref<Resource> cached = ResourceCache::get_ref(path);
+				if (cached->get_class() == t) {
+					cached->reset_state();
+					res = cached;
+				}
+			}
 
 
-		MissingResource *missing_resource = nullptr;
+			if (res.is_null()) {
+				//did not replace
+
+				Object *obj = ClassDB::instantiate(t);
+				if (!obj) {
+					if (ResourceLoader::is_creating_missing_resources_if_class_unavailable_enabled()) {
+						//create a missing resource
+						missing_resource = memnew(MissingResource);
+						missing_resource->set_original_class(t);
+						missing_resource->set_recording_properties(true);
+						obj = missing_resource;
+					} else {
+						error = ERR_FILE_CORRUPT;
+						ERR_FAIL_V_MSG(ERR_FILE_CORRUPT, local_path + ":Resource of unrecognized type in file: " + t + ".");
+					}
+				}
 
 
-		if (res.is_null()) {
-			//did not replace
-
-			Object *obj = ClassDB::instantiate(t);
-			if (!obj) {
-				if (ResourceLoader::is_creating_missing_resources_if_class_unavailable_enabled()) {
-					//create a missing resource
-					missing_resource = memnew(MissingResource);
-					missing_resource->set_original_class(t);
-					missing_resource->set_recording_properties(true);
-					obj = missing_resource;
-				} else {
+				r = Object::cast_to<Resource>(obj);
+				if (!r) {
+					String obj_class = obj->get_class();
 					error = ERR_FILE_CORRUPT;
 					error = ERR_FILE_CORRUPT;
-					ERR_FAIL_V_MSG(ERR_FILE_CORRUPT, local_path + ":Resource of unrecognized type in file: " + t + ".");
+					memdelete(obj); //bye
+					ERR_FAIL_V_MSG(ERR_FILE_CORRUPT, local_path + ":Resource type in resource field not a resource, type is: " + obj_class + ".");
 				}
 				}
-			}
 
 
-			Resource *r = Object::cast_to<Resource>(obj);
-			if (!r) {
-				String obj_class = obj->get_class();
-				error = ERR_FILE_CORRUPT;
-				memdelete(obj); //bye
-				ERR_FAIL_V_MSG(ERR_FILE_CORRUPT, local_path + ":Resource type in resource field not a resource, type is: " + obj_class + ".");
+				res = Ref<Resource>(r);
 			}
 			}
+		}
 
 
-			res = Ref<Resource>(r);
+		if (r) {
 			if (!path.is_empty()) {
 			if (!path.is_empty()) {
 				if (cache_mode != ResourceFormatLoader::CACHE_MODE_IGNORE) {
 				if (cache_mode != ResourceFormatLoader::CACHE_MODE_IGNORE) {
 					r->set_path(path, cache_mode == ResourceFormatLoader::CACHE_MODE_REPLACE); // If got here because the resource with same path has different type, replace it.
 					r->set_path(path, cache_mode == ResourceFormatLoader::CACHE_MODE_REPLACE); // If got here because the resource with same path has different type, replace it.

+ 36 - 0
core/io/resource_loader.cpp

@@ -272,6 +272,7 @@ Ref<Resource> ResourceLoader::_load(const String &p_path, const String &p_origin
 	}
 	}
 
 
 	load_paths_stack->resize(load_paths_stack->size() - 1);
 	load_paths_stack->resize(load_paths_stack->size() - 1);
+	res_ref_overrides.erase(load_nesting);
 	load_nesting--;
 	load_nesting--;
 
 
 	if (!res.is_null()) {
 	if (!res.is_null()) {
@@ -730,6 +731,40 @@ Ref<Resource> ResourceLoader::_load_complete_inner(LoadToken &p_load_token, Erro
 	}
 	}
 }
 }
 
 
+Ref<Resource> ResourceLoader::ensure_resource_ref_override_for_outer_load(const String &p_path, const String &p_res_type) {
+	ERR_FAIL_COND_V(load_nesting == 0, Ref<Resource>()); // It makes no sense to use this from nesting level 0.
+	const String &local_path = _validate_local_path(p_path);
+	HashMap<String, Ref<Resource>> &overrides = res_ref_overrides[load_nesting - 1];
+	HashMap<String, Ref<Resource>>::Iterator E = overrides.find(local_path);
+	if (E) {
+		return E->value;
+	} else {
+		Object *obj = ClassDB::instantiate(p_res_type);
+		ERR_FAIL_NULL_V(obj, Ref<Resource>());
+		Ref<Resource> res(obj);
+		if (!res.is_valid()) {
+			memdelete(obj);
+			ERR_FAIL_V(Ref<Resource>());
+		}
+		overrides[local_path] = res;
+		return res;
+	}
+}
+
+Ref<Resource> ResourceLoader::get_resource_ref_override(const String &p_path) {
+	DEV_ASSERT(p_path == _validate_local_path(p_path));
+	HashMap<int, HashMap<String, Ref<Resource>>>::Iterator E = res_ref_overrides.find(load_nesting);
+	if (!E) {
+		return nullptr;
+	}
+	HashMap<String, Ref<Resource>>::Iterator F = E->value.find(p_path);
+	if (!F) {
+		return nullptr;
+	}
+
+	return F->value;
+}
+
 bool ResourceLoader::exists(const String &p_path, const String &p_type_hint) {
 bool ResourceLoader::exists(const String &p_path, const String &p_type_hint) {
 	String local_path = _validate_local_path(p_path);
 	String local_path = _validate_local_path(p_path);
 
 
@@ -1222,6 +1257,7 @@ bool ResourceLoader::timestamp_on_load = false;
 thread_local int ResourceLoader::load_nesting = 0;
 thread_local int ResourceLoader::load_nesting = 0;
 thread_local WorkerThreadPool::TaskID ResourceLoader::caller_task_id = 0;
 thread_local WorkerThreadPool::TaskID ResourceLoader::caller_task_id = 0;
 thread_local Vector<String> *ResourceLoader::load_paths_stack;
 thread_local Vector<String> *ResourceLoader::load_paths_stack;
+thread_local HashMap<int, HashMap<String, Ref<Resource>>> ResourceLoader::res_ref_overrides;
 
 
 template <>
 template <>
 thread_local uint32_t SafeBinaryMutex<ResourceLoader::BINARY_MUTEX_TAG>::count = 0;
 thread_local uint32_t SafeBinaryMutex<ResourceLoader::BINARY_MUTEX_TAG>::count = 0;

+ 4 - 0
core/io/resource_loader.h

@@ -187,6 +187,7 @@ private:
 
 
 	static thread_local int load_nesting;
 	static thread_local int load_nesting;
 	static thread_local WorkerThreadPool::TaskID caller_task_id;
 	static thread_local WorkerThreadPool::TaskID caller_task_id;
+	static thread_local HashMap<int, HashMap<String, Ref<Resource>>> res_ref_overrides; // Outermost key is nesting level.
 	static thread_local Vector<String> *load_paths_stack; // A pointer to avoid broken TLS implementations from double-running the destructor.
 	static thread_local Vector<String> *load_paths_stack; // A pointer to avoid broken TLS implementations from double-running the destructor.
 	static SafeBinaryMutex<BINARY_MUTEX_TAG> thread_load_mutex;
 	static SafeBinaryMutex<BINARY_MUTEX_TAG> thread_load_mutex;
 	static HashMap<String, ThreadLoadTask> thread_load_tasks;
 	static HashMap<String, ThreadLoadTask> thread_load_tasks;
@@ -272,6 +273,9 @@ public:
 	static void set_create_missing_resources_if_class_unavailable(bool p_enable);
 	static void set_create_missing_resources_if_class_unavailable(bool p_enable);
 	_FORCE_INLINE_ static bool is_creating_missing_resources_if_class_unavailable_enabled() { return create_missing_resources_if_class_unavailable; }
 	_FORCE_INLINE_ static bool is_creating_missing_resources_if_class_unavailable_enabled() { return create_missing_resources_if_class_unavailable; }
 
 
+	static Ref<Resource> ensure_resource_ref_override_for_outer_load(const String &p_path, const String &p_res_type);
+	static Ref<Resource> get_resource_ref_override(const String &p_path);
+
 	static bool is_cleaning_tasks();
 	static bool is_cleaning_tasks();
 
 
 	static void initialize();
 	static void initialize();

+ 7 - 2
modules/gdscript/gdscript_analyzer.cpp

@@ -4299,7 +4299,8 @@ void GDScriptAnalyzer::reduce_preload(GDScriptParser::PreloadNode *p_preload) {
 
 
 			// Must load GDScript separately to permit cyclic references
 			// Must load GDScript separately to permit cyclic references
 			// as ResourceLoader::load() detects and rejects those.
 			// as ResourceLoader::load() detects and rejects those.
-			if (ResourceLoader::get_resource_type(p_preload->resolved_path) == "GDScript") {
+			const String &res_type = ResourceLoader::get_resource_type(p_preload->resolved_path);
+			if (res_type == "GDScript") {
 				Error err = OK;
 				Error err = OK;
 				Ref<GDScript> res = GDScriptCache::get_shallow_script(p_preload->resolved_path, err, parser->script_path);
 				Ref<GDScript> res = GDScriptCache::get_shallow_script(p_preload->resolved_path, err, parser->script_path);
 				p_preload->resource = res;
 				p_preload->resource = res;
@@ -4307,7 +4308,11 @@ void GDScriptAnalyzer::reduce_preload(GDScriptParser::PreloadNode *p_preload) {
 					push_error(vformat(R"(Could not preload resource script "%s".)", p_preload->resolved_path), p_preload->path);
 					push_error(vformat(R"(Could not preload resource script "%s".)", p_preload->resolved_path), p_preload->path);
 				}
 				}
 			} else {
 			} else {
-				p_preload->resource = ResourceLoader::load(p_preload->resolved_path);
+				Error err = OK;
+				p_preload->resource = ResourceLoader::load(p_preload->resolved_path, res_type, ResourceFormatLoader::CACHE_MODE_REUSE, &err);
+				if (err == ERR_BUSY) {
+					p_preload->resource = ResourceLoader::ensure_resource_ref_override_for_outer_load(p_preload->resolved_path, res_type);
+				}
 				if (p_preload->resource.is_null()) {
 				if (p_preload->resource.is_null()) {
 					push_error(vformat(R"(Could not preload resource file "%s".)", p_preload->resolved_path), p_preload->path);
 					push_error(vformat(R"(Could not preload resource file "%s".)", p_preload->resolved_path), p_preload->path);
 				}
 				}

+ 32 - 27
scene/resources/resource_format_text.cpp

@@ -191,8 +191,10 @@ Error ResourceLoaderText::_parse_ext_resource(VariantParser::Stream *p_stream, R
 }
 }
 
 
 Ref<PackedScene> ResourceLoaderText::_parse_node_tag(VariantParser::ResourceParser &parser) {
 Ref<PackedScene> ResourceLoaderText::_parse_node_tag(VariantParser::ResourceParser &parser) {
-	Ref<PackedScene> packed_scene;
-	packed_scene.instantiate();
+	Ref<PackedScene> packed_scene = ResourceLoader::get_resource_ref_override(local_path);
+	if (packed_scene.is_null()) {
+		packed_scene.instantiate();
+	}
 
 
 	while (true) {
 	while (true) {
 		if (next_tag.name == "node") {
 		if (next_tag.name == "node") {
@@ -664,39 +666,42 @@ Error ResourceLoaderText::load() {
 			return error;
 			return error;
 		}
 		}
 
 
-		Ref<Resource> cache = ResourceCache::get_ref(local_path);
-		if (cache_mode == ResourceFormatLoader::CACHE_MODE_REPLACE && cache.is_valid() && cache->get_class() == res_type) {
-			cache->reset_state();
-			resource = cache;
-		}
-
 		MissingResource *missing_resource = nullptr;
 		MissingResource *missing_resource = nullptr;
 
 
-		if (!resource.is_valid()) {
-			Object *obj = ClassDB::instantiate(res_type);
-			if (!obj) {
-				if (ResourceLoader::is_creating_missing_resources_if_class_unavailable_enabled()) {
-					missing_resource = memnew(MissingResource);
-					missing_resource->set_original_class(res_type);
-					missing_resource->set_recording_properties(true);
-					obj = missing_resource;
-				} else {
-					error_text += "Can't create sub resource of type: " + res_type;
+		resource = ResourceLoader::get_resource_ref_override(local_path);
+		if (resource.is_null()) {
+			Ref<Resource> cache = ResourceCache::get_ref(local_path);
+			if (cache_mode == ResourceFormatLoader::CACHE_MODE_REPLACE && cache.is_valid() && cache->get_class() == res_type) {
+				cache->reset_state();
+				resource = cache;
+			}
+
+			if (!resource.is_valid()) {
+				Object *obj = ClassDB::instantiate(res_type);
+				if (!obj) {
+					if (ResourceLoader::is_creating_missing_resources_if_class_unavailable_enabled()) {
+						missing_resource = memnew(MissingResource);
+						missing_resource->set_original_class(res_type);
+						missing_resource->set_recording_properties(true);
+						obj = missing_resource;
+					} else {
+						error_text += "Can't create sub resource of type: " + res_type;
+						_printerr();
+						error = ERR_FILE_CORRUPT;
+						return error;
+					}
+				}
+
+				Resource *r = Object::cast_to<Resource>(obj);
+				if (!r) {
+					error_text += "Can't create sub resource of type, because not a resource: " + res_type;
 					_printerr();
 					_printerr();
 					error = ERR_FILE_CORRUPT;
 					error = ERR_FILE_CORRUPT;
 					return error;
 					return error;
 				}
 				}
-			}
 
 
-			Resource *r = Object::cast_to<Resource>(obj);
-			if (!r) {
-				error_text += "Can't create sub resource of type, because not a resource: " + res_type;
-				_printerr();
-				error = ERR_FILE_CORRUPT;
-				return error;
+				resource = Ref<Resource>(r);
 			}
 			}
-
-			resource = Ref<Resource>(r);
 		}
 		}
 
 
 		Dictionary missing_resource_properties;
 		Dictionary missing_resource_properties;