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

Fix ctrl or win interfering with character inputs #149

Merged
merged 3 commits into from
Mar 8, 2023

Conversation

Vrixyz
Copy link
Contributor

@Vrixyz Vrixyz commented Feb 7, 2023

I'm building an app where I need users to enter a "at sign" (@), and input text is not allowing me to do that.

Looks like the issue is from bevy_egui.

My changes appear to fix it, but I didn't make a bigger background check on why that condition was there originally.

Other context as to how I ended up to this solution: https://discord.com/channels/691052431525675048/1072569924972728320

@mvlabat
Copy link
Owner

mvlabat commented Feb 14, 2023

Hey! Thanks for the PR.
The reason behind the ctrl/win condition is that we need to pass different events when a user presses a hotkey (such as Ctrl+A). I'm afraid we can't simply remove that, otherwise we'd also add a character input when a hotkey is pressed, which is undesired.

Could you provide more details on the issue? I don't understand what exactly prevents users to enter @ (which I believe is done with Shift+2).

@Vrixyz
Copy link
Contributor Author

Vrixyz commented Feb 14, 2023

On (most?) AZERTY PC keyboard, the character @ is done via altgr + 0

image
image curtesy of wikipedia https://en.wikipedia.org/wiki/AZERTY

@Vrixyz
Copy link
Contributor Author

Vrixyz commented Feb 14, 2023

A path toward the resolution of this issue would be to add tests for preventing the issue you're describing : "no wrong character input when hotkey is pressed".

@Vrixyz
Copy link
Contributor Author

Vrixyz commented Feb 14, 2023

After some research, I think looking into egui winit integration (and maybe others) can be enlightening:

@Vrixyz
Copy link
Contributor Author

Vrixyz commented Feb 14, 2023

After a more thorough reading of the code, I'm starting to doubt my keyboard...

I want to emphasize that the egui examples are allowing me to input an @ correctly, so there's probably something to fix in bevy_egui somewhere.

I'll fire up a debugger when I have time and report here my findings.

@Vrixyz
Copy link
Contributor Author

Vrixyz commented Feb 24, 2023

With my fix as of 6ab53df:

on PC:

  • altrgr-0 ('@' on my azerty) prints the correct character '@': bug fixed 🎉
  • ctrl-A is supported normally (selects everything, no characters added): no regression 🎉
  • ctrl-C + ctrl-V is supported normally (copies and pastes the correct characters): no regression 🎉

variables and event received on windows for a altgr-0:

[src\systems.rs:244] event = ReceivedCharacter {
    id: WindowId(
        00000000-0000-0000-0000-000000000000,
    ),
    char: '@',
}
[src\systems.rs:245] modifiers = Modifiers {
    alt: true,
    ctrl: true,
    shift: false,
    mac_cmd: false,
    command: true,
}
[src\systems.rs:246] (command, shift, ctrl, alt, mac_cmd) = (
    true,
    false,
    true,
    true,
    false,
)

on Mac:

  • altrgr-0 ('@' on my azerty) prints the correct character '@': bug fixed 🎉
  • ctrl-A prints an A 😭 then selects all
  • ctrl-C prints C 😭 then copies
  • ctrl-V prints V 😭 then copies

events received on mac for a ctrl-A:

[src/systems.rs:244] event = ReceivedCharacter {
    id: WindowId(
        00000000-0000-0000-0000-000000000000,
    ),
    char: 'a',
}
[src/systems.rs:245] modifiers = Modifiers {
    alt: false,
    ctrl: false,
    shift: false,
    mac_cmd: true,
    command: true,
}
[src/systems.rs:246] (command, shift, ctrl, alt, mac_cmd) = (
    true,
    false,
    false,
    false,
    true,
)

src/systems.rs Show resolved Hide resolved
@Vrixyz
Copy link
Contributor Author

Vrixyz commented Feb 24, 2023

Following my tests, I think my lastest comment https://github.com/mvlabat/bevy_egui/pull/149/files#r1117103529 would satisfy best worlds. Please tell me if you are aware of problematic hotkeys or anything!

I asked for aditional reviews on egui discord given the difficulty to test for side effects: https://discord.com/channels/900275882684477440/904461220592119849/1078692430687248444

@doup
Copy link

doup commented Mar 1, 2023

Does this fix work on Windows? (I'm on macOS) I would assume that when processing the received characters for ctr+c it'll print c.

src/systems.rs Outdated
@@ -240,7 +240,7 @@ pub fn process_input_system(
}
}

if !ctrl && !win {
if !mac_cmd {
Copy link

Choose a reason for hiding this comment

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

Does it make sense to express it as?

Suggested change
if !mac_cmd {
if !command {

Which means ctrl in Windows and cmd in macOS.

Copy link
Contributor Author

@Vrixyz Vrixyz Mar 1, 2023

Choose a reason for hiding this comment

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

in theory I agree, but the problem is that ctrl is true when using altrgr (maybe that's the actual root issue 🤔) I guess it wouldn't work then.

(see #149 (comment))

Copy link
Owner

Choose a reason for hiding this comment

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

https://en.wikipedia.org/wiki/AltGr_key#Ctrl+Alt

Ah well, I guess that's why. AltGr implies ctrl+alt

Copy link

Choose a reason for hiding this comment

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

Then that could be discarded? It's command only if it's ctrl, not ctrl+alt?

Copy link
Owner

@mvlabat mvlabat Mar 1, 2023

Choose a reason for hiding this comment

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

only if it's ctrl, not ctrl+alt

how about !command || ctrl && alt && cfg!(target_os = "windows")?

I believe this behaviour is specific only to Windows. Please correct me if I'm wrong, but I believe that in Linux altgr doesn't map to ctrl+alt (I don't have a way to test that atm).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to my manual testing, your suggested change works fine for windows and mac 🎉

I might be able to test the behaviour on a spare raspberry pi but not sure when.

Copy link

Choose a reason for hiding this comment

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

I meant that maybe alt can be included in the command variable computation?

Then here just leave !command. I mean this because what happens in windows if you do altgr+c/altgr+v? Does it work as as copy/paste since altgr maps to ctrl+alt (if I understood correctly).

Copy link

Choose a reason for hiding this comment

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

Following the changes in #156, I mean to integrate it like this:

let command = if cfg!(target_os = "macos") {
    mac_cmd
} else {
    // Only when `ctrl` is pressed; this skips `altgr` which maps to `ctrl+alt`
    ctrl && !alt
};

And then just doing !control should work, no? It'll ignore shortcuts, and in macOS chars like ctrl+a will skip because the inner !event.char.is_control() check. But, in macOS this will allow ctrl+_ usages that actually produce some character, like: ctrl+´ => ".

Copy link
Owner

Choose a reason for hiding this comment

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

@doup I'd prefer the altgr behaviour to be limited just to Windows, as I haven't found any proof that it works like that on any other platform. I haven't invested enough time to understand how ctrl+_ works in MacOS, but on my ANSI MacBook keyboard I haven't found a single button that produced a character in combination with ctrl+_.

I'll stick to this solution for the time being, but I'm open to revisiting it if any other issues are found.

@Vrixyz
Copy link
Contributor Author

Vrixyz commented Mar 1, 2023

Does this fix work on Windows? (I'm on macOS) I would assume that when processing the received characters for ctr+c it'll print c.

Interestingly, yes it works on windows correctly (copies the selected text without printing additional ones), I expected the same as you.

@mvlabat
Copy link
Owner

mvlabat commented Mar 1, 2023

@doup it seems to me that the behaviour mac users would expect is for a hotkey to do nothing. For example, when I press ctrl+c in the Notes app on mac, it just does nothing (neither prints c, nor copies the text).
At the same time I wouldn't mind if both ctrl+c and cmd+c worked as a hotkey for copying regardless of OS.

So I think we are ok if in the scenario when a user presses ctrl+c on mac, egui would either copy or do nothing, but we definitely don't want it put the c character and copy.

@doup
Copy link

doup commented Mar 1, 2023

Uhmm, that's true, ctr+_[1] is not processed in macOS. But, that would mean that this change should fail, but it doesn't (or not always).

The thing is that while processing ev_received_character unicode control characters are skipped in the inner if. I've modified char_input_events example in Bevy to show if a character is control or not, and reports ctr+a as a control character. But, not all combinations of ctr+_ are reported as control characters, so it doesn't always skip the inner if.

For example ctrl+, in macOS doesn't print the comma; but in this PR it does. Maybe this should be fixed via cfg! instead?

let skip_character_input = if cfg!(target_os = "macos") { mac_cmd || ctrl } else { false };

Although, it would be interesting to see if there is a way to discern ctrl and altgr in Windows; instead of using false.

[1]: _ as in "any character".

@doup
Copy link

doup commented Mar 1, 2023

Prrfff, I noticed that on my keyboard ctrl+, doesn't print, but ctrl+´ prints as "; so ctrl+_ is not always ignored in macOS. 🤯

Other stuff I've found:

  • ctrl+b works as arrow left
  • ctrl+n as arrow down (not always?)
  • ctrl+v goes to the end, -1 char (?)
  • ctrl+h backspace

Hahaha, this is weird. Anyway, it's probably OK to just ignore all ctrl+_ keys in macOS?
(until someone opens an issue saying he can't use ctrl+h to delete characters anymore 🙈 )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants