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 text editing keybinds #265

Merged
merged 1 commit into from
Mar 2, 2024

Conversation

eero-lehtinen
Copy link
Contributor

Copy, paste, cut, undo, redo and maybe others were bound to WIN instead of CTRL outside of macOS, which was wrong. It broke after updating to the latest Bevy. This pr binds them back to CTRL.

I also have to prevent text input when win is pressed because the command check doesn't work by itself anymore.

I don't have a mac available right now, but I assume it was broken for macOS too, because copy is CMD-C on macOS. The egui docs also tell you to use the same thing for command as mac_cmd.

@@ -163,7 +163,7 @@ pub fn process_input_system(
win,
} = *input_resources.modifier_keys_state;
let mac_cmd = if *context_params.is_macos { win } else { false };
let command = if !*context_params.is_macos { win } else { ctrl };
let command = if *context_params.is_macos { win } else { ctrl };
Copy link
Contributor

@Vrixyz Vrixyz Feb 27, 2024

Choose a reason for hiding this comment

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

yes, that fix seems correct!

I really wish we could make some tests on that :o ; it's a pain to verify

@@ -266,7 +266,7 @@ pub fn process_input_system(
}
}

if !command || !*context_params.is_macos && ctrl && alt {
if !command && !win || !*context_params.is_macos && ctrl && alt {
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
if !command && !win || !*context_params.is_macos && ctrl && alt {
if !command || !*context_params.is_macos && ctrl && alt {

I think adding && !win to the condition makes it so that we no longer ignore character input on such hotkeys as Ctrl + C on Windows (i.e. on pressing Ctrl + C it'll both copy and paste the c character)

Copy link
Contributor Author

@eero-lehtinen eero-lehtinen Mar 2, 2024

Choose a reason for hiding this comment

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

The logic is a bit weird because it's negated, but it definitely works correctly as I tested it on Linux and it works the same on Windows.

It checks that both Win and Ctrl are not pressed before typing characters. I wouldn't want to type anything if pressing e.g Win+Tab or Ctrl+C or even both at the same time.

Copy link
Owner

Choose a reason for hiding this comment

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

Yeah, my bad, I for some reason confused win (a modifier key) with the Windows platform. I surely haven't had enough sleep today 😄

@mvlabat
Copy link
Owner

mvlabat commented Mar 2, 2024

Thank you for the PR!

@mvlabat mvlabat merged commit a1f01a9 into mvlabat:main Mar 2, 2024
39 checks passed
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.

None yet

3 participants