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

GDScript: Support % in shorthand for get_node #61440

Merged
merged 1 commit into from
May 31, 2022

Conversation

vnen
Copy link
Member

@vnen vnen commented May 26, 2022

The % is used in scene unique nodes. Now % can also be used instead of $ for the shorthand, besides being allowed generally anywhere in the path as the prefix for a node name.

This is the syntax in GDScript to use the feature introduced in #60298.

Samples of the valid syntax:

var hey = $Child/Hey
var howdy = $Child/Hey/SubChild/Howdy

print(%Hey == hey)
print($%Hey == hey)
print(%"Hey" == hey)
print($"%Hey" == hey)
print($%"Hey" == hey)
print(%Hey/%Howdy == howdy)
print($%Hey/%Howdy == howdy)
print($"%Hey/%Howdy" == howdy)

A similar thing could be done for 3.x since it doesn't break compatibility.

Closes godotengine/godot-proposals#4583

@vnen vnen added this to the 4.0 milestone May 26, 2022
@vnen vnen requested a review from a team as a code owner May 26, 2022 16:14
@Mickeon
Copy link
Contributor

Mickeon commented May 26, 2022

I don't know about allowing the lone %. It sounds nice and cool on paper, but I personally find $% to be more explanatory.

@fire-forge
Copy link
Contributor

fire-forge commented May 26, 2022

Awesome! I tested it and it works great.

This also closes godotengine/godot-proposals#4583

@Mickeon
Copy link
Contributor

Mickeon commented May 27, 2022

Actually, my reasoning goes further than that. I would personally hold off the lone % operator as it does affect all of GDScript2. It's been fine for the $ operator, as accessing Nodes is fairly common in Godot, but introducing another wholy lone operator that does around the same thing, but only works in a "Scene & Node" context may be excessively cramped.

That's also because, in GDScript, % is already used in two wholy different contexts:

  • Modulus
  • String formatting

Introducing a third, completely unrelated purpose for it, I find to really be too much.

And that's not even mentioning, should godotengine/godot-proposals#4593 be implemented a certain way (and it really should, it'd be a massive usability improvement), autocompletion may display even if the context does not warrant it.

@vnen
Copy link
Member Author

vnen commented May 27, 2022

@Mickeon I don't think it's that much of a problem. This context is quite different from the others where % is used, it should be clear from the start that's a different thing. Completion should not be affected by this as it should only show nodes when % is used as a prefix operator.

If there's a lot of the demand I could still change it, but for what I've seen most people are okay with the syntax I implemented here.

@KoBeWi
Copy link
Member

KoBeWi commented May 27, 2022

Could you maybe add better support for dropping node path?
godot windows tools 64_uZW6jIerMr
It should drop without quotes and $.

The relevant code is here:

for (int i = 0; i < nodes.size(); i++) {
if (i > 0) {
text_to_drop += ", ";
}
NodePath np = nodes[i];
Node *node = get_node(np);
if (!node) {
continue;
}
String path;
if (node->is_unique_name_in_owner()) {
path = "%" + node->get_name();
} else {
path = sn->get_path_to(node);
}
for (const String &segment : path.split("/")) {
if (!segment.is_valid_identifier()) {
path = path.c_escape().quote(quote_style);
break;
}
}
text_to_drop += "$" + path;
}
}

@reduz
Copy link
Member

reduz commented May 27, 2022

@vnen This is awesome, you rock!

The `%` is used in scene unique nodes. Now `%` can also be used instead
of `$` for the shorthand, besides being allowed generally anywhere in
the path as the prefix for a node name.
@vnen vnen force-pushed the gdscript-scene-unique-nodes branch from 0c70a5f to eba3e0a Compare May 27, 2022 16:46
@vnen vnen requested a review from a team as a code owner May 27, 2022 16:46
@vnen
Copy link
Member Author

vnen commented May 27, 2022

@KoBeWi I updated the code to improve the drag & drop. Not sure if this works well with instanced scenes, but it can be improved further in the future.

@akien-mga akien-mga merged commit 68bf4eb into godotengine:master May 31, 2022
@akien-mga
Copy link
Member

Thanks!

@vnen vnen deleted the gdscript-scene-unique-nodes branch June 2, 2022 09:44
@zenvoid
Copy link
Contributor

zenvoid commented Jun 2, 2022

It's been fine for the $ operator, as accessing Nodes is fairly common in Godot, but introducing another wholy lone operator that does around the same thing, but only works in a "Scene & Node" context may be excessively cramped.

That's also because, in GDScript, % is already used in two wholy different contexts:

  • Modulus
  • String formatting

Introducing a third, completely unrelated purpose for it, I find to really be too much.

I agree. While it shouldn't be a problem in practical terms, it's starting to look too much like Perl and I find it ugly. "Ugly" being completely subjective, other people seem to like it and that's fine. Not a big problem for me, I can live with it. Just trying to publicly manifest how much I hate this new syntax ;)

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

Successfully merging this pull request may close these issues.

Allow accessing Scene Unique Nodes without quotes in GDScript
7 participants