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

Add a "current_rotation" variable to the console #8080

Merged
merged 10 commits into from
Oct 14, 2018

Conversation

tombomp
Copy link
Contributor

@tombomp tombomp commented Oct 10, 2018

This is an implementation of this feature request https://openrct2.org/forums/topic/3404-console-addition/

It adds a current_rotation variable whose get is equal to get_current_rotation() and set uses window_rotate_camera to rotate it to the desired value (from 0-3, although it works for any value it's just % 4)

This is my first github pull request and everything and I'm kind of rusty with C++ stuff too so obviously appreciate criticism of style and everything.

This is a value from 0-3 showing current rotaton of main window, 3 being the default and lower going counter clockwise. Possible to set as well.
uint8_t currentRotation = get_current_rotation();
rct_window* mainWindow = window_get_main();
// as there are only 4 possible rotations, make any input go to 0-3 to avoid unneccessary work
int32_t newRotation = int_val[0] % 4;
Copy link
Member

Choose a reason for hiding this comment

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

The comment and code don't match here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I meant if you passed a value like 20 then it would otherwise go through 5x ish the iterations necessary. But really it doesn't make sense to accept values outside the 0-3 range so I should probably just print a message saying the accepted range instead?

Copy link
Member

Choose a reason for hiding this comment

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

That sounds fine, but you need to limit it to [0, 3] range first. So far you only limited it to [-3, 3].

while (newRotation != currentRotation)
{
window_rotate_camera(mainWindow, -1);
newRotation++;
Copy link
Member

Choose a reason for hiding this comment

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

I'm confused, why is this done in a loop here rather than simply passing the direction user wanted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

window_rotate_camera only takes a 1 or -1 to turn clockwise or counterclockwise. I was assuming that was the best function to use and so I'd have to call it multiple times.

Copy link
Member

Choose a reason for hiding this comment

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

If it is indeed like so, then that case that's a bug in this function's design. It should take an enum class containing forward and backward enumeration value and there should be another function setting the rotation directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm happy to change that function so it works like you said. I just didn't want to immediately change around the other functions because I didn't want to interfere with other people's stuff.

Copy link
Member

Choose a reason for hiding this comment

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

window_rotate_camera accepts a rotation value, I'm not sure where you got the part "only takes a 1 or -1 to turn clockwise or counterclockwise" from.

Choose a reason for hiding this comment

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

window_rotate_camera accepts a rotation value, I'm not sure where you got the part "only takes a 1 or -1 to turn clockwise or counterclockwise" from.

probably from here? https://github.com/OpenRCT2/OpenRCT2/blob/develop/src/openrct2/interface/Window.cpp#L951

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I was reading the comment above it and taking it maybe too literally as well as not thinking hard enough - the function itself works fine and as expected with numbers other than 1 and -1 (ie -2 is 2 steps counterclockwise).

So I could just replace the loops with window_rotate_camera(mainWindow, currentRotation - newRotation); which I've tested and works fine and should be fine once I've fixed the other issue above. Sorry for dragging this out, I appreciate the help.

- Made get work correctly (missed brackets)
- Made set cleaner (1 call no loops)
- Made modulous correct (no negative numbers)
@tombomp
Copy link
Contributor Author

tombomp commented Oct 12, 2018

I'm going to be away for a few days but I added a new commit that hopefully fixes the issues you identified. Again I really appreciate the help and reviewing and I'm sorry for making heavy work of a simple thing.

@@ -5,6 +5,7 @@
- Feature: [#7956, #7964] Add sprite font glyphs for Hungarian and some Czech letters.
- Feature: [#7971] Toolbox option to open custom content folder.
- Feature: [#7980] Allow data path for RCT1 to be specified by a command line argument.
- Feature: New console variable "current_rotation" to get or set view rotation
Copy link
Member

Choose a reason for hiding this comment

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

Add the PR number and a full stop.

- Feature: [#8080] New console variable "current_rotation" to get and set view rotation.

This is a value from 0-3 showing current rotaton of main window, 3 being the default and lower going counter clockwise. Possible to set as well.
uint8_t currentRotation = get_current_rotation();
rct_window* mainWindow = window_get_main();
// there are only 4 possible rotations (0-3)
int32_t newRotation = int_val[0] & 3;
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps it would be better to reject arguments out of range and report usage in such case. It wouldn't let users develop bad habits of passing invalid values.

@janisozaur
Copy link
Member

I rebased this on top of now-merged #8078. There's one minor thing that could be improved, but I plan on merging it once CIs are green. Feel free to submit a PR to address my comment.

uint8_t currentRotation = get_current_rotation();
rct_window* mainWindow = window_get_main();
int32_t newRotation = int_val[0];
if(newRotation < 0 || newRotation > 3) {
Copy link
Member

Choose a reason for hiding this comment

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

Please format properly.

@janisozaur janisozaur merged commit 18307e3 into OpenRCT2:develop Oct 14, 2018
janisozaur added a commit that referenced this pull request Mar 13, 2019
- Feature: [#4418] Allow steep slopes on the side-friction roller coaster.
- Feature: [#7726] Add shortcut to advance one tick.
- Feature: [#7956, #7964] Add sprite font glyphs for Hungarian and some Czech letters.
- Feature: [#7971] Toolbox option to open custom content folder.
- Feature: [#7980] Allow data path for RCT1 to be specified by a command line argument.
- Feature: [#8073] Auto-upload minidumps to backtrace.io (optional, MSVC/Windows only)
- Feature: [#8078] Add save_park command to in-game console.
- Feature: [#8080] New console variable "current_rotation" to get or set view rotation.
- Feature: [#8098] Glyph for Russian rouble sign.
- Feature: [#8099] Add Powered Launch mode to Inverted RC (for RCT1 parity).
- Feature: [#8190] Allow building footpaths on 'corner down' terrain.
- Feature: [#8191] Allow building on-ride photos and water S-bends on the Water Coaster.
- Feature: [#8259] Add say command to in-game console.
- Feature: [#8374] Add replay system.
- Feature: [#8377] Add option to adjust amount of autosaves to keep.
- Feature: [#8458] Add sprite sorting benchmark.
- Feature: [#8583] Add boosters to water coaster.
- Feature: [#8648] Add optional chat button to top toolbar in multiplayer games.
- Feature: [#8652] Add network window including a graph for data usage visualisation.
- Feature: [#8670] Add ability to download missing objects when loading a park.
- Change: [#7961] Add new object types: station, terrain surface, and terrain edge.
- Change: [#8222] The climate setting has been moved from objective options to scenario options.
- Change: [#8718] Allow TARMAC object to be removed when running the `remove_unused_objects` command.
- Change: [#8718] No longer require the generic scenery groups and tarmac footpath to be checked when creating a scenario.
- Change: [#8734] Disable kick button in multiplayer window when unable to use it.
- Fix: [#3832] Changing the colour scheme of track pieces does not work in multiplayer.
- Fix: [#4094] Coasters with long flat-to-steep pieces offer them in diagonal mode (original bug).
- Fix: [#5684] Player list can desync between clients and server and can crash.
- Fix: [#6191] OpenRCT2 fails to run when the path has an emoji in it.
- Fix: [#7439] Placement messages have mixed strings
- Fix: [#7473] Disabling sound effects also disables "Disable audio on focus loss".
- Fix: [#7536] Android builds fail to start.
- Fix: [#7689] Deleting 0-tile maze gives a MONEY32_UNDEFINED (negative) refund.
- Fix: [#7828] Copied entrances and exits stay when demolishing ride.
- Fix: [#7945] Client IP address is logged as `(null)` in server logs.
- Fix: [#7952] Performance drop caused by code refactor.
- Fix: [#7954] Key validation fails on Windows due to non-ASCII user / player name.
- Fix: [#7975] Inspection flag not cleared for rides which are set to never be inspected (original bug).
- Fix: [#7985] Giant Screenshot ignores 'Map rendering' settings.
- Fix: [#7987] Broken track designs increase money by MONEY32_UNDEFINED.
- Fix: [#7991] Scenery and footpaths on Construction Rights tiles can be deleted using Clear Scenery.
- Fix: [#8034] Vanilla sprites are broken when making screenshots from command line.
- Fix: [#8045] Crash when switching between languages.
- Fix: [#8062] In multiplayer warnings for unstable cheats are shown when disabling them.
- Fix: [#8090] Maze designs saved incorrectly.
- Fix: [#8101] Title sequences window flashes after opening.
- Fix: [#8120] Crash trying to place peep spawn outside of map.
- Fix: [#8121] Crash Renaming park with server logging enabled.
- Fix: [#8139] Buying land costs money when the game is in "no money" mode.
- Fix: [#8141] Attempting to build entrance/exit on station 2 does not work.
- Fix: [#8142] Reliability of mazes and crooked houses can go below 100%.
- Fix: [#8187] Cannot set land ownership over ride entrances or exits in sandbox mode.
- Fix: [#8200] Incorrect behaviour when removing entrances and exits that are on the same tile.
- Fix: [#8204] Crash when tile element has no surface elements.
- Fix: [#8264] Rides and scenery placeable outside of map with ZC and Sandbox mode enabled.
- Fix: [#8335] Rides with arbitrary ride types can crash the game when they break down.
- Fix: [#8358] Infinite loop when changing vehicle count on stopped ride.
- Fix: [#8402] Crash closing a window in some cases.
- Fix: [#8431] Crash when game action logging is enabled.
- Fix: [#8433] Crash if master server response is not valid JSON.
- Fix: [#8434] Crash if curl_easy_init fails.
- Fix: [#8443] Crash when selecting the current vehicle for ride that has none available.
- Fix: [#8456] Junior booster track piece doesn't connect properly.
- Fix: [#8464] Crash on game shutdown.
- Fix: [#8469] Crash modifying colour on hacked rides.
- Fix: [#8508] Underground roto-drop is not going up.
- Fix: [#8555] Multiplayer window text limits are not computed properly.
- Fix: [#8572] Steel Twister track pieces ID 64 and 65 drawn incorrectly.
- Fix: [#8585] Part of track missing on air powered vertical coaster.
- Fix: [#8588] Guest list scrolling breaks above ~2000 guests.
- Fix: [#8591] Game loop does not run at a consistent tick rate of 40 Hz.
- Fix: [#8647] Marketing campaigns check for entry fees below £1 (original bug).
- Fix: [#8653] Crash when peeps attempt to enter a ride with no vehicles.
- Fix: [#8720] Desync due to boats colliding with ghost pieces.
- Fix: [#8739] Savegame from original game crashes when cruising through map.
- Fix: [#8742] Access violation in vehicle_update_sound_params.
- Fix: [#8804] Raising water shows money effect at the bottom rather than new height.
- Fix: [#8811] Some fields in the sv6 save file not being copied correctly.
- Fix: [#8824] Invalid read in footpath_chain_ride_queue.
- Improved: [#2940] Allow mouse-dragging to set patrol area (Singleplayer only).
- Improved: [#7730] Draw extreme vertical and lateral Gs red in the ride window's graph tab.
- Improved: [#7930] Automatically create folders for custom content.
- Improved: [#7980] Show the full path of the scenario in the scenario select window.
- Improved: [#7993] Allow assigning a keyboard shortcut for opening the tile inspector.
- Improved: [#8107] Support Discord release of RCT2.
- Improved: [#8491] Highlight entrance and exit with different colours in track design previews.
- Improved: Almost completely new Hungarian translation.
- Removed: [#7929] Support for scenario text objects.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants