Variant: Add support for duplicating Objects

To do so, Object gets a new virtual `object_duplicate` (`duplicate` already taken by incompatible implementations in derived classes) which is implemented in `Node` and `Resource`. This allows fixing godotengine#36372 properly in `ClassDB` by duplicating Objects that need it to avoid using the same one as default value. (So far it seems only Path2D/Path3D's `curve` property uses this.) Fixes godotengine#36372. Fixes godotengine#36650. Supersedes godotengine#36644 and godotengine#36656.
akien-mga · Jun 9, 2020 · 1b3bfb4 · 1b3bfb4
1 parent ae21b5d
commit 1b3bfb4
Show file tree

Hide file tree

Showing 7 changed files with 33 additions and 8 deletions.
diff --git a/core/class_db.cpp b/core/class_db.cpp
@@ -1386,7 +1386,17 @@ Variant ClassDB::class_get_default_property_value(const StringName &p_class, con
 	if (r_valid != nullptr) {
 		*r_valid = true;
 	}
-	return default_values[p_class][p_property];
+
+	Variant var = default_values[p_class][p_property];
+
+	// Some properties have an object instance as default value (e.g. Path2D::curve).
+	// We don't want the default value to point to the same instance for different
+	// objects, so we have to duplicate it.
+	// And while at it, we might also duplicate any Variant to avoid sharing Dictionary
+	// or Array default values.
+	var = var.duplicate(true);
+
+	return var;
 }
 
 RWLock *ClassDB::lock = nullptr;

diff --git a/core/object.h b/core/object.h
@@ -662,6 +662,8 @@ class Object {
 	void notification(int p_notification, bool p_reversed = false);
 	String to_string();
 
+	virtual Object *object_duplicate() const { return (Object *)this; };
+
 	//used mainly by script, get and set all INCLUDING string
 	virtual Variant getvar(const Variant &p_key, bool *r_valid = nullptr) const;
 	virtual void setvar(const Variant &p_key, const Variant &p_value, bool *r_valid = nullptr);

diff --git a/core/resource.cpp b/core/resource.cpp
@@ -233,6 +233,13 @@ Ref<Resource> Resource::duplicate(bool p_subresources) const {
 	return Ref<Resource>(r);
 }
 
+Object *Resource::object_duplicate() const {
+	// Doesn't work for some reason, not sure why.
+	//Resource *res = duplicate(true).ptr();
+	Resource *res = (Resource *)ClassDB::instance(get_class());
+	return res;
+}
+
 void Resource::_set_path(const String &p_path) {
 	set_path(p_path, false);
 }

diff --git a/core/resource.h b/core/resource.h
@@ -101,10 +101,12 @@ class Resource : public Reference {
 	void set_subindex(int p_sub_index);
 	int get_subindex() 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);
 	void configure_for_local_scene(Node *p_for_scene, Map<Ref<Resource>, Ref<Resource>> &remap_cache);
 
+	virtual Ref<Resource> duplicate(bool p_subresources = false) const;
+	virtual Object *object_duplicate() const;
+
 	void set_local_to_scene(bool p_enable);
 	bool is_local_to_scene() const;
 	virtual void setup_local_to_scene();

diff --git a/core/variant_op.cpp b/core/variant_op.cpp
@@ -4032,14 +4032,12 @@ Variant Variant::iter_get(const Variant &r_iter, bool &r_valid) const {
 Variant Variant::duplicate(bool deep) const {
 	switch (type) {
 		case OBJECT: {
-			/*  breaks stuff :(
-			if (deep && !_get_obj().ref.is_null()) {
-				Ref<Resource> resource = _get_obj().ref;
-				if (resource.is_valid()) {
-					return resource->duplicate(true);
+			if (deep) {
+				Object *obj = get_validated_object();
+				if (obj) {
+					return obj->object_duplicate();
 				}
 			}
-			*/
 			return *this;
 		} break;
 		case DICTIONARY:

diff --git a/scene/main/node.cpp b/scene/main/node.cpp
@@ -2324,6 +2324,10 @@ Node *Node::duplicate_and_reown(const Map<Node *, Node *> &p_reown_map) const {
 	return node;
 }
 
+Object *Node::object_duplicate() const {
+	return duplicate(DUPLICATE_USE_INSTANCING | DUPLICATE_SIGNALS | DUPLICATE_GROUPS | DUPLICATE_SCRIPTS);
+}
+
 static void find_owned_by(Node *p_by, Node *p_node, List<Node *> *p_owned) {
 	if (p_node->get_owner() == p_by) {
 		p_owned->push_back(p_node);

diff --git a/scene/main/node.h b/scene/main/node.h
@@ -370,6 +370,8 @@ class Node : public Object {
 	Node *duplicate_from_editor(Map<const Node *, Node *> &r_duplimap) const;
 #endif
 
+	virtual Object *object_duplicate() const;
+
 	// used by editors, to save what has changed only
 	void set_scene_instance_state(const Ref<SceneState> &p_state);
 	Ref<SceneState> get_scene_instance_state() const;