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

Ignore extra/unknown SDL_Keymod bits (mod_) #882

Merged
merged 2 commits into from
Sep 10, 2019

Conversation

kolen
Copy link
Contributor

@kolen kolen commented Apr 9, 2019

#881

Changed behavior of handling "key modifier" bits unknown to us in event.key.keysym.mod (SDL_Keymod). Current behavior, before this change, is:

  • If event is keydown, and any unknown bit is set to 1, treat whole bitmask as "no modifier keys" (like mod = 0)
  • If event is keyup, and any unknown bit is set to 0, panic

After this change, any extraneous bits are ignored.

I created this PR for illustrative purposes for discussion in #881, I don't know if it's a right way to handle mod_ bits, and if it's ready to merge. I didn't test it (however tests pass) Superficially tested by building an app against modified version, keyboard input works, including some modifier keys (not tested all modifier keys).

@@ -1619,13 +1619,6 @@ impl Event {
}} // close unsafe & match
}

pub fn unwrap_keymod(keymod_option: Option<keyboard::Mod>) -> keyboard::Mod {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's pub, so I'm not sure if it can be removed without causing breaking change. It might be not intended as pub, however, or might be not visible from outside of crate.

Copy link
Member

Choose a reason for hiding this comment

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

It is currently visible as you can see here: https://docs.rs/sdl2/0.32.2/sdl2/event/enum.Event.html#method.unwrap_keymod

However I don't see why it was made public since nothing returns an Option<Mod> anymore to my knowledge. Maybe keep it, but annotate it with #[deprecate]? See https://github.com/rust-lang/rfcs/blob/master/text/1270-deprecation.md

Also, can you put a note of the deprecation in the changelog? That would be helpful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Restored this method, added deprecation, added an entry to changelog (I didn't mark deprecation with bold font because probably no one is using this API, so it's low importance deprecation).

Tried to build an app against modified version, at least keyboard input works, including detection of pressed shift and control keys (not tested all modifiers).

Also added tests for unknown bits.

@kolen kolen force-pushed the ignore-extra-sdl-keymod-bits branch from 1c5c36b to 45a00ca Compare April 14, 2019 14:20
@kolen kolen force-pushed the ignore-extra-sdl-keymod-bits branch from 45a00ca to da4269d Compare April 14, 2019 15:20
@Cobrand Cobrand merged commit 39230f8 into Rust-SDL2:master Sep 10, 2019
sypwex pushed a commit to sypwex/rust-sdl2 that referenced this pull request Jun 2, 2024
Ignore extra/unknown SDL_Keymod bits (mod_)
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.

2 participants