Sfoglia il codice sorgente

Implement textual ext/subresource IDs.

* Friendlier with version control.
* Generates pseudo unique IDs, to minimize conflicts when merging, but still
  user readable (so, not UUID).
* Eventually will also allow to have more precisely named sub-resources in
  imported files.
* This will allow better reloading on changes (including resources already
  loaded) as well as better keeping track of changes on the DCC.
* Keeps backward compatibility with the old formats.
* Binary and text format version incremented to mark breakage in forward
  compatibility.
reduz 4 anni fa
parent
commit
75755beeee

+ 42 - 10
core/io/resource.cpp

@@ -33,6 +33,7 @@
 #include "core/core_string_names.h"
 #include "core/io/file_access.h"
 #include "core/io/resource_loader.h"
+#include "core/math/math_funcs.h"
 #include "core/object/script_language.h"
 #include "core/os/os.h"
 #include "scene/main/node.h" //only so casting works
@@ -94,12 +95,43 @@ String Resource::get_path() const {
 	return path_cache;
 }
 
-void Resource::set_subindex(int p_sub_index) {
-	subindex = p_sub_index;
+String Resource::generate_scene_unique_id() {
+	// Generate a unique enough hash, but still user-readable.
+	// If it's not unique it does not matter because the saver will try again.
+	OS::Date date = OS::get_singleton()->get_date();
+	OS::Time time = OS::get_singleton()->get_time();
+	uint32_t hash = hash_djb2_one_32(OS::get_singleton()->get_ticks_usec());
+	hash = hash_djb2_one_32(date.year, hash);
+	hash = hash_djb2_one_32(date.month, hash);
+	hash = hash_djb2_one_32(date.day, hash);
+	hash = hash_djb2_one_32(time.hour, hash);
+	hash = hash_djb2_one_32(time.minute, hash);
+	hash = hash_djb2_one_32(time.second, hash);
+	hash = hash_djb2_one_32(Math::rand(), hash);
+
+	static constexpr uint32_t characters = 5;
+	static constexpr uint32_t char_count = ('z' - 'a');
+	static constexpr uint32_t base = char_count + ('9' - '0');
+	String id;
+	for (uint32_t i = 0; i < characters; i++) {
+		uint32_t c = hash % base;
+		if (c < char_count) {
+			id += String::chr('a' + c);
+		} else {
+			id += String::chr('0' + (c - char_count));
+		}
+		hash /= base;
+	}
+
+	return id;
+}
+
+void Resource::set_scene_unique_id(const String &p_id) {
+	scene_unique_id = p_id;
 }
 
-int Resource::get_subindex() const {
-	return subindex;
+String Resource::get_scene_unique_id() const {
+	return scene_unique_id;
 }
 
 void Resource::set_name(const String &p_name) {
@@ -350,8 +382,8 @@ bool Resource::is_translation_remapped() const {
 
 #ifdef TOOLS_ENABLED
 //helps keep IDs same number when loading/saving scenes. -1 clears ID and it Returns -1 when no id stored
-void Resource::set_id_for_path(const String &p_path, int p_id) {
-	if (p_id == -1) {
+void Resource::set_id_for_path(const String &p_path, const String &p_id) {
+	if (p_id == "") {
 		ResourceCache::path_cache_lock.write_lock();
 		ResourceCache::resource_path_cache[p_path].erase(get_path());
 		ResourceCache::path_cache_lock.write_unlock();
@@ -362,15 +394,15 @@ void Resource::set_id_for_path(const String &p_path, int p_id) {
 	}
 }
 
-int Resource::get_id_for_path(const String &p_path) const {
+String Resource::get_id_for_path(const String &p_path) const {
 	ResourceCache::path_cache_lock.read_lock();
 	if (ResourceCache::resource_path_cache[p_path].has(get_path())) {
-		int result = ResourceCache::resource_path_cache[p_path][get_path()];
+		String result = ResourceCache::resource_path_cache[p_path][get_path()];
 		ResourceCache::path_cache_lock.read_unlock();
 		return result;
 	} else {
 		ResourceCache::path_cache_lock.read_unlock();
-		return -1;
+		return "";
 	}
 }
 #endif
@@ -414,7 +446,7 @@ Resource::~Resource() {
 
 HashMap<String, Resource *> ResourceCache::resources;
 #ifdef TOOLS_ENABLED
-HashMap<String, HashMap<String, int>> ResourceCache::resource_path_cache;
+HashMap<String, HashMap<String, String>> ResourceCache::resource_path_cache;
 #endif
 
 RWLock ResourceCache::lock;

+ 7 - 6
core/io/resource.h

@@ -59,7 +59,7 @@ private:
 
 	String name;
 	String path_cache;
-	int subindex = 0;
+	String scene_unique_id;
 
 	virtual bool _use_builtin_script() const { return true; }
 
@@ -105,8 +105,9 @@ public:
 	virtual void set_path(const String &p_path, bool p_take_over = false);
 	String get_path() const;
 
-	void set_subindex(int p_sub_index);
-	int get_subindex() const;
+	static String generate_scene_unique_id();
+	void set_scene_unique_id(const String &p_id);
+	String get_scene_unique_id() const;
 
 	virtual Ref<Resource> duplicate(bool p_subresources = false) const;
 	Ref<Resource> duplicate_for_local_scene(Node *p_for_scene, Map<Ref<Resource>, Ref<Resource>> &remap_cache);
@@ -140,8 +141,8 @@ public:
 
 #ifdef TOOLS_ENABLED
 	//helps keep IDs same number when loading/saving scenes. -1 clears ID and it Returns -1 when no id stored
-	void set_id_for_path(const String &p_path, int p_id);
-	int get_id_for_path(const String &p_path) const;
+	void set_id_for_path(const String &p_path, const String &p_id);
+	String get_id_for_path(const String &p_path) const;
 #endif
 
 	Resource();
@@ -156,7 +157,7 @@ class ResourceCache {
 	static RWLock lock;
 	static HashMap<String, Resource *> resources;
 #ifdef TOOLS_ENABLED
-	static HashMap<String, HashMap<String, int>> resource_path_cache; // each tscn has a set of resource paths and IDs
+	static HashMap<String, HashMap<String, String>> resource_path_cache; // Each tscn has a set of resource paths and IDs.
 	static RWLock path_cache_lock;
 #endif // TOOLS_ENABLED
 	friend void unregister_core_types();

+ 51 - 35
core/io/resource_format_binary.cpp

@@ -84,9 +84,10 @@ enum {
 	OBJECT_EXTERNAL_RESOURCE = 1,
 	OBJECT_INTERNAL_RESOURCE = 2,
 	OBJECT_EXTERNAL_RESOURCE_INDEX = 3,
-	//version 2: added 64 bits support for float and int
-	//version 3: changed nodepath encoding
-	FORMAT_VERSION = 3,
+	// Version 2: added 64 bits support for float and int.
+	// Version 3: changed nodepath encoding.
+	// Version 4: new string ID for ext/subresources, breaks forward compat.
+	FORMAT_VERSION = 4,
 	FORMAT_VERSION_CAN_RENAME_DEPS = 1,
 	FORMAT_VERSION_NO_NODEPATH_PROPERTY = 3,
 };
@@ -311,7 +312,14 @@ Error ResourceLoaderBinary::parse_variant(Variant &r_v) {
 				} break;
 				case OBJECT_INTERNAL_RESOURCE: {
 					uint32_t index = f->get_32();
-					String path = res_path + "::" + itos(index);
+					String path;
+
+					if (using_named_scene_ids) { // New format.
+						ERR_FAIL_INDEX_V((int)index, internal_resources.size(), ERR_PARSE_ERROR);
+						path = internal_resources[index].path;
+					} else {
+						path += res_path + "::" + itos(index);
+					}
 
 					//always use internal cache for loading internal resources
 					if (!internal_index_cache.has(path)) {
@@ -320,7 +328,6 @@ Error ResourceLoaderBinary::parse_variant(Variant &r_v) {
 					} else {
 						r_v = internal_index_cache[path];
 					}
-
 				} break;
 				case OBJECT_EXTERNAL_RESOURCE: {
 					//old file format, still around for compatibility
@@ -378,7 +385,6 @@ Error ResourceLoaderBinary::parse_variant(Variant &r_v) {
 					ERR_FAIL_V(ERR_FILE_CORRUPT);
 				} break;
 			}
-
 		} break;
 		case VARIANT_CALLABLE: {
 			r_v = Callable();
@@ -659,15 +665,17 @@ Error ResourceLoaderBinary::load() {
 
 		//maybe it is loaded already
 		String path;
-		int subindex = 0;
+		String id;
 
 		if (!main) {
 			path = internal_resources[i].path;
 
 			if (path.begins_with("local://")) {
 				path = path.replace_first("local://", "");
-				subindex = path.to_int();
+				id = path;
 				path = res_path + "::" + path;
+
+				internal_resources.write[i].path = path; // Update path.
 			}
 
 			if (cache_mode == ResourceFormatLoader::CACHE_MODE_REUSE) {
@@ -722,7 +730,7 @@ Error ResourceLoaderBinary::load() {
 			if (path != String() && 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_subindex(subindex);
+			r->set_scene_unique_id(id);
 		}
 
 		if (!main) {
@@ -879,7 +887,11 @@ void ResourceLoaderBinary::open(FileAccess *p_f) {
 	print_bl("type: " + type);
 
 	importmd_ofs = f->get_64();
-	for (int i = 0; i < 14; i++) {
+	uint32_t flags = f->get_32();
+	if (flags & ResourceFormatSaverBinaryInstance::FORMAT_FLAG_NAMED_SCENE_IDS) {
+		using_named_scene_ids = true;
+	}
+	for (int i = 0; i < 13; i++) {
 		f->get_32(); //skip a few reserved fields
 	}
 
@@ -1269,11 +1281,7 @@ void ResourceFormatSaverBinaryInstance::_pad_buffer(FileAccess *f, int p_bytes)
 	}
 }
 
-void ResourceFormatSaverBinaryInstance::_write_variant(const Variant &p_property, const PropertyInfo &p_hint) {
-	write_variant(f, p_property, resource_set, external_resources, string_map, p_hint);
-}
-
-void ResourceFormatSaverBinaryInstance::write_variant(FileAccess *f, const Variant &p_property, Set<RES> &resource_set, Map<RES, int> &external_resources, Map<StringName, int> &string_map, const PropertyInfo &p_hint) {
+void ResourceFormatSaverBinaryInstance::write_variant(FileAccess *f, const Variant &p_property, Map<RES, int> &resource_map, Map<RES, int> &external_resources, Map<StringName, int> &string_map, const PropertyInfo &p_hint) {
 	switch (p_property.get_type()) {
 		case Variant::NIL: {
 			f->store_32(VARIANT_NIL);
@@ -1492,13 +1500,13 @@ void ResourceFormatSaverBinaryInstance::write_variant(FileAccess *f, const Varia
 				f->store_32(OBJECT_EXTERNAL_RESOURCE_INDEX);
 				f->store_32(external_resources[res]);
 			} else {
-				if (!resource_set.has(res)) {
+				if (!resource_map.has(res)) {
 					f->store_32(OBJECT_EMPTY);
 					ERR_FAIL_MSG("Resource was not pre cached for the resource section, most likely due to circular reference.");
 				}
 
 				f->store_32(OBJECT_INTERNAL_RESOURCE);
-				f->store_32(res->get_subindex());
+				f->store_32(resource_map[res]);
 				//internal resource
 			}
 
@@ -1526,8 +1534,8 @@ void ResourceFormatSaverBinaryInstance::write_variant(FileAccess *f, const Varia
 					continue;
 				*/
 
-				write_variant(f, E->get(), resource_set, external_resources, string_map);
-				write_variant(f, d[E->get()], resource_set, external_resources, string_map);
+				write_variant(f, E->get(), resource_map, external_resources, string_map);
+				write_variant(f, d[E->get()], resource_map, external_resources, string_map);
 			}
 
 		} break;
@@ -1536,7 +1544,7 @@ void ResourceFormatSaverBinaryInstance::write_variant(FileAccess *f, const Varia
 			Array a = p_property;
 			f->store_32(uint32_t(a.size()));
 			for (int i = 0; i < a.size(); i++) {
-				write_variant(f, a[i], resource_set, external_resources, string_map);
+				write_variant(f, a[i], resource_map, external_resources, string_map);
 			}
 
 		} break;
@@ -1816,7 +1824,8 @@ Error ResourceFormatSaverBinaryInstance::save(const String &p_path, const RES &p
 
 	save_unicode_string(f, p_resource->get_class());
 	f->store_64(0); //offset to import metadata
-	for (int i = 0; i < 14; i++) {
+	f->store_32(FORMAT_FLAG_NAMED_SCENE_IDS);
+	for (int i = 0; i < 13; i++) {
 		f->store_32(0); // reserved
 	}
 
@@ -1886,37 +1895,43 @@ Error ResourceFormatSaverBinaryInstance::save(const String &p_path, const RES &p
 	// save internal resource table
 	f->store_32(saved_resources.size()); //amount of internal resources
 	Vector<uint64_t> ofs_pos;
-	Set<int> used_indices;
+	Set<String> used_unique_ids;
 
 	for (List<RES>::Element *E = saved_resources.front(); E; E = E->next()) {
 		RES r = E->get();
 		if (r->get_path() == "" || r->get_path().find("::") != -1) {
-			if (r->get_subindex() != 0) {
-				if (used_indices.has(r->get_subindex())) {
-					r->set_subindex(0); //repeated
+			if (r->get_scene_unique_id() != "") {
+				if (used_unique_ids.has(r->get_scene_unique_id())) {
+					r->set_scene_unique_id("");
 				} else {
-					used_indices.insert(r->get_subindex());
+					used_unique_ids.insert(r->get_scene_unique_id());
 				}
 			}
 		}
 	}
 
+	Map<RES, int> resource_map;
+	int res_index = 0;
 	for (List<RES>::Element *E = saved_resources.front(); E; E = E->next()) {
 		RES r = E->get();
 		if (r->get_path() == "" || r->get_path().find("::") != -1) {
-			if (r->get_subindex() == 0) {
-				int new_subindex = 1;
-				if (used_indices.size()) {
-					new_subindex = used_indices.back()->get() + 1;
+			if (r->get_scene_unique_id() == "") {
+				String new_id;
+
+				while (true) {
+					new_id = r->get_class() + "_" + Resource::generate_scene_unique_id();
+					if (!used_unique_ids.has(new_id)) {
+						break;
+					}
 				}
 
-				r->set_subindex(new_subindex);
-				used_indices.insert(new_subindex);
+				r->set_scene_unique_id(new_id);
+				used_unique_ids.insert(new_id);
 			}
 
-			save_unicode_string(f, "local://" + itos(r->get_subindex()));
+			save_unicode_string(f, "local://" + r->get_scene_unique_id());
 			if (takeover_paths) {
-				r->set_path(p_path + "::" + itos(r->get_subindex()), true);
+				r->set_path(p_path + "::" + r->get_scene_unique_id(), true);
 			}
 #ifdef TOOLS_ENABLED
 			r->set_edited(false);
@@ -1926,6 +1941,7 @@ Error ResourceFormatSaverBinaryInstance::save(const String &p_path, const RES &p
 		}
 		ofs_pos.push_back(f->get_position());
 		f->store_64(0); //offset in 64 bits
+		resource_map[r] = res_index++;
 	}
 
 	Vector<uint64_t> ofs_table;
@@ -1941,7 +1957,7 @@ Error ResourceFormatSaverBinaryInstance::save(const String &p_path, const RES &p
 		for (List<Property>::Element *F = rd.properties.front(); F; F = F->next()) {
 			Property &p = F->get();
 			f->store_32(p.name_idx);
-			_write_variant(p.value, F->get().pi);
+			write_variant(f, p.value, resource_map, external_resources, string_map, F->get().pi);
 		}
 	}
 

+ 5 - 2
core/io/resource_format_binary.h

@@ -60,6 +60,7 @@ class ResourceLoaderBinary {
 		RES cache;
 	};
 
+	bool using_named_scene_ids = false;
 	bool use_sub_threads = false;
 	float *progress = nullptr;
 	Vector<ExtResource> external_resources;
@@ -150,14 +151,16 @@ class ResourceFormatSaverBinaryInstance {
 	};
 
 	static void _pad_buffer(FileAccess *f, int p_bytes);
-	void _write_variant(const Variant &p_property, const PropertyInfo &p_hint = PropertyInfo());
 	void _find_resources(const Variant &p_variant, bool p_main = false);
 	static void save_unicode_string(FileAccess *f, const String &p_string, bool p_bit_on_len = false);
 	int get_string_index(const String &p_string);
 
 public:
+	enum {
+		FORMAT_FLAG_NAMED_SCENE_IDS = 1
+	};
 	Error save(const String &p_path, const RES &p_resource, uint32_t p_flags = 0);
-	static void write_variant(FileAccess *f, const Variant &p_property, Set<RES> &resource_set, Map<RES, int> &external_resources, Map<StringName, int> &string_map, const PropertyInfo &p_hint = PropertyInfo());
+	static void write_variant(FileAccess *f, const Variant &p_property, Map<RES, int> &resource_map, Map<RES, int> &external_resources, Map<StringName, int> &string_map, const PropertyInfo &p_hint = PropertyInfo());
 };
 
 class ResourceFormatSaverBinary : public ResourceFormatSaver {

+ 3 - 3
editor/project_manager.cpp

@@ -494,13 +494,13 @@ private:
 						if (!f) {
 							set_message(TTR("Couldn't create project.godot in project path."), MESSAGE_ERROR);
 						} else {
-							f->store_line("[gd_resource type=\"Environment\" load_steps=2 format=2]");
+							f->store_line("[gd_resource type=\"Environment\" load_steps=2 format=3]");
 							f->store_line("");
-							f->store_line("[sub_resource type=\"Sky\" id=1]");
+							f->store_line("[sub_resource type=\"Sky\" id=\"1\"]");
 							f->store_line("");
 							f->store_line("[resource]");
 							f->store_line("background_mode = 2");
-							f->store_line("sky = SubResource( 1 )");
+							f->store_line("sky = SubResource( \"1\" )");
 							memdelete(f);
 						}
 					}

+ 103 - 92
scene/resources/resource_format_text.cpp

@@ -35,8 +35,9 @@
 #include "core/io/resource_format_binary.h"
 #include "core/version.h"
 
-//version 2: changed names for basis, aabb, Vectors, etc.
-#define FORMAT_VERSION 2
+// Version 2: changed names for Basis, AABB, Vectors, etc.
+// Version 3: new string ID for ext/subresources, breaks forward compat.
+#define FORMAT_VERSION 3
 
 #include "core/io/dir_access.h"
 #include "core/version.h"
@@ -56,22 +57,23 @@ Ref<Resource> ResourceLoaderText::get_resource() {
 Error ResourceLoaderText::_parse_sub_resource_dummy(DummyReadData *p_data, VariantParser::Stream *p_stream, Ref<Resource> &r_res, int &line, String &r_err_str) {
 	VariantParser::Token token;
 	VariantParser::get_token(p_stream, token, line, r_err_str);
-	if (token.type != VariantParser::TK_NUMBER) {
-		r_err_str = "Expected number (sub-resource index)";
+	if (token.type != VariantParser::TK_NUMBER && token.type != VariantParser::TK_STRING) {
+		r_err_str = "Expected number (old style) or string (sub-resource index)";
 		return ERR_PARSE_ERROR;
 	}
 
-	int index = token.value;
+	String unique_id = token.value;
 
-	if (!p_data->resource_map.has(index)) {
+	if (!p_data->resource_map.has(unique_id)) {
 		Ref<DummyResource> dr;
 		dr.instantiate();
-		dr->set_subindex(index);
-		p_data->resource_map[index] = dr;
-		p_data->resource_set.insert(dr);
+		dr->set_scene_unique_id(unique_id);
+		p_data->resource_map[unique_id] = dr;
+		uint32_t im_size = p_data->resource_index_map.size();
+		p_data->resource_index_map.insert(dr, im_size);
 	}
 
-	r_res = p_data->resource_map[index];
+	r_res = p_data->resource_map[unique_id];
 
 	VariantParser::get_token(p_stream, token, line, r_err_str);
 	if (token.type != VariantParser::TK_PARENTHESIS_CLOSE) {
@@ -85,12 +87,12 @@ Error ResourceLoaderText::_parse_sub_resource_dummy(DummyReadData *p_data, Varia
 Error ResourceLoaderText::_parse_ext_resource_dummy(DummyReadData *p_data, VariantParser::Stream *p_stream, Ref<Resource> &r_res, int &line, String &r_err_str) {
 	VariantParser::Token token;
 	VariantParser::get_token(p_stream, token, line, r_err_str);
-	if (token.type != VariantParser::TK_NUMBER) {
-		r_err_str = "Expected number (sub-resource index)";
+	if (token.type != VariantParser::TK_NUMBER && token.type != VariantParser::TK_STRING) {
+		r_err_str = "Expected number (old style sub-resource index) or String (ext-resource ID)";
 		return ERR_PARSE_ERROR;
 	}
 
-	int id = token.value;
+	String id = token.value;
 
 	ERR_FAIL_COND_V(!p_data->rev_external_resources.has(id), ERR_PARSE_ERROR);
 
@@ -108,14 +110,14 @@ Error ResourceLoaderText::_parse_ext_resource_dummy(DummyReadData *p_data, Varia
 Error ResourceLoaderText::_parse_sub_resource(VariantParser::Stream *p_stream, Ref<Resource> &r_res, int &line, String &r_err_str) {
 	VariantParser::Token token;
 	VariantParser::get_token(p_stream, token, line, r_err_str);
-	if (token.type != VariantParser::TK_NUMBER) {
-		r_err_str = "Expected number (sub-resource index)";
+	if (token.type != VariantParser::TK_NUMBER && token.type != VariantParser::TK_STRING) {
+		r_err_str = "Expected number (old style sub-resource index) or string";
 		return ERR_PARSE_ERROR;
 	}
 
-	int index = token.value;
-	ERR_FAIL_COND_V(!int_resources.has(index), ERR_INVALID_PARAMETER);
-	r_res = int_resources[index];
+	String id = token.value;
+	ERR_FAIL_COND_V(!int_resources.has(id), ERR_INVALID_PARAMETER);
+	r_res = int_resources[id];
 
 	VariantParser::get_token(p_stream, token, line, r_err_str);
 	if (token.type != VariantParser::TK_PARENTHESIS_CLOSE) {
@@ -129,16 +131,16 @@ Error ResourceLoaderText::_parse_sub_resource(VariantParser::Stream *p_stream, R
 Error ResourceLoaderText::_parse_ext_resource(VariantParser::Stream *p_stream, Ref<Resource> &r_res, int &line, String &r_err_str) {
 	VariantParser::Token token;
 	VariantParser::get_token(p_stream, token, line, r_err_str);
-	if (token.type != VariantParser::TK_NUMBER) {
-		r_err_str = "Expected number (sub-resource index)";
+	if (token.type != VariantParser::TK_NUMBER && token.type != VariantParser::TK_STRING) {
+		r_err_str = "Expected number (old style sub-resource index) or String (ext-resource ID)";
 		return ERR_PARSE_ERROR;
 	}
 
-	int id = token.value;
+	String id = token.value;
 
 	if (!ignore_resource_parsing) {
 		if (!ext_resources.has(id)) {
-			r_err_str = "Can't load cached ext-resource #" + itos(id);
+			r_err_str = "Can't load cached ext-resource id: " + id;
 			return ERR_PARSE_ERROR;
 		}
 
@@ -409,7 +411,7 @@ Error ResourceLoaderText::load() {
 
 		String path = next_tag.fields["path"];
 		String type = next_tag.fields["type"];
-		int index = next_tag.fields["id"];
+		String id = next_tag.fields["id"];
 
 		if (path.find("://") == -1 && path.is_rel_path()) {
 			// path is relative to file being loaded, so convert to a resource path
@@ -453,14 +455,14 @@ Error ResourceLoaderText::load() {
 			} else {
 #ifdef TOOLS_ENABLED
 				//remember ID for saving
-				res->set_id_for_path(local_path, index);
+				res->set_id_for_path(local_path, id);
 #endif
 			}
 
 			er.cache = res;
 		}
 
-		ext_resources[index] = er;
+		ext_resources[id] = er;
 
 		error = VariantParser::parse_tag(&stream, lines, error_text, next_tag, &rp);
 
@@ -489,15 +491,15 @@ Error ResourceLoaderText::load() {
 
 		if (!next_tag.fields.has("id")) {
 			error = ERR_FILE_CORRUPT;
-			error_text = "Missing 'index' in external resource tag";
+			error_text = "Missing 'id' in external resource tag";
 			_printerr();
 			return error;
 		}
 
 		String type = next_tag.fields["type"];
-		int id = next_tag.fields["id"];
+		String id = next_tag.fields["id"];
 
-		String path = local_path + "::" + itos(id);
+		String path = local_path + "::" + id;
 
 		//bool exists=ResourceCache::has(path);
 
@@ -575,7 +577,7 @@ Error ResourceLoaderText::load() {
 		int_resources[id] = res; //always assign int resources
 		if (do_assign && cache_mode != ResourceFormatLoader::CACHE_MODE_IGNORE) {
 			res->set_path(path, cache_mode == ResourceFormatLoader::CACHE_MODE_REPLACE);
-			res->set_subindex(id);
+			res->set_scene_unique_id(id);
 		}
 
 		if (progress && resources_total > 0) {
@@ -736,7 +738,7 @@ void ResourceLoaderText::get_dependencies(FileAccess *p_f, List<String> *p_depen
 
 		if (!next_tag.fields.has("id")) {
 			error = ERR_FILE_CORRUPT;
-			error_text = "Missing 'index' in external resource tag";
+			error_text = "Missing 'id' in external resource tag";
 			_printerr();
 			return;
 		}
@@ -814,7 +816,7 @@ Error ResourceLoaderText::rename_dependencies(FileAccess *p_f, const String &p_p
 			}
 
 			String path = next_tag.fields["path"];
-			int index = next_tag.fields["id"];
+			String id = next_tag.fields["id"];
 			String type = next_tag.fields["type"];
 
 			bool relative = false;
@@ -833,7 +835,7 @@ Error ResourceLoaderText::rename_dependencies(FileAccess *p_f, const String &p_p
 				path = base_path.path_to_file(path);
 			}
 
-			fw->store_line("[ext_resource path=\"" + path + "\" type=\"" + type + "\" id=" + itos(index) + "]");
+			fw->store_line("[ext_resource path=\"" + path + "\" type=\"" + type + "\" id=\"" + id + "\"]");
 
 			tag_end = f->get_position();
 		}
@@ -1015,7 +1017,7 @@ Error ResourceLoaderText::save_as_binary(FileAccess *p_f, const String &p_path)
 
 		String path = next_tag.fields["path"];
 		String type = next_tag.fields["type"];
-		int index = next_tag.fields["id"];
+		String id = next_tag.fields["id"];
 
 		bs_save_unicode_string(wf.f, type);
 		bs_save_unicode_string(wf.f, path);
@@ -1025,7 +1027,7 @@ Error ResourceLoaderText::save_as_binary(FileAccess *p_f, const String &p_path)
 		dr.instantiate();
 		dr->set_path("res://dummy" + itos(lindex)); //anything is good to detect it for saving as external
 		dummy_read.external_resources[dr] = lindex;
-		dummy_read.rev_external_resources[index] = dr;
+		dummy_read.rev_external_resources[id] = dr;
 
 		error = VariantParser::parse_tag(&stream, lines, error_text, next_tag, &rp);
 
@@ -1069,7 +1071,7 @@ Error ResourceLoaderText::save_as_binary(FileAccess *p_f, const String &p_path)
 
 			if (!next_tag.fields.has("id")) {
 				error = ERR_FILE_CORRUPT;
-				error_text = "Missing 'index' in external resource tag";
+				error_text = "Missing 'id' in external resource tag";
 				_printerr();
 				return error;
 			}
@@ -1114,7 +1116,7 @@ Error ResourceLoaderText::save_as_binary(FileAccess *p_f, const String &p_path)
 			if (assign != String()) {
 				Map<StringName, int> empty_string_map; //unused
 				bs_save_unicode_string(wf2, assign, true);
-				ResourceFormatSaverBinaryInstance::write_variant(wf2, value, dummy_read.resource_set, dummy_read.external_resources, empty_string_map);
+				ResourceFormatSaverBinaryInstance::write_variant(wf2, value, dummy_read.resource_index_map, dummy_read.external_resources, empty_string_map);
 				prop_count++;
 
 			} else if (next_tag.name != String()) {
@@ -1175,7 +1177,7 @@ Error ResourceLoaderText::save_as_binary(FileAccess *p_f, const String &p_path)
 
 			Map<StringName, int> empty_string_map; //unused
 			bs_save_unicode_string(wf2, name, true);
-			ResourceFormatSaverBinaryInstance::write_variant(wf2, value, dummy_read.resource_set, dummy_read.external_resources, empty_string_map);
+			ResourceFormatSaverBinaryInstance::write_variant(wf2, value, dummy_read.resource_index_map, dummy_read.external_resources, empty_string_map);
 			prop_count++;
 		}
 
@@ -1394,10 +1396,10 @@ String ResourceFormatSaverTextInstance::_write_resources(void *ud, const RES &p_
 
 String ResourceFormatSaverTextInstance::_write_resource(const RES &res) {
 	if (external_resources.has(res)) {
-		return "ExtResource( " + itos(external_resources[res]) + " )";
+		return "ExtResource( \"" + external_resources[res] + "\" )";
 	} else {
 		if (internal_resources.has(res)) {
-			return "SubResource( " + itos(internal_resources[res]) + " )";
+			return "SubResource( \"" + internal_resources[res] + "\" )";
 		} else if (res->get_path().length() && res->get_path().find("::") == -1) {
 			if (res->get_path() == local_path) { //circular reference attempt
 				return "null";
@@ -1426,8 +1428,11 @@ void ResourceFormatSaverTextInstance::_find_resources(const Variant &p_variant,
 					ERR_PRINT("Circular reference to resource being saved found: '" + local_path + "' will be null next time it's loaded.");
 					return;
 				}
-				int index = external_resources.size();
-				external_resources[res] = index;
+
+				// Use a numeric ID as a base, because they are sorted in natural order before saving.
+				// This increases the chances of thread loading to fetch them first.
+				String id = itos(external_resources.size() + 1) + "_" + Resource::generate_scene_unique_id();
+				external_resources[res] = id;
 				return;
 			}
 
@@ -1513,11 +1518,11 @@ Error ResourceFormatSaverTextInstance::save(const String &p_path, const RES &p_r
 		takeover_paths = false;
 	}
 
-	// save resources
+	// Save resources.
 	_find_resources(p_resource, true);
 
 	if (packed_scene.is_valid()) {
-		//add instances to external resources if saving a packed scene
+		// Add instances to external resources if saving a packed scene.
 		for (int i = 0; i < packed_scene->get_state()->get_node_count(); i++) {
 			if (packed_scene->get_state()->is_node_instance_placeholder(i)) {
 				continue;
@@ -1525,8 +1530,8 @@ Error ResourceFormatSaverTextInstance::save(const String &p_path, const RES &p_r
 
 			Ref<PackedScene> instance = packed_scene->get_state()->get_node_instance(i);
 			if (instance.is_valid() && !external_resources.has(instance)) {
-				int index = external_resources.size();
-				external_resources[instance] = index;
+				int index = external_resources.size() + 1;
+				external_resources[instance] = itos(index) + "_" + Resource::generate_scene_unique_id(); // Keep the order for improved thread loading performance.
 			}
 		}
 	}
@@ -1537,12 +1542,6 @@ Error ResourceFormatSaverTextInstance::save(const String &p_path, const RES &p_r
 			title += "type=\"" + p_resource->get_class() + "\" ";
 		}
 		int load_steps = saved_resources.size() + external_resources.size();
-		/*
-		if (packed_scene.is_valid()) {
-			load_steps+=packed_scene->get_node_count();
-		}
-		//no, better to not use load steps from nodes, no point to that
-		*/
 
 		if (load_steps > 1) {
 			title += "load_steps=" + itos(load_steps) + " ";
@@ -1550,51 +1549,61 @@ Error ResourceFormatSaverTextInstance::save(const String &p_path, const RES &p_r
 		title += "format=" + itos(FORMAT_VERSION) + "";
 
 		f->store_string(title);
-		f->store_line("]\n"); //one empty line
+		f->store_line("]\n"); // One empty line.
 	}
 
 #ifdef TOOLS_ENABLED
-	//keep order from cached ids
-	Set<int> cached_ids_found;
-	for (Map<RES, int>::Element *E = external_resources.front(); E; E = E->next()) {
-		int cached_id = E->key()->get_id_for_path(local_path);
-		if (cached_id < 0 || cached_ids_found.has(cached_id)) {
-			E->get() = -1; //reset
+	// Keep order from cached ids.
+	Set<String> cached_ids_found;
+	for (Map<RES, String>::Element *E = external_resources.front(); E; E = E->next()) {
+		String cached_id = E->key()->get_id_for_path(local_path);
+		if (cached_id == "" || cached_ids_found.has(cached_id)) {
+			int sep_pos = E->get().find("_");
+			if (sep_pos != -1) {
+				E->get() = E->get().substr(0, sep_pos + 1); // Keep the order found, for improved thread loading performance.
+			} else {
+				E->get() = "";
+			}
+
 		} else {
 			E->get() = cached_id;
 			cached_ids_found.insert(cached_id);
 		}
 	}
-	//create IDs for non cached resources
-	for (Map<RES, int>::Element *E = external_resources.front(); E; E = E->next()) {
-		if (cached_ids_found.has(E->get())) { //already cached, go on
+	// Create IDs for non cached resources.
+	for (Map<RES, String>::Element *E = external_resources.front(); E; E = E->next()) {
+		if (cached_ids_found.has(E->get())) { // Already cached, go on.
 			continue;
 		}
 
-		int attempt = 1; //start from one, more readable format
-		while (cached_ids_found.has(attempt)) {
-			attempt++;
+		String attempt;
+		while (true) {
+			attempt = E->get() + Resource::generate_scene_unique_id();
+			if (!cached_ids_found.has(attempt)) {
+				break;
+			}
 		}
 
 		cached_ids_found.insert(attempt);
 		E->get() = attempt;
-		//update also in resource
+		// Update also in resource.
 		Ref<Resource> res = E->key();
 		res->set_id_for_path(local_path, attempt);
 	}
 #else
-	//make sure to start from one, as it makes format more readable
-	for (Map<RES, int>::Element *E = external_resources.front(); E; E = E->next()) {
-		E->get() = E->get() + 1;
+	// Make sure to start from one, as it makes format more readable.
+	int counter = 1;
+	for (Map<RES, String>::Element *E = external_resources.front(); E; E = E->next()) {
+		E->get() = itos(counter++);
 	}
 #endif
 
 	Vector<ResourceSort> sorted_er;
 
-	for (Map<RES, int>::Element *E = external_resources.front(); E; E = E->next()) {
+	for (Map<RES, String>::Element *E = external_resources.front(); E; E = E->next()) {
 		ResourceSort rs;
 		rs.resource = E->key();
-		rs.index = E->get();
+		rs.id = E->get();
 		sorted_er.push_back(rs);
 	}
 
@@ -1603,23 +1612,23 @@ Error ResourceFormatSaverTextInstance::save(const String &p_path, const RES &p_r
 	for (int i = 0; i < sorted_er.size(); i++) {
 		String p = sorted_er[i].resource->get_path();
 
-		f->store_string("[ext_resource path=\"" + p + "\" type=\"" + sorted_er[i].resource->get_save_class() + "\" id=" + itos(sorted_er[i].index) + "]\n"); //bundled
+		f->store_string("[ext_resource path=\"" + p + "\" type=\"" + sorted_er[i].resource->get_save_class() + "\" id=\"" + sorted_er[i].id + "\"]\n"); // Bundled.
 	}
 
 	if (external_resources.size()) {
-		f->store_line(String()); //separate
+		f->store_line(String()); // Separate.
 	}
 
-	Set<int> used_indices;
+	Set<String> used_unique_ids;
 
 	for (List<RES>::Element *E = saved_resources.front(); E; E = E->next()) {
 		RES res = E->get();
 		if (E->next() && (res->get_path() == "" || res->get_path().find("::") != -1)) {
-			if (res->get_subindex() != 0) {
-				if (used_indices.has(res->get_subindex())) {
-					res->set_subindex(0); //repeated
+			if (res->get_scene_unique_id() != "") {
+				if (used_unique_ids.has(res->get_scene_unique_id())) {
+					res->set_scene_unique_id(""); // Repeated.
 				} else {
-					used_indices.insert(res->get_subindex());
+					used_unique_ids.insert(res->get_scene_unique_id());
 				}
 			}
 		}
@@ -1631,31 +1640,35 @@ Error ResourceFormatSaverTextInstance::save(const String &p_path, const RES &p_r
 		bool main = (E->next() == nullptr);
 
 		if (main && packed_scene.is_valid()) {
-			break; //save as a scene
+			break; // Save as a scene.
 		}
 
 		if (main) {
 			f->store_line("[resource]");
 		} else {
 			String line = "[sub_resource ";
-			if (res->get_subindex() == 0) {
-				int new_subindex = 1;
-				if (used_indices.size()) {
-					new_subindex = used_indices.back()->get() + 1;
+			if (res->get_scene_unique_id() == "") {
+				String new_id;
+				while (true) {
+					new_id = res->get_class() + "_" + Resource::generate_scene_unique_id();
+
+					if (!used_unique_ids.has(new_id)) {
+						break;
+					}
 				}
 
-				res->set_subindex(new_subindex);
-				used_indices.insert(new_subindex);
+				res->set_scene_unique_id(new_id);
+				used_unique_ids.insert(new_id);
 			}
 
-			int idx = res->get_subindex();
-			line += "type=\"" + res->get_class() + "\" id=" + itos(idx);
-			f->store_line(line + "]");
+			String id = res->get_scene_unique_id();
+			line += "type=\"" + res->get_class() + "\" id=\"" + id;
+			f->store_line(line + "\"]");
 			if (takeover_paths) {
-				res->set_path(p_path + "::" + itos(idx), true);
+				res->set_path(p_path + "::" + id, true);
 			}
 
-			internal_resources[res] = idx;
+			internal_resources[res] = id;
 #ifdef TOOLS_ENABLED
 			res->set_edited(false);
 #endif
@@ -1663,7 +1676,6 @@ Error ResourceFormatSaverTextInstance::save(const String &p_path, const RES &p_r
 
 		List<PropertyInfo> property_list;
 		res->get_property_list(&property_list);
-		//property_list.sort();
 		for (List<PropertyInfo>::Element *PE = property_list.front(); PE; PE = PE->next()) {
 			if (skip_editor && PE->get().name.begins_with("__editor")) {
 				continue;
@@ -1704,7 +1716,7 @@ Error ResourceFormatSaverTextInstance::save(const String &p_path, const RES &p_r
 	}
 
 	if (packed_scene.is_valid()) {
-		//if this is a scene, save nodes and connections!
+		// If this is a scene, save nodes and connections!
 		Ref<SceneState> state = packed_scene->get_state();
 		for (int i = 0; i < state->get_node_count(); i++) {
 			StringName type = state->get_node_type(i);
@@ -1812,7 +1824,6 @@ Error ResourceFormatSaverTextInstance::save(const String &p_path, const RES &p_r
 	}
 
 	f->close();
-	//memdelete(f);
 
 	return OK;
 }

+ 9 - 12
scene/resources/resource_format_text.h

@@ -58,10 +58,8 @@ class ResourceLoaderText {
 
 	bool ignore_resource_parsing = false;
 
-	//Map<String,String> remaps;
-
-	Map<int, ExtResource> ext_resources;
-	Map<int, RES> int_resources;
+	Map<String, ExtResource> ext_resources;
+	Map<String, RES> int_resources;
 
 	int resources_total = 0;
 	int resource_current = 0;
@@ -77,7 +75,6 @@ class ResourceLoaderText {
 	mutable int lines = 0;
 
 	Map<String, String> remaps;
-	//void _printerr();
 
 	static Error _parse_sub_resources(void *p_self, VariantParser::Stream *p_stream, Ref<Resource> &r_res, int &line, String &r_err_str) { return reinterpret_cast<ResourceLoaderText *>(p_self)->_parse_sub_resource(p_stream, r_res, line, r_err_str); }
 	static Error _parse_ext_resources(void *p_self, VariantParser::Stream *p_stream, Ref<Resource> &r_res, int &line, String &r_err_str) { return reinterpret_cast<ResourceLoaderText *>(p_self)->_parse_ext_resource(p_stream, r_res, line, r_err_str); }
@@ -92,9 +89,9 @@ class ResourceLoaderText {
 
 	struct DummyReadData {
 		Map<RES, int> external_resources;
-		Map<int, RES> rev_external_resources;
-		Set<RES> resource_set;
-		Map<int, RES> resource_map;
+		Map<String, RES> rev_external_resources;
+		Map<RES, int> resource_index_map;
+		Map<String, RES> resource_map;
 	};
 
 	static Error _parse_sub_resource_dummys(void *p_self, VariantParser::Stream *p_stream, Ref<Resource> &r_res, int &line, String &r_err_str) { return _parse_sub_resource_dummy((DummyReadData *)(p_self), p_stream, r_res, line, r_err_str); }
@@ -168,14 +165,14 @@ class ResourceFormatSaverTextInstance {
 
 	Set<RES> resource_set;
 	List<RES> saved_resources;
-	Map<RES, int> external_resources;
-	Map<RES, int> internal_resources;
+	Map<RES, String> external_resources;
+	Map<RES, String> internal_resources;
 
 	struct ResourceSort {
 		RES resource;
-		int index = 0;
+		String id;
 		bool operator<(const ResourceSort &p_right) const {
-			return index < p_right.index;
+			return id.naturalnocasecmp_to(p_right.id) < 0;
 		}
 	};