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.

Sign up for GitHub

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

Jump to bottom

Create onready variables when dropping nodes and holding Ctrl #60708

Merged
merged 1 commit into from
May 17, 2022
Merged

Create onready variables when dropping nodes and holding Ctrl #60708

merged 1 commit into from
May 17, 2022

Conversation

timothyqiu
Copy link
Member

Like Ctrl drop files adds preload to the path, this PR makes Ctrl drop nodes inserts onready variable code.

Peek.2022-05-02.21-03.mp4

Proof of concept and closes godotengine/godot-proposals#4482

@timothyqiu timothyqiu requested a review from a team as a code owner May 2, 2022 13:12
@akien-mga akien-mga requested a review from a team May 2, 2022 13:26
@akien-mga akien-mga added this to the 4.0 milestone May 2, 2022
@KoBeWi
Copy link
Member

KoBeWi commented May 2, 2022

Any chance it could add also a type hint when they are enabled in editor settings?
So e.g.
var animation_player = $AnimationPlayer
becomes
var animation_player = $AnimationPlayer as AnimationPlayer
It's mostly to help autocompletion.

Also do you handle name conflicts automatically? It's possible to have two same-named nodes at different branches.

@timothyqiu
Copy link
Member Author

timothyqiu commented May 2, 2022 •
edited
Loading

Any chance it could add also a type hint when they are enabled in editor settings?

It's included. See 0:18 in the video.

@onready var animation_player := $AnimationPlayer as AnimationPlayer

Also do you handle name conflicts automatically?

No, it's always the snake_cased node name. I think it's easier to let the user decide the name in this case. It's hard to find a strategy that satisfies everyone.

In the end, it's usually those parts other than the variable name that are quite repetitive and annoying.

@KoBeWi
Copy link
Member

KoBeWi commented May 2, 2022

It's included. See 0:18 in the video.

Ah, I didn't watch it whole, sorry.

No, it's always the snake_cased node name. I think it's easier to let the user decide the name in this case.

Makes sense.

@timothyqiu timothyqiu mentioned this pull request May 8, 2022
Merged
@timothyqiu timothyqiu requested a review from a team as a code owner May 8, 2022 08:23
@timothyqiu
Copy link
Member Author

timothyqiu commented May 8, 2022 •
edited
Loading

KoBeWi
KoBeWi reviewed May 16, 2022
View reviewed changes
core/string/ustring.cpp Outdated Show resolved Hide resolved
KoBeWi
KoBeWi reviewed May 16, 2022
View reviewed changes
core/string/ustring.cpp Outdated Show resolved Hide resolved
KoBeWi
KoBeWi reviewed May 16, 2022
View reviewed changes
editor/plugins/script_text_editor.cpp
Comment on lines +1557 to +1577
for (const String &segment : path.split("/")) {
if (!segment.is_valid_identifier()) {
path = path.c_escape().quote(quote_style);
break;
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part is great. If you copied it below, you could also solve #33317

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that is worth a dedicated PR. It changes the meaning of normal node dropping from inserting a string containing the node's path to inserting the path after $.

@timothyqiu timothyqiu requested a review from a team as a code owner May 17, 2022 03:58
@akien-mga akien-mga merged commit 1d5e662 into godotengine:master May 17, 2022
@akien-mga
Copy link
Member

Thanks!

@timothyqiu timothyqiu deleted the drop-onready branch May 17, 2022 12:14
@Jummit Jummit mentioned this pull request May 18, 2022
Merged
Jummit added a commit to Jummit/godot that referenced this pull request May 18, 2022
@Jummit
Use % when dropping unique scene nodes into script
1101f6c 
This expands uppon godotengine#60708, using `get_node("%NodeName")` for nodes that
have a unique scene name to avoid having to change the onready
statements when the paths of the nodes change.
akien-mga pushed a commit to akien-mga/godot that referenced this pull request Jul 3, 2022
@Jummit @akien-mga
Use % when dropping unique scene nodes into script
63f35d2 
This expands uppon godotengine#60708, using `get_node("%NodeName")` for nodes that
have a unique scene name to avoid having to change the onready
statements when the paths of the nodes change.

(cherry picked from commit 1101f6c)
@hamoom
Copy link

hamoom commented Aug 5, 2022 •
edited
Loading

any way to make this work on MacOS? holding ctrl acts as a right click.

Riordan-DC pushed a commit to Riordan-DC/godot that referenced this pull request Jan 24, 2023
@Jummit
Use % when dropping unique scene nodes into script
2eb3bca 
This expands uppon godotengine#60708, using `get_node("%NodeName")` for nodes that
have a unique scene name to avoid having to change the onready
statements when the paths of the nodes change.

(cherry picked from commit 1101f6c)
@foxwellm
Copy link

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

@jrfilms89
Copy link

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

For mac users of the future(compared to this thread), you must left click and drag the node from the scene to your script, but before you drop it in, hold CMD(command). It does not work if you hold CMD before you left click and drag. Only after the left click and drag has occurred.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KoBeWi KoBeWi KoBeWi approved these changes

Assignees
No one assigned
Labels
enhancement topic:editor usability
Projects
None yet
Milestone
4.0
Development

Successfully merging this pull request may close these issues.

Create onready variables when dropping nodes into script editor and holding Ctrl
6 participants
@timothyqiu @KoBeWi @akien-mga @hamoom @foxwellm @jrfilms89