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

Create onready variables when dropping nodes into script editor and holding Ctrl #4482

Closed
timothyqiu opened this issue May 2, 2022 · 12 comments · Fixed by godotengine/godot#60708
Milestone

Comments

@timothyqiu
Copy link
Member

Describe the project you are working on

GUI tutorial

Describe the problem or limitation you are having in your project

Repeatedly typing onready var name := $Path/To/Node as NodeType for various nodes.

Dragging nodes into the script editor currently only inserts "Path/To/NodeA", "Path/To/NodeB".

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

When holding Ctrl, dropping nodes into the script editor creates and initializes onready variables for the dropped nodes.

It's similar to how file dropping works: when holding Ctrl, it drops a preload() function call instead of a path string.

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

Peek.2022-05-02.21-03.mp4

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

What to drop is in C++ code. It cannot be worked around, at least not a few lines.

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

Seems impossible.

@Mickeon
Copy link

Mickeon commented May 3, 2022

What's the use case of the regular drop, in the first place? I assume it's for being inserted in get_node(), but I find weird it doesn't include it or $ automatically, although it's consistent with FileSystem file dropping.

Anyway, perhaps, going along with this proposal, dropping inside of a function while holding CTRL should not insert the @onready? Since, the annotation would pretty much be discarded on those occasions anyhow.

@h0lley
Copy link

h0lley commented May 4, 2022

any reason for as Type instead of : Type =?

@timothyqiu
Copy link
Member Author

any reason for as Type instead of : Type =?

For these onready variables, normally, the type name can be deduced from the variable name:

# Looks quite busy and repetitive.
@onready var animation_player: AnimationPlayer = $Path/To/AnimationPlayer

# You can safely ignore the right side of assignment.
@onready var animation_player := $Path/To/AnimationPlayer as AnimationPlayer

@timothyqiu
Copy link
Member Author

dropping inside of a function while holding CTRL should not insert the @onready?

To me, this could be a dedicated feature.

Dropping is not context aware currently. There are more things to handle in order to be "correct", e.g. :

  • Dropping inside|identifier
  • Dropping "inside| a string" or comment
  • Dropping as a parameter of get_node(|). What if multiple nodes are dropped in this case?
  • What to do if it's dropped inside a nested class?
  • etc...

@YuriSizov
Copy link
Contributor

any reason for as Type instead of : Type =?

For these onready variables, normally, the type name can be deduced from the variable name:

# Looks quite busy and repetitive.

@onready var animation_player: AnimationPlayer = $Path/To/AnimationPlayer



# You can safely ignore the right side of assignment.

@onready var animation_player := $Path/To/AnimationPlayer as AnimationPlayer

Those behave differently. Personally, I prefer having the type at the operator because it helps to catch errors earlier. Although, at least in some cases in 3.x, code support works better with the second option (but this feels like a bug).

@timothyqiu
Copy link
Member Author

Those behave differently.

Ah, I thought they have the same behavior. What's the difference?

@YuriSizov
Copy link
Contributor

YuriSizov commented May 5, 2022

Those behave differently.

Ah, I thought they have the same behavior. What's the difference?

When you have a type at the operator and try to assign an invalid node path, it will fail there. In the second option the type is deduced from the value, and the value is cast to the target type. In case of incompatibility it will just be turned into a null and happily assigned.

As a result, in the first case errors can be detected at assignment, and in the second case only when you try to do something with the variable (which can happen much later, depending on what you need a node reference for).

PS. Although, as I've said, in 3.x a deduced type provides better code support for some reason, which is counter intuitive, but is a reason why we chose it in some projects.

@Mickeon
Copy link

Mickeon commented May 5, 2022

PS. Although, as I've said, in 3.x a deduced type provides better code support for some reason, which is counter intuitive, but is a reason why we chose it in some projects.

Oh yeah, I had noticed it too! For some reason letting the Node type be inferred with as [NameOfTheClass] gives better autocompletion when writing in a Script outside of the Scene the Script has been assigned into. If it isn't already, I hope it is reported as a bug.

@KoBeWi
Copy link
Member

KoBeWi commented May 16, 2022

If it isn't already, I hope it is reported as a bug.

Yes: #39228

The : Type syntax is ok, but it generates unsafe lines. Although it doesn't really matter much.

@foxwellm
Copy link

For any MacOS users. You can drag from Scene to Script with left click. But before you drop, hold down control.

@djpeach
Copy link

djpeach commented Jun 14, 2023

For any MacOS users. You can drag from Scene to Script with left click. But before you drop, hold down control.

My goodness, thank you. Was very frustrated this did not work

iMarv added a commit to iMarv/godot-vscode-plugin that referenced this issue May 24, 2024
The right-click action, that drops the node-path into your clipboard
already provides close to the same utility of the drag-n-drop feature -
only missing a `$`.
To complement this and to add another QoL change, I refactored the
drop-edit functionality to include a full `@onready` template. This will
now create a variable name based on the node-name and provides the full
convention, inspired by the same feature in the godot editor: godotengine/godot-proposals#4482
@michaelgundlach
Copy link

For any MacOS users. You can drag from Scene to Script with left click. But before you drop, hold down control.

For me on an M1 Mac, it was 'Command', not 'control'. The problem is that Command-click deselects the node you had selected, or selects a second node if one is already selected, thus pasting either 0 or 2 variables. Pressing Command after the click works as intended in all cases.

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

Successfully merging a pull request may close this issue.

10 participants