Rename keys like `LAlt` to `AltLeft` #8792

jim-ec · 2023-06-08T14:21:51Z

Objective

The KeyCode enum cases LWin and RWin are too opinionated because they are also assigned meaning by non-Windows operating systems. macOS calls the keys completely different.

Solution

Match winits approach naming convention.

Migration Guide

Migrate by replacing:

LAlt → AltLeft
RAlt → AltRight
LBracket → BracketLeft
RBracket → BracketRight
LControl → ControlLeft
RControl → ControlRight
LShift → ShiftLeft
RShift → ShiftRight
LWin → SuperLeft
RWin → SuperRight

MrGVSV

While this differs from winit's KeyCode naming, I think this makes things a bit clearer for non-Windows users.

I might be biased though: #3240 (my first ever issue for Bevy)

alice-i-cecile

LGTM, but if possible we should add a doc alias to the enum variant. No idea if that's supported though :)

CGMossa · 2023-06-08T15:04:45Z

This needs a little more careful consideration.

mockersf · 2023-06-08T15:17:17Z

This is also part of the winit update (#8745). LWin will be called SuperLeft on the next winit version.

jim-ec · 2023-06-08T15:17:43Z

I could add doc aliases to the enum cases. Something like #[doc(alias("RWin", "RMeta"))]. I just verified that it works.

alice-i-cecile · 2023-06-08T15:47:31Z

Great, I like the doc aliases regardless of the final decision here.

alice-i-cecile · 2023-06-08T18:11:00Z

crates/bevy_input/src/keyboard.rs

+    /// Generic keyboards usually display this key with the *Microsoft Windows* logo.
+    /// Apple keyboards call this key the *Command Key* and display it using the ⌘ character.
+    #[doc(alias("LWin", "LMeta", "LLogo"))]
+    LSuper,


Suggested change

LSuper,

SuperLeft,

We should align with #8745 to avoid conflicts (and keep consistency with upstream).

Shouldn't we then also rename LAlt, LBracket, LControl, and LShift to LeftAlt etc.?

AltLeft, but yes, I agree. I think that the increased clarity is worth the extra characters.

I'm sure @mockersf has opinions though :p

In that case we should also adjust the doc aliases to WinLeft etc.

I'm sure @mockersf has opinions though :p

If we change one, we should change them all... but no opinion on wether to do it now or with winit update later

alice-i-cecile · 2023-06-08T18:11:11Z

crates/bevy_input/src/keyboard.rs

+    /// Generic keyboards usually display this key with the *Microsoft Windows* logo.
+    /// Apple keyboards call this key the *Command Key* and display it using the ⌘ character.
+    #[doc(alias("RWin", "RMeta", "RLogo"))]
+    RSuper,


Suggested change

RSuper,

SuperRight,

Vrixyz · 2023-06-08T21:42:57Z

As author of in #8792, I saw this PR linked, I'm ok with handling the potential conflicts due to merging this PR :)

The PR description mentions winit's ModifiersKeys ; but as hinted in the comments, I think I'd prefer to keep our namings matched to winit's KeyCode.

While this differs from winit's KeyCode naming

I'm alright with temporary winit difference, if consensus is that it's clearer. If there's no issue with #8745, namings should be back to similar after we update to winit 0.29 (hopefully bevy 0.12).

jim-ec · 2023-06-08T22:20:27Z

So I renamed the key codes to SuperLeft etc. to be consistent with winit.

MrGVSV · 2023-06-09T15:06:01Z

Also, the title and description (at least the migration guide) should probably be updated to make the changelog work easier for 0.11

TimJentzsch · 2023-06-12T21:23:44Z

crates/bevy_input/src/keyboard.rs

+    /// The `Left Alt` key. Maps to `Left Option` on Mac.
+    AltLeft,


Very minor thing: The doc string is now the reverse of the actual name.
Not sure if this matters though.

Yeah I'm not a fan of winit's naming here but it makes sense to be consistent with them

Yeah I think this is fine.

alice-i-cecile · 2023-06-15T01:37:01Z

Alright, merging this in as no one else has expressed opposition after more socialization and there's consensus that these names are clearer :)

Rename LWin and RWin to LSuper and RSuper

2bf5ec5

MrGVSV approved these changes Jun 8, 2023

View reviewed changes

alice-i-cecile added A-Input Player input via keyboard, mouse, gamepad, and more C-Usability A targeted quality-of-life change that makes Bevy easier to use M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide labels Jun 8, 2023

alice-i-cecile approved these changes Jun 8, 2023

View reviewed changes

Add doc aliases Win, Meta, and Logo

8e7bd2b

alice-i-cecile reviewed Jun 8, 2023

View reviewed changes

Rename to AltLeft ect.

b5969ae

jim-ec force-pushed the rename-win-key branch from 947bc94 to b5969ae Compare June 8, 2023 22:18

alice-i-cecile approved these changes Jun 9, 2023

View reviewed changes

jim-ec changed the title ~~Rename LWin and RWin to LSuper and RSuper~~ Rename keys like LAlt to AltLeft Jun 9, 2023

alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Jun 10, 2023

alice-i-cecile requested a review from mockersf June 10, 2023 23:20

TimJentzsch reviewed Jun 12, 2023

View reviewed changes

TimJentzsch approved these changes Jun 13, 2023

View reviewed changes

alice-i-cecile added this pull request to the merge queue Jun 15, 2023

Merged via the queue into bevyengine:main with commit 13f50c7 Jun 15, 2023

jim-ec deleted the rename-win-key branch June 15, 2023 11:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rename keys like `LAlt` to `AltLeft` #8792

Rename keys like `LAlt` to `AltLeft` #8792

jim-ec commented Jun 8, 2023 •

edited

Loading

MrGVSV left a comment

alice-i-cecile left a comment

CGMossa commented Jun 8, 2023

mockersf commented Jun 8, 2023 •

edited

Loading

jim-ec commented Jun 8, 2023

alice-i-cecile commented Jun 8, 2023

alice-i-cecile Jun 8, 2023

jim-ec Jun 8, 2023

alice-i-cecile Jun 8, 2023

alice-i-cecile Jun 8, 2023

jim-ec Jun 8, 2023

mockersf Jun 8, 2023

alice-i-cecile Jun 8, 2023

Vrixyz commented Jun 8, 2023 •

edited

Loading

jim-ec commented Jun 8, 2023 •

edited

Loading

MrGVSV commented Jun 9, 2023

TimJentzsch Jun 12, 2023

MrGVSV Jun 12, 2023

alice-i-cecile Jun 12, 2023

alice-i-cecile commented Jun 15, 2023 •

edited

Loading

		/// The `Left Alt` key. Maps to `Left Option` on Mac.
		AltLeft,

Rename keys like LAlt to AltLeft #8792

Rename keys like LAlt to AltLeft #8792

Conversation

jim-ec commented Jun 8, 2023 • edited Loading

Objective

Solution

Migration Guide

MrGVSV left a comment

Choose a reason for hiding this comment

alice-i-cecile left a comment

Choose a reason for hiding this comment

CGMossa commented Jun 8, 2023

mockersf commented Jun 8, 2023 • edited Loading

jim-ec commented Jun 8, 2023

alice-i-cecile commented Jun 8, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Vrixyz commented Jun 8, 2023 • edited Loading

jim-ec commented Jun 8, 2023 • edited Loading

MrGVSV commented Jun 9, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alice-i-cecile commented Jun 15, 2023 • edited Loading

Rename keys like `LAlt` to `AltLeft` #8792

Rename keys like `LAlt` to `AltLeft` #8792

jim-ec commented Jun 8, 2023 •

edited

Loading

mockersf commented Jun 8, 2023 •

edited

Loading

Vrixyz commented Jun 8, 2023 •

edited

Loading

jim-ec commented Jun 8, 2023 •

edited

Loading

alice-i-cecile commented Jun 15, 2023 •

edited

Loading