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

Keyboard shortcuts for track construction #5200

Merged
merged 1 commit into from
Mar 3, 2017

Conversation

JoelTroch
Copy link
Contributor

This PR add some keyboard shortcuts for track construction (especially for custom designs). I tried my best to optimize and have a clean code as much as possible.

Related issue: #1730

if (gScreenFlags & SCREEN_FLAGS_TITLE_DEMO)
return;

rct_window * w = window_find_by_class(WC_RIDE_CONSTRUCTION);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rather see this code in the construction window file.

Copy link
Contributor

Choose a reason for hiding this comment

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

(Reason being that the widgets[9] etc is difficult to maintain)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean move the whole code to src/openrct2/windows/ride_construction.c ?

@JoelTroch
Copy link
Contributor Author

I have refactored the "turn left" keyboard shortcut, can you please tell me if it's better that way ? If it's good then I'll do the same for the rest

@@ -537,4 +537,6 @@ void game_command_remove_track(sint32 *eax, sint32 *ebx, sint32 *ecx, sint32 *ed
void game_command_set_maze_track(sint32 *eax, sint32 *ebx, sint32 *ecx, sint32 *edx, sint32 *esi, sint32 *edi, sint32 *ebp);
void game_command_set_brakes_speed(sint32 *eax, sint32 *ebx, sint32 *ecx, sint32 *edx, sint32 *esi, sint32 *edi, sint32 *ebp);

void window_ride_construction_keyboard_shortcut_turn_left();
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be in window.h

return;
}

switch (_currentTrackCurve) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Indentation of the switch cases is inconsistent.

@marijnvdwerf
Copy link
Contributor

Made a few style-related comments.

@IntelOrca Can you go over window_ride_construction_keyboard_shortcut_turn_left implementation-wise?

@JoelTroch
Copy link
Contributor Author

Methods declaration moved from track.h to window.h and fixed the bad coding style for switch statements.

I'm doing the rest while waiting for @IntelOrca 's review implementation-wise.

@Nubbie
Copy link
Contributor

Nubbie commented Feb 12, 2017

Maybe could squash them all together in the end and insert a 'Fix: XXXX'? :p

@JoelTroch
Copy link
Contributor Author

@Nubbie Commits have been squashed ^^

@JoelTroch JoelTroch force-pushed the keyboard-track-construct branch 2 times, most recently from 9c3d4d3 to 952e34f Compare February 15, 2017 23:30
@Broxzier
Copy link
Member

My laptop has a Numpad that I have disabled most of the time, with some of the keys having double meaning (8/up, 4/left, 6/right, 2/down, 7/home, 9/page up, 1/end, 3/page down), but this doesn't take into account whether I have Numpad enabled or disabled. With Numpad enabled I expect Num 9 to act only as Num 9, and with Numpad disabled I expect it to act only act as the page up key (in my case of course).

@JoelTroch
Copy link
Contributor Author

If I understood correctly, you want "Keypad 8" (I'm using the term "Keypad" because that's how SDL2 call them) and "Page Up" to act the same ?

@Broxzier
Copy link
Member

@JoelTroch No, exactly the opposite.

img_20170217_150620201

In other applications when I have Numpad disabled (Numlock off), Keypad 9 acts like Page Up, and when Numpad is enabled, it acts as "Keypad 9" (In a text editor, it would either scroll up, or write down the "9").
In OpenRCT2 however, it always acts as if Numpad is disabled, and zooms out when I press "Keypad 9", even when Numpad is enabled.

This issue has probably been here before this PR too, I just only found out about it now.

@marijnvdwerf
Copy link
Contributor

This issue should probably get solved by #3908. Unfortunately that PR is rather stuck.

@JoelTroch
Copy link
Contributor Author

@Broxzier I see what you mean, I have the same layout as you.

After a very quick Google search, it might be related to Unicode input but someone more expert with input stuff should confirm or deny my sentence.

@janisozaur
Copy link
Member

What's the status of this?

@JoelTroch
Copy link
Contributor Author

It can be merged whenever the main OpenRCT2 developers desire to do so.

About the issue mentioned by @Broxzier , this isn't in the scope of this PR but @marijnvdwerf mentioned that #3908 could fix it if it's rebased and free of merge conflicts.

@janisozaur janisozaur added pending rebase PR needs to be rebased. pending review testing required PR needs to be tested before it is merged. labels Feb 27, 2017
@janisozaur
Copy link
Member

@JoelTroch can you rebase?

@JoelTroch
Copy link
Contributor Author

@janisozaur Done ^^

@janisozaur janisozaur removed the pending rebase PR needs to be rebased. label Feb 28, 2017
contributors.md Outdated
@@ -55,6 +55,7 @@ Includes all git commit authors. Aliases are GitHub user names.
* Niels NTG Poldervaart (Niels-NTG) - Misc.
* (zaxcav) - Improvements to original pathfinding algorithm.
* Hielke Morsink (Broxzier) - Tile inspector, misc.
* Joël Troch (Shepard62FR) - Keyboard shortcuts for ride construction.
Copy link
Member

Choose a reason for hiding this comment

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

Your Github username is JoelTroch, not Shepard62FR.

@@ -1,5 +1,6 @@
0.0.7 (in development)
------------------------------------------------------------------------
- Feature: [#1730] Add keyboard shortcuts for track construction.
Copy link
Member

Choose a reason for hiding this comment

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

- Feature: [#1730] Keyboard shortcuts for track construction.

@JoelTroch
Copy link
Contributor Author

Requested changes applied

@JoelTroch JoelTroch force-pushed the keyboard-track-construct branch 2 times, most recently from 1fcbde5 to e4c01f4 Compare March 2, 2017 22:09
@janisozaur
Copy link
Member

@Gymnasiast I looked briefly at the code and it looked simple enough for me not to expect any bugs in there. There was still a misindented line or two there. I trust your judgement.

@JoelTroch
Copy link
Contributor Author

JoelTroch commented Mar 2, 2017

@janisozaur I fixed one of the misaligned line, let me know if there are more.

UPDATE : Removed trailing whitespaces

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing required PR needs to be tested before it is merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants