-
-
Notifications
You must be signed in to change notification settings - Fork 22k
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
Fix InputEvent actions. #8936
Fix InputEvent actions. #8936
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.
LGTM
core/os/input_event.cpp
Outdated
} | ||
bool InputEvent::is_action_released(const StringName &p_action) const { | ||
|
||
if (is_action(p_action)) { | ||
return !is_pressed(); |
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.
Sorry if I'm being clueless on this matter but.... Why is !is_pressed() used for action_released?
Shouldn't release be when pressed was true and then pressed becomes false? Otherwise as long as I'm not pressing the key, action_released() will always return true... which seems wrong
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.
No, you're calling this on individual InputEvents
. That would be the case if it was implemented this way for Input.is_action_released(action)
, yes.
But since this is for events, there's just no way is_action_released
will return true if the event is something other than specified in the action.
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.
Well, no, there is only one pressed=false
event when the key is released. So, this is going to catch just that one
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.
oh ok, thanks for the explanation :)
core/os/input_event.cpp
Outdated
@@ -62,10 +62,16 @@ bool InputEvent::is_action(const StringName &p_action) const { | |||
|
|||
bool InputEvent::is_action_pressed(const StringName &p_action) const { | |||
|
|||
return false; // InputMap::get_singleton()->event_is_action(Ref<InputEvent>(this),p_action); | |||
if (is_action(p_action)) { | |||
return is_pressed(); |
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.
Actually, just thought about it, I guess you might want to add && !is_echo()
here, but I'm not completely sure about that.
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.
Mhh, I'll check how 2.1 handled 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.
Yeah, definitely should be ignoring echo events. ^^
The `InputEvent::is_action(pressed|released)` methods weren't implemented yet. Also fixed a typo in `InputDefault` that prevented `Input.is_action(pressed|released)` from working.
The
InputEvent::is_action(pressed|released)
methods weren't implemented yet.Also fixed a typo in
InputDefault
that preventedInput.is_action(pressed|released)
from working.Fixes #8914