Implement Scene Unique Nodes

[reduz]

Implements godotengine/godot-proposals#4096

• Nodes can be marked unique to the scene in the editor (or via code).
• Unique nodes can be accessed via the % prefix at any point in the path. From that point in the path (depending on whether the scene of the path is), the unique node will be fetched.
• Implementation is very optimal, as these nodes are cached.

Screenshots:

To make any node unique, this new option was added:

Which results in the node being marked with an icon:

This allows the node to be accessed from anywhere within the scene (note, $ Syntax not updated, required GDScript changes as a follow-up PR):

If more than one node is made unique with the unique name, a warning will be shown:

Note Backporting this PR to Godot 3.x should be relatively easy if anyone is interested, it does not rely on anything specific from master branch.

[Zireael07]

How does this work if I have a player scene instantiated in the main game scene, and I mark the player unique (from the main game scene, not within player scene)?
Will I be able to access player everywhere (since everything is instantiated in the main game scene) or not (because e.g. some things are spawned from code, not within editor)?

[reduz]

@Zireael07 This is not what this feature is for. If you want to do this, just add the player to a group (player) and retrieve it from there. This is for code that needs to fetch nodes within the same scene, but this scene can be instantiated multiple times.

[Zireael07]

Ah, ok. So it should likely not allow me to try to set something that's instantiated/foreign as scene unique, if it doesn't already? (Haven't looked at the changes)

[h0lley]

I feel the RMB context menu is a good place for this option and don't think it should get a toggle button in the scene panel like visibility.

Reasoning:

• Visibility toggling is something you do frequently - for instance to quickly test things out - so the quick access makes sense.
• Giving a node a scene unique identifier however is something you do much more deliberately and generally as a permanent change - you are unlikely to want to touch it afterwards. You'll also likely do it much less frequently - mostly during the initial setup of the scene - so it's probably not worth cluttering the scene panel for.

Although the indicator that a node is scene unique can probably be improved.
There is also the possibility to make this node name use bold or colored text.
And/or the %-symbol could show in front of the node name to mimic its usage in code.

Btw, does this also resolve godotengine/godot-proposals#996 ?

[reduz]

@h0lley It can always work like the lock button, whereas its only visible when enabled, and if you click it it gets disabled.

[RandomShaper]

I guess it'd be good that at runtime there's a check on debug about multiple nodes with the same unique name present in the tree.

Also, it'd be good to specify the behavior when that happens, like, "the most recent one wins" (which seems the easiest to implement?).

Nevermind. I thought I was reading a proposal. 😅 I'll check the code later.

[Byteron]

I still think this is going to be abused as a replacement for @onready var unique_node = $UniqueNode and then everyone is gonna complain why their code is so slow.

[TokageItLab]

AnimationTrack might want to indicate that it is a unique path. However, there is the problem of not being able to handle the case of a unique path that has a non-unique path as its parent, so there is no need to hustle to work around that.

It seems to me that we will need to add an option to import animations as unique paths later in the AnimationLibrary importer, etc., but that would be outside the scope of this PR.

[h0lley]

I still think this is going to be abused as a replacement for @onready var unique_node = $UniqueNode and then everyone is gonna complain why their code is so slow.

this would not be abuse - it's part of what this feature is intended for. doing this is completely fine.

it would have no relevant impact on performance when using literal NodePath syntax: get_node(^"%MyNode") or $%MyNode.
performance wise, this is already almost identical with straight references, and then on top of that, if I understand correctly scene unique nodes are cached, so it would literally be identical to references.

it can have an impact on performance when get_node() is being passed a String that still needs to be converted to NodePath, but that's a different issue about making people aware about that fact (or making the GDScript compiler smarter about it).

[reduz]

@TokageItLab Right, this should be added to the importer in a subsequent PR I guess

[reduz]

@h0lley Okay, used the icon system we already have, feels more consistent now.

[h0lley]

@h0lley Okay, used the icon system we already have, feels more consistent now.

I think it looks good, although if we want to do it like that, I reckon the option should be put in the top tool bar - closeby

Then there would be no need to have this outlier toggle button in the RMB context menu.

[reduz]

@h0lley The toolbar is for options related to the viewport, which is not the case here. Other options that appear as icons (whether there are connected signals, as an example) are not there either. I think it should be good as is for now, we can see after merged if better ideas come up or if its good enough.

[markdibarry]

This is a welcome change. As a C# user, this is a very tedious part of setting up and changing a scene, so I appreciate a more terse syntax.
To be clear, the following would be the C# equivalent?: GetNode<Node>("%MyNode");

[reduz]

@markdibarry yup, that should work fine in C#

[CassianoBelniak]

Is this feature limited to get_node(...)?

It would be nice to have something like %Node or $%Node to follow the pattern of $Node.

[markdibarry]

@CassianoBelniak

This allows the node to be accessed from anywhere within the scene (note, $ Syntax not updated, required GDScript changes as a follow-up PR):

[reduz]

@timothyqiu alright, fixed the first case. The second one will probably need a bit more doc when tutorial is written.

[akien-mga]

Looks good.

[akien-mga]

Thanks!

[boruok]

problem with getting reference in grandchild node in 3.5beta:

root
- turn_manager
- ...
- canvas_layer
- - game_ui
- - - minimap

access to turn_manager from minimap returns null
access to turn_manager from game_ui returns node
this is expected result?

Sets this node's name as a unique name in its owner. This allows the node to be accessed as %Name instead of the full path, from any node within that scene.

so, better description would be: any node within that scene, but doesn't grandchild and lower levels?

edit: added editor version.

[clayjohn]

@boruok sounds like you may want to file a bug report. Make sure to include detailed information that others can use to reproduce the issue you are facing.

[KoBeWi]

This is not a bug, but likely a misuse of the feature. Show your full scene and how you try to access the nodes.
Note that unique names work within the same scene, so if your minimap is an instance, you need to do "../%turn_manager"

[boruok]

@KoBeWi, hmm, i actually find what was wrong: saved branch scene usage, if i make that branch make local reference works, should i open issue?

[timothyqiu]

@boruok This is intended. They are two different scenes after you saved the branch.

[AndersonDeMatos]

I don't know if this is the best place to say this or if it should be a new proposal, but I think it should be harder to disable the '%' prefix than simply clicking it, because sometimes we just want to select the node in the scene tree and we unintentionally click the '%' and disable it. All the times that it happened to me I saw it happening, but there may be cases when we don't see it and it causes errors on the code when trying to get those nodes by their unique names with the '%' prefix. And it is annoying having to enable it again every time it happens. I think it should be as "difficult" to disable it as it is to enable it, so we don't disable it unintentionally.
Another thing that annoys me is that the '%' symbol appears in front (or behind) the name of the node name when the space for the name is shorter than the node name. This does not happen with other icons, like the one for the script or for the signal. I think it's because the '%' symbol is not a proper icon, but its only a Unicode character (I guess). It should have its own icon, even if its an SVG with an image of a '%' symbol.

[AThousandShips]

This isn't the place for this indeed, please open a discussion or proposal for it, this is a closed PR that has been inactive for over a year so this isn't the place for feedback