Merge pull request #83343 from warriormaster12/node-property-duplication

Fix export variable of type Node pointing to a wrong child node when duplicating
godotengine · Dec 8, 2023 · 13305d3 · 13305d3
2 parents dfe0f58 + de1dc6c
commit 13305d3
Show file tree

Hide file tree

Showing 2 changed files with 77 additions and 38 deletions.
diff --git a/scene/main/node.cpp b/scene/main/node.cpp
@@ -2528,44 +2528,6 @@ Node *Node::_duplicate(int p_flags, HashMap<const Node *, Node *> *r_duplimap) c
 		}
 	}
 
-	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);
-			}
-		}
-	}
-
 	if (get_name() != String()) {
 		node->set_name(get_name());
 	}
@@ -2631,6 +2593,62 @@ Node *Node::_duplicate(int p_flags, HashMap<const Node *, Node *> *r_duplimap) c
 		}
 	}
 
+	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 {
+				// If property points to a node which is owned by a node we are duplicating, update its path.
+				if (value.get_type() == Variant::OBJECT) {
+					Node *property_node = Object::cast_to<Node>(value);
+					if (property_node && is_ancestor_of(property_node)) {
+						value = current_node->get_node_or_null(get_path_to(property_node));
+					}
+				} 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 && is_ancestor_of(property_node)) {
+								arr[i] = current_node->get_node_or_null(get_path_to(property_node));
+							}
+						}
+						value = arr;
+					}
+				}
+				current_node->set(name, value);
+			}
+		}
+	}
+
 	return node;
 }
 

diff --git a/scene/property_utils.cpp b/scene/property_utils.cpp
@@ -74,10 +74,31 @@ Variant PropertyUtils::get_property_default_value(const Object *p_object, const
 			const SceneState::PackState &ia = states_stack[i];
 			bool found = false;
 			Variant value_in_ancestor = ia.state->get_property_value(ia.node, p_property, found);
+			const Vector<String> &deferred_properties = ia.state->get_node_deferred_nodepath_properties(ia.node);
 			if (found) {
 				if (r_is_valid) {
 					*r_is_valid = true;
 				}
+				// Replace properties stored as NodePaths with actual Nodes.
+				// Otherwise, the property value would be considered as overridden.
+				if (deferred_properties.has(p_property)) {
+					if (value_in_ancestor.get_type() == Variant::ARRAY) {
+						Array paths = value_in_ancestor;
+
+						bool valid = false;
+						Array array = node->get(p_property, &valid);
+						ERR_CONTINUE(!valid);
+						array = array.duplicate();
+
+						array.resize(paths.size());
+						for (int j = 0; j < array.size(); j++) {
+							array.set(j, node->get_node_or_null(paths[j]));
+						}
+						value_in_ancestor = array;
+					} else {
+						value_in_ancestor = node->get_node_or_null(value_in_ancestor);
+					}
+				}
 				return value_in_ancestor;
 			}
 			// Save script for later