Overhaul `Resource::duplicate()`

Thanks to a refactor, `Resource::duplicate_for_local_scene()` and `Resource::duplicate()` are now both users of the same, parametrized, implementation.

`Resource::duplicate()` now honors deepness in a more consistent and predictable fashion. `Resource::duplicate_deep()` is added (instead of just adding a parameter to the former, for compatibility needs).

The behavior after this change is as follows:
  - Deep (`deep=true`, formerly `subresources=true`):
    - Previously, only resources found as direct property values of the one to copy would be, recursively, duplicated.
    - Now, in addition, arrays and dictionaries are walked so the copy is truly deep, and only local subresources found across are copied.
    - Previously, subresources would be duplicated as many times as being referenced throughout the main resource.
    - Now, each subresource is only duplicated once and from that point, a referenced to that single copy is used. That's the enhanced behavior that `duplicate_for_local_scene()` already featured.
    - The behavior with respect to packed arrays is still duplication.
    - Formerly, arrays and dictionaries were recursive duplicated, with resources ignored.
    - Now, arrays and dictionaries are recursive duplicated, with resources duplicated.
    - When doing it through `duplicate_deep()`, there's a` deep_subresources_mode` parameter, with various possibilites to control if no resources are duplicated (so arrays, etc. are, but keeping referencing the originals), if only the internal ones are (resources with no non-local path, the default), or if all of them are. The default is to copy every subresource, just like `duplicate(true)`.
  - Not deep (`deep=false`, formerly `subresources=false`): <a name="resource-shallow"></a>
    - Previously, the first level of resources found as direct property values would be duplicated unconditionally. Packed arrays, arrays and dictionaries were non-recursively duplicated.
    - Now, no subresource found at any level in any form will be duplicated, but the original reference kept instead. Packed arrays, arrays and dictionaries are referenced, not duplicated at all.
    - Now, resources found as values of always-duplicate properties are duplicated, recursively or not matching what was requested for the root call.

This commit also changes what's the virtual method to override to customize the duplication (now it's the protected `_duplicate()` instead of the public `duplicate()`).

core/io/image.cpp

@@ -4262,7 +4262,7 @@ Image::Image(const uint8_t *p_mem_png_jpg, int p_len) {
 	}
 }
 
-Ref<Resource> Image::duplicate(bool p_subresources) const {
+Ref<Resource> Image::_duplicate(const DuplicateParams &p_params) const {
 	Ref<Image> copy;
 	copy.instantiate();
 	copy->_copy_internals_from(*this);

core/io/image.h

@@ -244,6 +244,8 @@ public:
 	static Ref<Image> (*basis_universal_unpacker_ptr)(const uint8_t *p_data, int p_size);
 
 protected:
+	virtual Ref<Resource> _duplicate(const DuplicateParams &p_params) const override;
+
 	static void _bind_methods();
 
 private:
@@ -425,8 +427,6 @@ public:
 	void convert_ra_rgba8_to_rg();
 	void convert_rgba8_to_bgra8();
 
-	virtual Ref<Resource> duplicate(bool p_subresources = false) const override;
-
 	UsedChannels detect_used_channels(CompressSource p_source = COMPRESS_SOURCE_GENERIC) const;
 	void optimize_channels();
 

core/io/resource.cpp

@@ -266,16 +266,43 @@ void Resource::reload_from_file() {
 	copy_from(s);
 }
 
-Variant Resource::_duplicate_recursive_for_local_scene(const Variant &p_variant, Node *p_for_scene, HashMap<Ref<Resource>, Ref<Resource>> &p_remap_cache) {
+Variant Resource::_duplicate_recursive(const Variant &p_variant, const DuplicateParams &p_params, uint32_t p_usage) const {
+	// Anything other than object can be simply skipped in case of a shallow copy.
+	if (!p_params.deep && p_variant.get_type() != Variant::OBJECT) {
+		return p_variant;
+	}
+
 	switch (p_variant.get_type()) {
 		case Variant::OBJECT: {
 			const Ref<Resource> &sr = p_variant;
-			if (sr.is_valid() && sr->is_local_to_scene()) {
-				if (p_remap_cache.has(sr)) {
-					return p_remap_cache[sr];
+			bool should_duplicate = false;
+			if (sr.is_valid()) {
+				if ((p_usage & PROPERTY_USAGE_ALWAYS_DUPLICATE)) {
+					should_duplicate = true;
+				} else if ((p_usage & PROPERTY_USAGE_NEVER_DUPLICATE)) {
+					should_duplicate = false;
+				} else if (p_params.local_scene) {
+					should_duplicate = sr->is_local_to_scene();
 				} else {
-					const Ref<Resource> &dupe = sr->duplicate_for_local_scene(p_for_scene, p_remap_cache);
-					p_remap_cache[sr] = dupe;
+					switch (p_params.subres_mode) {
+						case RESOURCE_DEEP_DUPLICATE_NONE: {
+							should_duplicate = false;
+						} break;
+						case RESOURCE_DEEP_DUPLICATE_INTERNAL: {
+							should_duplicate = p_params.deep && sr->is_built_in();
+						} break;
+						case RESOURCE_DEEP_DUPLICATE_ALL: {
+							should_duplicate = p_params.deep;
+						} break;
+					}
+				}
+			}
+			if (should_duplicate) {
+				if (p_params.remap_cache->has(sr)) {
+					return p_params.remap_cache->get(sr);
+				} else {
+					const Ref<Resource> &dupe = sr->_duplicate(p_params);
+					p_params.remap_cache->insert(sr, dupe);
 					return dupe;
 				}
 			} else {
@@ -290,7 +317,7 @@ Variant Resource::_duplicate_recursive_for_local_scene(const Variant &p_variant,
 			}
 			dst.resize(src.size());
 			for (int i = 0; i < src.size(); i++) {
-				dst[i] = _duplicate_recursive_for_local_scene(src[i], p_for_scene, p_remap_cache);
+				dst[i] = _duplicate_recursive(src[i], p_params);
 			}
 			return dst;
 		} break;
@@ -303,8 +330,8 @@ Variant Resource::_duplicate_recursive_for_local_scene(const Variant &p_variant,
 			for (const Variant &k : src.get_key_list()) {
 				const Variant &v = src[k];
 				dst.set(
-						_duplicate_recursive_for_local_scene(k, p_for_scene, p_remap_cache),
-						_duplicate_recursive_for_local_scene(v, p_for_scene, p_remap_cache));
+						_duplicate_recursive(k, p_params),
+						_duplicate_recursive(v, p_params));
 			}
 			return dst;
 		} break;
@@ -326,16 +353,20 @@ Variant Resource::_duplicate_recursive_for_local_scene(const Variant &p_variant,
 	}
 }
 
-Ref<Resource> Resource::duplicate_for_local_scene(Node *p_for_scene, HashMap<Ref<Resource>, Ref<Resource>> &p_remap_cache) {
+Ref<Resource> Resource::_duplicate(const DuplicateParams &p_params) const {
+	ERR_FAIL_COND_V_MSG(p_params.local_scene && p_params.subres_mode != RESOURCE_DEEP_DUPLICATE_MAX, Ref<Resource>(), "Duplication for local-to-scene can't specify a deep duplicate mode.");
+
 	List<PropertyInfo> plist;
 	get_property_list(&plist);
 
 	Ref<Resource> r = Object::cast_to<Resource>(ClassDB::instantiate(get_class()));
 	ERR_FAIL_COND_V(r.is_null(), Ref<Resource>());
 
-	p_remap_cache[this] = r;
+	p_params.remap_cache->insert(Ref<Resource>(this), r);
 
-	r->local_scene = p_for_scene;
+	if (p_params.local_scene) {
+		r->local_scene = p_params.local_scene;
+	}
 
 	// Duplicate script first, so the scripted properties are considered.
 	r->set_script(get_script());
@@ -349,22 +380,21 @@ Ref<Resource> Resource::duplicate_for_local_scene(Node *p_for_scene, HashMap<Ref
 		}
 
 		Variant p = get(E.name);
-
-		bool should_recurse = true;
-		if ((E.usage & PROPERTY_USAGE_NEVER_DUPLICATE) && ((Ref<Resource>)p).is_valid()) {
-			should_recurse = false;
-		}
-
-		if (should_recurse) {
-			p = _duplicate_recursive_for_local_scene(p, p_for_scene, p_remap_cache);
-		}
-
+		p = _duplicate_recursive(p, p_params, E.usage);
 		r->set(E.name, p);
 	}
 
 	return r;
 }
 
+Ref<Resource> Resource::duplicate_for_local_scene(Node *p_for_scene, HashMap<Ref<Resource>, Ref<Resource>> &p_remap_cache) const {
+	DuplicateParams params;
+	params.deep = true;
+	params.local_scene = p_for_scene;
+	params.remap_cache = &p_remap_cache;
+	return _duplicate(params);
+}
+
 void Resource::_find_sub_resources(const Variant &p_variant, HashSet<Ref<Resource>> &p_resources_found) {
 	switch (p_variant.get_type()) {
 		case Variant::ARRAY: {
@@ -418,53 +448,24 @@ void Resource::configure_for_local_scene(Node *p_for_scene, HashMap<Ref<Resource
 	}
 }
 
-Ref<Resource> Resource::duplicate(bool p_subresources) const {
-	List<PropertyInfo> plist;
-	get_property_list(&plist);
-
-	Ref<Resource> r = static_cast<Resource *>(ClassDB::instantiate(get_class()));
-	ERR_FAIL_COND_V(r.is_null(), Ref<Resource>());
-
-	for (const PropertyInfo &E : plist) {
-		if (!(E.usage & PROPERTY_USAGE_STORAGE)) {
-			continue;
-		}
-		Variant p = get(E.name);
-
-		switch (p.get_type()) {
-			case Variant::Type::DICTIONARY:
-			case Variant::Type::ARRAY:
-			case Variant::Type::PACKED_BYTE_ARRAY:
-			case Variant::Type::PACKED_COLOR_ARRAY:
-			case Variant::Type::PACKED_INT32_ARRAY:
-			case Variant::Type::PACKED_INT64_ARRAY:
-			case Variant::Type::PACKED_FLOAT32_ARRAY:
-			case Variant::Type::PACKED_FLOAT64_ARRAY:
-			case Variant::Type::PACKED_STRING_ARRAY:
-			case Variant::Type::PACKED_VECTOR2_ARRAY:
-			case Variant::Type::PACKED_VECTOR3_ARRAY:
-			case Variant::Type::PACKED_VECTOR4_ARRAY: {
-				r->set(E.name, p.duplicate(p_subresources));
-			} break;
-
-			case Variant::Type::OBJECT: {
-				if (!(E.usage & PROPERTY_USAGE_NEVER_DUPLICATE) && (p_subresources || (E.usage & PROPERTY_USAGE_ALWAYS_DUPLICATE))) {
-					Ref<Resource> sr = p;
-					if (sr.is_valid()) {
-						r->set(E.name, sr->duplicate(p_subresources));
-					}
-				} else {
-					r->set(E.name, p);
-				}
-			} break;
+Ref<Resource> Resource::duplicate(bool p_deep) const {
+	HashMap<Ref<Resource>, Ref<Resource>> remap_cache;
+	DuplicateParams params;
+	params.deep = p_deep;
+	params.subres_mode = RESOURCE_DEEP_DUPLICATE_INTERNAL;
+	params.remap_cache = &remap_cache;
+	return _duplicate(params);
+}
 
-			default: {
-				r->set(E.name, p);
-			}
-		}
-	}
+Ref<Resource> Resource::duplicate_deep(ResourceDeepDuplicateMode p_deep_subresources_mode) const {
+	ERR_FAIL_INDEX_V(p_deep_subresources_mode, RESOURCE_DEEP_DUPLICATE_MAX, Ref<Resource>());
 
-	return r;
+	HashMap<Ref<Resource>, Ref<Resource>> remap_cache;
+	DuplicateParams params;
+	params.deep = true;
+	params.subres_mode = p_deep_subresources_mode;
+	params.remap_cache = &remap_cache;
+	return _duplicate(params);
 }
 
 void Resource::_set_path(const String &p_path) {
@@ -611,7 +612,13 @@ void Resource::_bind_methods() {
 
 	ClassDB::bind_method(D_METHOD("emit_changed"), &Resource::emit_changed);
 
-	ClassDB::bind_method(D_METHOD("duplicate", "subresources"), &Resource::duplicate, DEFVAL(false));
+	ClassDB::bind_method(D_METHOD("duplicate", "deep"), &Resource::duplicate, DEFVAL(false));
+	ClassDB::bind_method(D_METHOD("duplicate_deep", "deep_subresources_mode"), &Resource::duplicate_deep, DEFVAL(RESOURCE_DEEP_DUPLICATE_INTERNAL));
+
+	BIND_ENUM_CONSTANT(RESOURCE_DEEP_DUPLICATE_NONE);
+	BIND_ENUM_CONSTANT(RESOURCE_DEEP_DUPLICATE_INTERNAL);
+	BIND_ENUM_CONSTANT(RESOURCE_DEEP_DUPLICATE_ALL);
+
 	ADD_SIGNAL(MethodInfo("changed"));
 	ADD_SIGNAL(MethodInfo("setup_local_to_scene_requested"));
 

core/io/resource.h

@@ -54,9 +54,24 @@ class Resource : public RefCounted {
 	GDCLASS(Resource, RefCounted);
 
 public:
+	enum ResourceDeepDuplicateMode {
+		RESOURCE_DEEP_DUPLICATE_NONE,
+		RESOURCE_DEEP_DUPLICATE_INTERNAL,
+		RESOURCE_DEEP_DUPLICATE_ALL,
+		RESOURCE_DEEP_DUPLICATE_MAX
+	};
+
 	static void register_custom_data_to_otdb() { ClassDB::add_resource_base_extension("res", get_class_static()); }
 	virtual String get_base_extension() const { return "res"; }
 
+protected:
+	struct DuplicateParams {
+		bool deep = false;
+		ResourceDeepDuplicateMode subres_mode = RESOURCE_DEEP_DUPLICATE_MAX;
+		Node *local_scene = nullptr;
+		HashMap<Ref<Resource>, Ref<Resource>> *remap_cache = nullptr;
+	};
+
 private:
 	friend class ResBase;
 	friend class ResourceCache;
@@ -83,7 +98,7 @@ private:
 
 	SelfList<Resource> remapped_list;
 
-	Variant _duplicate_recursive_for_local_scene(const Variant &p_variant, Node *p_for_scene, HashMap<Ref<Resource>, Ref<Resource>> &p_remap_cache);
+	Variant _duplicate_recursive(const Variant &p_variant, const DuplicateParams &p_params, uint32_t p_usage = 0) const;
 	void _find_sub_resources(const Variant &p_variant, HashSet<Ref<Resource>> &p_resources_found);
 
 protected:
@@ -104,6 +119,8 @@ protected:
 	GDVIRTUAL1C(_set_path_cache, String);
 	GDVIRTUAL0(_reset_state);
 
+	virtual Ref<Resource> _duplicate(const DuplicateParams &p_params) const;
+
 public:
 	static Node *(*_get_local_scene_func)(); //used by editor
 	static void (*_update_configuration_warning)(); //used by editor
@@ -131,8 +148,9 @@ public:
 	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, HashMap<Ref<Resource>, Ref<Resource>> &p_remap_cache);
+	Ref<Resource> duplicate(bool p_deep = false) const;
+	Ref<Resource> duplicate_deep(ResourceDeepDuplicateMode p_deep_subresources_mode = RESOURCE_DEEP_DUPLICATE_INTERNAL) const;
+	Ref<Resource> duplicate_for_local_scene(Node *p_for_scene, HashMap<Ref<Resource>, Ref<Resource>> &p_remap_cache) const;
 	void configure_for_local_scene(Node *p_for_scene, HashMap<Ref<Resource>, Ref<Resource>> &p_remap_cache);
 
 	void set_local_to_scene(bool p_enable);
@@ -168,6 +186,8 @@ public:
 	~Resource();
 };
 
+VARIANT_ENUM_CAST(Resource::ResourceDeepDuplicateMode);
+
 class ResourceCache {
 	friend class Resource;
 	friend class ResourceLoader; //need the lock

doc/classes/Resource.xml

@@ -50,15 +50,24 @@
 		</method>
 		<method name="duplicate" qualifiers="const">
 			<return type="Resource" />
-			<param index="0" name="subresources" type="bool" default="false" />
+			<param index="0" name="deep" type="bool" default="false" />
 			<description>
 				Duplicates this resource, returning a new resource with its [code]export[/code]ed or [constant PROPERTY_USAGE_STORAGE] properties copied from the original.
-				If [param subresources] is [code]false[/code], a shallow copy is returned; nested resources within subresources are not duplicated and are shared with the original resource (with one exception; see below). If [param subresources] is [code]true[/code], a deep copy is returned; nested subresources will be duplicated and are not shared (with two exceptions; see below).
-				[param subresources] is usually respected, with the following exceptions:
-				- Subresource properties with the [constant PROPERTY_USAGE_ALWAYS_DUPLICATE] flag are always duplicated.
+				If [param deep] is [code]false[/code], a [b]shallow[/b] copy is returned: nested [Array], [Dictionary], and [Resource] properties are not duplicated and are shared with the original resource.
+				If [param deep] is [code]true[/code], a [b]deep[/b] copy is returned: all nested arrays, dictionaries, and packed arrays are also duplicated (recursively). Any [Resource] found inside will only be duplicated if it's local, like [constant RESOURCE_DEEP_DUPLICATE_INTERNAL] used with [method duplicate_deep].
+				The following exceptions apply:
+				- Subresource properties with the [constant PROPERTY_USAGE_ALWAYS_DUPLICATE] flag are always duplicated (recursively or not, depending on [param deep]).
 				- Subresource properties with the [constant PROPERTY_USAGE_NEVER_DUPLICATE] flag are never duplicated.
-				- Subresources inside [Array] and [Dictionary] properties are never duplicated.
 				[b]Note:[/b] For custom resources, this method will fail if [method Object._init] has been defined with required parameters.
+				[b]Note:[/b] When duplicating with [param deep] set to [code]true[/code], each resource found, including the one on which this method is called, will be only duplicated once and referenced as many times as needed in the duplicate. For instance, if you are duplicating resource A that happens to have resource B referenced twice, you'll get a new resource A' referencing a new resource B' twice.
+			</description>
+		</method>
+		<method name="duplicate_deep" qualifiers="const">
+			<return type="Resource" />
+			<param index="0" name="deep_subresources_mode" type="int" enum="Resource.ResourceDeepDuplicateMode" default="1" />
+			<description>
+				Duplicates this resource, deeply, like [method duplicate][code](true)[/code], with extra control over how subresources are handled.
+				[param deep_subresources_mode] must be one of the values from [enum ResourceDeepDuplicateMode].
 			</description>
 		</method>
 		<method name="emit_changed">
@@ -176,4 +185,15 @@
 			</description>
 		</signal>
 	</signals>
+	<constants>
+		<constant name="RESOURCE_DEEP_DUPLICATE_NONE" value="0" enum="ResourceDeepDuplicateMode">
+			No subresorces at all are duplicated. This is useful even in a deep duplication to have all the arrays and dictionaries duplicated but still pointing to the original resources.
+		</constant>
+		<constant name="RESOURCE_DEEP_DUPLICATE_INTERNAL" value="1" enum="ResourceDeepDuplicateMode">
+			Only subresources without a path or with a scene-local path will be duplicated.
+		</constant>
+		<constant name="RESOURCE_DEEP_DUPLICATE_ALL" value="2" enum="ResourceDeepDuplicateMode">
+			Every subresource found will be duplicated, even if it has a non-local path. In other words, even potentially big resources stored separately will be duplicated.
+		</constant>
+	</constants>
 </class>