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

Better support for displaced entrances and exits #7031

Merged
merged 5 commits into from
Feb 6, 2018

Conversation

Broxzier
Copy link
Member

Rides have a list of XY locations for the entrances and exits of every station, and a list of station heights which basically acts as the Z axis. At many places the game assumes that the entrances are there, making them not work properly when they have been moved. As a result, building a working queue from the entrance is only possible by copying over a path from the actual station, and peeps will try to navigate to the original location.

This PR makes detached ride entrances usable by making the path tool search for them on the known tile when placing a path. Instead of checking for the expected height, it loops over the elements on tile XY and checks for the ride index instead (which it didn't do before, so it happens you have two entrances at the same tile, it would always choose the first in the list, even if it's from another ride!).

To make entrances and exits usable that have been moved, I've added a button to the Tile Inspector which can update the location that the ride has saved.

This PR only takes care of the assumption for when building queues, but there are many more (example: mechanics don't get called when the ride exit is not where the ride thinks it is). I'd like to leave those for later, preferably after the next release.

uint8 rideIndex = *queueChainPtr;
Ride *ride = get_ride(rideIndex);
if (ride->type == RIDE_TYPE_NULL) {
for (uint8 * queueChainPtr = _footpathQueueChain; queueChainPtr < _footpathQueueChainNext; queueChainPtr++)
Copy link
Member

Choose a reason for hiding this comment

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

This should use range-based for and there's little reason for queueChainPtr and rideIndex to coexist,

Copy link
Member

Choose a reason for hiding this comment

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

Oh, it doesn't loop through all the elements, but only up to a pointer somewhere into this array. Nevermind then.

{
rct_tile_element * const entranceElement = map_get_nth_element_at(x, y, elementIndex);

if (!entranceElement || tile_element_get_type(entranceElement) != TILE_ELEMENT_TYPE_ENTRANCE)
Copy link
Contributor

Choose a reason for hiding this comment

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

if (entranceElement == nullptr || ...


if (flags & GAME_COMMAND_FLAG_APPLY)
{
Ride * ride = get_ride(entranceElement->properties.entrance.ride_index);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should check for nullptr.

@rwjuk rwjuk added the pending rebase PR needs to be rebased. label Jan 17, 2018
@rwjuk
Copy link
Contributor

rwjuk commented Jan 17, 2018

Left a couple of comments regarding null checks, otherwise LGTM (will need a rebase).

@Broxzier
Copy link
Member Author

I've changed the ptr check and rebased. Since the rest of the document checked pointers by using !ptr instead of ptr == nullptr too, I've decided to change them all and run clang-format.

@rwjuk rwjuk removed the pending rebase PR needs to be rebased. label Jan 17, 2018
*
* A full copy of the GNU General Public License can be found in licence.txt
*****************************************************************************/
* OpenRCT2, an open source clone of Roller Coaster Tycoon 2.
Copy link
Member

Choose a reason for hiding this comment

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

?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is just what clang-format does - we should probably tweak it not to do it.

Copy link
Member Author

Choose a reason for hiding this comment

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

clang-format screws up the copyright notice at the top, and adds those spaces. After comparing it with other files I noticed that they all have one space before the lines, so I left that change in.

@@ -89,9 +89,7 @@ sint32 tile_inspector_insert_corrupt_at(sint32 x, sint32 y, sint16 elementIndex,
{
// Make sure there is enough space for the new element
if (!map_check_free_elements_and_reorganise(1))
{
Copy link
Member

Choose a reason for hiding this comment

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

Unnecessary change.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's more consistent, though.

@Gymnasiast
Copy link
Member

At many places the game assumes that the entrances are there, making them not work properly when they have been moved

Not true. When loading a save, the game checks if all entrances and exits are actually at the set locations, and updates those locations if they are not there. This can only fail when there are multiple entrance/exit buildings.

@Broxzier
Copy link
Member Author

Not true. When loading a save, the game checks if all entrances and exits are actually at the set locations, and updates those locations if they are not there. This can only fail when there are multiple entrance/exit buildings.

Most players will try to build a path from an entrance or exit right after placing it instead of reloading the saved game first. With this PR they are usable right away.

@Gymnasiast
Copy link
Member

Gymnasiast commented Jan 26, 2018

This does not update station_heights, which might cause problems with pathfinding. On the other hand, this value is shared by the entrance and exit, so it's hard to say whether it should update that value or not. Basically, any event where the entrance and exit are not at the some level will cause trouble.

@Broxzier
Copy link
Member Author

The exit needs to match the station height for a mechanic to find fix the ride, but guests don't care about the actual entrance height apparently. They're able to find the entrance when it's higher up.

@Gymnasiast
Copy link
Member

Hm, so perhaps a good idea to only update it when moving the exit?

@Broxzier
Copy link
Member Author

I thought about doing that, but then I want it to be a temporary fix, until the mechanic AI is improved. I'd like to get rid of all assumptions of the station height matching the entrance and exit heights. Something like this: https://gist.github.com/Broxzier/9fec1b15ddc23f59ddc24a1d69688b2d
But I'll have to look into this in more detail. I think that better fits in its own PR (which then should remove the temporary fix).

@Gymnasiast
Copy link
Member

I think that's a good idea, yes. Can you put in the temporary hack with a TODO note?

@Broxzier
Copy link
Member Author

Done so, and rebased. I also did a quick test to make sure it actually works.

Copy link
Member

@Gymnasiast Gymnasiast left a comment

Choose a reason for hiding this comment

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

I still don't like the unnecessary formatting, but I'm going to let that slide.

AFAIC, this can be merged after rebasing.

The original code assumes the entrance is at the exact same height as the
station, which is not always the case with hacked rides.
With this, entrances and exits that have been moved away from its original XY
location can be made usable. Only one entrance or exit will be usable per
station.
This changes all similar checks in the TileInspector file to use `ptr == nullptr` instead of `!ptr`
and applies the coding style.
@Broxzier Broxzier merged commit b682324 into OpenRCT2:develop Feb 6, 2018
@Broxzier Broxzier deleted the hacked-entrances branch February 25, 2018 11:30
janisozaur added a commit that referenced this pull request Mar 18, 2018
- Feature: [#2893] Object selection filters for items from RCT1, Added Attractions and Loopy Landscapes.
- Feature: [#3505] Allow up to 1024 items per scenery tab.
- Feature: [#3510] Auto-append extension if none is specified.
- Feature: [#3994] Show bottom toolbar with map tooltip (theme option).
- Feature: [#4184] Add command and cheat to alter the date.
- Feature: [#4906] Add follow sprite command to title sequences.
- Feature: [#4984] Add option to highlight path issues: full bins, vandalism & vomit.
- Feature: [#5826] Add the show_limits command to show map data counts and limits.
- Feature: [#6078] Game now converts mp.dat to SC21.SC4 (Mega Park) automatically.
- Feature: [#6125] Path can now be placed in park entrances.
- Feature: [#6181] Map generator now allows adjusting random terrain and tree placement in Simplex Noise tab.
- Feature: [#6235] Add drawing debug option for showing visuals when and where blocks of the screen are painted.
- Feature: [#6290] Arabic translation (experimental).
- Feature: [#6292] Allow building queue lines in the Scenario Editor.
- Feature: [#6295] TrueType fonts are now rendered with light font hinting by default.
- Feature: [#6307] Arrows are now shown when placing park entrances.
- Feature: [#6313] Add keyboard shortcut for toggle gridlines.
- Feature: [#6324] Add command to deselect unused objects from the object selection.
- Feature: [#6325] Allow using g1.dat from RCT Classic.
- Feature: [#6329] Render level crossings when the Miniature Railway crossed a path.
- Feature: [#6338] Virtual floor to help positioning objects vertically.
- Feature: [#6353] Show custom RCT1 scenarios in New Scenario window.
- Feature: [#6411] Add command to remove the park fence.
- Feature: [#6414] Raise maximum launch speed of the Corkscrew RC back to 96 km/h (for RCT1 parity).
- Feature: [#6433] Turn 'unlock all prices' into a regular (non-cheat, persistent) option.
- Feature: [#6516] Ability to search by filename in the object selection window.
- Feature: [#6530] Land rights tool no longer blocks when a tile is not for purchase.
- Feature: [#6568] Add smooth nearest neighbour scaling.
- Feature: [#6651, #6658] Integrate Discord Rich Presence.
- Feature: [#6709] The New Ride window now shows available vehicles for a ride type.
- Feature: [#6731] Object indexing progress is now reported via command line output.
- Feature: [#6779] On-ride photo segment for Splash Boats.
- Feature: [#6838] Ability to auto-pause server when no clients are connected.
- Feature: [#7031] Better support for displaced ride entrances and exits.
- Feature: Add search box to track design window.
- Feature: Allow using object files from RCT Classic.
- Feature: Title sequences now testable in-game.
- Feature: Vehicles with matching capabilities are now always switchable.
- Feature: Add search box to track design window.
- Feature: Add load scenario command to title sequences.
- Fix: [#816] In the map window, there are more peeps flickering than there are selected (original bug).
- Fix: [#996, #2589, #2875] Viewport scrolling no longer shakes or gets stuck.
- Fix: [#1185] Close button colour of prompt windows does not match.
- Fix: [#1833, #4937, #6138] 'Too low!' warning when building rides and shops on the lowest land level (original bug).
- Fix: [#2254] Edge scrolling horizontally now has the same speed as vertical edge scrolling.
- Fix: [#2607] Rain rendered incorrectly in additional viewport.
- Fix: [#3171] Guests entering from the corner of the tile in Amity Airfield (original bug).
- Fix: [#3330] Current number of passengers overflows when over 255 (original bug).
- Fix: [#4760] Asia - Great Wall of China and South America - Rio Carnival have incorrect guest entry points (original bug).
- Fix: [#4953, #6277] Unable to advertise to master servers over IPv6.
- Fix: [#4991] Inverted helices can be built on the Lay Down RC, but are not drawn.
- Fix: [#5190] Cannot build Wild Mouse - Flying Dutchman Gold Mine.
- Fix: [#5224] Multiplayer window is not closed when server shuts down.
- Fix: [#5228] Top toolbar disappears when opening SC4 file.
- Fix: [#5261] Deleting a banner sign after copy/pasting it will crash the game.
- Fix: [#5398] Attempting to place Mini Maze.TD4 results in weird behaviour and crashes.
- Fix: [#5417] Hacked Crooked House tracked rides do not dispatch vehicles.
- Fix: [#5445] Patrol area not imported from RCT1 saves and scenarios.
- Fix: [#5585] Inconsistent zooming with mouse wheel.
- Fix: [#5609] Vehicle switching may cause '0 cars per train' to be set.
- Fix: [#5636] Pausing the game shows mute button as active.
- Fix: [#5741] Land rights indicators disappear when switching views.
- Fix: [#5761] Mini coaster doesn't appear despite being selected.
- Fix: [#5788] Empty scenario names cause invisible entries in scenario list.
- Fix: [#5809] Support Steam RCT1 file layout when loading CSG images.
- Fix: [#5838] Crash when saving very large track designs.
- Fix: [#5901] Placing peep spawn not synced across multiplayer.
- Fix: [#6101] Rides remain in ride list window briefly after demolition.
- Fix: [#6114] Crash when using a non-LL CSG1.DAT.
- Fix: [#6115] Random title screen music not random on launch.
- Fix: [#6118, #6245, #6366] Tracked animated vehicles not animating.
- Fix: [#6129] Guest List summary not updating after a ride rename.
- Fix: [#6133] Construction rights not shown after selecting buy mode.
- Fix: [#6188] Viewports not being clipped properly when zoomed out in OpenGL mode.
- Fix: [#6193] All rings in Space Rings use the same secondary colour.
- Fix: [#6196, #6223] Guest's energy underflows and never decreases.
- Fix: [#6198] You cannot cancel RCT1 directory selection.
- Fix: [#6199] Inverted Hairpin Coaster vehicle tab is not centred.
- Fix: [#6202] Guests can break occupied benches (original bug).
- Fix: [#6251] Splash Boats renders flat-to-25-degree pieces in tunnels incorrectly.
- Fix: [#6261, #6344, #6520] Broken pathfinding after removing park entrances with the tile inspector.
- Fix: [#6271] Wrong booster speed tooltip text.
- Fix: [#6293] Restored interface sounds while gameplay is paused.
- Fix: [#6301] Track list freezes after deleting track in Track Manager.
- Fix: [#6308] Cannot create title sequence if title sequences folder does not exist.
- Fix: [#6314] Imported SV4 files do not mark their scenarios as completed.
- Fix: [#6318] Cannot sack staff that have not been placed.
- Fix: [#6320] Crash when CSS1.DAT is absent.
- Fix: [#6331] Scenery costs nothing in track designs.
- Fix: [#6358] HTTP requests can point to invalid URL string.
- Fix: [#6360] Off-by-one filenames when exporting all sprites.
- Fix: [#6388] Construction rights tool erroneously enabled in some RCT1 scenarios even when no rights are available.
- Fix: [#6413] Maze previews only showing scenery.
- Fix: [#6423] Importing parks containing names with Polish characters.
- Fix: [#6423] Polish characters now correctly drawn when using the sprite font.
- Fix: [#6445] Guests' favourite ride improperly set when importing from RCT1 or AA.
- Fix: [#6452] Scenario text cut off when switching between 32 and 64-bit builds.
- Fix: [#6460] Crash when reading corrupt object files.
- Fix: [#6481] Can't take screenshots of parks with colons in the name.
- Fix: [#6500] Failure to load resources when config file is missing.
- Fix: [#6547] The server log is not logged if the server name contains CJK.
- Fix: [#6593] Cannot hire entertainers when default scenery groups are not selected (original bug).
- Fix: [#6657] Guest list is missing tracking icon after reopening.
- Fix: [#6803] Symbolic links to directories are not descended by FileScanner.
- Fix: [#6830] Crash when using mountain tool due to ride with no ride entry.
- Fix: [#6833] Shops in corrupted files not imported correctly.
- Fix: [#6846] Zoom level in some ride overview windows was erroneously set too high.
- Fix: [#6904] Manually added multiplayer servers not saved.
- Fix: [#7003] Building sloped paths through flat paths with clearance checks off causes glitches.
- Fix: [#7011] Swinging and bobsleigh cars going backwards swing in the wrong direction (original bug).
- Fix: [#7042, #7077] Paths sometimes disconnect when building them with clearance checks off.
- Fix: [#7125] No entry signs not correctly handled in pathfinding.
- Fix: [#7223] Vehicle mass not correctly recalculated when using remove all guests cheat.
- Fix: [#7229] Exploding guests cheat causes rides to get stuck and freezes game.
- Fix: [#7295] peep_should_go_on_ride_again() checked balloon colour instead of toilet need.
- Fix: [#7301] Sprite compiler dithering checks transparency of wrong pixel.
- Fix: Infinite loop when removing scenery elements with >127 base height.
- Fix: Ghosting of transparent map elements when the viewport is moved in OpenGL mode.
- Fix: Clear IME buffer after committing composed text.
- Fix: RCT1 mazes with wooden fences not imported correctly.
- Fix: Title sequence editor now gracefully fails to preview a title sequence and lets the user know with an error message.
- Fix: When preset title sequence fails to load, the preset will forcibly be changed to the first sequence to successfully load.
- Fix: Remove consecutive thoughts about a ride being demolished.
- Fix: Water raft vehicles stop spinning when going up slopes.
- Fix: Incorrect spin is applied to coasters on S-bends and other turns.
- Improved: [#5962] Use AVX2 instruction set where supported, resulting in a performance boost.
- Improved: [#5964] Use SSE 4.1 instruction set where supported, resulting in a performance boost.
- Improved: [#6186] Transparent menu items now draw properly in OpenGL mode.
- Improved: [#6218] Speed up game start up time by saving scenario index to file.
- Improved: [#6242] Prevent scenery aging and grass growth causing tile invalidation unless necessary - slight performance boost.
- Improved: [#6423] Polish is now rendered using the sprite font, rather than TTF.
- Improved: [#6746] Draw friction wheels instead of chain lift on Looping Roller Coaster stations.
- Improved: Load/save window now refreshes list if native file dialog is closed/cancelled.
- Improved: Major translation updates for Japanese and Polish.
- Improved: Added 24x24, 48x48, and 96x96 icon resolutions.
- Technical: [#6384] On macOS, address NSFileHandlingPanel deprecation by using NSModalResponse instead.
- Technical: [#6772] RCT2 interop removed.
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