Skip to content

Update Shortcut class reference #107252

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

Merged
merged 1 commit into from
Jun 8, 2025

Conversation

GlitchedCode922
Copy link
Contributor

@GlitchedCode922 GlitchedCode922 requested a review from a team as a code owner June 7, 2025 08:55
@AThousandShips AThousandShips added enhancement documentation cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release cherrypick:4.4 Considered for cherry-picking into a future 4.4.x release labels Jun 7, 2025
@AThousandShips AThousandShips added this to the 4.x milestone Jun 7, 2025
@Mickeon Mickeon requested a review from bruvzg June 7, 2025 16:00
Copy link
Member

@Mickeon Mickeon left a comment

Choose a reason for hiding this comment

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

My thoughts stand, but the description in this PR is an obvious improvement.

Ideally all commits on this PR should be squashed, but I have a hunch this was done online through Github's editor.

key_event.command_or_control_autoremap = true # Swaps ctrl for Command on Mac.
save_shortcut.set_events([key_event])

func _input(event:InputEvent) -> void:
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't be _shortcut_input used instead?

Copy link
Member

Choose a reason for hiding this comment

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

It also shouldn't use static typing in hindsight. Not in the class reference, at least.

Suggested change
func _input(event:InputEvent) -> void:
func _input(event) -> void:

Regardless, I believe not. _input should also work @bruvzg

Copy link
Contributor Author

@GlitchedCode922 GlitchedCode922 Jun 7, 2025

Choose a reason for hiding this comment

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

I tested the code, it also works with _input

Copy link
Member

@bruvzg bruvzg Jun 7, 2025

Choose a reason for hiding this comment

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

It also shouldn't use static typing in hindsight.

event: InputEvent is fine, and this is what Godot autocompletion will add for func _input.

Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't be _shortcut_input used instead?

The only difference is order of execution, if none of the nodes mark input call as handled it will call shortcut_input and then call unhandeled_input. It depends on the situation, and any should be fine for the example.

But it should have accept_event() after print line to suppress other input methods.

Copy link
Contributor Author

@GlitchedCode922 GlitchedCode922 Jun 7, 2025

Choose a reason for hiding this comment

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

accept_event() only exists in Control nodes. The documentation example is not necessarily a Control and there may be other nodes listening to this shortcut too.

Copy link
Member

@Mickeon Mickeon Jun 7, 2025

Choose a reason for hiding this comment

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

event: InputEvent is fine, and this is what Godot autocompletion will add for func _input.

A grand majority of the class reference explicitly avoids static typing for brevity. Someday that could change, but this should be consistent for now.

The documentation example is not necessarily a Control and there may be other nodes listening to this shortcut too.

Most of the times there's only one Node that accepts a shortcut, though. All nodes can use get_viewport().set_input_as_handled() for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If static typing is not used in the class reference, -> void should also be removed

@GlitchedCode922
Copy link
Contributor Author

GlitchedCode922 commented Jun 7, 2025

but I have a hunch this was done online through Github's editor

Yes, most of the commits were done in the web editor.

@akien-mga akien-mga modified the milestones: 4.x, 4.5 Jun 8, 2025
@akien-mga akien-mga removed the cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release label Jun 8, 2025
@akien-mga akien-mga merged commit 77f2623 into godotengine:master Jun 8, 2025
20 checks passed
@akien-mga
Copy link
Member

Thanks!

@GlitchedCode922 GlitchedCode922 deleted the shortcuts branch June 9, 2025 07:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherrypick:4.4 Considered for cherry-picking into a future 4.4.x release documentation enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing docs for how to work with and usage Shortcuts
6 participants