From 74cc9e8d93f1ad73bdec528928255493341a2f26 Mon Sep 17 00:00:00 2001 From: kobewi Date: Fri, 21 Jun 2024 15:45:55 +0200 Subject: [PATCH] Fix storing of Node Array properties --- scene/property_utils.cpp | 18 ++++++++++++ tests/scene/test_node.h | 59 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 75 insertions(+), 2 deletions(-) diff --git a/scene/property_utils.cpp b/scene/property_utils.cpp index db17f9d64364..94a037bd9b81 100644 --- a/scene/property_utils.cpp +++ b/scene/property_utils.cpp @@ -44,6 +44,7 @@ bool PropertyUtils::is_property_value_different(const Object *p_object, const Va // This must be done because, as some scenes save as text, there might be a tiny difference in floats due to numerical error. return !Math::is_equal_approx((float)p_a, (float)p_b); } else if (p_a.get_type() == Variant::NODE_PATH && p_b.get_type() == Variant::OBJECT) { + // With properties of type Node, left side is NodePath, while right side is Node. const Node *base_node = Object::cast_to(p_object); const Node *target_node = Object::cast_to(p_b); if (base_node && target_node) { @@ -51,6 +52,23 @@ bool PropertyUtils::is_property_value_different(const Object *p_object, const Va } } + if (p_a.get_type() == Variant::ARRAY && p_b.get_type() == Variant::ARRAY) { + const Node *base_node = Object::cast_to(p_object); + Array array1 = p_a; + Array array2 = p_b; + + if (base_node && !array1.is_empty() && array2.size() == array1.size() && array1[0].get_type() == Variant::NODE_PATH && array2[0].get_type() == Variant::OBJECT) { + // Like above, but NodePaths/Nodes are inside arrays. + for (int i = 0; i < array1.size(); i++) { + const Node *target_node = Object::cast_to(array2[i]); + if (!target_node || array1[i] != base_node->get_path_to(target_node)) { + return true; + } + } + return false; + } + } + // For our purposes, treating null object as NIL is the right thing to do const Variant &a = p_a.get_type() == Variant::OBJECT && (Object *)p_a == nullptr ? Variant() : p_a; const Variant &b = p_b.get_type() == Variant::OBJECT && (Object *)p_b == nullptr ? Variant() : p_b; diff --git a/tests/scene/test_node.h b/tests/scene/test_node.h index 93ef3fe7282f..05764d8f29fd 100644 --- a/tests/scene/test_node.h +++ b/tests/scene/test_node.h @@ -68,6 +68,10 @@ class TestNode : public Node { ClassDB::bind_method(D_METHOD("set_exported_node", "node"), &TestNode::set_exported_node); ClassDB::bind_method(D_METHOD("get_exported_node"), &TestNode::get_exported_node); ADD_PROPERTY(PropertyInfo(Variant::OBJECT, "exported_node", PROPERTY_HINT_NODE_TYPE, "Node"), "set_exported_node", "get_exported_node"); + + ClassDB::bind_method(D_METHOD("set_exported_nodes", "node"), &TestNode::set_exported_nodes); + ClassDB::bind_method(D_METHOD("get_exported_nodes"), &TestNode::get_exported_nodes); + ADD_PROPERTY(PropertyInfo(Variant::ARRAY, "exported_nodes", PROPERTY_HINT_TYPE_STRING, "24/34:Node"), "set_exported_nodes", "get_exported_nodes"); } private: @@ -84,11 +88,15 @@ class TestNode : public Node { int physics_process_counter = 0; Node *exported_node = nullptr; + Array exported_nodes; List *callback_list = nullptr; void set_exported_node(Node *p_node) { exported_node = p_node; } Node *get_exported_node() const { return exported_node; } + + void set_exported_nodes(const Array &p_nodes) { exported_nodes = p_nodes; } + Array get_exported_nodes() const { return exported_nodes; } }; TEST_CASE("[SceneTree][Node] Testing node operations with a very simple scene tree") { @@ -500,7 +508,16 @@ TEST_CASE("[SceneTree][Node]Exported node checks") { node->add_child(child); child->set_owner(node); + Node *child2 = memnew(Node); + child2->set_name("Child2"); + node->add_child(child2); + child2->set_owner(node); + + Array children; + children.append(child); + node->set("exported_node", child); + node->set("exported_nodes", children); SUBCASE("Property of duplicated node should point to duplicated child") { GDREGISTER_CLASS(TestNode); @@ -512,8 +529,7 @@ TEST_CASE("[SceneTree][Node]Exported node checks") { memdelete(dup); } - SUBCASE("Saving instance with exported node should not store the unchanged property") { - node->set_process_mode(Node::PROCESS_MODE_ALWAYS); + SUBCASE("Saving instance with exported nodes should not store the unchanged property") { Ref ps; ps.instantiate(); ps->pack(node); @@ -548,6 +564,45 @@ TEST_CASE("[SceneTree][Node]Exported node checks") { CHECK_FALSE(is_wrong); } + SUBCASE("Saving instance with exported nodes should store property if changed") { + Ref ps; + ps.instantiate(); + ps->pack(node); + + String scene_path = TestUtils::get_temp_path("test_scene.tscn"); + ps->set_path(scene_path); + + Node *root = memnew(Node); + + Node *sub_child = ps->instantiate(PackedScene::GEN_EDIT_STATE_MAIN); + root->add_child(sub_child); + sub_child->set_owner(root); + + sub_child->set("exported_node", sub_child->get_child(1)); + + children = Array(); + children.append(sub_child->get_child(1)); + sub_child->set("exported_nodes", children); + + Ref ps2; + ps2.instantiate(); + ps2->pack(root); + + scene_path = TestUtils::get_temp_path("new_test_scene2.tscn"); + ResourceSaver::save(ps2, scene_path); + memdelete(root); + + int stored_properties = 0; + Ref fa = FileAccess::open(scene_path, FileAccess::READ); + while (!fa->eof_reached()) { + const String line = fa->get_line(); + if (line.begins_with("exported_node")) { + stored_properties++; + } + } + CHECK_EQ(stored_properties, 2); + } + memdelete(node); }