-
-
Notifications
You must be signed in to change notification settings - Fork 22.5k
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
Update Shortcut
class reference
#107252
Conversation
There was a problem hiding this 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.
doc/classes/Shortcut.xml
Outdated
key_event.command_or_control_autoremap = true # Swaps ctrl for Command on Mac. | ||
save_shortcut.set_events([key_event]) | ||
|
||
func _input(event:InputEvent) -> void: |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
func _input(event:InputEvent) -> void: | |
func _input(event) -> void: |
Regardless, I believe not. _input
should also work @bruvzg
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
735ead8
to
c9025c2
Compare
Yes, most of the commits were done in the web editor. |
f75fa90
to
34b07b2
Compare
34b07b2
to
3a30a1c
Compare
Thanks! |
Closes godotengine/godot-docs#11000