Merge pull request #108507 from dementive/optimize-scene-tree-groups

Optimize scene tree groups

modules/navigation_2d/2d/nav_mesh_generator_2d.cpp

@@ -246,12 +246,12 @@ void NavMeshGenerator2D::set_generator_parsers(LocalVector<NavMeshGeometryParser
 }
 
 void NavMeshGenerator2D::generator_parse_source_geometry_data(Ref<NavigationPolygon> p_navigation_mesh, Ref<NavigationMeshSourceGeometryData2D> p_source_geometry_data, Node *p_root_node) {
-	List<Node *> parse_nodes;
+	Vector<Node *> parse_nodes;
 
 	if (p_navigation_mesh->get_source_geometry_mode() == NavigationPolygon::SOURCE_GEOMETRY_ROOT_NODE_CHILDREN) {
 		parse_nodes.push_back(p_root_node);
 	} else {
-		p_root_node->get_tree()->get_nodes_in_group(p_navigation_mesh->get_source_geometry_group_name(), &parse_nodes);
+		parse_nodes = p_root_node->get_tree()->get_nodes_in_group(p_navigation_mesh->get_source_geometry_group_name());
 	}
 
 	Transform2D root_node_transform = Transform2D();

modules/navigation_3d/3d/nav_mesh_generator_3d.cpp

@@ -278,12 +278,12 @@ void NavMeshGenerator3D::set_generator_parsers(LocalVector<NavMeshGeometryParser
 }
 
 void NavMeshGenerator3D::generator_parse_source_geometry_data(const Ref<NavigationMesh> &p_navigation_mesh, Ref<NavigationMeshSourceGeometryData3D> p_source_geometry_data, Node *p_root_node) {
-	List<Node *> parse_nodes;
+	Vector<Node *> parse_nodes;
 
 	if (p_navigation_mesh->get_source_geometry_mode() == NavigationMesh::SOURCE_GEOMETRY_ROOT_NODE_CHILDREN) {
 		parse_nodes.push_back(p_root_node);
 	} else {
-		p_root_node->get_tree()->get_nodes_in_group(p_navigation_mesh->get_source_group_name(), &parse_nodes);
+		parse_nodes = p_root_node->get_tree()->get_nodes_in_group(p_navigation_mesh->get_source_group_name());
 	}
 
 	Transform3D root_node_transform = Transform3D();

scene/2d/canvas_modulate.cpp

@@ -117,8 +117,7 @@ PackedStringArray CanvasModulate::get_configuration_warnings() const {
 	PackedStringArray warnings = Node2D::get_configuration_warnings();
 
 	if (is_in_canvas && is_visible_in_tree()) {
-		List<Node *> nodes;
-		get_tree()->get_nodes_in_group("_canvas_modulate_" + itos(get_canvas().get_id()), &nodes);
+		Vector<Node *> nodes = get_tree()->get_nodes_in_group("_canvas_modulate_" + itos(get_canvas().get_id()));
 
 		if (nodes.size() > 1) {
 			warnings.push_back(RTR("Only one visible CanvasModulate is allowed per canvas.\nWhen there are more than one, only one of them will be active. Which one is undefined."));

scene/main/scene_tree.cpp

@@ -1581,22 +1581,20 @@ Node *SceneTree::get_first_node_in_group(const StringName &p_group) {
 	return E->value.nodes[0];
 }
 
-void SceneTree::get_nodes_in_group(const StringName &p_group, List<Node *> *p_list) {
+Vector<Node *> SceneTree::get_nodes_in_group(const StringName &p_group) {
 	_THREAD_SAFE_METHOD_
 	HashMap<StringName, Group>::Iterator E = group_map.find(p_group);
 	if (!E) {
-		return;
+		return {};
 	}
 
 	_update_group_order(E->value); //update order just in case
 	int nc = E->value.nodes.size();
 	if (nc == 0) {
-		return;
-	}
-	Node **ptr = E->value.nodes.ptrw();
-	for (int i = 0; i < nc; i++) {
-		p_list->push_back(ptr[i]);
+		return {};
 	}
+
+	return E->value.nodes;
 }
 
 void SceneTree::_flush_delete_queue() {

scene/main/scene_tree.h

@@ -409,7 +409,7 @@ public:
 
 	void queue_delete(Object *p_object);
 
-	void get_nodes_in_group(const StringName &p_group, List<Node *> *p_list);
+	Vector<Node *> get_nodes_in_group(const StringName &p_group);
 	Node *get_first_node_in_group(const StringName &p_group);
 	bool has_group(const StringName &p_identifier) const;
 	int get_node_count_in_group(const StringName &p_group) const;

scene/main/shader_globals_override.cpp

@@ -227,8 +227,7 @@ void ShaderGlobalsOverride::_get_property_list(List<PropertyInfo> *p_list) const
 
 void ShaderGlobalsOverride::_activate() {
 	ERR_FAIL_NULL(get_tree());
-	List<Node *> nodes;
-	get_tree()->get_nodes_in_group(SceneStringName(shader_overrides_group_active), &nodes);
+	Vector<Node *> nodes = get_tree()->get_nodes_in_group(SceneStringName(shader_overrides_group_active));
 	if (nodes.is_empty()) {
 		//good we are the only override, enable all
 		active = true;

scene/main/viewport.cpp

@@ -4375,8 +4375,7 @@ Camera2D *Viewport::get_camera_2d() const {
 
 void Viewport::assign_next_enabled_camera_2d(const StringName &p_camera_group) {
 	ERR_MAIN_THREAD_GUARD;
-	List<Node *> camera_list;
-	get_tree()->get_nodes_in_group(p_camera_group, &camera_list);
+	Vector<Node *> camera_list = get_tree()->get_nodes_in_group(p_camera_group);
 
 	Camera2D *new_camera = nullptr;
 	for (Node *E : camera_list) {

tests/scene/test_node.h

@@ -160,16 +160,15 @@ TEST_CASE("[SceneTree][Node] Testing node operations with a very simple scene tr
 	}
 
 	SUBCASE("Node should be accessible via group") {
-		List<Node *> nodes;
-		SceneTree::get_singleton()->get_nodes_in_group("nodes", &nodes);
+		Vector<Node *> nodes = SceneTree::get_singleton()->get_nodes_in_group("nodes");
 		CHECK(nodes.is_empty());
 
 		node->add_to_group("nodes");
 
-		SceneTree::get_singleton()->get_nodes_in_group("nodes", &nodes);
+		nodes = SceneTree::get_singleton()->get_nodes_in_group("nodes");
 		CHECK_EQ(nodes.size(), 1);
-		List<Node *>::Element *E = nodes.front();
-		CHECK_EQ(E->get(), node);
+		Node *E = nodes.get(0);
+		CHECK_EQ(E, node);
 	}
 
 	SUBCASE("Node should be possible to find") {
@@ -394,11 +393,10 @@ TEST_CASE("[SceneTree][Node] Testing node operations with a more complex simple
 	}
 
 	SUBCASE("Nodes should be accessible via their groups") {
-		List<Node *> nodes;
-		SceneTree::get_singleton()->get_nodes_in_group("nodes", &nodes);
+		Vector<Node *> nodes = SceneTree::get_singleton()->get_nodes_in_group("nodes");
 		CHECK(nodes.is_empty());
 
-		SceneTree::get_singleton()->get_nodes_in_group("other_nodes", &nodes);
+		nodes = SceneTree::get_singleton()->get_nodes_in_group("other_nodes");
 		CHECK(nodes.is_empty());
 
 		node1->add_to_group("nodes");
@@ -406,34 +404,34 @@ TEST_CASE("[SceneTree][Node] Testing node operations with a more complex simple
 		node1_1->add_to_group("nodes");
 		node1_1->add_to_group("other_nodes");
 
-		SceneTree::get_singleton()->get_nodes_in_group("nodes", &nodes);
+		nodes = SceneTree::get_singleton()->get_nodes_in_group("nodes");
 		CHECK_EQ(nodes.size(), 2);
 
-		List<Node *>::Element *E = nodes.front();
-		CHECK_EQ(E->get(), node1);
-		E = E->next();
-		CHECK_EQ(E->get(), node1_1);
+		Node *E = nodes.get(0);
+		CHECK_EQ(E, node1);
+		E = nodes.get(1);
+		CHECK_EQ(E, node1_1);
 
 		// Clear and try again with the other group.
 		nodes.clear();
 
-		SceneTree::get_singleton()->get_nodes_in_group("other_nodes", &nodes);
+		nodes = SceneTree::get_singleton()->get_nodes_in_group("other_nodes");
 		CHECK_EQ(nodes.size(), 2);
 
-		E = nodes.front();
-		CHECK_EQ(E->get(), node1_1);
-		E = E->next();
-		CHECK_EQ(E->get(), node2);
+		E = nodes.get(0);
+		CHECK_EQ(E, node1_1);
+		E = nodes.get(1);
+		CHECK_EQ(E, node2);
 
 		// Clear and try again with the other group and one node removed.
 		nodes.clear();
 
 		node1->remove_from_group("nodes");
-		SceneTree::get_singleton()->get_nodes_in_group("nodes", &nodes);
+		nodes = SceneTree::get_singleton()->get_nodes_in_group("nodes");
 		CHECK_EQ(nodes.size(), 1);
 
-		E = nodes.front();
-		CHECK_EQ(E->get(), node1_1);
+		E = nodes.get(0);
+		CHECK_EQ(E, node1_1);
 	}
 
 	SUBCASE("Nodes added as siblings of another node should be right next to it") {
@@ -472,12 +470,11 @@ TEST_CASE("[SceneTree][Node] Testing node operations with a more complex simple
 		node1->add_to_group("nodes");
 		node1->replace_by(node2, true);
 
-		List<Node *> nodes;
-		SceneTree::get_singleton()->get_nodes_in_group("nodes", &nodes);
+		Vector<Node *> nodes = SceneTree::get_singleton()->get_nodes_in_group("nodes");
 		CHECK_EQ(nodes.size(), 1);
 
-		List<Node *>::Element *E = nodes.front();
-		CHECK_EQ(E->get(), node2);
+		Node *E = nodes.get(0);
+		CHECK_EQ(E, node2);
 	}
 
 	SUBCASE("Duplicating a node should also duplicate the children") {