Make ButtonInput<Key> more robust to ghost inputs & invalid states. - #25490
Make ButtonInput<Key> more robust to ghost inputs & invalid states.#25490CyberspaceDreamn wants to merge 3 commits into
ButtonInput<Key> more robust to ghost inputs & invalid states.#25490Conversation
…Key>` more robust to unusual key states.
alice-i-cecile
added
C-Bug
| // Release all cached input to avoid having stuck input when switching between windows in os | ||
| if !keyboard_focus_lost_reader.is_empty() { | ||
| keycode_input.release_all(); | ||
| key_input.release_all(); |
There was a problem hiding this comment.
Yeah, I agree with this choice. It introduces different weirdness but I think that this is the way.
| mut key_input: ResMut<ButtonInput<Key>>, | ||
| mut keyboard_input_reader: MessageReader<KeyboardInput>, | ||
| mut keyboard_focus_lost_reader: MessageReader<KeyboardFocusLost>, | ||
| mut held_key_codes: Local<HashMap<KeyCode, Vec<Key>>>, |
There was a problem hiding this comment.
A comment on these Locals laying out the core strategy would be really helpful for following the central logic.
| let just_pressed = key_input.just_pressed(logical_key.clone()); | ||
| key_input.press(logical_key.clone()); | ||
|
|
||
| if !just_pressed { |
There was a problem hiding this comment.
I'm pretty sure we need to clear just_released in the same fashion.
There was a problem hiding this comment.
I intentionally set just_released because an event releasing the button was just sent & the docs are a bit ambiguous on the intent for when just_released is true.
pub fn just_released(&self, input: T) -> bool
Returns true if the input has been released during the current frame.
Note: This function does not imply information regarding the current state of ButtonInput::pressed or ButtonInput::just_pressed.
It does make more sense though for a ButtonInput to only be released when it is no longer held.
| @@ -172,6 +174,8 @@ pub fn keyboard_input_system( | |||
| mut key_input: ResMut<ButtonInput<Key>>, | |||
| mut keyboard_input_reader: MessageReader<KeyboardInput>, | |||
| mut keyboard_focus_lost_reader: MessageReader<KeyboardFocusLost>, | |||
There was a problem hiding this comment.
The local maps aren't cleared during KeyboardFocusLost :)
alice-i-cecile
left a comment
alice-i-cecile
left a comment
There was a problem hiding this comment.
Good changes, and an important bug fix. There's a couple of holes to patch still though :)
alice-i-cecile
added
S-Waiting-on-Author
- Ensure `ButtonInput<Key>` is only released when no longer being held. - Improve documentation of the recently added `KeyCode` -> `Key` reference counting. - Clear `keyboard_input_system`'s local cache of held keys on `KeyboardFocusLost`. - This avoids a very rare bug that could cause some buttons to become sticky. - Add more unit tests to `bevy_input::keyboard`.
2 participants
Objective
Generally make
ButtonInput<Key>more tolerant of edge-cases, like aKeybeing held down by multipleKeyCodes,Keys being held down byKeyCodes that are no longer held, keyboard layout swaps, & linux locale hotkey weirdness.Fixes #25483, fixes #25484.
Probably also #25041, but I can't test on macos.
Solution
ButtonInput<Key>is now cleared when a window loses keyboard focus, even if allKeyCodes were already releasedButtonInput<Key>now keeps track of whichKeyCodes arepresseddown with eachKeyKeys only remember aKeyCodewhile thatKeyCodeispressedKeyCodes pressing aKeyare released, thatKeyis releasedTesting
Manually, with a
cargo run --example multiple_text_inputspatched to logButtonInput<Key>'s state, by doing things likeKeyCodethat maps to a differentKeyon press & releaseButtonInput<Key>stateNone of these work well/at all on main & they all do with this patch.
New unit tests that replicate most of the above.