Browse Source

Avoid duplicating signals from scene instances into packed scenes

cixil 11 months ago
parent
commit
8a42e3d3ef
3 changed files with 76 additions and 4 deletions
  1. 1 1
      core/object/object.h
  2. 12 3
      scene/resources/packed_scene.cpp
  3. 63 0
      tests/scene/test_packed_scene.h

+ 1 - 1
core/object/object.h

@@ -572,7 +572,7 @@ public:
 		CONNECT_PERSIST = 2, // hint for scene to save this connection
 		CONNECT_PERSIST = 2, // hint for scene to save this connection
 		CONNECT_ONE_SHOT = 4,
 		CONNECT_ONE_SHOT = 4,
 		CONNECT_REFERENCE_COUNTED = 8,
 		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 {
 	struct Connection {

+ 12 - 3
scene/resources/packed_scene.cpp

@@ -1034,6 +1034,7 @@ Error SceneState::_parse_node(Node *p_owner, Node *p_node, int p_parent_idx, Has
 }
 }
 
 
 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) {
 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())) {
 	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;
 		return OK;
 	}
 	}
@@ -1054,7 +1055,14 @@ Error SceneState::_parse_connections(Node *p_owner, Node *p_node, HashMap<String
 		for (const Node::Connection &F : conns) {
 		for (const Node::Connection &F : conns) {
 			const Node::Connection &c = F;
 			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 (c.flags & CONNECT_INHERITED) {
 				continue;
 				continue;
 			}
 			}
 
 
@@ -1220,9 +1228,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++) {
 	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) {
 		if (err) {
 			return err;
 			return err;
 		}
 		}

+ 63 - 0
tests/scene/test_packed_scene.h

@@ -56,6 +56,69 @@ TEST_CASE("[PackedScene] Pack Scene and Retrieve State") {
 	memdelete(scene);
 	memdelete(scene);
 }
 }
 
 
+TEST_CASE("[PackedScene] Signals 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.
+		Ref<PackedScene> packed_scene;
+		packed_scene.instantiate();
+		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.
+		Ref<PackedScene> packed_scene;
+		packed_scene.instantiate();
+		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") {
 TEST_CASE("[PackedScene] Clear Packed Scene") {
 	// Create a scene to pack.
 	// Create a scene to pack.
 	Node *scene = memnew(Node);
 	Node *scene = memnew(Node);