Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to register nodes globally with a unique identifier #2812

Closed
RebelliousX opened this issue Jun 2, 2021 · 28 comments
Closed

Allow to register nodes globally with a unique identifier #2812

RebelliousX opened this issue Jun 2, 2021 · 28 comments

Comments

@RebelliousX
Copy link

RebelliousX commented Jun 2, 2021

Describe the project you are working on

This proposal is not related to any specific project. In fact, I believe
it will enhance productivity for all types of projects, whether they
are related to UI designing, or scene management in general.

Describe the problem or limitation you are having in your project

There is a need to "easily" access any object whether it is down or up in
hierarchy, or even in a different sub tree of nodes without relying on
relative or absolute paths. I believe adding a reference to an object,
and de-referencing its value would get us the object no matter where it
resides or even if the user changes the scene hierarchy layout.

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

There is always a rigid requirement to pay attention to the scene layout and its hierarchy.
For example, "call down and signal up", modifying scripts to change absolute / relative paths
when scene layout change.

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

The proposal is to add a property to every object or node type.
That unique property is just a reference or a "Unique ID" for that
object which is added to a global and hidden (from the user)
set/hash map for quick lookup. All the user needs to do to get
an object or a node is to ask for that unique ID.

The unique ID can be auto-generated string with the name of the object
or even can be modified by the user if he/she wishes to. Well, as long
as it remains unique.

Benefits:

  • Eliminate user confusion when accessing nodes or objects.
    There is no need to remember "call down, signal up".
  • Absolute and relative paths won't matter when accessing an object.
  • Future modification to a project's layout of nodes and their heirarchy
    won't require painfull editing of scripts of changed absolute or relative
    paths.
  • To relate more to what users of other game engines are familiar with.
    For example, Game Maker Studio can access any object easily, and Unity
    -I believe- has reference objects too.

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

No, because if any change to the project while in the middle or later stages of development will
require lots of script editing and going bug hunting which can waste lots of time.

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

I don't think this functionality can be added using an Addon.

How to access the object using a unique ID?

Any suitable good way with general consensus and acceptance would be good.
I suggest using double dollar sign $$"uniqueID" or a function similar to
get_node by UID.

And lastly, I believe this won't break any of current functionality at all,
since all current behaviors are retained. And accessing by using a unique id
is just optional.

Edit:
To clarify more, the hash map is just mapping the unique ID which can be set as a string (while designing, before running the project) to the newly created instance of the object at Runtime, the global hash map is filled at runtime using UID "a string" as a key, to reference an object (which can vary from one runtime to another) the value is get_instance_id(). If the object is not yet created, the value would be null anyways.

Then user can access the object, say a Node2D with UID of "Node2D_001" to access it: $$"Node2D_001" no matter where that node is nested 20 levels deep up or down, which will return the object itself, which can be implemented say instance_from_id(hashmap["Node2D_001"]), this process should be hidden from the user.

@YuriSizov
Copy link
Contributor

Because scenes are reusable assets, nodes in them cannot be marked with a unique ID (you can instance each scene multiple times therefore you'd have multiple nodes with the same "unique" ID). If you want global access to some of your nodes, consider creating an autoloaded manager and registering your nodes there with a script.

Otherwise, using hierarchy is the only way to go, and rigidness is a feature of it, not a limitation.

@YuriSizov YuriSizov changed the title Easier way to reference objects! Allow to register nodes globally with a unique identifier Jun 2, 2021
@RebelliousX
Copy link
Author

Because scenes are reusable assets, nodes in them cannot be marked with a unique ID (you can instance each scene multiple times therefore you'd have multiple nodes with the same "unique" ID). If you want global access to some of your nodes, consider creating an autoloaded manager and registering your nodes there with a script.

Otherwise, using hierarchy is the only way to go, and rigidness is a feature of it, not a limitation.

Well, whether we are instancing (to be reused) or duplicating (cloning) a scene (to make it unique), a scene object (in C++) behind the scenes (no pun intended) is just an object that has different memory location. Accessing that object using its memory
location would save lots of frustration.

Besides, I disagree that rigidness is a freature and not a limitation. Scene layout and scripts should go hand in hand, that is,
modifying scene layout should not interfere with scripts or require editing at all! This is the rigidness I am talking about. :)

@Xrayez
Copy link
Contributor

Xrayez commented Jun 2, 2021

Otherwise, using hierarchy is the only way to go, and rigidness is a feature of it, not a limitation.

I think your presumption is false in this regard.


This is kind of already possible, except that unique identifier is not a property of Object (which could be technically exposed as a read-only property):

extends Node2D

func _ready():
	var object = $Sprite
	var unique_id = object.get_instance_id()
	var object_from_id = instance_from_id(unique_id)
	assert(object == object_from_id)

You can pass unique IDs that way and the objects are retrieved from ClassDB. Also, this approach has the benefit of not returning "freed objects" (you may have heard that you could use is_instance_valid() method for this to handle "previously freed objects" problem).

That said, I like the idea behind this proposal, because core does have features which could help implementing in a way to resolve further limitations.

@YuriSizov
Copy link
Contributor

YuriSizov commented Jun 2, 2021

Accessing that object using its memory location would save lots of frustration.

Of course, and objects in memory have IDs already. If you want to reference them using that, you can, but that ID will vary run to run. I was under the impression you wanted to set it yourself in the editor beforehand.

Scene layout and scripts should go hand in hand, that is, modifying scene layout should not interfere with scripts or require editing at all!

Scripts are a way to bring scenes to life. Of course scripts should be aware of the node structure they control. But that only applies to the nodes that this script has direct access to. Cross-branch access is considered a bad-practice and a global manager object is how you do it properly. You indeed should not make your scripts in one branch be aware about the particular node structure in another. You create APIs for that and abstract your access behind them.

@RebelliousX
Copy link
Author

RebelliousX commented Jun 2, 2021

Of course, and objects in memory have IDs already. If you want to reference them using that, you can, but that ID will vary run to run. I was under the impression you wanted to set it yourself in the editor beforehand.

Sorry for the misunderstanding, I did not realize that global instance of objects already exists in Godot according to @Xrayez code snippet, because of my ignorance of that, I suggested a way to implement it (in C++) using a hash map with Keys as a unique id string which can be modified by user if he/she wish to, and the value would be the object itself.

Edit: To clarify more, the hash map is just mapping the unique ID which can be set as a string (while designing, before running the project) to the newly created instance of the object at Runtime, the global hash map is filled at runtime using UID "a string" as a key, to reference an object (which can vary from one runtime to another) the value is get_instance_id(). If the object is not yet created, the value would be null anyways.

Then user can access the object, say a Node2D with UID of "Node2D_001" to access it: $$"Node2D_001" no matter where that node is nested 20 levels deep up or down, which will return the object itself, which can be implemented say instance_from_id(hashmap["Node2D_001"]), this process should be hidden from the user.

@KoBeWi
Copy link
Member

KoBeWi commented Jun 2, 2021

I don't think this functionality can be added using an Addon.

Except it can. You can use SceneTree.node_added signal for that. Just connect it to some autoload node and you have access to every created node. You can easily assign them your own unique IDs and store them in some Dictionary.

The unique ID can be auto-generated string with the name of the object or even can be modified by the user if he/she wishes to.

This sounds like something that everyone would want to have different. Auto-generated names might not suit everyone's needs or expectations, which makes such functionality better to be an addon.

@RebelliousX
Copy link
Author

I don't think this functionality can be added using an Addon.

Except it can. You can use SceneTree.node_added signal for that. Just connect it to some autoload node and you have access to every created node. You can easily assign them your own unique IDs and store them in some Dictionary.

No, that will complicate things instead of simplifying them. Besides, doing that in gdscript will be dog slow! In this proposal we are trying to attach a reference (UID string) in the editor (at compile time) to a reference an object yet to be created in runtime (instance id) to get an object of that reference.

And as for the UID name, as I suggested, it can be auto-generated or / and give the option to the user to name it. The same way nodes are named automatically (or can be renamed) when added to the editor.

@KoBeWi
Copy link
Member

KoBeWi commented Jun 2, 2021

No, that will complicate things instead of simplifying them.

How? You can write such addon under 50 lines and then you don't need to touch it anymore. Its functionality can be whatever you want, so if you make it simple, it will be simple.

Besides, doing that in gdscript will be dog slow!

It won't, unless you are spawning thousand of nodes very often.

@RebelliousX
Copy link
Author

No, that will complicate things instead of simplifying them.

How? You can write such addon under 50 lines and then you don't need to touch it anymore. Its functionality can be whatever you want, so if you make it simple, it will be simple.

Because we can't use instance_from_id() and get_instance_id() in the editor (nodes are not running yet) unless we make all nodes using tool keyword in their scripts. In this proposal, this needs to work in editor and runtime.

I am not saying it is impossible as an addon, maybe. But I really believe the benefits of this proposal to be added to the engine
natively outweighs any other solution. Also, not all users will be aware of such addon.

@YuriSizov
Copy link
Contributor

How can this work in the editor? In the editor scenes are not aware of each other. And within a single scene you should use node paths and node relations.

@AaronRecord
Copy link

AaronRecord commented Jun 2, 2021

You might be interested in Node.find_node(), although it's performance is less good than a hash map would be. Also #1048 which I think is a better solution to this problem.

@SilencedPerson
Copy link

90% of the time when i wish could look up nodes in the global scope, i am looking for just 1 specific node.
What about just having group that returns single object?
let's call it a tag. It would always return last object assigned under this tag, if there is none, it returns null. If there is already node with that tag and increment argument is TRUE, add number behind it. otherwise just overwrite it.

SceneTree.get_node_by_tag("Cutscene")

Node.tag_as("BulletManager",increment = true)
Node.tag_as("BulletManager",increment = true)
Node.tag_as("BulletManager",increment = true)

Get_tree().get_node_by_tag("BulletManager")
Get_tree().get_node_by_tag("BulletManager1")
Get_tree().get_node_by_tag("BulletManager2")
Get_tree().get_node_by_tag("John Doe")

Node.remove_tag("CurrentlySelectedWindow")

I think this is sufficient solution to the problem, as you can always use get_node("whatever") to get the children, or even concatenate the owner's tag to their, for example Get_tree().get_node_by_tag("BulletManager2Fireball4")

@YuriSizov
Copy link
Contributor

What about just having group that returns single object?

Since we already have node groups, I don't see why we need another system that 100% overlaps with it.

@KoBeWi
Copy link
Member

KoBeWi commented Jun 2, 2021

btw in 4.0 there is a method to get the first node in a group. Groups are pretty performant, so using them as tags is totally possible.

@SilencedPerson
Copy link

btw in 4.0 there is a method to get the first node in a group. Groups are pretty performant, so using them as tags is totally possible.

ah, that's a step in the right direction then, however, i would still like to have an ability to add number to the group if there is anything assigned to it already. just that little change could make all the other other ways of getting nodes redundant.

@RebelliousX
Copy link
Author

RebelliousX commented Jun 2, 2021

The thing is, with making an addon for this or not, again, I don't think it is simple enough of just accessing nodes in the scene tree.
If we try to make an addon, even if we are successfully able to access the nodes, I don't think an addon is able to modify all objects to add a unique ID (or unique name) property to them. Accessing a node by its name won't cut it since another node with the same name on a different sibling branch for example might have the same name.

Recently, I was working with a GUI app for Android, it is not a game. It has lots of controls created at run time to read and write
data fields in many controls. For example, a simple CheckBox can be nested in 3 to 4 or more VBox or HBox containers, and might have its own different control or parent as well. And if we try to get the value of that CheckBox inside an OK button for example, we have to refer to it either using a relative or absolute path, which can be really LONG path!! And this is true even if the path is down the hierarchy, say a parent try to access the text value of a LineEdit nested deep 10 different containers and controls. If we change the layout slightly we have to modify the path inside the script. Or even worse, make a signal to save its state in some AutoLoad signleton.

Again, it is not about an addon is enough, sure making a dictionary in gdscript and add all nodes in the scene tree seems to be easy, But that is only at runtime not while inside editor or editing a script of random object. I don't think an addon can do that.

Yet, I admit, naming the proposed property as Unique ID can be misleading, since we already have global ID for object / instance. maybe Unique Name is better (that is, unlike a node's name, it should be guaranteed there is no name clashes even in different scene tree branches).

What is better?

onready var MainPage = $Background/MainScreen/MainPage
onready var DeleteTransactionPage = $Background/MainScreen/DeleteTransactionPage
onready var NewAssetPage = $Background/MainScreen/NewAssetPage
onready var DeleteAssetPage = $Background/MainScreen/DeleteAssetPage
onready var cmbAssetsList = $Background/MainScreen/MainPage/AssetsContainer/AssetsList
onready var transactionListContainer = $Background/MainScreen/MainPage/Transactions/TransactionsList

or

onready var MainPage = $$"MainPage_001"
onready var DeleteTransactionPage = $$"DeleteTransactionPage_001"
onready var NewAssetPage = $$"NewAssetPage_001"
onready var DeleteAssetPage = $$"DeleteAssetPage_001"
onready var cmbAssetsList = $$"AssetsList_001"
onready var transactionListContainer = $$"TransactionsList_001"

Now even if we change the layout or where the controls that needed to be accessed are located or nested inside a hundred horizontal or vertical containers. We would be able to access them without changing a single line of code! I hope that makes it clearer.

@KoBeWi
Copy link
Member

KoBeWi commented Jun 2, 2021

This already works:

onready var MainPage = find_node("MainPage")
onready var DeleteTransactionPage = find_node("DeleteTransactionPage")
onready var NewAssetPage = find_node("NewAssetPage")
onready var DeleteAssetPage = find_node("DeleteAssetPage")
onready var cmbAssetsList = find_node("AssetsList")
onready var transactionListContainer = find_node("TransactionsList")

@RebelliousX
Copy link
Author

RebelliousX commented Jun 3, 2021

using find_node can be costly since it needs most of the time to be recursive to search for a node! Add to that we have to determine if we own that node or not. What if the node is inside another sibiling branch? That would slow everything down. a HashMap lookup would be a lot faster.

This is what the documentation says about find_node

Node find_node ( String mask, bool recursive=true, bool owned=true ) const
Finds a descendant of this node whose name matches mask as in String.match (i.e. case-sensitive, but "*" matches zero or more characters and "?" matches any single character except ".").
Note: It does not match against the full path, just against individual node names.
If owned is true, this method only finds nodes whose owner is this node. This is especially important for scenes instantiated through a script, because those scenes don't have an owner.
Note: As this method walks through all the descendants of the node, it is the slowest way to get a reference to another node. Whenever possible, consider using get_node instead. To avoid using find_node too often, consider caching the node reference into a variable.

At the end, the docs still recommends to cache the node into a variable which puts back to square 1.

@KoBeWi
Copy link
Member

KoBeWi commented Jun 3, 2021

But this is what onready is for. It caches the result of find_node(), so it runs only once. This way it's cheap to use.

HashMap will only make real difference if you access the node very often. But such nodes are usually stored in variables anyways (onready).

@RebelliousX
Copy link
Author

This is true in this case only which is just an example. But to try to find a node that is created at runtime to access its properties inside the gameloop would kill the performance for sure.

I am not sure if you guys see the benefits of this proposal or not?! But I certainly see lots of pros (listed in first post) and minimal cons.

@KoBeWi
Copy link
Member

KoBeWi commented Jun 3, 2021

Well, personally I don't see any benefits. I have created tens of Godot projects and never needed such feature. get_node(), groups and signals are enough pretty much every case and most of users will agree with that. If you need to rely on find_node() or some global map of every node in tree then you are doing something wrong, i.e. there is problem with your project structure not the engine.

But to try to find a node that is created at runtime to access its properties inside the gameloop would kill the performance for sure.

It won't. Again, any node that is accessed often (per frame or per multiple frames or, like, thrice in lifetime) should be stored in a variable (btw accessing variable is faster than using global hashmap). The only cost is when the node is being created and it's minimal. You can instance 10000 scenes with scripts that have ~10 onready variables and simply instancing them will take the most time, accessing nodes is just negligible.

As for cons, you are proposing a complex system that solves already solved problem. And it's not performance-free either, from what I understand every node would need to have a generated unique id, which then needs to be stored (though it's probably not expensive).

I remember there was similar issue in the past: godotengine/godot#12541

@dalexeev
Copy link
Member

dalexeev commented Jun 3, 2021

There is a need to "easily" access any object whether it is down or up in
hierarchy, or even in a different sub tree of nodes without relying on
relative or absolute paths. I believe adding a reference to an object,
and de-referencing its value would get us the object no matter where it
resides or even if the user changes the scene hierarchy layout.

This is possible:

# global.gd
var node1
var node2
var node3

# node1.gd
func _ready():
    Global.node1 = self

# another.gd
if Global.node1:
    print(Global.node1)

However, this indicates some problems in the architecture of your project. Global relationships should be as small as possible.

  • To relate more to what users of other game engines are familiar with.
    For example, Game Maker Studio can access any object easily

This is more of a disadvantage than an advantage, because it encourages bad code. But even the existing capabilities don't stop you from writing Game Maker-style code.

In any case, I see no need to create another redundant system. Perhaps my words will sound rude, but learn to use the existing functionality before asking to create something that would be more understandable and convenient for you personally.

@RebelliousX
Copy link
Author

Well, personally I don't see any benefits. I have created tens of Godot projects and never needed such feature.

I am sorry to hear that your small projects were as simple as the tutorial found on godot docs, otherwise, you would find the benefits of this proposal. But I don't judge other people work. I am sorry that you failed to see that implementing this will make scripts and scenes more cohesive together, changing one in anyway won't require the other to be changed.

There is a need to "easily" access any object whether it is down or up in
hierarchy, or even in a different sub tree of nodes without relying on
relative or absolute paths. I believe adding a reference to an object,
and de-referencing its value would get us the object no matter where it
resides or even if the user changes the scene hierarchy layout.

This is possible:

# global.gd
var node1
var node2
var node3

# node1.gd
func _ready():
    Global.node1 = self

# another.gd
if Global.node1:
    print(Global.node1)

However, this indicates some problems in the architecture of your project. Global relationships should be as small as possible.

  • To relate more to what users of other game engines are familiar with.
    For example, Game Maker Studio can access any object easily

This is more of a disadvantage than an advantage, because it encourages bad code. But even the existing capabilities don't stop you from writing Game Maker-style code.

In any case, I see no need to create another redundant system. Perhaps my words will sound rude, but learn to use the existing functionality before asking to create something that would be more understandable and convenient for you personally.

Clearly you did not read any of my posts, forgot or you decided to ignore them. Look a few posts up about how it is not about making an addon to do this since it can't be done in an addon, all objects should have a unique id string baked into them. You code sinppet is flawed and has nothing to do with what I am trying to achieve. Sorry.

Again this is just a proposal, I still firmly believe that this will bring benefits and eliminate most of the limitations of interactions between scene management and scripts. But if people found this useless, I don't want to waste my precious time explaining over and over. I already did waste lots my time. Peace out.

@AaronRecord
Copy link

AaronRecord commented Jun 3, 2021

It sounds like your problems are that the node paths are rigid (break when a node's path changes) and that node paths can get very long, which are slow to write and look messy. I mentioned #1048 earlier because it solves both of these problems:

  • No hard coded name or path is necessary, it's a reference to a specific node which won't break if the node's path changes
  • You don't have to write out long paths by hand, no messy scripts

Plus then you don't need to manage unique IDs for each node, it gets handled automatically

@dalexeev
Copy link
Member

dalexeev commented Jun 3, 2021

If you need references within one scene, you can do this:

export(NodePath) var MainPagePath
export(NodePath) var DeleteTransactionPagePath
export(NodePath) var NewAssetPagePath
export(NodePath) var DeleteAssetPagePath
export(NodePath) var cmbAssetsListPath
export(NodePath) var transactionListContainerPath

onready var MainPage = get_node(MainPagePath)
onready var DeleteTransactionPage = get_node(DeleteTransactionPagePath)
onready var NewAssetPage = get_node(NewAssetPagePath)
onready var DeleteAssetPage = get_node(DeleteAssetPagePath)
onready var cmbAssetsList = get_node(cmbAssetsListPath)
onready var transactionListContainer = get_node(transactionListContainerPath)

The paths will be stored in the scene file and updated automatically. You only need to specify them once in the editor. There is a proposal to add the ability to export Nodes at once, instead of NodePaths.

@RebelliousX
Copy link
Author

RebelliousX commented Jun 3, 2021

@LightningAA Yes, something similar to that, but still NodePath is still rigid because it can change by the user and that will require changes to scripts as well, I am trying to eliminate that.

Long time ago, heck 6 years ago, I did open an issue godotengine/godot#3163
" Renaming anything in Godot should rename their references #3163" many people liked the idea, at the end it was dropped.
In that issue, I posted a video about a youtuber who made NodPaths dynamic and changing them in editor made them change in script automatically. Unfortunately he did not share his code and did not make PR for that.

IMAGE ALT TEXT HERE

@dalexeev
Copy link
Member

dalexeev commented Jun 3, 2021

made NodPaths dynamic and changing them in editor made them change in script automatically

I don't know how it was in older versions, but now it works. When you move or rename a node in the hierarchy, the exported variable is updated automatically.

@AaronRecord
Copy link

AaronRecord commented Jun 3, 2021

Here's the new proposal for auto updating references btw: #899

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants