-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Rename KeyCode
to PhysicalKey
#11692
Conversation
crates/bevy_input/src/keyboard.rs
Outdated
@@ -162,7 +162,7 @@ pub enum NativeKeyCode { | |||
/// | |||
/// ## Usage | |||
/// | |||
/// It is used as the generic `T` value of an [`ButtonInput`] to create a `Res<Input<KeyCode>>`. | |||
/// It is used as the generic `T` value of an [`ButtonInput`] to create a `Res<ButtonInput<PhysicalKeyCode>>`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a tiiiny bit out of scope but probably an oversight from a previous PR.
It looks like your PR is a breaking change, but you didn't provide a migration guide. Could you add some context on what users should update when this change get released in a new version of Bevy? |
Seems like the comment that's the basis of the corresponding item in #11052 mentions |
Yeah probably right, PhysicalKey makes sense for consistency with LogicalKey |
253f5ce
to
b50c1eb
Compare
If we're going to change this, I prefer |
KeyCode
to PhysicalKeyCode
KeyCode
to PhysicalKey
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm currently eating dinner/on my phone but I'd like to avoid the logical/physical key naming scheme that winit chose. See discord
|
@mockersf in bevy 0.12 and prior versions |
I was initially against this. But I looked at how winit arranged their PhysicalKey. They keep both Logical key is |
In this case I think we should consider options based on a clean slate, instead of worrying about migrations. Personally, I agree with @mockersf. Logical/physical keys seems too confusing, especially for new devs. I think KeyCode + Input Action Map is the right conceptual design. |
Part of #11052
What this Pr addresses
In bevy 0.12,
KeyCode
was referring to logical key (user layout dependant key), as it changes in bevy 0.13, changing the naming makes this behavioral change more obvious to users.Technically: it's just a rename of
KeyCode
toPhysicalKey
.Controversial
More discussion on discord: https://discord.com/channels/691052431525675048/768253008416342076/1203993423506571285
We're copying winit's type, and winit's wording is
KeyCode
.I agree keeping the same type name as winit makes it more obvious which type we're bringing in from winit. (in winit documentation,
KeyCode
is addressed as "Code representing the location of a physical key").And there is a
PhysicalKey
deeper down the lower level.. (andKeyCode
is a crossplatform abstraction over it).Migration Guide
KeyCode
. If you were previously usingKeyCode
to handle logical keys (to support user's keyboard layout), you might want to reach for the eventKeyboardInput
and itslogical_key
, or the eventReceivedCharacter
.Res<ButtonInput<PhysicalKey>>
to handle physical keys.