godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.12k stars 69 forks source link

Actually implement Scene Groups #10357

Open h0lley opened 1 month ago

h0lley commented 1 month ago

Describe the project you are working on

A game where I have multiple levels inside of the scene tree at the same time. While the player can only ever look at one level, there's important processing taking place in other levels the player may come back to later. It would be very useful if a level could call onto all nodes of a certain group that are only its own children.

Describe the problem or limitation you are having in your project

Groups are now separated into Scene Groups and Global Groups. The way the GUI is currently arranged, it strongly implies that Scene Groups are a special type of group that are scoped to a scene at runtime. However, this would be incorrect, and all that Scene Groups currently are is "an editor-only way of listing certain groups only in the currently edited scene". To be honest, I don't see much of an application for that, and find the cost of misleading users to expect different behavior at runtime to be much greater.

If Scene Groups would actually be scoped to a scene similar (just like) how scene unique node names already are, then there would be a real application and use for them.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Similar to how we have create_tween() in both SceneTree and Node we'd also have group-related functions like call_group() and get_nodes_in_group() in Node. These then only return nodes of the specified group that are inside of their scene (most likely based on the scope of the owner reference).

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

Already covered above.

However, in case this proposal finds no traction, it falls back to at least changing the current editor GUI to be less misleading.

Current GUI:

image

Proposed:

image

The little image indicates that this is a group that is being used by at least one node in the currently edited scene. In addition, these groups would be sorted to the top of the list. Bonus: Clicking this icon could put a group filter (e.g. g:my_group_a) into the Scene panel's search bar. This should be more than enough to provide the user with everything that was practical about the old (current) GUI without implying that there are two types of groups.

As a SceneTree feature, there really are just global groups, and this GUI reflects that properly. Henceforth, all groups would be what "Global Groups" are currently and would be serialized in project.godot, giving the user a complete overview of groups used in the project from the new tab in project settings.

If this enhancement will not be used often, can it be worked around with a few lines of script?

We could of course filter nodes that are returned by a global get_nodes_in_group() ourselves. Perhaps by doing something unwieldy like this:

func get_local_nodes_in_group(group_name: StringName) -> Array[Node]:
    return get_nodes_in_group(group_name).filter(func (node):
        return node.owner == self)

But that seems rather inflexible and possibly costly if it's being run frequently. Inflexible mostly because it also could only be run from the root node of a scene, so it wouldn't match up with, for example, the % accessor.

The entirety of the node groups feature is already purely QoL that can be worked around, so personally I'd be fine with sticking to the current feature-set and only really see the GUI improvement as something important.

Is there a reason why this should be core and not an add-on in the asset library?

Groups already are core.

CarpenterBlue commented 1 month ago

Possibly better work around is to implement group on the owner that's just instance_id.

Then just do: In the _ready() on the owner: add_to_group(str(get_instance_id())) or in the descendants add_to_group(str(owner.get_instance_id())) and whenever you need to get the group: get_tree().get_nodes_in_group(str(owner.get_instance_id()))

If you need more specificity, then add prefix/suffix. I would still prefer there being real solution for this like you proposed. We need both this and the slotted nodes that were proposed in #5475.

Mickeon commented 1 month ago

This sounds a lot like Scene Unique Nodes, due to allowing to access to the grouped node from anywhere within the scene. Except, it's essentially infinite states instead of just "Unique" or "Not unique"

Nor that I like unique nodes, or this proposal a little bit due to memory concerns. Just an observation, though. I would avoid to add features that overlap.