Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A key can be pressed or not pressed. This is the status of the key, not an event. The
InputState::keys_down
attribute contains a collection of any key being held down at any given frame.InputState::key_down(Key)
returns true if a key is being held down. So far so good.A key being pressed or released is an event, i.e. the change from being pressed to not being pressed, or from not being pressed to being pressed. While
InputState::key_released(Key
) returns true correctly only when the key was released on a given frame,InputState::key_pressed(Key)
doesn't behave that way, and instead returns true as long as the key is held down.Besides not allowing this method to be used to detect the actual key press, another unintended consequence is that on the frame where the key is released, sometimes both events will be there, and thus both key_pressed and key_released will return true.
The example in the second commit of this PR helps seeing this. Before applying the patch to InputState, press and hold the letter A, then release. First you'll see that during the key being pressed, not only you get true from
key_down()
but also fromkey_press()
. Also if you try to do this a few times, you will see that sometimes the message "pressed" is printed AFTER the message "released" (note there's no release messages in the following screenshot, they key is being held down)The root of the problem is the difference in meaning between pressed/released events in egui/eframe and egui-winit. For egui-winit, keyboard events mean something different, as implemented on State::on_keyboard_input. Keys pressed are added as Event::Key with pressed true (and this applies while the key is held down).
The patch on the first commit is very simple, and it just removes from the raw input events the key press if this key was already in the set of keys pressed from the previous frame. It of course leaves the keys_pressed set unaltered, so that the key can still be queried for being held down (there are only "held" messages in between a "pressed" and a "released" message in the following screenshot)
The API to the user doesn't change, but with this fix key_pressed will change behaviour. If someone was using key_pressed to detect if a key was being held down, this will stop working. The fix would be trivial though, just changing the API call from key_pressed() to key_down(); the parameters are all the same.