bevyengine / bevy

A refreshingly simple data-driven game engine built in Rust
https://bevyengine.org
Apache License 2.0
36.24k stars 3.57k forks source link

Phantom key presses on window focus change (alt+tab, etc) #13299

Closed hut closed 5 months ago

hut commented 6 months ago

Bevy version

0.13.2

Relevant system information

ArchLinux with i3wm window manager alongside KDE using X11

What you did

I press Alt+Tab to change focus to my bevy application.

What went wrong

I expected this to change focus to my application, and nothing else.

What happend: It caused a key press event of the TAB key and triggered the application action associated with the TAB key.

This happens with any key that is used for focus switching. For example, if I use my window manager's "alt+w" key binding to switch to the workspace of the game, it registers as a key press of the "w" key, which causes my character to unintentionally move forward.

Worse yet, those keys get stuck in a "permanently pressed" state, triggering the actions forever, until I press the respective key once again.

This is driving me crazy as I switch focus back/forth between my bevy application a lot.

Additional information / Cause

This is caused by bevy erroneously interpreting winit's "synthetic" key presses as real key presses.

If I add a check for is_synthetic in bevy's winit KeyboardInput event handler:

WindowEvent::KeyboardInput { ref event, is_synthetic, .. } => {
    if !is_synthetic {
        if event.state.is_pressed() {
            if let Some(char) = &event.text {
                let char = char.clone();
                app.send_event(ReceivedCharacter { window, char });
            }
        }
        app.send_event(converters::convert_keyboard_input(event, window));
    }
}

then the phantom key press events disappear in my application.

This is likely not the full solution though. While the phantom key press events are a clear bug, the phantom key release events might serve a purpose and possibly should be exposed. See https://github.com/rust-windowing/winit/issues/3543#issuecomment-1973024673

If you decide to not filter out synthetic keys, please at least expose the fact that they are synthetic to the user so we can filter them out ourselves. This would be suboptimal solution, but better than nothing.

If I can help in any way with solving this, don't hesitate to ask.

Thank you.

P.S. here's an analogous fix in another UI framework: https://github.com/lapce/floem/pull/387

hut commented 5 months ago

Another analogous PR in the UI framework egui: https://github.com/emilk/egui/issues/4513

I would happily make a PR to fix this in bevy. Just say the word.

SpecificProtagonist commented 5 months ago

Thanks for your investigation! A PR would be great. Ignoring synthetic events should fix #12273 as well (EDIT: only if #12878 doesn't get merged) (see also #12372).

There are multiple related issues around how keys behave when focus is lost/gained – behavior differs between platforms (and keys). Documenting that would be great, but doesn't need to happen in this PR.

alice-i-cecile commented 5 months ago

Seconding the request for a PR :) Just make sure to branch on top of https://github.com/bevyengine/bevy/pull/13678, which is the adopted form of the PR linked above.