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

Keyboard keycode to qwerty char #782

Closed
wants to merge 2 commits into from

Conversation

Jerald
Copy link
Contributor

@Jerald Jerald commented Nov 3, 2020

Adds functions that convert a KeyCode into the equivalent textual char on a qwerty keyboard, based on the status of shift at the time.

In addition, fixed a typo in the enum variant names where "Asterisk" was incorrectly called "Asterix".

@Jerald Jerald force-pushed the keyboard-keycode-to-char branch from a03f1cb to c49cffd Compare November 3, 2020 20:53
@karroffel karroffel added C-Feature A new feature, making something new possible A-Input Player input via keyboard, mouse, gamepad, and more labels Nov 3, 2020
/// let c3 = KeyCode::End.to_qwerty_char(true);
/// assert_eq!(c3, None);
/// ```
pub fn to_qwerty_char(self, shift_down: bool) -> Option<char> {
Copy link
Member

Choose a reason for hiding this comment

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

I think calling this qwerty isn't quite correct. It seems like this would work equally well on an AZERTY keyboard. How about just to_char()?

Copy link
Member

Choose a reason for hiding this comment

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

The chars don't match for Key1 to Key0 between azerty and qwerty.
For example, with azerty you have to press shift to get the number

Copy link
Member

Choose a reason for hiding this comment

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

Arg gotcha gotcha. in that case idk if hard-coding qwerty is the right answer. I don't want to optimize bevy apis for specific regions.

Maybe we should just create a new event for ReceivedCharacter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Realistically, the "correct" approach would be to have different keysets/locales which you run key codes through and which each have conversions. Localization in general is a remarkably complex problem...

Because of this, I was imagining this as a sort of "stop-gap" solution until the proper work would be done.

But looking at that winit event you linked, implementing a new input thing which hooks into that event to supply direct text would likely be the better and more cross-platform solution...

/// let c3 = KeyCode::End.to_qwerty_char(true);
/// assert_eq!(c3, None);
/// ```
pub fn to_qwerty_char(self, shift_down: bool) -> Option<char> {
Copy link
Member

Choose a reason for hiding this comment

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

I think we should probably remove this method. We don't need two ways to get characters with/without shift.

How about just to_char() and to_char_with_shift()

@smokku
Copy link
Member

smokku commented Nov 5, 2020

We should not be implementing key to character mapping ourselves - it is a maintenance nightmare.
Instead we should rely on information provided by the operating system - its the job of OS to maintain the keyboard layout abstraction - and modern OSes are very good at it.

i.e. I'd rather see CharacterEvent dispatched by bevy_winit than doing some stop-gap in bevy_keyboard.

@Jerald Jerald mentioned this pull request Nov 6, 2020
@Jerald
Copy link
Contributor Author

Jerald commented Nov 6, 2020

I've now opened a new PR (#804) based on the winit ReceivedCharacter event that cart mentioned above.

Closing this PR in favour of that one.

@Jerald Jerald closed this Nov 6, 2020
@Jerald Jerald mentioned this pull request Nov 6, 2020
@Jerald
Copy link
Contributor Author

Jerald commented Nov 6, 2020

I've also created #805 based on discord feedback, which is a more window-focused alternative to #804.

Only one of #804 or #805 should realistically be merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Input Player input via keyboard, mouse, gamepad, and more C-Feature A new feature, making something new possible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants