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

Ensure minimum modifiers are pressed when matching actions #59343

Merged
merged 1 commit into from
Mar 20, 2022

Conversation

madmiraal
Copy link
Contributor

#54173 modified the action_match methods, but dropped the check for matching the minimum modifiers when a key or button is pressed.
Before:

Key code = get_keycode_with_modifiers();
Key event_code = key->get_keycode_with_modifiers();
match = get_keycode() == key->get_keycode() && (!key->is_pressed() || (code & event_code) == code);

After:
match &= get_modifiers_mask() == key->get_modifiers_mask();

This PR reintroduces that check.

Fixes #57943

Copy link
Contributor

@EricEzaM EricEzaM left a comment

Choose a reason for hiding this comment

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

Based on your comment/analysis of the issue here, I think this is the correct fix. Even with exact being false, pressing D should not trigger an action with event Ctrl+D. The minimum modifiers should be respected. Pressing Ctrl+Shift+D however, would still trigger Ctrl+D if exact is false, which is the desired behaviour I believe.

However, looking back at the doco which was written, it probably is not worded 100% correct. I added this feature (exact matches) but did not write the doco - so that's at least partially my fault.

If exact_match is false, it ignores the input modifiers for InputEventKey and InputEventMouseButton events, and the direction for InputEventJoypadMotion events.

It would probably be more correct to say

If exact_match is false, it ignores additional input modifiers for InputEventKey... [etc]

@madmiraal
Copy link
Contributor Author

Updated the documentation as suggested.

@akien-mga akien-mga merged commit cb51df3 into godotengine:master Mar 20, 2022
@akien-mga
Copy link
Member

Thanks for the in depth review and fix! It's not easy to wrap one's head around all the subtleties of input with modifiers :)

@madmiraal madmiraal deleted the fix-57943 branch March 20, 2022 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants