Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix storing of Node Array properties #93430

Merged
merged 1 commit into from
Jun 22, 2024
Merged

Fix storing of Node Array properties #93430

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions scene/property_utils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,31 @@ 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<Node>(p_object);
const Node *target_node = Object::cast_to<Node>(p_b);
if (base_node && target_node) {
return p_a != base_node->get_path_to(target_node);
}
}

if (p_a.get_type() == Variant::ARRAY && p_b.get_type() == Variant::ARRAY) {
const Node *base_node = Object::cast_to<Node>(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<Node>(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;
Expand Down
59 changes: 57 additions & 2 deletions tests/scene/test_node.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -84,11 +88,15 @@ class TestNode : public Node {
int physics_process_counter = 0;

Node *exported_node = nullptr;
Array exported_nodes;

List<Node *> *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") {
Expand Down Expand Up @@ -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);
Expand All @@ -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<PackedScene> ps;
ps.instantiate();
ps->pack(node);
Expand Down Expand Up @@ -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<PackedScene> 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<PackedScene> 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<FileAccess> 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);
}

Expand Down
Loading