Merge pull request #75459 from kleonc/node-fix-find-children

Fix recursive `Node.find_children`
godotengine · Jun 19, 2023 · cce100a · cce100a
2 parents 080d471 + d159123
commit cce100a
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
@@ -1665,25 +1665,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
@@ -308,22 +308,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") {