Skip to content

Commit

Permalink
fix bug where signals from scene instances are included in packed scenes
Browse files Browse the repository at this point in the history
  • Loading branch information
Cecilia Carter committed Sep 22, 2024
1 parent e4e024a commit 3320750
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 5 deletions.
2 changes: 1 addition & 1 deletion core/object/object.h
Original file line number Diff line number Diff line change
Expand Up @@ -572,7 +572,7 @@ class Object {
CONNECT_PERSIST = 2, // hint for scene to save this connection
CONNECT_ONE_SHOT = 4,
CONNECT_REFERENCE_COUNTED = 8,
CONNECT_INHERITED = 16, // Used in editor builds.
CONNECT_INHERITED = 16, // Whether or not the connection is in an instance of a scene
};

struct Connection {
Expand Down
17 changes: 14 additions & 3 deletions scene/resources/packed_scene.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1033,7 +1033,10 @@ Error SceneState::_parse_node(Node *p_owner, Node *p_node, int p_parent_idx, Has
return OK;
}


Error SceneState::_parse_connections(Node *p_owner, Node *p_node, HashMap<StringName, int> &name_map, HashMap<Variant, int, VariantHasher, VariantComparator> &variant_map, HashMap<Node *, int> &node_map, HashMap<Node *, int> &nodepath_map) {

// Ignore nodes that are within a scene instance
if (p_node != p_owner && p_node->get_owner() && p_node->get_owner() != p_owner && !p_owner->is_editable_instance(p_node->get_owner())) {
return OK;
}
Expand All @@ -1054,10 +1057,17 @@ Error SceneState::_parse_connections(Node *p_owner, Node *p_node, HashMap<String
for (const Node::Connection &F : conns) {
const Node::Connection &c = F;

if (!(c.flags & CONNECT_PERSIST)) { //only persistent connections get saved
// Don't save connections that are not persistent
if (!(c.flags & CONNECT_PERSIST)) {
continue;
}

// Don't include signals that are from scene instances
// (They are already saved in the scenes themselves)
if (bool(c.flags & CONNECT_INHERITED)) {
continue;
}

// only connections that originate or end into main saved scene are saved
// everything else is discarded

Expand Down Expand Up @@ -1220,9 +1230,10 @@ Error SceneState::_parse_connections(Node *p_owner, Node *p_node, HashMap<String
}
}

// Recursively parse child connections
for (int i = 0; i < p_node->get_child_count(); i++) {
Node *c = p_node->get_child(i);
Error err = _parse_connections(p_owner, c, name_map, variant_map, node_map, nodepath_map);
Node *child = p_node->get_child(i);
Error err = _parse_connections(p_owner, child, name_map, variant_map, node_map, nodepath_map);
if (err) {
return err;
}
Expand Down
62 changes: 61 additions & 1 deletion tests/scene/test_packed_scene.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
#define TEST_PACKED_SCENE_H

#include "scene/resources/packed_scene.h"

#include "tests/test_macros.h"

namespace TestPackedScene {
Expand All @@ -56,6 +55,67 @@ TEST_CASE("[PackedScene] Pack Scene and Retrieve State") {
memdelete(scene);
}

TEST_CASE("[PackedScene] Test That Correct Signals are Preserved when Packing Scene") {
// Create main scene
// root
// `- sub_node (local)
// `- sub_scene (instance of another scene)
// `- sub_scene_node (owned by sub_scene)
Node *main_scene_root = memnew(Node);
Node *sub_node = memnew(Node);
Node *sub_scene_root = memnew(Node);
Node *sub_scene_node = memnew(Node);

main_scene_root->add_child(sub_node);
sub_node->set_owner(main_scene_root);

sub_scene_root->add_child(sub_scene_node);
sub_scene_node->set_owner(sub_scene_root);

main_scene_root->add_child(sub_scene_root);
sub_scene_root->set_owner(main_scene_root);

SUBCASE("Signals that should be saved") {
int main_flags = Object::CONNECT_PERSIST;
// sub node to a node in main scene
sub_node->connect("ready", callable_mp(main_scene_root, &Node::is_ready), main_flags);
// subscene root to a node in main scene
sub_scene_root->connect("ready", callable_mp(main_scene_root, &Node::is_ready), main_flags);
//subscene root to subscene root (connected within main scene)
sub_scene_root->connect("ready", callable_mp(sub_scene_root, &Node::is_ready), main_flags);

// Pack the scene.
PackedScene packed_scene;
const Error err = packed_scene.pack(main_scene_root);
CHECK(err == OK);

// Make sure the right connections are in packed scene
Ref<SceneState> state = packed_scene.get_state();
CHECK_EQ(state->get_connection_count(), 3);
}

SUBCASE("Signals that should not be saved") {
int subscene_flags = Object::CONNECT_PERSIST | Object::CONNECT_INHERITED;
// subscene node to itself
sub_scene_node->connect("ready", callable_mp(sub_scene_node, &Node::is_ready), subscene_flags);
// subscene node to subscene root
sub_scene_node->connect("ready", callable_mp(sub_scene_root, &Node::is_ready), subscene_flags);
//subscene root to subscene root (connected within sub scene)
sub_scene_root->connect("ready", callable_mp(sub_scene_root, &Node::is_ready), subscene_flags);

// Pack the scene.
PackedScene packed_scene;
const Error err = packed_scene.pack(main_scene_root);
CHECK(err == OK);

// Make sure the right connections are in packed scene
Ref<SceneState> state = packed_scene.get_state();
CHECK_EQ(state->get_connection_count(), 0);
}

memdelete(main_scene_root);
}

TEST_CASE("[PackedScene] Clear Packed Scene") {
// Create a scene to pack.
Node *scene = memnew(Node);
Expand Down

0 comments on commit 3320750

Please sign in to comment.