Remove the requirement for node paths to be unique

[thimenesup]

Describe the project you are working on

Any Godot Project

Describe the problem or limitation you are having in your project

Nothing specific to this, but this should bring more performance.

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

Node names being unique is an arbitrary decision that hinders more than it helps, since you must handle a potential case for it whenever adding or renaming node, which has a cost, specially generating a new unique node name, allocating and interning the new String, specially costly in say, instancing hundreds of projectiles, where their name doesn't matter in the slightless.

How would you refer to a specific node in the SceneTree?
Simple, by index, guaranteed to be unique and O(1) and its already implemented, get_child()

What about NodePaths?
NodePaths could just be changed to be an array of indices.

What happens to get_node()
Nothing really, you just should expect it to return the first node that matches the expected Name/NodePath, if any.

How does the editor handle these changes?
Well, everything is supposed to be abstracted away by the NodePaths and the changes should work transparently.
The end user wouldn't really handle indices manually to refer to nodes, just the editor internally by itself.

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

See above.

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

Must be a core engine change.

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

See above.

[Calinou]

See also godotengine/godot#27608.

[YuriSizov]

The end user wouldn't really handle indices manually to refer to nodes, just the editor internally by itself.

If I understand correctly, you propose that at runtime there are no names and therefore you cannot refer to nodes by their name and path. But that's very often the case, and indices are not guaranteed to be the same every time when you do something dynamically. Which is what the names are for: you don't micromanage your nodes and don't think about their order unless it's really important.

---

As for the general idea, we need to base solutions on some real life examples. If you claim some performance improvements, you need to prove that it's actually possible in a real project. Without a proof of concept this will unlikely be taken by anyone.

If your problem is with spawning a big number of projectiles, naming nodes is likely not even the main actor in the performance loss. To solve that case there is a swarm system proposal by reduz, if you're interested #2380

[MaaaxiKing]

Node names being unique

Node names aren't unique if they're not siblings:

 ┖╴Game
    ┠╴Game
    ┖╴Game2

There are 2 nodes named "Game".

[thimenesup]

If I understand correctly, you propose that at runtime there are no names and therefore you cannot refer to nodes by their name and path. But that's very often the case...

Read again the proposal, users would still be able to refer to nodes using get_node() with names.

and indices are not guaranteed to be the same every time when you do something dynamically.

Indices are guaranteed to be the same when you instance a scene, in which case you already hold a reference to the node and therefore you won't need to refer to it by name.

If your problem is with spawning a big number of projectiles, naming nodes is likely not even the main actor in the performance loss.

That was just an example, adding and removing nodes from the tree is just slow, but with this change it could be faster.

Node names aren't unique if they're not siblings:

So? Read the proposal again.

[YuriSizov]

Read again the proposal, users would still be able to refer to nodes using get_node() with names.

I've read it a bunch of times, but I still don't understand how deep do you want to change things. You talk about replacing names with indices even in the editor internals for some reason. But you also mention that names would still work as before.
When where would the optimization come from? Just from not checking if the names conflict?

We can use RNG when naming the unnamed nodes then and reasonably avoid conflicts this way, without actually checking and assigning sequential numbers. We can also leave this to the user and add a parameter to add_child to not check for name uniqueness. You can then name your nodes however you like, ensuring uniqueness, without wasting precious time to check it in the tree. This way it still works to solve your problem without breaking the core of the engine.

Indices are guaranteed to be the same when you instance a scene, in which case you already hold a reference to the node and therefore you won't need to refer to it by name.

See, if you want to rely on indices you can already do that, but you also mention replacing nodepaths with indices. But node paths exist for convenience and so you don't have to know the order of your nodes precisely to find them. So you can't really replace nodepaths with indices without inconveniencing those who need them.

And then you mention that the editor would convert them to indices automatically, which doesn't make sense to do at all. If you have nodes in the editor, you cannot have them unnamed, how'd you work with the tree then? The only time when it makes sense to have unnamed nodes is at runtime. And forcing named nodes and their paths to turn into indices can really screw with projects. That really depends on what you're doing. Say, I put a bunch of nodes on a branch, and then Y-sort and reorder them at runtime. I have no idea what their indices are after that, so I have to make sure to get their references before doing that for things not to break. This creates all sorts of inconveniences, removes all the guarantees, and forces two different behaviors for getting the nodes if we indeed silently replace paths with indices.

That was just an example, adding and removing nodes from the tree is just slow, but with this change it could be faster.

You still have to prove it, with a project and not with a synthetic test too. You may remove the need for the engine to check for names, but create more overhead for the developers who need to be more careful about how they refer to the nodes.

---

PS. If you see people misunderstanding you, it's in your interest to try and explain yourself better instead of referring them to read the thing again. Also please be mindful of the Code of Conduct and the requirement to be respectful.

[dalexeev]

It's not very good when several different nodes have the same absolute path. Performance isn't important here. If you have tens of thousands of nodes in the tree, then performance problems will not only be due to checking the uniqueness of the node name when adding it to the tree.

Instead of constantly using get_node or $ (for example in _process), you should cache the node reference to a variable.

[thimenesup]

I've read it a bunch of times, but I still don't understand how deep do you want to change things. You talk about replacing names with indices even in the editor internals for some reason.

The editor stores serialized references to nodes with this format NodePath("Panel/Buttons/Button2")
With the change of node paths not longer being unique, to guarantee that the editor would still store an unique reference to the node you want, it would use indices instead as in NodePath(3/0/2)

That is, only internal change, the user would STILL just be able to get nodes by name... OR by selecting the node from the UI in case of assigning it using an exported NodePath.

you cannot have them unnamed, how'd you work with the tree then?

Nodes STILL have a name, how its that hard to grasp this, just think how Unity handles GameObjects or Unreal Actors,
you CAN refer them by name, you CAN manipulate them without using indices, and those objects CAN share the same name.

... adding and removing nodes from the tree is just slow, but with this change it could be faster. ... You still have to prove it

While it obviously has to be profiled, it doesn't that much theorizing that having to call this whenever a node is added, has a cost

Which now my question is, what is the benefit of having unique node path names? Why are we paying that cost?

dalexeev
Read the proposal again, because this only mentions performance when adding/renaming nodes.

[YuriSizov]

Ok, we're getting there.

Which now my question is, what is the benefit of having unique node path names? Why are we paying that cost?

I can think of serialization being one reason. If serialized nodes aren't uniquely identifiable it can lead to consistency problems.

Which now my question is, what is the benefit of having unique node path names? Why are we paying that cost?

Then why would there be any internal change to the editor. When it's fetching nodes by name, it doesn't do any renaming or adding.

---

I think you've mixed a few things together leading to this confusion. It's a good question whether the nodes need to have a unique name or not. But there is no need to change things to using indices internally or externally. Everything can just work as before, except yes, get_node would only return the first match it can. So there would probably be a get_nodes method to get all the possibilities and then you would use an index on that.

[dalexeev]

Which now my question is, what is the benefit of having unique node path names? Why are we paying that cost?

dalexeev
Read the proposal again, because this only mentions performance when adding/renaming nodes.

NodePaths are designed to look like file paths. Let's first convince the OS developers to remove the "stupid restriction" and allow two or more different files in the same folder to have the same name, and only then we will discuss such a change in Godot. 😄

[thimenesup]

I can think of serialization being one reason. If serialized nodes aren't uniquely identifiable it can lead to consistency problems.

Thats why I said that serialization would need to change from using name-based NodePaths to index based, guarantees to still have uniquely identifiable nodes.

Let's first convince the OS developers to remove the "stupid restriction" and allow two or more different files in the same folder to have the same name, and only then we will discuss such a change in Godot.

Yeah but the underlying OS filesystem isn't an arbitrary choice made by Godot.

Unique Node Paths/Names on the other hand ARE, which upon inspection it may not have been a good idea, but the good news is that we could fix that.

[YuriSizov]

Thats why I said that serialization would need to change from using name-based NodePaths to index based, guarantees to still have uniquely identifiable nodes.

Serialized scenes don't use node paths to uniquely identify the nodes, they use the name of the node and the name of the parent.

[node name="Panel" type="ColorRect" parent="."]
...

[node name="TabContainer" type="TabContainer" parent="Layout"]

Using indices here would be just using 0, 1, 2, 3 which is similar to not using anything and relying on their order.

[thimenesup]

So whats the problem? Thats already how the editor handles serialized resources by the way.

[YuriSizov]

So whats the problem? Thats already how the editor handles serialized resources by the way.

That's how it used to work, and that led to a lot of consistency problems. We moved to a UID system recently precisely to fix it. So some UID system would need to be introduced here as well, and it's not indices. And since we already have names, we can just keep them unique, you know.

[thimenesup]

The consistency problems of resources root from moving/removing/renaming external files outside of the editor, but the nodes from a scene are internal to that file and the editor should easily be able to handle the changes.

Again, im not sure what are all these artificial roadblocks, as I already said, both Unity and Unreal do it this way.

[YuriSizov]

The consistency problems of resources root from moving/removing/renaming external files outside of the editor, but the nodes from a scene are internal to that file and the editor should easily be able to handle the changes.

That's incorrect, serialization problems with resources can happen just from opening them, and with internal sub-resources as well (why would it matter, they still rely on sequential IDs?).

Again, im not sure what are all these artificial roadblocks, as I already said, both Unity and Unreal do it this way.

Neither Unity nor Unreal rely on scene hierarchy the way Godot does. Also, neither of those engines shares the codebase with Godot for this to even be relevant. Superficial comparisons aren't useful.

---

Let's not get carried away from the problem you are trying to solve. You proposed a solution, but it may not be the solution. It doesn't mean that the problem cannot be solved otherwise. Is there a use-case for instancing a lot of objects and experiencing some downtime due to the name checks? Maybe. Allowing ambiguous node paths is not the only solution.

Like I've suggested, we can just add a way to disable the check and leave it for the user to handle. You'd still have to name the nodes (but that seems to be the case here anyway), but the engine won't do supposedly expensive check for uniqueness because you already ensure that when naming them, and have the complete control over the situation. This will still solve the problem, but it also won't have a huge effect on the rest of the system and whatever "arbitrary" limitations we might have.

And there is still the swarm proposal for non-Node based entities which would be light-weight and not as taxing on the scene tree. That also solves the use-cases you've presented.

[dalexeev]

Nothing specific to this, but this should bring more performance.

since you must handle a potential case for it whenever adding or renaming node, which has a cost, specially generating a new unique node name, allocating and interning the new String, specially costly in say, instancing hundreds of projectiles

Is the problem you are trying to solve really that important? IMO this is something from the category of premature optimizations. Typically, a single node does not have so many children that adding a new node or renaming it would cause performance problems.

where their name doesn't matter in the slightless

If the name is not important, then the issue with renaming is solved automatically: just do not rename these nodes. As for the rest of the "problem", there is a special type of node name containing the @ symbol, you cannot give such names, these names are used when no name is given and they are guaranteed to be unique. I don't know if this case is checked, but optimization is really possible here (if needed), for such nodes it is possible not to check the names of siblings when added to the tree.

[thimenesup]

That's incorrect, serialization problems with resources can happen just from opening them, and with internal sub-resources as well (why would it matter, they still rely on sequential IDs?).

Occur from what? An engine bug? A random cosmic ray flipping an important bit causing memory corruption?

Neither Unity nor Unreal rely on scene hierarchy the way Godot does. ... Superficial comparisons aren't useful.

The scene hierarchy of both is extremely similar if not identical, all three of them use a tree structure.

Like I've suggested, we can just add a way to disable the check and leave it for the user to handle ...

I don't think this change needs to left some legacy behind, this is not C strings.

Is the problem you are trying to solve really that important? IMO this is something from the category of premature optimizations. Typically, a single node does not have so many children that adding a new node or renaming it would cause performance problems.

This isn't a premature optimization, this isn't really and edge case, this happens to every node you add to the scene tree.
A node can have a LOT of childrens, again, think you are instancing many objects and adding them to the scene tree root.

If the name is not important, then the issue with renaming is solved automatically: just do not rename these nodes.

If you instance an scene multiple times and add them to the same parent, the root nodes will rename themselves because of the arbirtrary enforced uniqueness.

[theraot]

The editor stores serialized references to nodes with this format NodePath("Panel/Buttons/Button2")
With the change of node paths not longer being unique, to guarantee that the editor would still store an unique reference to the node you want, it would use indices instead as in NodePath(3/0/2)

You are introducing a new cost here. If the NodePath store indexes instead of names, every time I change the order of nodes in the scene in the editor, Godot has to search all the NodePaths and update their indexes so they still refer to the same node. Well, that, or allow NodePaths to break.

Alright, hold on. I know that is only "in the editor". However, the editor is a Godot application. And I remind you people can write addons. What if an addon moves things in the scene tree? The editor would need a way to get a notification when nodes in the scene tree moved. And thus moving a node would have to emit a signal. Also adding or removing, anything that can changes indexes.

---

I would also like to point out that the text based formats are good for version control. It makes merging diffs affordable. However, if I have repository shared with a coworker and we both manipulate a scene… The correct indexes are going to be neither or ours, because the correct indexes would have to take into account what we both did.

In fact, these problems can be completely hidden. For example, we have that node path you mention (NodePath(3/0/2))… And I insert a node, so Godot updates it for me, from NodePath(3/0/2) to NodePath(3/0/3). And my coworker also inserts a different node, so Godot updates it for them, also from NodePath(3/0/2) to NodePath(3/0/3). And we merge, and... Hey! No conflict! Except it should be NodePath(3/0/4). And we would figure out too late.

These ARE consistency problems.

(I'm working with a Behavior Tree system, by the way, and inserting nodes to add steps in a sequence is a reasonable thing to do. Although I would prefer that each person works on a different scene, it happens).

---

... adding and removing nodes from the tree is just slow, but with this change it could be faster. ... You still have to prove it

While it obviously has to be profiled, it doesn't that much theorizing that having to call this whenever a node is added, has a cost

Oh, that? Let us find what is it calling:

https://github.com/godotengine/godot/blob/master/scene/main/scene_tree.cpp#L123

A function that emits a signal, I see. If that is the cost you want to get rid of, I think your proposal should be to remove that signal. So, let us find why that signal exists (blame took me to the commit, which took me to the issue):

godotengine/godot#18102

Ah. They added a signal so Godot can get a notification when an addon renamed a node! (Sounds familiar)

You know what, I like that. I'm all in favor of improving our capability to extend Godot. Because these kind of changes allow for more powerful addons, and thus, addons can provide more functionality, which - in turn - does not have be in core. Ultimately keeping Godot lightweight. I'm completely in favor of empowering addon creators.

---

Alright, if this has become a performance problem for you…

I remind you that if you are adding nodes at runtime, you don't have to give them names. Furthermore, if you want to give them names, if you it before adding them to the scene tree, that signal will not trigger. Thus:

1. Don't give names to nodes you add in runtime if you can help it. In fact, as you are already aware, you can use indexes with get_child. Edit: If you just added a node, you are holding a reference to it. Store it. So you don't have to call a function to get it. get_child would be what I suggest if you are adding too many, which - in my mind - is when you could have a performance issue (I mean, why store an array, if you have the scene tree).
2. If you have to give them names, give them names before adding them.

Beyond that, I would need to know more about the particulars of why this is a performance problem for you. There could be an alternative. For example - what comes to mind - is that you can add other properties to your nodes, which do not trigger signals when they change.

[thimenesup]

You are introducing a new cost here. If the NodePath store indexes instead of names, every time I change the order of nodes in the scene in the editor, Godot has to search all the NodePaths and update their indexes so they still refer to the same node. Well, that, or allow NodePaths to break.

So basically the same behaviour of when you rename a node?

Alright, hold on. I know that is only "in the editor". However, the editor is a Godot application. And I remind you people can write addons. What if an addon moves things in the scene tree? The editor would need a way to get a notification when nodes in the scene tree moved. And thus moving a node would have to emit a signal. Also adding or removing, anything that can changes indexes.

This behaviour you are describing is already implemented and there is a signal for it, in fact, my addon needed it but it wasn't exposed until I bothered to do a pull request for it

In fact, these problems can be completely hidden. For example, we have that node path you mention (NodePath(3/0/2))… And I insert a node, so Godot updates it for me, from NodePath(3/0/2) to NodePath(3/0/3). And my coworker also inserts a different node, so Godot updates it for them, also from NodePath(3/0/2) to NodePath(3/0/3). And we merge, and... Hey! No conflict! Except it should be NodePath(3/0/4).

What do you mean? If you add a new child node in both files they should have the same NodePath. If the nodes are different there should be a merge conflict anyways.

A function that emits a signal, I see. If that is the cost you want to get rid of

Its not only the signal, you are completely ignoring the functions that make sure that the node name is unique, which is called whenever a node is added, not only renamed.

[theraot]

@thimenesup

Perhaps I misunderstood. Weren't you suggesting to change the serialization of the scene to not use names but indexes instead? If it is how I imagined, two branches adding nodes on the same path would merge, but it would be wrong.

Anyway, now that you point _validate_child_name and _generate_serial_child_name I see that would be a problem when adding an instanced scene. What I was saying of adding node without name still stands, but if we are instancing a scene, it comes with a name. And if we are adding a bunch of instanced scenes that adds up.

At least you can remove the name before adding the instanced scene to the scene tree. So that the execution goes here: https://github.com/godotengine/godot/blob/master/scene/main/node.cpp#L967

That does nothing for the children nodes, of course, but there is where we would be using $ or get_node, relying on the names… Yes, removing the requirement for names to be unique would be a solution.

---

Ok, counter proposal:

• Keep NodePath as it is. So that get_node does not need to change, nor any language using it (e.g. convert names to indexes).
• Change p_force_human_readable to p_keep_name, and make it true the default.
• If p_keep_name is false always generate an "@" name. Never checks if the name already exists.
• If p_keep_name is true:
  • Tool builds will continue to use serial child names. So that we don't get "@" names in the editor when adding nodes.
  • Not Tool Debug builds: Will allow repeated names. But will check for them, and in case of collision will push a warning telling you that you might not be able to access those nodes, and suggesting to set p_keep_name to false instead. The warning should only happen when there is a collision so it does not happen all the time (e.g. when instancing a scene).
  • Not Tool Release builds: Will allow repeated names. Only the first matching will be accesible via get_node. No check is made. This way the release build is faster.

Note that the situation where we would get the warning, is a situation when we currently would have the node renamed, and we would not be able to predict the node path. If we follow the instruction and set p_keep_name to false, we would have the node renamed, all the same. If we ignore the warning, then we better not be trying to get the node from get_node, because it won't work.

[thimenesup]

That suggestion doesn't make any sense, why would unique names be a toggle? You either enforce it 100% or you can't trust that that your names are guaranteed to be unique when you toggle it on.

[theraot]

@thimenesup In my counter-proposal no option is offering a guarantee of unique names. You won't be able to trust names are unique. I don't know what toggle are you talking about.

Do you mean that I say a tool build (e.g. Godot editor) would continue to use serial names? That is just to give a nice default to the user (e.g. "AudioStreamPlayer", "AudioStreamPlayer2", "AudioStreamPlayer3" and so on). It does not guarantees they are unique. You could, and would be encouraged to, rename them to whatever you want, including the same name.

Do you mean passing true to p_keep_name? That is just Godot does not change the name. The names are what you tell them to be, and if they are unique or not, is up to you. So, perhaps you mean passing false to p_keep_name? That would generate @ names (no check involved for uniqueness), those are just so the node get names that do not interfere with whatever you are doing. Perhaps you would prefer removing @ names entirely?

See, no option is intended to give you unique names.

Is the warning what bothers you? That is to help beginners not shot themselves in the foot by not properly naming their nodes. Which is another reason to generate serial names (it would be a bad user experience to add nodes with repeated names just to complain to the user that there are repeated names).

And this is what you say on the title of your proposal, we "remove the requirement for node paths to be unique", therefore we can't trust that names are guaranteed to be unique. So I don't understand what bothers you.

[thimenesup]

Now I see you mean when creating a node, that there by default but optionally it should generate an human readable name, yeah that makes sense.

[Calinou]

Closing due to lack of support (see the above comments and reactions on OP).