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

Add shorthand expr.$NodePath for expr.get_node("NodePath") #1776

Open
dalexeev opened this issue Nov 5, 2020 · 19 comments
Open

Add shorthand expr.$NodePath for expr.get_node("NodePath") #1776

dalexeev opened this issue Nov 5, 2020 · 19 comments

Comments

@dalexeev
Copy link
Member

dalexeev commented Nov 5, 2020

Describe the project you are working on:
A game

Describe the problem or limitation you are having in your project:
When I want to create onready variables for nested nodes, I have to repeat the path prefix:

onready var item1 = $Scroll/Grid/Item1
onready var item2 = $Scroll/Grid/Item2
onready var item3 = $Scroll/Grid/Item3

Obviously, this is not good. For example, the path to the Grid node may change and must be changed in all variables.

I can replace this with

onready var grid = $Scroll/Grid
onready var item1 = grid.get_node("Item1")
onready var item2 = grid.get_node("Item2")
onready var item3 = grid.get_node("Item3")

But I would like to use a more compact syntax.

Describe the feature / enhancement and how it helps to overcome the problem or limitation:
Add shorthand expr$NodePath for expr.get_node("NodePath"). Then we can do this:

onready var grid = $Scroll/Grid
onready var item1 = grid$Item1
onready var item2 = grid$Item2
onready var item3 = grid$Item3

or:

onready var grid = $Scroll/Grid
onready var item1 = grid.$Item1
onready var item2 = grid.$Item2
onready var item3 = grid.$Item3

Describe how your proposal will work, with code, pseudocode, mockups, and/or diagrams:
I haven't looked at how this is implemented, so I don't know.

If this enhancement will not be used often, can it be worked around with a few lines of script?:
It is possible to attach the script directly to the Grid node, but this option is not always acceptable.

Is there a reason why this should be core and not an add-on in the asset library?:
This cannot be done with an add-on, because it affects the GDScript syntax.

@YuriSizov
Copy link
Contributor

Not sure if that's the best possible syntax, but I agree with this being a problem.

@Calinou
Copy link
Member

Calinou commented Nov 5, 2020

If we were to allow this, I think grid.$Item1 (with a .) would make more sense. That said, I can imagine this being difficult to parse.

@KoBeWi
Copy link
Member

KoBeWi commented Nov 5, 2020

Obviously, this is not good. For example, the path to the Grid node may change and must be changed in all variables.

This is a matter of using Replace All, so it's not really a big problem.

@dalexeev
Copy link
Member Author

dalexeev commented Nov 5, 2020

it's not really a big problem

Yes, you're right, this is just syntactic sugar. But this is a logical continuation of the $NodePath syntax, which is also syntactic sugar.

@Tyrannosaurus1234
Copy link

Tyrannosaurus1234 commented Nov 6, 2020

You can do $"parentNode/childNode" to provide the $ operator with a nodePath that includes slashes. Doing $parentNode/childNode, to me, sounds like asking for trouble from a syntax standpoint. To reduce boilerplate and increase portability in your paths, you can, theoretically, do something like:

onready var gridPath = "thisNode/grid"
onready var item1 = $gridPath+"/item1"
onready var item1 = $gridPath+"/item2"
onready var item1 = $gridPath+"/item3"

@dalexeev
Copy link
Member Author

dalexeev commented Nov 6, 2020

You can do $"parentNode/childNode" to provide the $ operator with a nodePath that includes slashes.

@Tyrannosaurus1234 You may not have known this, but it is not necessary, it even works without the quotes.

var x = "path/to/node"

# In 3.2 these options are equivalent:
$path/to/node
$"path/to/node"
get_node(x)

# In an alternate reality:
$"path/to/node"
$x
$(x)
$("path/to/" + "node")

Basically, I like the option with parentheses because it doesn't break anything.

var grid_path = "Scroll/Grid/"
onready var item1 = $(grid_path + "Item1")

But my option is still more concise:

onready var grid = $Scroll/Grid
onready var item1 = grid$Item1

@Calinou
Copy link
Member

Calinou commented Nov 6, 2020

For reference, quotes with $ are only required for relative paths that use .. or absolute paths:

$"../Player"
$"/root/Game/Level"

@vnen
Copy link
Member

vnen commented Mar 2, 2021

For reference, quotes with $ are only required for relative paths that use .. or absolute paths

Or when the node name contains spaces or special characters (i.e. when it's not a valid identifier).

That said, I would prefer Calinou's suggestion of using node.$ChildPath.

@dalexeev
Copy link
Member Author

dalexeev commented Mar 3, 2021

That said, I would prefer Calinou's suggestion of using node.$ChildPath.

I'm satisfied with both options. If you think the second is better, I'll agree with you.

@Bromeon
Copy link

Bromeon commented Mar 3, 2021

Is this a frequent use case?

Personally I've made the experience that most of the time, I'm requesting a node relative to the current one.
If I wanted to access a sub-node of an unrelated node directly, it would often indicate a lack of encapsulation (my node needs to know the inner structure of the other node, and changing that breaks unrelated code).

@dalexeev
Copy link
Member Author

dalexeev commented Mar 3, 2021

Is this a frequent use case?

Quite often in UI scenes, because many intermediate nodes are used, especially containers. In other types of scenes, too, there are sometimes many levels of nesting. I don't use 3D, but I saw from the screenshots how many nodes people have.

Plus:

Yes, you're right, this is just syntactic sugar. But this is a logical continuation of the $NodePath syntax, which is also syntactic sugar.

@h0lley
Copy link

h0lley commented Feb 25, 2022

is this really purely syntactic sugar?

I grabbed
https://github.com/Zylann/gdscript_performance
added a couple more $ tests, ran it in the current alpha, and here is the ouput:

-------------------
The following times are in microseconds, for one occurence of a test.

For time: 0.035us
Running micro benchmarks...
get_node() where only one child...
    0.508us (1000000 iterations)
get_node() where only one child with $...
    0.165us (1000000 iterations)
get_node() where 10 children...
    0.522us (1000000 iterations)
get_node() where 10 children with $...
    0.167us (1000000 iterations)
get_node() 3 parents deep having 10 children...
    0.984us (1000000 iterations)
get_node() 3 parents deep having 10 children with $...
    0.206us (1000000 iterations)
-------------------
Saving results...
Done.

feel free to test yourself: project4.zip

as you can see, the $ versions have significantly better performance.
I suspect this is due to the fact that $ does not take variable node paths, and so perhaps they are being indexed?

so my conclusion is unless node paths are variable, get_node() should generally be avoided, and so it is really unfortunate that right now, $ cannot be called on a node reference.

my_node.$ChildNode would be cool.
if syntax highlighting still works with the node path portion being distinctly colored, I think readability will be just fine.

@dalexeev
Copy link
Member Author

is this really purely syntactic sugar?

as you can see, the $ versions have significantly better performance.

I suspect this is due to the fact that $ does not take variable node paths

Yes, I read somewhere that the difference between get_node and $ is that in the second case, constant precompiled NodePaths are used, and in the first case, the String -> NodePath conversion is additionally performed each time.

Try also comparing with the get_node(@"NodePath") option (get_node(^"NodePath") in 4.0-dev).

Yes, it's not exactly syntactic sugar, but it's more shorthand and syntactic sugar than an optimization tool.

@h0lley
Copy link

h0lley commented Feb 25, 2022

didn't knew about the ^ thing!
yes, get_node(^"NodePath") and $NodePath have exactly the same performance.

still very much in favor of this proposal though.

@BaddRadish
Copy link

two things here:

  1. $() could simply /not/ take indexed path, so that wouldnt break anything. in fact get_node("some_literal") should smartly index but oh well.
  2. $/path works fine for me, so really you only need quotes for ../ which i find deeply disturbing.

@LuisCarli
Copy link

The more I’m using Godot, the more I’m relying on nodes, get_node and on the $ shortcut.

I’ve tried to use variable.$NodeName before, because I thought $NodeName worked as if it was being replaced by a get_node(^”NodeName”) call when the script was parsed. So it should work anywhere a get_node call was being used.

I think this proposal is a logical generalization of the current $ shortcut. And I think the $ shortcut is useful enough for this generalization to be important.

@h0lley
Copy link

h0lley commented May 10, 2023

meanwhile scene unique nodes where introduced with their own shorthand, so one would now expect variable.%NodeName to work in the same way.

@dalexeev dalexeev changed the title Add shorthand expr$NodePath for expr.get_node("NodePath") Add shorthand expr.$NodePath for expr.get_node("NodePath") Apr 29, 2024
@Jean-Low
Copy link

This is useful syntax clarity for using heavy composition with nodes. I have a damage system that all creatures in my game have, this is represented by a node with the same script inside each creature scene. Using if(body.$DamageSystem): inside collision signals is really intuitive, instead i need to rely on get_node() for now.

@crotron
Copy link

crotron commented Aug 28, 2024

I would also like to add a use case that I'm currently working with. I have a bunch of code in various places that is doing more or less the same bunch of function calls on the children of different child nodes. For example:

# Instance 1
$"Menu/Items".is_focused = false
$"Menu/Items/Clickbox".mouse_filter = MOUSE_FILTER_IGNORE
$"Menu/Items/Sprite2D".use_parent_material = false # etc

# Instance 2
$"Monster".is_focused = false
$"Monster/Clickbox".mouse_filter = MOUSE_FILTER_IGNORE
$"Monster/Sprite2D".use_parent_material = false # etc

(there are a bunch more lines similar in structure to the above that I removed for brevity)

I'm looking to refactor my code to implement the common behavior in a helper function, in the spirit of DRY. Something like the following:

func unfocus(thing):
	thing.is_focused = false
	thing.get_node('Clickbox').mouse_filter = MOUSE_FILTER_IGNORE
	thing.get_node('Sprite2D').use_parent_material = false #etc

I can then call it with thing = $"Menu/Items" for Instance 1 and thing = $"Monster" for Instance 2. This works, but because what I'm trying to work on is now a variable and not self, I can't call $ inside the helper function and must instead use get_node as above. I think that being able to use .$ in this context would improve readability.

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