Overhaul `Variant::duplicate()` for resources

This in the scope of a duplication triggered via any type in the `Variant` realm. that is, the following: `Variant` itself, `Array` and `Dictionary`. That includes invoking `duplicate()` from scripts.

A `duplicate_deep(deep_subresources_mode)` method is added to `Variant`, `Array` and `Dictionary` (for compatibility reasons, simply adding an extra parameter was not possible). The default value for it is `RESOURCE_DEEP_DUPLICATE_NONE`, which is like calling `duplicate(true)`.

Remarks:
- The results of copying resources via those `Variant` types are exactly the same as if the copy were initiated from the `Resource` type at C++.
- In order to keep some separation between `Variant` and the higher-level animal which is `Resource`, `Variant` still contains the original code for that, so it's self-sufficient unless there's a `Resource` involved. Once the deep copy finds a `Resource` that has to be copied according to the duplication parameters, the algorithm invokes the `Resource` duplication machinery. When the stack is unwind back to a nesting level `Variant` can handle, `Variant` duplication logic keeps functioning.

While that is good from a responsibility separation standpoint, that would have a caveat: `Variant` would not be aware of the mapping between original and duplicate subresources and so wouldn't be able to keep preventing multiple duplicates.

To avoid that, this commit also introduces a wormwhole, a sharing mechanism by which `Variant` and `Resource` can collaborate in managing the lifetime of the original-to-duplicates map. The user-visible benefit is that the overduplicate prevention works as broadly as the whole `Variant` entity being copied, including all nesting levels, regardless how disconnected the data members containing resources may be across al the nesting levels. In other words, despite the aforementioned division of duties between `Variant` and `Resource` duplication logic, the duplicates map is shared among them. It's created when first finding a `Resource` and, however how deep the copy was working at that point, the map kept alive unitl the stack is unwind to the root user call, until the first step of the recursion.

Thanks to that common map of duplicates, this commit is able to fix the issue that `Resource::duplicate_for_local_scene()` used to ignore overridden duplicate logic.

core/io/resource.cpp

@@ -294,15 +294,20 @@ Variant Resource::_duplicate_recursive(const Variant &p_variant, const Duplicate
 						case RESOURCE_DEEP_DUPLICATE_ALL: {
 							should_duplicate = p_params.deep;
 						} break;
+						default: {
+							DEV_ASSERT(false);
+						}
 					}
 				}
 			}
 			if (should_duplicate) {
-				if (p_params.remap_cache->has(sr)) {
-					return p_params.remap_cache->get(sr);
+				if (thread_duplicate_remap_cache->has(sr)) {
+					return thread_duplicate_remap_cache->get(sr);
 				} else {
-					const Ref<Resource> &dupe = sr->_duplicate(p_params);
-					p_params.remap_cache->insert(sr, dupe);
+					const Ref<Resource> &dupe = p_params.local_scene
+							? sr->duplicate_for_local_scene(p_params.local_scene, *thread_duplicate_remap_cache)
+							: sr->_duplicate(p_params);
+					thread_duplicate_remap_cache->insert(sr, dupe);
 					return dupe;
 				}
 			} else {
@@ -356,20 +361,31 @@ Variant Resource::_duplicate_recursive(const Variant &p_variant, const Duplicate
 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.");
 
+	DuplicateRemapCacheT *remap_cache_backup = thread_duplicate_remap_cache;
+
+// These are for avoiding potential duplicates that can happen in custom code
+// from participating in the same duplication session (remap cache).
+#define BEFORE_USER_CODE thread_duplicate_remap_cache = nullptr;
+#define AFTER_USER_CODE thread_duplicate_remap_cache = remap_cache_backup;
+
 	List<PropertyInfo> plist;
 	get_property_list(&plist);
 
+	BEFORE_USER_CODE
 	Ref<Resource> r = Object::cast_to<Resource>(ClassDB::instantiate(get_class()));
+	AFTER_USER_CODE
 	ERR_FAIL_COND_V(r.is_null(), Ref<Resource>());
 
-	p_params.remap_cache->insert(Ref<Resource>(this), r);
+	thread_duplicate_remap_cache->insert(Ref<Resource>(this), r);
 
 	if (p_params.local_scene) {
 		r->local_scene = p_params.local_scene;
 	}
 
 	// Duplicate script first, so the scripted properties are considered.
+	BEFORE_USER_CODE
 	r->set_script(get_script());
+	AFTER_USER_CODE
 
 	for (const PropertyInfo &E : plist) {
 		if (!(E.usage & PROPERTY_USAGE_STORAGE)) {
@@ -379,20 +395,49 @@ Ref<Resource> Resource::_duplicate(const DuplicateParams &p_params) const {
 			continue;
 		}
 
+		BEFORE_USER_CODE
 		Variant p = get(E.name);
+		AFTER_USER_CODE
+
 		p = _duplicate_recursive(p, p_params, E.usage);
+
+		BEFORE_USER_CODE
 		r->set(E.name, p);
+		AFTER_USER_CODE
 	}
 
 	return r;
+
+#undef BEFORE_USER_CODE
+#undef AFTER_USER_CODE
 }
 
-Ref<Resource> Resource::duplicate_for_local_scene(Node *p_for_scene, HashMap<Ref<Resource>, Ref<Resource>> &p_remap_cache) const {
+Ref<Resource> Resource::duplicate_for_local_scene(Node *p_for_scene, DuplicateRemapCacheT &p_remap_cache) const {
+#ifdef DEBUG_ENABLED
+	// The only possibilities for the remap cache passed being valid are these:
+	// a) It's the same already used as the one of the thread. That happens when this function
+	//    is called within some recursion level within a duplication.
+	// b) There's no current thread remap cache, which means this function is acting as an entry point.
+	// This check failing means that this function is being called as an entry point during an ongoing
+	// duplication, likely due to custom instantiation or setter code. It would be an engine bug because
+	// code starting or joining a duplicate session must ensure to exit it temporarily when making calls
+	// that may in turn invoke such custom code.
+	if (thread_duplicate_remap_cache && &p_remap_cache != thread_duplicate_remap_cache) {
+		ERR_PRINT("Resource::duplicate_for_local_scene() called during an ongoing duplication session. This is an engine bug.");
+	}
+#endif
+
+	DuplicateRemapCacheT *remap_cache_backup = thread_duplicate_remap_cache;
+	thread_duplicate_remap_cache = &p_remap_cache;
+
 	DuplicateParams params;
 	params.deep = true;
 	params.local_scene = p_for_scene;
-	params.remap_cache = &p_remap_cache;
-	return _duplicate(params);
+	const Ref<Resource> &dupe = _duplicate(params);
+
+	thread_duplicate_remap_cache = remap_cache_backup;
+
+	return dupe;
 }
 
 void Resource::_find_sub_resources(const Variant &p_variant, HashSet<Ref<Resource>> &p_resources_found) {
@@ -421,7 +466,7 @@ void Resource::_find_sub_resources(const Variant &p_variant, HashSet<Ref<Resourc
 	}
 }
 
-void Resource::configure_for_local_scene(Node *p_for_scene, HashMap<Ref<Resource>, Ref<Resource>> &p_remap_cache) {
+void Resource::configure_for_local_scene(Node *p_for_scene, DuplicateRemapCacheT &p_remap_cache) {
 	List<PropertyInfo> plist;
 	get_property_list(&plist);
 
@@ -449,23 +494,89 @@ void Resource::configure_for_local_scene(Node *p_for_scene, HashMap<Ref<Resource
 }
 
 Ref<Resource> Resource::duplicate(bool p_deep) const {
-	HashMap<Ref<Resource>, Ref<Resource>> remap_cache;
+	DuplicateRemapCacheT remap_cache;
+	bool started_session = false;
+	if (!thread_duplicate_remap_cache) {
+		thread_duplicate_remap_cache = &remap_cache;
+		started_session = true;
+	}
+
 	DuplicateParams params;
 	params.deep = p_deep;
 	params.subres_mode = RESOURCE_DEEP_DUPLICATE_INTERNAL;
-	params.remap_cache = &remap_cache;
-	return _duplicate(params);
+	const Ref<Resource> &dupe = _duplicate(params);
+
+	if (started_session) {
+		thread_duplicate_remap_cache = nullptr;
+	}
+
+	return dupe;
 }
 
 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>());
 
-	HashMap<Ref<Resource>, Ref<Resource>> remap_cache;
+	DuplicateRemapCacheT remap_cache;
+	bool started_session = false;
+	if (!thread_duplicate_remap_cache) {
+		thread_duplicate_remap_cache = &remap_cache;
+		started_session = true;
+	}
+
 	DuplicateParams params;
 	params.deep = true;
 	params.subres_mode = p_deep_subresources_mode;
-	params.remap_cache = &remap_cache;
-	return _duplicate(params);
+	const Ref<Resource> &dupe = _duplicate(params);
+
+	if (started_session) {
+		thread_duplicate_remap_cache = nullptr;
+	}
+
+	return dupe;
+}
+
+Ref<Resource> Resource::_duplicate_from_variant(bool p_deep, ResourceDeepDuplicateMode p_deep_subresources_mode, int p_recursion_count) const {
+	// A call without deep duplication would have been early-rejected at Variant::duplicate() unless it's the root call.
+	DEV_ASSERT(!(p_recursion_count > 0 && p_deep_subresources_mode == RESOURCE_DEEP_DUPLICATE_NONE));
+
+	// When duplicating from Variant, this function may be called multiple times from
+	// different parts of the data structure being copied. Therefore, we need to create
+	// a remap cache instance in a way that can be shared among all of the calls.
+	// Whatever Variant, Array or Dictionary that initiated the call chain will eventually
+	// claim it, when the stack unwinds up to the root call.
+	// One exception is that this is the root call.
+
+	if (p_recursion_count == 0) {
+		if (p_deep) {
+			return duplicate_deep(p_deep_subresources_mode);
+		} else {
+			return duplicate(false);
+		}
+	}
+
+	if (thread_duplicate_remap_cache) {
+		Resource::DuplicateRemapCacheT::Iterator E = thread_duplicate_remap_cache->find(Ref<Resource>(this));
+		if (E) {
+			return E->value;
+		}
+	} else {
+		thread_duplicate_remap_cache = memnew(DuplicateRemapCacheT);
+	}
+
+	DuplicateParams params;
+	params.deep = p_deep;
+	params.subres_mode = p_deep_subresources_mode;
+
+	const Ref<Resource> dupe = _duplicate(params);
+
+	return dupe;
+}
+
+void Resource::_teardown_duplicate_from_variant() {
+	if (thread_duplicate_remap_cache) {
+		memdelete(thread_duplicate_remap_cache);
+		thread_duplicate_remap_cache = nullptr;
+	}
 }
 
 void Resource::_set_path(const String &p_path) {
@@ -615,9 +726,10 @@ void Resource::_bind_methods() {
 	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);
+	// For the bindings, it's much more natural to expose this enum from the Variant realm via Resource.
+	ClassDB::bind_integer_constant(get_class_static(), StringName("ResourceDeepDuplicateMode"), "RESOURCE_DEEP_DUPLICATE_NONE", RESOURCE_DEEP_DUPLICATE_NONE);
+	ClassDB::bind_integer_constant(get_class_static(), StringName("ResourceDeepDuplicateMode"), "RESOURCE_DEEP_DUPLICATE_INTERNAL", RESOURCE_DEEP_DUPLICATE_INTERNAL);
+	ClassDB::bind_integer_constant(get_class_static(), StringName("ResourceDeepDuplicateMode"), "RESOURCE_DEEP_DUPLICATE_ALL", RESOURCE_DEEP_DUPLICATE_ALL);
 
 	ADD_SIGNAL(MethodInfo("changed"));
 	ADD_SIGNAL(MethodInfo("setup_local_to_scene_requested"));

core/io/resource.h

@@ -54,13 +54,6 @@ 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"; }
 
@@ -69,7 +62,6 @@ protected:
 		bool deep = false;
 		ResourceDeepDuplicateMode subres_mode = RESOURCE_DEEP_DUPLICATE_MAX;
 		Node *local_scene = nullptr;
-		HashMap<Ref<Resource>, Ref<Resource>> *remap_cache = nullptr;
 	};
 
 private:
@@ -98,6 +90,9 @@ private:
 
 	SelfList<Resource> remapped_list;
 
+	using DuplicateRemapCacheT = HashMap<Ref<Resource>, Ref<Resource>>;
+	static thread_local inline DuplicateRemapCacheT *thread_duplicate_remap_cache = nullptr;
+
 	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);
 
@@ -150,6 +145,8 @@ public:
 
 	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_from_variant(bool p_deep, ResourceDeepDuplicateMode p_deep_subresources_mode, int p_recursion_count) const;
+	static void _teardown_duplicate_from_variant();
 	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);
 
@@ -186,7 +183,7 @@ public:
 	~Resource();
 };
 
-VARIANT_ENUM_CAST(Resource::ResourceDeepDuplicateMode);
+VARIANT_ENUM_CAST(ResourceDeepDuplicateMode);
 
 class ResourceCache {
 	friend class Resource;

core/variant/array.cpp

@@ -37,7 +37,6 @@
 #include "core/templates/vector.h"
 #include "core/variant/callable.h"
 #include "core/variant/dictionary.h"
-#include "core/variant/variant.h"
 
 struct ArrayPrivate {
 	SafeRefCount refcount;
@@ -518,10 +517,14 @@ const Variant &Array::get(int p_idx) const {
 }
 
 Array Array::duplicate(bool p_deep) const {
-	return recursive_duplicate(p_deep, 0);
+	return recursive_duplicate(p_deep, RESOURCE_DEEP_DUPLICATE_NONE, 0);
 }
 
-Array Array::recursive_duplicate(bool p_deep, int recursion_count) const {
+Array Array::duplicate_deep(ResourceDeepDuplicateMode p_deep_subresources_mode) const {
+	return recursive_duplicate(true, p_deep_subresources_mode, 0);
+}
+
+Array Array::recursive_duplicate(bool p_deep, ResourceDeepDuplicateMode p_deep_subresources_mode, int recursion_count) const {
 	Array new_arr;
 	new_arr._p->typed = _p->typed;
 
@@ -531,12 +534,19 @@ Array Array::recursive_duplicate(bool p_deep, int recursion_count) const {
 	}
 
 	if (p_deep) {
+		bool is_call_chain_end = recursion_count == 0;
+
 		recursion_count++;
 		int element_count = size();
 		new_arr.resize(element_count);
 		Variant *write = new_arr._p->array.ptrw();
 		for (int i = 0; i < element_count; i++) {
-			write[i] = get(i).recursive_duplicate(true, recursion_count);
+			write[i] = get(i).recursive_duplicate(true, p_deep_subresources_mode, recursion_count);
+		}
+
+		// Variant::recursive_duplicate() may have created a remap cache by now.
+		if (is_call_chain_end) {
+			Resource::_teardown_duplicate_from_variant();
 		}
 	} else {
 		new_arr._p->array = _p->array;

core/variant/array.h

@@ -31,6 +31,7 @@
 #pragma once
 
 #include "core/typedefs.h"
+#include "core/variant/variant_deep_duplicate.h"
 
 #include <climits>
 #include <initializer_list>
@@ -164,7 +165,8 @@ public:
 	Variant pop_at(int p_pos);
 
 	Array duplicate(bool p_deep = false) const;
-	Array recursive_duplicate(bool p_deep, int recursion_count) const;
+	Array duplicate_deep(ResourceDeepDuplicateMode p_deep_subresources_mode = RESOURCE_DEEP_DUPLICATE_INTERNAL) const;
+	Array recursive_duplicate(bool p_deep, ResourceDeepDuplicateMode p_deep_subresources_mode, int recursion_count) const;
 
 	Array slice(int p_begin, int p_end = INT_MAX, int p_step = 1, bool p_deep = false) const;
 	Array filter(const Callable &p_callable) const;

core/variant/dictionary.cpp

@@ -569,7 +569,11 @@ const Variant *Dictionary::next(const Variant *p_key) const {
 }
 
 Dictionary Dictionary::duplicate(bool p_deep) const {
-	return recursive_duplicate(p_deep, 0);
+	return recursive_duplicate(p_deep, RESOURCE_DEEP_DUPLICATE_NONE, 0);
+}
+
+Dictionary Dictionary::duplicate_deep(ResourceDeepDuplicateMode p_deep_subresources_mode) const {
+	return recursive_duplicate(true, p_deep_subresources_mode, 0);
 }
 
 void Dictionary::make_read_only() {
@@ -581,7 +585,7 @@ bool Dictionary::is_read_only() const {
 	return _p->read_only != nullptr;
 }
 
-Dictionary Dictionary::recursive_duplicate(bool p_deep, int recursion_count) const {
+Dictionary Dictionary::recursive_duplicate(bool p_deep, ResourceDeepDuplicateMode p_deep_subresources_mode, int recursion_count) const {
 	Dictionary n;
 	n._p->typed_key = _p->typed_key;
 	n._p->typed_value = _p->typed_value;
@@ -592,9 +596,16 @@ Dictionary Dictionary::recursive_duplicate(bool p_deep, int recursion_count) con
 	}
 
 	if (p_deep) {
+		bool is_call_chain_end = recursion_count == 0;
+
 		recursion_count++;
 		for (const KeyValue<Variant, Variant> &E : _p->variant_map) {
-			n[E.key.recursive_duplicate(true, recursion_count)] = E.value.recursive_duplicate(true, recursion_count);
+			n[E.key.recursive_duplicate(true, p_deep_subresources_mode, recursion_count)] = E.value.recursive_duplicate(true, p_deep_subresources_mode, recursion_count);
+		}
+
+		// Variant::recursive_duplicate() may have created a remap cache by now.
+		if (is_call_chain_end) {
+			Resource::_teardown_duplicate_from_variant();
 		}
 	} else {
 		for (const KeyValue<Variant, Variant> &E : _p->variant_map) {

core/variant/dictionary.h

@@ -35,6 +35,7 @@
 #include "core/templates/local_vector.h"
 #include "core/templates/pair.h"
 #include "core/variant/array.h"
+#include "core/variant/variant_deep_duplicate.h"
 
 class Variant;
 
@@ -98,7 +99,8 @@ public:
 	Array values() const;
 
 	Dictionary duplicate(bool p_deep = false) const;
-	Dictionary recursive_duplicate(bool p_deep, int recursion_count) const;
+	Dictionary duplicate_deep(ResourceDeepDuplicateMode p_deep_subresources_mode = RESOURCE_DEEP_DUPLICATE_INTERNAL) const;
+	Dictionary recursive_duplicate(bool p_deep, ResourceDeepDuplicateMode p_deep_subresources_mode, int recursion_count) const;
 
 	void set_typed(const ContainerType &p_key_type, const ContainerType &p_value_type);
 	void set_typed(uint32_t p_key_type, const StringName &p_key_class_name, const Variant &p_key_script, uint32_t p_value_type, const StringName &p_value_class_name, const Variant &p_value_script);

core/variant/variant.h

@@ -61,6 +61,7 @@
 #include "core/variant/array.h"
 #include "core/variant/callable.h"
 #include "core/variant/dictionary.h"
+#include "core/variant/variant_deep_duplicate.h"
 
 class Object;
 class RefCounted;
@@ -612,7 +613,8 @@ public:
 
 	void zero();
 	Variant duplicate(bool p_deep = false) const;
-	Variant recursive_duplicate(bool p_deep, int recursion_count) const;
+	Variant duplicate_deep(ResourceDeepDuplicateMode p_deep_subresources_mode = RESOURCE_DEEP_DUPLICATE_INTERNAL) const;
+	Variant recursive_duplicate(bool p_deep, ResourceDeepDuplicateMode p_deep_subresources_mode, int recursion_count) const;
 
 	/* Built-In Methods */
 

core/variant/variant_call.cpp

@@ -2398,6 +2398,7 @@ static void _register_variant_builtin_methods_misc() {
 	bind_method(Dictionary, keys, sarray(), varray());
 	bind_method(Dictionary, values, sarray(), varray());
 	bind_method(Dictionary, duplicate, sarray("deep"), varray(false));
+	bind_method(Dictionary, duplicate_deep, sarray("deep_subresources_mode"), varray(RESOURCE_DEEP_DUPLICATE_INTERNAL));
 	bind_method(Dictionary, get, sarray("key", "default"), varray(Variant()));
 	bind_method(Dictionary, get_or_add, sarray("key", "default"), varray(Variant()));
 	bind_method(Dictionary, set, sarray("key", "value"), varray());
@@ -2456,6 +2457,7 @@ static void _register_variant_builtin_methods_array() {
 	bind_method(Array, bsearch_custom, sarray("value", "func", "before"), varray(true));
 	bind_method(Array, reverse, sarray(), varray());
 	bind_method(Array, duplicate, sarray("deep"), varray(false));
+	bind_method(Array, duplicate_deep, sarray("deep_subresources_mode"), varray(RESOURCE_DEEP_DUPLICATE_INTERNAL));
 	bind_method(Array, slice, sarray("begin", "end", "step", "deep"), varray(INT_MAX, 1, false));
 	bind_method(Array, filter, sarray("method"), varray());
 	bind_method(Array, map, sarray("method"), varray());

core/variant/variant_deep_duplicate.h

@@ -0,0 +1,41 @@
+/**************************************************************************/
+/*  variant_deep_duplicate.h                                              */
+/**************************************************************************/
+/*                         This file is part of:                          */
+/*                             GODOT ENGINE                               */
+/*                        https://godotengine.org                         */
+/**************************************************************************/
+/* Copyright (c) 2014-present Godot Engine contributors (see AUTHORS.md). */
+/* Copyright (c) 2007-2014 Juan Linietsky, Ariel Manzur.                  */
+/*                                                                        */
+/* Permission is hereby granted, free of charge, to any person obtaining  */
+/* a copy of this software and associated documentation files (the        */
+/* "Software"), to deal in the Software without restriction, including    */
+/* without limitation the rights to use, copy, modify, merge, publish,    */
+/* distribute, sublicense, and/or sell copies of the Software, and to     */
+/* permit persons to whom the Software is furnished to do so, subject to  */
+/* the following conditions:                                              */
+/*                                                                        */
+/* The above copyright notice and this permission notice shall be         */
+/* included in all copies or substantial portions of the Software.        */
+/*                                                                        */
+/* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,        */
+/* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF     */
+/* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. */
+/* IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY   */
+/* CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,   */
+/* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE      */
+/* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.                 */
+/**************************************************************************/
+
+#pragma once
+
+// This would be ideally declared nested in Variant, but that would cause circular
+// includes with Array and Dictionary, for instance.
+// Also, this enum is be exposed via Resource.
+enum ResourceDeepDuplicateMode {
+	RESOURCE_DEEP_DUPLICATE_NONE,
+	RESOURCE_DEEP_DUPLICATE_INTERNAL,
+	RESOURCE_DEEP_DUPLICATE_ALL,
+	RESOURCE_DEEP_DUPLICATE_MAX
+};

core/variant/variant_setget.cpp

@@ -29,9 +29,10 @@
 /**************************************************************************/
 
 #include "variant_setget.h"
-
 #include "variant_callable.h"
 
+#include "core/io/resource.h"
+
 struct VariantSetterGetterInfo {
 	void (*setter)(Variant *base, const Variant *value, bool &valid);
 	void (*getter)(const Variant *base, Variant *value);
@@ -1969,26 +1970,33 @@ Variant Variant::iter_get(const Variant &r_iter, bool &r_valid) const {
 }
 
 Variant Variant::duplicate(bool p_deep) const {
-	return recursive_duplicate(p_deep, 0);
+	return recursive_duplicate(p_deep, RESOURCE_DEEP_DUPLICATE_NONE, 0);
+}
+
+Variant Variant::duplicate_deep(ResourceDeepDuplicateMode p_deep_subresources_mode) const {
+	ERR_FAIL_INDEX_V(p_deep_subresources_mode, RESOURCE_DEEP_DUPLICATE_MAX, Variant());
+	return recursive_duplicate(true, p_deep_subresources_mode, 0);
 }
 
-Variant Variant::recursive_duplicate(bool p_deep, int recursion_count) const {
+Variant Variant::recursive_duplicate(bool p_deep, ResourceDeepDuplicateMode p_deep_subresources_mode, int recursion_count) const {
 	switch (type) {
 		case OBJECT: {
-			/*  breaks stuff :(
-			if (p_deep && !_get_obj().ref.is_null()) {
-				Ref<Resource> resource = _get_obj().ref;
-				if (resource.is_valid()) {
-					return resource->duplicate(true);
-				}
+			// If the root target of duplicate() is a Resource, we can't early-reject because that
+			// resource itself must be duplicated, much as if Resource::duplicate() had been called.
+			if (p_deep_subresources_mode == RESOURCE_DEEP_DUPLICATE_NONE && recursion_count > 0) {
+				return *this;
+			}
+			Resource *res = Object::cast_to<Resource>(_get_obj().obj);
+			if (res) {
+				return res->_duplicate_from_variant(p_deep, p_deep_subresources_mode, recursion_count);
+			} else {
+				return *this;
 			}
-			*/
-			return *this;
 		} break;
 		case DICTIONARY:
-			return operator Dictionary().recursive_duplicate(p_deep, recursion_count);
+			return operator Dictionary().recursive_duplicate(p_deep, p_deep_subresources_mode, recursion_count);
 		case ARRAY:
-			return operator Array().recursive_duplicate(p_deep, recursion_count);
+			return operator Array().recursive_duplicate(p_deep, p_deep_subresources_mode, recursion_count);
 		case PACKED_BYTE_ARRAY:
 			return operator Vector<uint8_t>().duplicate();
 		case PACKED_INT32_ARRAY:

doc/classes/Array.xml

@@ -332,7 +332,16 @@
 			<param index="0" name="deep" type="bool" default="false" />
 			<description>
 				Returns a new copy of the array.
-				By default, a [b]shallow[/b] copy is returned: all nested [Array] and [Dictionary] elements are shared with the original array. Modifying them in one array will also affect them in the other.[br]If [param deep] is [code]true[/code], a [b]deep[/b] copy is returned: all nested arrays and dictionaries are also duplicated (recursively).
+				By default, a [b]shallow[/b] copy is returned: all nested [Array], [Dictionary], and [Resource] elements are shared with the original array. Modifying them in one array will also affect them in the other.
+				If [param deep] is [code]true[/code], a [b]deep[/b] copy is returned: all nested arrays and dictionaries are also duplicated (recursively). Any [Resource] is still shared with the original array, though.
+			</description>
+		</method>
+		<method name="duplicate_deep" qualifiers="const">
+			<return type="Array" />
+			<param index="0" name="deep_subresources_mode" type="int" default="1" />
+			<description>
+				Duplicates this array, 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 Resource.ResourceDeepDuplicateMode]. By default, only internal resources will be duplicated (recursively).
 			</description>
 		</method>
 		<method name="erase">

doc/classes/Dictionary.xml

@@ -187,7 +187,17 @@
 			<return type="Dictionary" />
 			<param index="0" name="deep" type="bool" default="false" />
 			<description>
-				Creates and returns a new copy of the dictionary. If [param deep] is [code]true[/code], inner [Dictionary] and [Array] keys and values are also copied, recursively.
+				Returns a new copy of the dictionary.
+				By default, a [b]shallow[/b] copy is returned: all nested [Array], [Dictionary], and [Resource] keys and values are shared with the original dictionary. Modifying them in one dictionary will also affect them in the other.
+				If [param deep] is [code]true[/code], a [b]deep[/b] copy is returned: all nested arrays and dictionaries are also duplicated (recursively). Any [Resource] is still shared with the original dictionary, though.
+			</description>
+		</method>
+		<method name="duplicate_deep" qualifiers="const">
+			<return type="Dictionary" />
+			<param index="0" name="deep_subresources_mode" type="int" default="1" />
+			<description>
+				Duplicates this dictionary, 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 Resource.ResourceDeepDuplicateMode]. By default, only internal resources will be duplicated (recursively).
 			</description>
 		</method>
 		<method name="erase">

doc/classes/Resource.xml

@@ -64,7 +64,7 @@
 		</method>
 		<method name="duplicate_deep" qualifiers="const">
 			<return type="Resource" />
-			<param index="0" name="deep_subresources_mode" type="int" enum="Resource.ResourceDeepDuplicateMode" default="1" />
+			<param index="0" name="deep_subresources_mode" type="int" enum="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].

scene/main/node.cpp

@@ -3160,7 +3160,11 @@ void Node::_duplicate_properties(const Node *p_root, const Node *p_original, Nod
 			continue;
 		}
 
-		Variant value = p_original->get(name).duplicate(true);
+		Variant value = p_original->get(name);
+		// To keep classic behavior, because, in contrast, nowadays a resource would be duplicated.
+		if (value.get_type() != Variant::OBJECT) {
+			value = value.duplicate(true);
+		}
 
 		if (E.usage & PROPERTY_USAGE_ALWAYS_DUPLICATE) {
 			Resource *res = Object::cast_to<Resource>(value);