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.

Sign up for GitHub

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
Merged

Fix ctrl or win interfering with character inputs #149

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6ab53df
Fix ctrl or win interfering with character inputs
Vrixyz Feb 7, 2023
4b5fe95
condition on mac_cmd to check for problematic hotkeys
Vrixyz Feb 24, 2023
1fafb14
apply suggested logic
Vrixyz Mar 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/systems.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ pub fn process_input_system(
}
}

if !ctrl && !win {
Vrixyz marked this conversation as resolved.
Show resolved Hide resolved
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 •
edited
Loading

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

@vladbat00 vladbat00 Mar 1, 2023 •
edited
Loading

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.

for event in input_events.ev_received_character.iter() {
if !event.char.is_control() {
input_resources
Expand Down