Merge pull request #91329 from warriormaster12/dont-do-things-twice

Fix a performance regression when duplicating a node

scene/main/node.cpp

@@ -2770,45 +2770,6 @@ Node *Node::_duplicate(int p_flags, HashMap<const Node *, Node *> *r_duplimap) c
 			parent->move_child(dup, pos);
 		}
 	}
-
-	for (List<const Node *>::Element *N = node_tree.front(); N; N = N->next()) {
-		Node *current_node = node->get_node(get_path_to(N->get()));
-		ERR_CONTINUE(!current_node);
-
-		if (p_flags & DUPLICATE_SCRIPTS) {
-			bool is_valid = false;
-			Variant scr = N->get()->get(script_property_name, &is_valid);
-			if (is_valid) {
-				current_node->set(script_property_name, scr);
-			}
-		}
-
-		List<PropertyInfo> plist;
-		N->get()->get_property_list(&plist);
-
-		for (const PropertyInfo &E : plist) {
-			if (!(E.usage & PROPERTY_USAGE_STORAGE)) {
-				continue;
-			}
-			String name = E.name;
-			if (name == script_property_name) {
-				continue;
-			}
-
-			Variant value = N->get()->get(name).duplicate(true);
-
-			if (E.usage & PROPERTY_USAGE_ALWAYS_DUPLICATE) {
-				Resource *res = Object::cast_to<Resource>(value);
-				if (res) { // Duplicate only if it's a resource
-					current_node->set(name, res->duplicate());
-				}
-
-			} else {
-				current_node->set(name, value);
-			}
-		}
-	}
-
 	return node;
 }
 
@@ -2820,7 +2781,7 @@ Node *Node::duplicate(int p_flags) const {
 		_duplicate_signals(this, dupe);
 	}
 
-	_duplicate_properties_node(this, this, dupe);
+	_duplicate_properties(this, this, dupe, p_flags);
 
 	return dupe;
 }
@@ -2831,7 +2792,8 @@ Node *Node::duplicate_from_editor(HashMap<const Node *, Node *> &r_duplimap) con
 }
 
 Node *Node::duplicate_from_editor(HashMap<const Node *, Node *> &r_duplimap, const HashMap<Ref<Resource>, Ref<Resource>> &p_resource_remap) const {
-	Node *dupe = _duplicate(DUPLICATE_SIGNALS | DUPLICATE_GROUPS | DUPLICATE_SCRIPTS | DUPLICATE_USE_INSTANTIATION | DUPLICATE_FROM_EDITOR, &r_duplimap);
+	int flags = DUPLICATE_SIGNALS | DUPLICATE_GROUPS | DUPLICATE_SCRIPTS | DUPLICATE_USE_INSTANTIATION | DUPLICATE_FROM_EDITOR;
+	Node *dupe = _duplicate(flags, &r_duplimap);
 
 	// This is used by SceneTreeDock's paste functionality. When pasting to foreign scene, resources are duplicated.
 	if (!p_resource_remap.is_empty()) {
@@ -2843,7 +2805,7 @@ Node *Node::duplicate_from_editor(HashMap<const Node *, Node *> &r_duplimap, con
 	// if the emitter node comes later in tree order than the receiver
 	_duplicate_signals(this, dupe);
 
-	_duplicate_properties_node(this, this, dupe);
+	_duplicate_properties(this, this, dupe, flags);
 
 	return dupe;
 }
@@ -2897,34 +2859,58 @@ void Node::remap_nested_resources(Ref<Resource> p_resource, const HashMap<Ref<Re
 }
 #endif
 
-// Duplicates properties that store a Node.
-// This has to be called after nodes have been duplicated since
-// only then do we get a full picture of how the duplicated node tree looks like.
-void Node::_duplicate_properties_node(const Node *p_root, const Node *p_original, Node *p_copy) const {
+// Duplicate node's properties.
+// This has to be called after nodes have been duplicated since there might be properties
+// of type Node that can be updated properly only if duplicated node tree is complete.
+void Node::_duplicate_properties(const Node *p_root, const Node *p_original, Node *p_copy, int p_flags) const {
 	List<PropertyInfo> props;
-	p_copy->get_property_list(&props);
+	p_original->get_property_list(&props);
+	StringName script_property_name = CoreStringNames::get_singleton()->_script;
+	if (p_flags & DUPLICATE_SCRIPTS) {
+		bool is_valid = false;
+		Variant scr = p_original->get(script_property_name, &is_valid);
+		if (is_valid) {
+			p_copy->set(script_property_name, scr);
+		}
+	}
 	for (const PropertyInfo &E : props) {
 		if (!(E.usage & PROPERTY_USAGE_STORAGE)) {
 			continue;
 		}
-		String name = E.name;
+		const StringName name = E.name;
+
+		if (name == script_property_name) {
+			continue;
+		}
+
 		Variant value = p_original->get(name).duplicate(true);
-		if (value.get_type() == Variant::OBJECT) {
-			Node *property_node = Object::cast_to<Node>(value);
-			if (property_node && (p_root == property_node || p_root->is_ancestor_of(property_node))) {
-				value = p_copy->get_node_or_null(p_original->get_path_to(property_node));
-				p_copy->set(name, value);
+
+		if (E.usage & PROPERTY_USAGE_ALWAYS_DUPLICATE) {
+			Resource *res = Object::cast_to<Resource>(value);
+			if (res) { // Duplicate only if it's a resource
+				p_copy->set(name, res->duplicate());
 			}
-		} else if (value.get_type() == Variant::ARRAY) {
-			Array arr = value;
-			if (arr.get_typed_builtin() == Variant::OBJECT) {
-				for (int i = 0; i < arr.size(); i++) {
-					Node *property_node = Object::cast_to<Node>(arr[i]);
-					if (property_node && (p_root == property_node || p_root->is_ancestor_of(property_node))) {
-						arr[i] = p_copy->get_node_or_null(p_original->get_path_to(property_node));
+		} else {
+			if (value.get_type() == Variant::OBJECT) {
+				Node *property_node = Object::cast_to<Node>(value);
+				Variant out_value = value;
+				if (property_node && (p_root == property_node || p_root->is_ancestor_of(property_node))) {
+					out_value = p_copy->get_node_or_null(p_original->get_path_to(property_node));
+				}
+				p_copy->set(name, out_value);
+			} else if (value.get_type() == Variant::ARRAY) {
+				Array arr = value;
+				if (arr.get_typed_builtin() == Variant::OBJECT) {
+					for (int i = 0; i < arr.size(); i++) {
+						Node *property_node = Object::cast_to<Node>(arr[i]);
+						if (property_node && (p_root == property_node || p_root->is_ancestor_of(property_node))) {
+							arr[i] = p_copy->get_node_or_null(p_original->get_path_to(property_node));
+						}
 					}
+					value = arr;
+					p_copy->set(name, value);
 				}
-				value = arr;
+			} else {
 				p_copy->set(name, value);
 			}
 		}
@@ -2933,7 +2919,7 @@ void Node::_duplicate_properties_node(const Node *p_root, const Node *p_original
 	for (int i = 0; i < p_original->get_child_count(); i++) {
 		Node *copy_child = p_copy->get_child(i);
 		ERR_FAIL_NULL_MSG(copy_child, "Child node disappeared while duplicating.");
-		_duplicate_properties_node(p_root, p_original->get_child(i), copy_child);
+		_duplicate_properties(p_root, p_original->get_child(i), copy_child, p_flags);
 	}
 }
 

scene/main/node.h

@@ -266,7 +266,7 @@ private:
 	void _propagate_groups_dirty();
 	Array _get_node_and_resource(const NodePath &p_path);
 
-	void _duplicate_properties_node(const Node *p_root, const Node *p_original, Node *p_copy) const;
+	void _duplicate_properties(const Node *p_root, const Node *p_original, Node *p_copy, int p_flags) const;
 	void _duplicate_signals(const Node *p_original, Node *p_copy) const;
 	Node *_duplicate(int p_flags, HashMap<const Node *, Node *> *r_duplimap = nullptr) const;