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

keycodes #657

Draft
wants to merge 3 commits into
base: develop
Choose a base branch
from
Draft

keycodes #657

wants to merge 3 commits into from

Conversation

spatialfree
Copy link
Contributor

i took a stab at an issue i posted a while back: #486

and the following commit actually manages to do what i need for linux ^-^ ...but windows scan codes are different than x11's keycodes~ so that will need to be handled properly before merging these changes!

@maluoi
Copy link
Collaborator

maluoi commented Jul 6, 2023

Heyas, thanks for the PR here! I'm still thinking about this, but the shape of the API changes here don't feel right to me. This is mixing together the scan codes with the virtual key codes, when they're two rather separate ideas. Like, injecting a scan code should automatically inject the matching virtual key, and injecting a virtual key should automatically inject a scan code as well. Testing for a key should likewise be separate from testing for a scan code.

There may also need to be matching utility functions for users to convert to and from scan codes and virtual keys.

I'm also not terribly excited about using numbers for scan codes? While I understand that scan codes may not have good mappings to real names, it still feels like a really bad experience. Perhaps scan codes can be numbers, or an enumeration that maps QWERTY to scan codes?

On the technical side, a uint8_t would match a byte in C#.

@spatialfree
Copy link
Contributor Author

thanks for being so thorough! as it really helped me iterate on this~ a whole lot quicker! 👍

i made what seemed like some good tradeoffs of complexity clarity and ease of use inline with your feedback while also making it easier to plug in the windows platform side of things

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

2 participants