Fix recursive Node.find_children

godotengine · Apr 9, 2023 · d159123 · d159123
1 parent 23394be
commit d159123
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 16 deletions.
diff --git a/scene/main/node.cpp b/scene/main/node.cpp
@@ -1414,25 +1414,19 @@ TypedArray<Node> Node::find_children(const String &p_pattern, const String &p_ty
 			continue;
 		}
 
-		if (!p_pattern.is_empty()) {
-			if (!cptr[i]->data.name.operator String().match(p_pattern)) {
-				continue;
-			} else if (p_type.is_empty()) {
+		if (p_pattern.is_empty() || cptr[i]->data.name.operator String().match(p_pattern)) {
+			if (p_type.is_empty() || cptr[i]->is_class(p_type)) {
 				ret.append(cptr[i]);
-			}
-		}
+			} else if (cptr[i]->get_script_instance()) {
+				Ref<Script> scr = cptr[i]->get_script_instance()->get_script();
+				while (scr.is_valid()) {
+					if ((ScriptServer::is_global_class(p_type) && ScriptServer::get_global_class_path(p_type) == scr->get_path()) || p_type == scr->get_path()) {
+						ret.append(cptr[i]);
+						break;
+					}
 
-		if (cptr[i]->is_class(p_type)) {
-			ret.append(cptr[i]);
-		} else if (cptr[i]->get_script_instance()) {
-			Ref<Script> scr = cptr[i]->get_script_instance()->get_script();
-			while (scr.is_valid()) {
-				if ((ScriptServer::is_global_class(p_type) && ScriptServer::get_global_class_path(p_type) == scr->get_path()) || p_type == scr->get_path()) {
-					ret.append(cptr[i]);
-					break;
+					scr = scr->get_base_script();
 				}
-
-				scr = scr->get_base_script();
 			}
 		}
 

diff --git a/tests/scene/test_node.h b/tests/scene/test_node.h
@@ -267,22 +267,39 @@ TEST_CASE("[SceneTree][Node] Testing node operations with a more complex simple
 		Node *child = SceneTree::get_singleton()->get_root()->find_child("NestedNode", true, false);
 		CHECK_EQ(child, nullptr);
 
+		TypedArray<Node> children = SceneTree::get_singleton()->get_root()->find_children("NestedNode", "", true, false);
+		CHECK_EQ(children.size(), 0);
+
 		node1->set_name("Node1");
 		node2->set_name("Node2");
 		node1_1->set_name("NestedNode");
 
 		child = SceneTree::get_singleton()->get_root()->find_child("NestedNode", true, false);
 		CHECK_EQ(child, node1_1);
 
+		children = SceneTree::get_singleton()->get_root()->find_children("NestedNode", "", true, false);
+		CHECK_EQ(children.size(), 1);
+		CHECK_EQ(Object::cast_to<Node>(children[0]), node1_1);
+
 		// First node that matches with the name is node1.
 		child = SceneTree::get_singleton()->get_root()->find_child("Node?", true, false);
 		CHECK_EQ(child, node1);
 
+		children = SceneTree::get_singleton()->get_root()->find_children("Node?", "", true, false);
+		CHECK_EQ(children.size(), 2);
+		CHECK_EQ(Object::cast_to<Node>(children[0]), node1);
+		CHECK_EQ(Object::cast_to<Node>(children[1]), node2);
+
 		SceneTree::get_singleton()->get_root()->move_child(node2, 0);
 
 		// It should be node2, as it is now the first one in the tree.
 		child = SceneTree::get_singleton()->get_root()->find_child("Node?", true, false);
 		CHECK_EQ(child, node2);
+
+		children = SceneTree::get_singleton()->get_root()->find_children("Node?", "", true, false);
+		CHECK_EQ(children.size(), 2);
+		CHECK_EQ(Object::cast_to<Node>(children[0]), node2);
+		CHECK_EQ(Object::cast_to<Node>(children[1]), node1);
 	}
 
 	SUBCASE("Nodes should be accessible via their node path") {