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

Add support for scancode keyboard input #312

Merged

Conversation

TimJentzsch
Copy link
Collaborator

@TimJentzsch TimJentzsch commented Feb 4, 2023

Closes #307, closes #179.

This PR adds support for scan code keyboard input, i.e., using the key location instead of the logical key value.
This makes sense when you care about the physical location of the key rather than which letter it represents.

This is an important feature for games, as the classical WASD movement input should have keys that are closely together, in the triangle shape.
When a player uses the French AZERTY keyboard layout instead of the US QWERTY one, the ZQSD keys make more sense (which are on the same position as the WASD keys on the QWERTY layout).

You can now convert a ScanCode into InputKind to achieve this.
Furthermore, I added a helper enum QwertyKeyLocation, which makes it easy to select the correct scan codes based on their keys on the QWERTY keyboard layout.

The classical virtual dpad can now be defined like this:

VirtualDPad {
    up: QwertyScanCode::W.into(),
    down: QwertyScanCode::S.into(),
    left: QwertyScanCode::A.into(),
    right: QwertyScanCode::D.into(),
},

TODO

  • Add missing definitions for QwertyScanCode (mostly the Numpad keys).
  • Check scan codes on MacOS as they might be different. If they are, we need to create a second definition of QwertyScanCode which is selected based on the target_os.

Feedback Wanted

  • Do you have better ideas for the naming here? It's kind of confusing now that we have InputKind::Keyboard but also InputKind::KeyLocation. Also QwertyKeyLocation can probably be improved, maybe QwertyScanCode makes more sense here? I renamed it to QwertyScanCode.
  • Proof reading of the values for QwertyScanCode is appreciated, as it will be very hard to find errors here after merging.

@alice-i-cecile alice-i-cecile added the enhancement New feature or request label Feb 4, 2023
@TimJentzsch TimJentzsch marked this pull request as ready for review February 6, 2023 18:50
Copy link

@afonsolage afonsolage left a comment

Choose a reason for hiding this comment

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

I think it would be better to add another example for physical keyboard mapping, since the virtual_dpad example aims to example how to solve a different problem.

Using the new QueryScanCode may be confusing for new users.

@TimJentzsch
Copy link
Collaborator Author

@afonsolage I reverted the changes to the virtual_dpad example and created a new one. Please let me know if you think this works better :)

@TimJentzsch
Copy link
Collaborator Author

I created a mini Bevy app to check which key has which scan code: https://github.com/TimJentzsch/bevy_scan_code_test

It looks like the arrow key scan codes are different, I'll have to double check why

@janhohenheim
Copy link
Contributor

Did you verify that this behaves well in WASM? I had some surprises there recently when using ScanCodes 😅

@TimJentzsch
Copy link
Collaborator Author

Good point, I didn't test that yet.
We should do that before merging

@TimJentzsch
Copy link
Collaborator Author

Looks like the values are indeed different for WASM, I found a reference here: https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/keyCode#value_of_keycode

There might be some differences on WASM between the OS, which kinda sucks for us since we have the same compile target on WASM for all OS.
But it looks like most keys are the same so it should be fine.

Anyway, time to copy another batch of hex values... 🙃

@TimJentzsch
Copy link
Collaborator Author

@janhohenheim I noticed that for Foxtrot you sampled the scan codes by hand. Do you happen to have a MacOs device with Apple keyboard available for testing?

@janhohenheim
Copy link
Contributor

janhohenheim commented Feb 21, 2023

Could the VirtualDPad also support ScanCodes in its convenience methods, viz. wasdand arrow_keys?

@alice-i-cecile
Copy link
Contributor

I would like that, but I'm fine to defer it to another PR.

@janhohenheim
Copy link
Contributor

janhohenheim commented Feb 22, 2023

I'm using this branch in Foxtrot now to help you catch bugs and I found it to be broken on wasm: live demo.
As you can see, the mouse movement is still picked up correctly, but all keyboard inputs are ignored. This behavior is consistent across Windows 10 and macOS using Firefox and Chrome.
The native builds using ScanCodes and the wasm builds using KeyCodes are working as intended using the same branch.

For reference, this is how I setup my InputManagerBundle: https://github.com/janhohenheim/foxtrot/blob/a0dc82ab5cb5aeafb9ce020f2909c02ef82a92a5/src/player_control/actions.rs#L71

@TimJentzsch
Copy link
Collaborator Author

Thanks @janhohenheim for reporting this issue!

This was caused by a dumb copy/paste error which resulted in the macOS configuration being used on WASM.
I think I fixed it now, can you check if it works on Foxtrot?

VirtualDPad::wasd also uses scan codes now.
I didn't change the arrow keys, because they should work the same on all keyboard layouts, but I'm open to changing them as well if needed.

@janhohenheim
Copy link
Contributor

janhohenheim commented Feb 25, 2023

Cool, I'll check in an hour 🙂
Minor nitpick: your code calls the target "WASM", but the official capitalization is "Wasm" 😉
Update: it works now 🎉

@TimJentzsch
Copy link
Collaborator Author

I think the last remaining part is figuring out why some of the Windows/Linux scan codes appear to be wrong. At least the ones prefixed by e0_ seem to be different than the Set 01 spec I found.
Either the scan codes are not actually from Set 01 or I need to pull them from another source.

@alice-i-cecile alice-i-cecile added this to the 0.9 milestone Feb 27, 2023
@alice-i-cecile
Copy link
Contributor

Okay, code quality is solid. Going to test this out on my Windows machine and see what I get.

@alice-i-cecile
Copy link
Contributor

Windows data for you @TimJentzsch: https://gist.github.com/alice-i-cecile/bc86f697497a44228ab20ced8b463a3c

@TimJentzsch
Copy link
Collaborator Author

TimJentzsch commented Mar 1, 2023

The Windows keycodes seem to match what I got.

But it seems like Linux has different bindings, at least on my machine.
For example, I'm getting:

  • Left: 0x69
  • Up: 0x67
  • Right: 0x6a
  • Down: 0x6c

But I couldn't find this combination on the specs I found.
I also checked http://www.quadibloc.com/comp/scan.htm which was used for Foxtrot, but it doesn't seem to match the codes that I get.

@janhohenheim
Copy link
Contributor

@TimJentzsch I blindly copied values for Linux without testing them, so don't trust those too much 😉

@TimJentzsch
Copy link
Collaborator Author

I think I found the Linux values through this winit PR, it links to the Linux source directly: https://github.com/torvalds/linux/blob/master/include/uapi/linux/input-event-codes.h

From the few values I checked it seems to match up.

@janhohenheim
Copy link
Contributor

That has to be the most trustworthy source you could find 😄

@TimJentzsch
Copy link
Collaborator Author

@alice-i-cecile Linux added, this should be ready to review now.

@alice-i-cecile
Copy link
Contributor

This is as good as we're going to get for an initial impl: merging and shipping.

@alice-i-cecile alice-i-cecile merged commit fb88b4d into Leafwing-Studios:main Mar 8, 2023
@TimJentzsch TimJentzsch deleted the 307-scancode-support branch March 8, 2023 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for scan codes Support ScanCode input bindings
5 participants