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

Reorganise the order of shortcut keys in the Shortcut Keys window #17288

Merged
merged 3 commits into from Jan 28, 2023
Merged

Reorganise the order of shortcut keys in the Shortcut Keys window #17288

merged 3 commits into from Jan 28, 2023

Conversation

guljam
Copy link
Contributor

@guljam guljam commented May 26, 2022

(Sorry for the new PR. I did not know how to initialize the modified remote repository, so I created a new branch.)

Re-ordered default shortcuts like vanilla RCT2
This version considers the shortcut arrangement of plugins.

The plugin's shortcut keys have a 'orderIndex' value of -1 by default and are sorted in the same way as in the previous version.
The 'orderIndex' of default shortcut starts from 0 and follows the order defined in ShortcutOrder.

You can still change the shortcut order by change the lines of elements in the "std::vector shortcutOrder" of the ShortcutManager::RegisterDefaultShortcuts() function.

I don't understand what @IntelOrca said before "set 'orderIndex' to 10 or 100". So I sorted by index numbering primitively.

old:
oldshortcut

new:
newshorcut

@guljam guljam changed the title Fix messed up shortcuts Fix the order of shortcut keys messed up in the shortcut settings window May 26, 2022
@guljam guljam marked this pull request as draft May 26, 2022 23:55
@guljam guljam marked this pull request as ready for review May 27, 2022 00:01
@guljam guljam marked this pull request as draft May 27, 2022 00:02
@guljam guljam marked this pull request as ready for review May 27, 2022 01:46
src/openrct2-ui/input/Shortcuts.cpp Outdated Show resolved Hide resolved
src/openrct2-ui/input/Shortcuts.cpp Outdated Show resolved Hide resolved
src/openrct2-ui/input/Shortcuts.cpp Outdated Show resolved Hide resolved
src/openrct2-ui/input/ShortcutManager.h Outdated Show resolved Hide resolved
@guljam
Copy link
Contributor Author

guljam commented May 27, 2022

@Broxzier I was trying to fix the code you suggested
I found that my code is completely ignoring the data in the reset keys.

so I restore the code for the ShortcutManager::RegisterDefaultShortcuts function to the previous version and
I simply entered the orderindex in the last parameter.

This method is very tricky when changing the order of the code.
@IntelOrca expects that shortcuts will rarely change order, so I hardcoded them.

@guljam guljam requested a review from Broxzier May 27, 2022 14:53
Copy link
Member

@Broxzier Broxzier left a comment

Choose a reason for hiding this comment

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

Looks like the priorities are incrementing by one per shortcut. Why not store a variable, possible even inside the RegisterShortcut function, and let that handle the order. That saves a lot of magic numbers here.

src/openrct2-ui/input/ShortcutManager.h Outdated Show resolved Hide resolved
src/openrct2-ui/input/ShortcutManager.h Outdated Show resolved Hide resolved
src/openrct2-ui/input/ShortcutManager.h Outdated Show resolved Hide resolved
src/openrct2-ui/input/ShortcutManager.h Outdated Show resolved Hide resolved
src/openrct2-ui/input/ShortcutManager.h Outdated Show resolved Hide resolved
@guljam
Copy link
Contributor Author

guljam commented May 27, 2022

Looks like the priorities are incrementing by one per shortcut. Why not store a variable, possible even inside the RegisterShortcut function, and let that handle the order. That saves a lot of magic numbers here.

that's good idea.
I was stupid.
Just by doing Orderindex++, it is expected that the shortcut keys of the plugin will be sorted well. Let's fix the code.

@guljam
Copy link
Contributor Author

guljam commented May 27, 2022

@Broxzier
I've found an optimized solution with your suggestion and the behavior is:

  1. When ShortcutManager::RegisterShortcut is called, the shortcut's OrderIndex is given Shortcuts.size() -1
  2. When the InitialiseList() function of shortcutKeys.cpp is called, the list is reorderd by OrderIndex

The plugin shortcuts work nice too.
The order in which ShortcutManager::RegisterShortcut is called has become important.

@guljam guljam requested a review from Broxzier May 27, 2022 18:27
@Broxzier
Copy link
Member

Broxzier commented Jun 6, 2022

Please clean up this PR and mark comments as resolved if they have been addressed.

@guljam guljam requested a review from Broxzier June 6, 2022 19:40
@guljam
Copy link
Contributor Author

guljam commented Jun 6, 2022

Please clean up this PR and mark comments as resolved if they have been addressed.
Added "// clang-format on" comment to last line.

@tupaschoal
Copy link
Member

@guljam would you mind marking the conversations that finished as resolved?

@guljam
Copy link
Contributor Author

guljam commented Jun 29, 2022

@tupaschoal Oh, sorry. Does the conversation always have to end?

@tupaschoal
Copy link
Member

@tupaschoal Oh, sorry. Does the conversation always have to end?

We tend to assume that if it's open, it's because one of the parties involved is waiting for an action (a response, a fix, etc). When it is just a suggestion that maps directly to changing the code, then it becomes outdated and it's easy to see that it has been solved and mark as such. When it's a more abstract suggestion/comment, you're always free to ask the commenter whether they like the solution or to clarify, and this is when it stays unresolved.

src/openrct2-ui/input/ShortcutIds.h Outdated Show resolved Hide resolved
src/openrct2-ui/input/Shortcuts.cpp Outdated Show resolved Hide resolved
src/openrct2-ui/windows/ShortcutKeys.cpp Outdated Show resolved Hide resolved
src/openrct2-ui/windows/ShortcutKeys.cpp Outdated Show resolved Hide resolved
src/openrct2-ui/windows/ShortcutKeys.cpp Outdated Show resolved Hide resolved
@guljam guljam requested a review from Broxzier June 30, 2022 02:10
@Broxzier
Copy link
Member

I've now squashed and rebased this PR. I'll get to testing this and checking over the changes again soon.

@Broxzier Broxzier added changelog This issue/PR deserves a changelog entry. labels Jan 25, 2023
@Broxzier Broxzier changed the title Fix the order of shortcut keys messed up in the shortcut settings window Reorganise the order of shortcut keys in the Shortcut Keys window Jan 27, 2023
@Broxzier Broxzier removed the changelog This issue/PR deserves a changelog entry. label Jan 28, 2023
@ZehMatt ZehMatt added the changelog This issue/PR deserves a changelog entry. label Jan 28, 2023
Copy link
Member

@ZehMatt ZehMatt left a comment

Choose a reason for hiding this comment

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

Tested and works as expected

@Broxzier Broxzier merged commit 566c1f2 into OpenRCT2:develop Jan 28, 2023
@Broxzier Broxzier added this to the v0.4.4 milestone Jan 28, 2023
janisozaur added a commit that referenced this pull request Mar 28, 2023
- Feature: [#11269] Add properties for speed and length of vehicle animations.
- Feature: [#15849] Objectives can now be set for up to 50000 guests.
- Feature: [#18537] Add shift/control modifiers to window close buttons, closing all but the given window or all windows of the same type, respectively.
- Feature: [#18732] [Plugin] API to get the guests thoughts.
- Feature: [#18744] Cheat to allow using a regular path as a queue path.
- Feature: [#19023] Add Canadian French translation.
- Feature: [#19341] Add “All Scenery” tab to scenery window.
- Feature: [#19378] Add command to combine CSG1i.DAT and CSG1.DAT.
- Feature: [objects#226] Port RCT1 Corkscrew Coaster train.
- Feature: [objects#229] Port RCT1 go karts with helmets.
- Feature: [OpenMusic#20, OpenMusic#21] Added Blizzard and Extraterresterial ride music styles.
- Improved: [#11473] Hot reload for plug-ins now works on macOS.
- Improved: [#12466] RCT1 parks now use RCT1’s interest calculation algorithm.
- Improved: [#14635] Scenery window now shows up to 255 scenery groups.
- Improved: [#17288] Reorganise the order of shortcut keys in the Shortcut Keys window.
- Improved: [#18706] Ability to view the list of contributors in-game.
- Improved: [#18749] Ability to have 4 active awards for more than one month in a row.
- Improved: [#18826] [Plugin] Added all actions and their documentation to plugin API.
- Improved: [#18945] Languages can now fall back to other languages than English.
- Improved: [#18970] Trying to load a non-park save will now display a context error.
- Improved: [#18975] Add lift sprites for steep hills on the wooden roller coaster.
- Improved: [#19044] Added special thanks to RMC and Wiegand to the About page.
- Improved: [#19131] Track missing objects when selecting scenery groups in console.
- Improved: [#19253] Queue junctions drawn properly when using regular paths as queue. Note: Requires using tile inspector to indicate railings can be used at T or X junctions.
- Improved: [#19067] New Ride window now allows filtering similarly to Object Selection.
- Improved: [#19272] Scenery window now allows filtering similarly to Object Selection.
- Improved: [#19447] The control key now enables word jumping in text input fields.
- Improved: [#19463] Added ‘W’ and ‘Y’ with circumflex to sprite font (for Welsh).
- Improved: [#19549] Enable large address awareness for 32 bit Windows builds allowing to use 4 GiB of virtual memory.
- Improved: [#19668] Decreased the minimum map size from 13 to 3.
- Improved: [#19683] The delays for ride ratings to appear has been reduced drastically.
- Improved: [#19697] “Show guest purchases” will now work in multiplayer.
- Change: [#19018] Renamed actions to fit the naming scheme.
- Change: [#19091] [Plugin] Add game action information to callback arguments of custom actions.
- Change: [#19233] Reduce lift speed minimum and maximum values for “Classic Wooden Coaster”.
- Removed: [#19520] Support for Windows Vista systems.
- Fix: [#474] Mini golf window shows more players than there actually are (original bug).
- Fix: [#592] Window scrollbar not able to navigate to the end of large lists.
- Fix: [#7210] Land tile smoothing occurs with edge tiles (original bug).
- Fix: [#17996] Finances window not cleared when starting some .park scenarios.
- Fix: [#18260] Crash opening parks that have multiple tiles referencing the same banner entry.
- Fix: [#18467] “Selected only” Object Selection filter is active in Track Designs Manager, and cannot be toggled.
- Fix: [#18904] OpenRCT2 audio object accidentally exported in saves.
- Fix: [#18905] Ride Construction window theme is not applied correctly.
- Fix: [#18911] Mini Golf station does not draw correctly from all angles.
- Fix: [#18971] New Game does not prompt for save before quitting.
- Fix: [#18986] [Plugin] Sending remote scripts larger than 63KiB crashing all clients.
- Fix: [#18994] Title music doesn’t start after enabling master volume.
- Fix: [#19025] Park loan behaves inconsistently with non-round and out-of-bounds values.
- Fix: [#19026] Park loan is clamped to a 32-bit integer.
- Fix: [#19068] Guests may not join queues correctly.
- Fix: [#19091] [Plugin] Remote plugins in multiplayer servers do not unload properly.
- Fix: [#19112] Clearing the last character in the Object Selection filter does not properly reset it.
- Fix: [#19112] Text boxes not updated with empty strings in Track List, Server List, and Start Server windows.
- Fix: [#19114] [Plugin] ‘GameActionResult’ does not comply to API specification.
- Fix: [#19136] SV6 saves with experimental RCT1 paths not imported correctly.
- Fix: [#19243] .park scenarios don’t complete properly.
- Fix: [#19250] MusicObjects do not free their preview images.
- Fix: [#19292] Overflow in ‘totalRideValue’.
- Fix: [#19339] Incorrect import of crashed particles from SV4.
- Fix: [#19379] “No platforms” station style shows platforms on the Junior Roller Coaster.
- Fix: [#19380] Startup crash when no sequences are installed and random sequences are enabled.
- Fix: [#19391] String corruption caused by an improper buffer handling in ‘GfxWrapString’.
- Fix: [#19434, #19509] Object types added by OpenRCT2 do not get removed when executing ‘remove_unused_objects’.
- Fix: [#19475] Cannot increase loan when more than £1000 in debt.
- Fix: [#19493] SV4 saves not importing the correct vehicle colours.
- Fix: [#19517] Crash when peeps try to exit or enter hacked rides that have no waypoints specified.
- Fix: [#19524] Staff counter shows incorrect values if there are more than 32767 staff members.
- Fix: [#19574] Handle exits in null locations.
- Fix: [#19641, #19643] Missing water tile in Infernal Views’ and Six Flags Holland’s river.
qwzhaox pushed a commit to qwzhaox/OpenRCT2 that referenced this pull request Apr 10, 2023
- Feature: [OpenRCT2#11269] Add properties for speed and length of vehicle animations.
- Feature: [OpenRCT2#15849] Objectives can now be set for up to 50000 guests.
- Feature: [OpenRCT2#18537] Add shift/control modifiers to window close buttons, closing all but the given window or all windows of the same type, respectively.
- Feature: [OpenRCT2#18732] [Plugin] API to get the guests thoughts.
- Feature: [OpenRCT2#18744] Cheat to allow using a regular path as a queue path.
- Feature: [OpenRCT2#19023] Add Canadian French translation.
- Feature: [OpenRCT2#19341] Add “All Scenery” tab to scenery window.
- Feature: [OpenRCT2#19378] Add command to combine CSG1i.DAT and CSG1.DAT.
- Feature: [objects#226] Port RCT1 Corkscrew Coaster train.
- Feature: [objects#229] Port RCT1 go karts with helmets.
- Feature: [OpenMusic#20, OpenMusic#21] Added Blizzard and Extraterresterial ride music styles.
- Improved: [OpenRCT2#11473] Hot reload for plug-ins now works on macOS.
- Improved: [OpenRCT2#12466] RCT1 parks now use RCT1’s interest calculation algorithm.
- Improved: [OpenRCT2#14635] Scenery window now shows up to 255 scenery groups.
- Improved: [OpenRCT2#17288] Reorganise the order of shortcut keys in the Shortcut Keys window.
- Improved: [OpenRCT2#18706] Ability to view the list of contributors in-game.
- Improved: [OpenRCT2#18749] Ability to have 4 active awards for more than one month in a row.
- Improved: [OpenRCT2#18826] [Plugin] Added all actions and their documentation to plugin API.
- Improved: [OpenRCT2#18945] Languages can now fall back to other languages than English.
- Improved: [OpenRCT2#18970] Trying to load a non-park save will now display a context error.
- Improved: [OpenRCT2#18975] Add lift sprites for steep hills on the wooden roller coaster.
- Improved: [OpenRCT2#19044] Added special thanks to RMC and Wiegand to the About page.
- Improved: [OpenRCT2#19131] Track missing objects when selecting scenery groups in console.
- Improved: [OpenRCT2#19253] Queue junctions drawn properly when using regular paths as queue. Note: Requires using tile inspector to indicate railings can be used at T or X junctions.
- Improved: [OpenRCT2#19067] New Ride window now allows filtering similarly to Object Selection.
- Improved: [OpenRCT2#19272] Scenery window now allows filtering similarly to Object Selection.
- Improved: [OpenRCT2#19447] The control key now enables word jumping in text input fields.
- Improved: [OpenRCT2#19463] Added ‘W’ and ‘Y’ with circumflex to sprite font (for Welsh).
- Improved: [OpenRCT2#19549] Enable large address awareness for 32 bit Windows builds allowing to use 4 GiB of virtual memory.
- Improved: [OpenRCT2#19668] Decreased the minimum map size from 13 to 3.
- Improved: [OpenRCT2#19683] The delays for ride ratings to appear has been reduced drastically.
- Improved: [OpenRCT2#19697] “Show guest purchases” will now work in multiplayer.
- Change: [OpenRCT2#19018] Renamed actions to fit the naming scheme.
- Change: [OpenRCT2#19091] [Plugin] Add game action information to callback arguments of custom actions.
- Change: [OpenRCT2#19233] Reduce lift speed minimum and maximum values for “Classic Wooden Coaster”.
- Removed: [OpenRCT2#19520] Support for Windows Vista systems.
- Fix: [OpenRCT2#474] Mini golf window shows more players than there actually are (original bug).
- Fix: [OpenRCT2#592] Window scrollbar not able to navigate to the end of large lists.
- Fix: [OpenRCT2#7210] Land tile smoothing occurs with edge tiles (original bug).
- Fix: [OpenRCT2#17996] Finances window not cleared when starting some .park scenarios.
- Fix: [OpenRCT2#18260] Crash opening parks that have multiple tiles referencing the same banner entry.
- Fix: [OpenRCT2#18467] “Selected only” Object Selection filter is active in Track Designs Manager, and cannot be toggled.
- Fix: [OpenRCT2#18904] OpenRCT2 audio object accidentally exported in saves.
- Fix: [OpenRCT2#18905] Ride Construction window theme is not applied correctly.
- Fix: [OpenRCT2#18911] Mini Golf station does not draw correctly from all angles.
- Fix: [OpenRCT2#18971] New Game does not prompt for save before quitting.
- Fix: [OpenRCT2#18986] [Plugin] Sending remote scripts larger than 63KiB crashing all clients.
- Fix: [OpenRCT2#18994] Title music doesn’t start after enabling master volume.
- Fix: [OpenRCT2#19025] Park loan behaves inconsistently with non-round and out-of-bounds values.
- Fix: [OpenRCT2#19026] Park loan is clamped to a 32-bit integer.
- Fix: [OpenRCT2#19068] Guests may not join queues correctly.
- Fix: [OpenRCT2#19091] [Plugin] Remote plugins in multiplayer servers do not unload properly.
- Fix: [OpenRCT2#19112] Clearing the last character in the Object Selection filter does not properly reset it.
- Fix: [OpenRCT2#19112] Text boxes not updated with empty strings in Track List, Server List, and Start Server windows.
- Fix: [OpenRCT2#19114] [Plugin] ‘GameActionResult’ does not comply to API specification.
- Fix: [OpenRCT2#19136] SV6 saves with experimental RCT1 paths not imported correctly.
- Fix: [OpenRCT2#19243] .park scenarios don’t complete properly.
- Fix: [OpenRCT2#19250] MusicObjects do not free their preview images.
- Fix: [OpenRCT2#19292] Overflow in ‘totalRideValue’.
- Fix: [OpenRCT2#19339] Incorrect import of crashed particles from SV4.
- Fix: [OpenRCT2#19379] “No platforms” station style shows platforms on the Junior Roller Coaster.
- Fix: [OpenRCT2#19380] Startup crash when no sequences are installed and random sequences are enabled.
- Fix: [OpenRCT2#19391] String corruption caused by an improper buffer handling in ‘GfxWrapString’.
- Fix: [OpenRCT2#19434, OpenRCT2#19509] Object types added by OpenRCT2 do not get removed when executing ‘remove_unused_objects’.
- Fix: [OpenRCT2#19475] Cannot increase loan when more than £1000 in debt.
- Fix: [OpenRCT2#19493] SV4 saves not importing the correct vehicle colours.
- Fix: [OpenRCT2#19517] Crash when peeps try to exit or enter hacked rides that have no waypoints specified.
- Fix: [OpenRCT2#19524] Staff counter shows incorrect values if there are more than 32767 staff members.
- Fix: [OpenRCT2#19574] Handle exits in null locations.
- Fix: [OpenRCT2#19641, OpenRCT2#19643] Missing water tile in Infernal Views’ and Six Flags Holland’s river.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog This issue/PR deserves a changelog entry.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants