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

The keyboard shortcut for rotating the game view can be set to Enter or KP Enter, but not both #9690

Closed
jgottula opened this issue Jul 31, 2019 · 2 comments · Fixed by #10092
Labels
bug Something went wrong.

Comments

@jgottula
Copy link
Contributor

jgottula commented Jul 31, 2019

OS: Windows 8.1
Commit/Build: 77f1905


Original RCT2 allowed rotating the view via either the normal Enter key or the keypad Enter key, interchangeably.

OpenRCT2, in its present state, does not allow this. If the keyboard shortcut is bound to "Return" (the default), then only the normal Enter key will rotate the view. If the keyboard shortcut is instead bound to "Keypad Enter", then only the keypad Enter key will rotate the view. There does not appear to be any way to allow both Enter keys to trigger the action.


I did look for duplicate reports on this, but didn't find anything recent. Way back in 2014, issue #651 was filed, concerning this same problem, and there was a fix for it in #653. But in the years since, my understanding is that basically all of the relevant code has been refactored and rewritten, making that moot.

I have no idea when this became a problem again, but I would assume that it was probably when the current keyboard shortcut system code was written up. I could have done a big bisect to pin it down for sure; but it doesn't seem terribly important, since the reason for the issue in the current code is quite straightforward.

In the current code, the keyboard shortcut system works via a series of simple 1:1 mappings between a single SDL scancode (possibly OR'd with a modifier key) and the function to be executed when that scancode is matched. And of course, since there are two distinct SDL scancodes for normal Enter and keypad Enter (SDL_SCANCODE_RETURN and SDL_SCANCODE_KP_ENTER), only one or the other can be assigned to the mapping for SHORTCUT_ROTATE_VIEW_CLOCKWISE; not both.


So... it seems that a resolution to this issue would probably either involve (a) a more complicated keyboard shortcut system that allows multiple keys to be bound to the same action (which would be plenty of work, and would make the UI a bit more complicated for users, but would ultimately allow for greater flexibility); or (b) some special-casing for the Enter keys (which might be a bit ugly, but would be simpler). The latter approach appears to be more-or-less what was done way back in #653.

Any special-casing, if done, I reckon would probably be put into keyboard_shortcut_handle and/or keyboard_shortcut_handle_command, essentially spoofing equality for SDL_SCANCODE_RETURN and SDL_SCANCODE_KP_ENTER. And then some additional code would probably be desirable in KeyboardShortcuts::Set and/or KeyboardShortcuts::GetFromKey to force user input of SDL_SCANCODE_KP_ENTER in the keyboard shortcut change window to be stored as SDL_SCANCODE_RETURN, lest users think they are mapping two separate actions to Enter and KP Enter when in fact only the non-keypad one will actually do anything.


I don't have a PR for this at the moment; if you want me to write one up, I can take a stab at it.

Really though, at this point, I'm more interested to know which direction you guys would prefer to take this (in part because it's potentially arguable whether the two Enter keys should be treated interchangeably for keyboard shortcuts, depending on preference):

  • No fix
  • Special-casing the Enter scancodes in the keyboard shortcut system, unilaterally
  • Special-casing the Enter scancodes in the keyboard shortcut system, but only for the view rotation shortcuts in particular
  • Enhancing the keyboard shortcut system to allow multiple keys to be mapped to the same action
  • Something else
@ocalhoun6
Copy link
Contributor

This is possibly related to #9684

@jgottula
Copy link
Contributor Author

@ocalhoun6 Huh, that caused me to realize something interesting: the snippet of code that was added back in 2014 for the #653 fix (which created a SDLK_KP_ENTER <--> SDL_SCANCODE_RETURN equivalency of sorts) must have been applicable to all key input back then; but while that bit of code still exists in the game (albeit in slightly altered form), it is now only applicable to text input situations in windows in the UI, and explicitly not applicable to keyboard shortcut situations any longer.

Here's that snippet in the current codebase:

if (key == SDLK_KP_ENTER)
{
// Map Keypad enter to regular enter.
key = SDLK_RETURN;
}

A relatively recent commit, 5d5373b, added some new special cases of SDL_SCANCODE_RETURN <--> SDL_SCANCODE_KP_ENTER equivalency specifically for console and chat input situations, which are treated separately from the UI windows.

So it kinda-sorta sounds like there may have been an understanding back in 2014 that the #653 fix would apply to everything; but at some point in the intervening time, its effect was limited only to text input, and so now we have things like 5d5373b popping up to specifically catch instances where the equivalency is still desired.

Interesting...

@Gymnasiast Gymnasiast added the bug Something went wrong. label Aug 1, 2019
Gymnasiast pushed a commit that referenced this issue Oct 18, 2019
…keys

* Remapped keypad Enter to return scancode

Fix #9684: Entering custom size for water/land tool allows confirmation
with main enter key, but not numpad enter key

Fix #9690: The keyboard shortcut for rotating the game view can be set
to Enter or KP Enter, but not both

* Add changelog entry
janisozaur added a commit that referenced this issue Oct 28, 2019
- Feature: [#9285] Remember current group in scenario list window.
- Feature: [#9918] Increase image list capacity by about 100k units.
- Change: [#1349] Increase the number of ride music played simultaneously from 2 to 32.
- Fix: [#4927] Giant screenshot cut off at bottom and top.
- Fix: [#7572] Queue paths connect to regular paths through fences.
- Fix: [#7690] Problem with guests freezing on certain tiles of path.
- Fix: [#7883] Headless server log is stored incorrectly if server name contains CJK in Ubuntu
- Fix: [#8136] Excessive lateral G penalty is too excessive.
- Fix: [#8584] Duck spawning function does not check tiles with x or y coordinate of 0..64 (Original bug)
- Fix: [#9179] Crash when modifying a ride occasionally.
- Fix: [#9533] Door sounds not playing.
- Fix: [#9574] Text overflow in scenario objective window when using CJK languages.
- Fix: [#9603] Don't render audio when master volume is turned off.
- Fix: [#9625] Show correct cost in scenery selection.
- Fix: [#9669] The tile inspector shortcut key does not work with debugging tools disabled.
- Fix: [#9675] Guest entry point limit can be bypassed in scenario editor.
- Fix: [#9683] Cannot raise water level if part of the tool's area of effect is off of the map.
- Fix: [#9684] Entering custom size for water/land tool allows confirmation with main enter key, but not numpad enter key.
- Fix: [#9690] The keyboard shortcut for rotating the game view can be set to Enter or KP Enter, but not both.
- Fix: [#9717] Scroll bars do not render correctly when using OpenGL renderer.
- Fix: [#9729] Peeps do not take into account height difference when deciding to pathfind to a ride entrance (original bug).
- Fix: [#9902] Doors/Portcullis do not check to make sure doors are open causing double opens.
- Fix: [#9926] Africa - Oasis park has wrong peep spawn (original bug).
- Fix: [#9953] Crash when hacked rides attempt to find the closest mechanic.
- Fix: [#9955] Resizing map in while pause mode does not work and may result in freezes.
- Fix: [#9957] When using 'no money' cheat, guests complain of running out of cash.
- Fix: [#9970] Wait for quarter load fails.
- Fix: [#9994] Game action tick collision during server connect and map load.
- Fix: [#10017] Ghost elements influencing ride excitement.
- Fix: [#10036] Do not allocate large chunks of memory for save file classification.
- Fix: [#10106] Ride circuits should not be used for modes that do not support it.
- Fix: [#10149] Desync in headless mode with rides that create smoke particles.
- Improved: [#9466] Add the rain weather effect to the OpenGL renderer.
- Improved: [#9987] Minimum load rounding.
- Improved: [#10125] Better support for high DPI screens.
XplosiveLugnut pushed a commit to XplosiveLugnut/OpenRCT2 that referenced this issue Apr 27, 2020
…ed as different keys

* Remapped keypad Enter to return scancode

Fix OpenRCT2#9684: Entering custom size for water/land tool allows confirmation
with main enter key, but not numpad enter key

Fix OpenRCT2#9690: The keyboard shortcut for rotating the game view can be set
to Enter or KP Enter, but not both

* Add changelog entry
XplosiveLugnut pushed a commit to XplosiveLugnut/OpenRCT2 that referenced this issue Apr 27, 2020
- Feature: [OpenRCT2#9285] Remember current group in scenario list window.
- Feature: [OpenRCT2#9918] Increase image list capacity by about 100k units.
- Change: [OpenRCT2#1349] Increase the number of ride music played simultaneously from 2 to 32.
- Fix: [OpenRCT2#4927] Giant screenshot cut off at bottom and top.
- Fix: [OpenRCT2#7572] Queue paths connect to regular paths through fences.
- Fix: [OpenRCT2#7690] Problem with guests freezing on certain tiles of path.
- Fix: [OpenRCT2#7883] Headless server log is stored incorrectly if server name contains CJK in Ubuntu
- Fix: [OpenRCT2#8136] Excessive lateral G penalty is too excessive.
- Fix: [OpenRCT2#8584] Duck spawning function does not check tiles with x or y coordinate of 0..64 (Original bug)
- Fix: [OpenRCT2#9179] Crash when modifying a ride occasionally.
- Fix: [OpenRCT2#9533] Door sounds not playing.
- Fix: [OpenRCT2#9574] Text overflow in scenario objective window when using CJK languages.
- Fix: [OpenRCT2#9603] Don't render audio when master volume is turned off.
- Fix: [OpenRCT2#9625] Show correct cost in scenery selection.
- Fix: [OpenRCT2#9669] The tile inspector shortcut key does not work with debugging tools disabled.
- Fix: [OpenRCT2#9675] Guest entry point limit can be bypassed in scenario editor.
- Fix: [OpenRCT2#9683] Cannot raise water level if part of the tool's area of effect is off of the map.
- Fix: [OpenRCT2#9684] Entering custom size for water/land tool allows confirmation with main enter key, but not numpad enter key.
- Fix: [OpenRCT2#9690] The keyboard shortcut for rotating the game view can be set to Enter or KP Enter, but not both.
- Fix: [OpenRCT2#9717] Scroll bars do not render correctly when using OpenGL renderer.
- Fix: [OpenRCT2#9729] Peeps do not take into account height difference when deciding to pathfind to a ride entrance (original bug).
- Fix: [OpenRCT2#9902] Doors/Portcullis do not check to make sure doors are open causing double opens.
- Fix: [OpenRCT2#9926] Africa - Oasis park has wrong peep spawn (original bug).
- Fix: [OpenRCT2#9953] Crash when hacked rides attempt to find the closest mechanic.
- Fix: [OpenRCT2#9955] Resizing map in while pause mode does not work and may result in freezes.
- Fix: [OpenRCT2#9957] When using 'no money' cheat, guests complain of running out of cash.
- Fix: [OpenRCT2#9970] Wait for quarter load fails.
- Fix: [OpenRCT2#9994] Game action tick collision during server connect and map load.
- Fix: [OpenRCT2#10017] Ghost elements influencing ride excitement.
- Fix: [OpenRCT2#10036] Do not allocate large chunks of memory for save file classification.
- Fix: [OpenRCT2#10106] Ride circuits should not be used for modes that do not support it.
- Fix: [OpenRCT2#10149] Desync in headless mode with rides that create smoke particles.
- Improved: [OpenRCT2#9466] Add the rain weather effect to the OpenGL renderer.
- Improved: [OpenRCT2#9987] Minimum load rounding.
- Improved: [OpenRCT2#10125] Better support for high DPI screens.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something went wrong.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants