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

Fix railway/path clipping when on same height #6329

Merged
merged 11 commits into from
Nov 16, 2017

Conversation

JeroenDStout
Copy link
Contributor

Paths and railways built through one-another with zero-clearance show flicking. Changing the bounding box height stabilises it.

flicker

This change pushes the bounding box of straight railway and car ride pieces up by 1. This consistently puts them above regular pathways.

The change also pushes peeps up by 5 (instead of their default 3) to prevent them from causing depth fighting again. Although I have not checked every single possibility, as far as I can tell peeps are never in a situation where they depth fight with something that is 2 above them. Litter is pushed up by 1 to appear on top of the tracks.

This is all in anticipation of the future waiting-for-crossing PR, which is why I only changed straight track pieces. I imagine once waiting for crossing is in, allowing track pieces to be built onto paths can be enabled even without zero clearance.

@IntelOrca
Copy link
Contributor

Hmm, I the thing is - you should just be able to have it draw based on map element order on the tile, if two things share the same boundary box. But @janisozaur and @marijnvdwerf might have a better idea if that is possible.

@JeroenDStout
Copy link
Contributor Author

It is true that some sort of meta-ordering would be better, but peeps and litter already use this shifted method to always appear above paths (and peeps above litter). The boundary box is also not identical for both, the path & track just share the same z.

@Gymnasiast
Copy link
Member

I'm not sure if car ride should get the same treatment. Otherwise, it looks good to me.

Work to be done is indeed allowing building railways through paths, and adding proper crossing sprites (even if it's just two rails without track bed, that would look a lot better).

@JeroenDStout
Copy link
Contributor Author

I added the car ride because it is the only other track I could think of that I could conceivably see having crossings.

Personally I thought of this PR as a fix for the z-fighting in existing cases (I see a lot of parks with zero-clearance crossings), and save the waiting / custom sprites / 'legal' building for a future PR.

@IntelOrca
Copy link
Contributor

I am not against this change, and it can be at least a temporary solution for now until we fix ordering based on tile stack.

@JeroenDStout
Copy link
Contributor Author

JeroenDStout commented Sep 30, 2017

track

This inevitably grew a little bit. Using custom sprites we could support drawing railways through any path. I have modified the paint_session struct and related code to be aware of same-height paths. This can be used in the railway drawing code to change the visuals. This same-height code might have a lot of other uses for when it is useful to know if things are intersecting.

previousHeight = height;
session->PathElementOnSameHeight = 0;
rct_map_element * ittElement = map_element;
while (!map_element_is_last_for_tile(ittElement++))
Copy link
Member

Choose a reason for hiding this comment

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

ittElement?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, that was a shorthand I did not mean to push. I gave the variable a proper name in the next push.

@JeroenDStout
Copy link
Contributor Author

JeroenDStout commented Sep 30, 2017

track

More subtle indent sprites. The missing corner from the SPR_MINIATURE_RAILWAY_QUARTER_TURN_3_TILES_SW_SE has also been added.

Between tiles currently a small bit of track is visible. This can still be fixed by creating bespoke 'track going underneath' sprites for the (relatively few) sprites where this visually is a problem.

@marijnvdwerf
Copy link
Contributor

Not a fan as this should increase the testpaint failure rate. It doesn't seem to do so, but that seems to be a bug.

@marijnvdwerf
Copy link
Contributor

Also, the indenting of most comment blocks doesn't seem to match the indentation level of the surrounding code. And I've seen some lines with only whitespace.

@JeroenDStout
Copy link
Contributor Author

But does this mean the code should be altered to prevent a trip-up for testpaint? Because in a sense it is correct this is not drawing identical to vanilla.

I shall change the indent style, but I believe the code style does not say anything about whitespace lines.

@@ -571,23 +572,42 @@ static const uint32 miniature_railway_track_pieces_diag_25_deg_up[4] = {
SPR_MINIATURE_RAILWAY_DIAG_25_DEG_UP_S_N,
};

static uint32 miniature_railway_track_to_grooved(uint32 imageID)
{
return imageID - SPR_MINIATURE_RAILWAY_FLAT_SW_NE + SPR_G2_MINIATURE_RAILWAY_GROOVED_BEGIN;
Copy link
Member

Choose a reason for hiding this comment

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

imageId

@Gymnasiast
Copy link
Member

Is this ready, as far as you're concerned? Code looks good, only one minor line note.

@JeroenDStout
Copy link
Contributor Author

I think I realised a problem with this code - train corners have no overlay grooved pieces and will not be visible when intersecting with roads.

Long term I would be interested in a solution where the railway track just has a toggle for alternative track where the train also runs visible lower, like this:

toggle

...except with an appropriate transition piece.

In the short term I think the solution is just to add grooved sprites for all railway tracks. It will take me some time to do but it would be a good instant improvement for any park where railways intersect paths. I suggest the PR will be ready when those sprites are implemented, too.

@Gymnasiast
Copy link
Member

How about only handling straight track pieces for now?

@JeroenDStout
Copy link
Contributor Author

I am tempted to say yes but unless the path selectively is bumped up a z-level, it will consistently clip above all turns; so the rails will be invisible (certainly not on top) when-ever a track intersects with a path. Implementing that selective bumping would be adding a lot of logic to the path paint function which I feel does not belong there.

(And vainly from a showman perspective, showing this feature first in action with a turn in the track will make a bigger impression than adding more later.)

@Gymnasiast
Copy link
Member

I'm not sure I understand the need for handling turns, actually. Almost all real-life crossings are straight. At some point, we have to limit the scope and come up with something that can be merged for everyone to enjoy. I think it's better to settle for something more limited that works now. You can always add more stuff later on.

@JeroenDStout
Copy link
Contributor Author

I get where you are coming from, though I don't see the hurry! I will add the required code for bumping the path up a level when appropriate (so it doesn't clip above all tracks) and fix the few pixels of track sticking through the path every tile.

The reason I am big on corners was because the 2nd reaction after "great" to this, which I've seen, is people wanting tramlines to run through their entire park. But I'll save it for a later PR.

@JeroenDStout
Copy link
Contributor Author

Now works without wrong pixels in straight pieces:

crossing

Selective code in path means turns still work as in vanilla:

crossing2

@JeroenDStout
Copy link
Contributor Author

Now drawing in a different way, with extra sprites.

overlay

Going onto a path has a special lead-in (so the effect is less sudden), and it works on a variety of paths.

@pizza2004
Copy link
Contributor

pizza2004 commented Oct 5, 2017

Is there possibly a way to make the rails work without having the black lines around them? I mean, like, while still being visually distinct from the path underneath. I just think it looks a tad odd how they transition from having the lines over the path to not having them on their regular track.

@Gymnasiast
Copy link
Member

@pizza2004 I preferred the darker lines, but then some didn't. I guess it's just not possible to come up with something that will satisfy everyone. At this point my priority is to have it available for everyone, and we can always revisit the sprites later.

@HaasJona
Copy link
Contributor

HaasJona commented Oct 5, 2017

If you look at how rail transitions work in real life, the gaps for the wheel to go in are only on the inside on the rail. On the outside, the ground is usually relatively flush with the rail. I would therefore vote for the darker spots to be only on the inside of the rail.

@JeroenDStout
Copy link
Contributor Author

JeroenDStout commented Oct 5, 2017

Upon reflection I agree with @HaasJona, I think the far-most outer shadow could be tweaked to be slightly larger on the inside than outside. Would you be willing to edit the sprites again, @oli414 ?

I think the shadow mask should be self-explanatory.

@oli414
Copy link
Contributor

oli414 commented Oct 6, 2017

@JeroenDStout For sure. I'm working this whole weekend so I won't have the time to do it but I can make them next week.

Mind adding some reference images to take a look at @HaasJone or @JeroenDStout

@HaasJona
Copy link
Contributor

HaasJona commented Oct 6, 2017

@oli414
Copy link
Contributor

oli414 commented Oct 6, 2017

I see what you're getting at. I quite like your draft already, I'll try a few different things in that same vein. But I do think it is more visually representative of the actual situation. Thanks.

@oli414
Copy link
Contributor

oli414 commented Oct 10, 2017

@JeroenDStout The transparency thing doesn't appear to be working for me when grabbing your fork (Looks like it reads random bytes for the masks). But I got new sprites ready that I just need to have a quick look at in-game.

@JeroenDStout
Copy link
Contributor Author

@oli414 Ah, I forgot this branch doesn't include the fixes I made to the sprite importer. If you pull from #6362 you can use the executable that generates for sprite importing.

But I do hope to soon finish up #6362 so that work-around won't be necessary.

@Gymnasiast
Copy link
Member

This will need to be tested well with some NE parks to ensure it doesn't mess up edge cases w/r/t path drawing.

@JeroenDStout
Copy link
Contributor Author

@Gymnasiast Yes. The situations in which the behaviour changes, right now, is when both a TRACK_ELEM_FLAT and a path element are visible; and share the same xyz. Turns of any kind should not change drawing behaviour.

So the question is, are there situations where a TRACK_ELEM_FLAT intersects a path while not a railway; and is the path drawing on top of the track "more wrong" than the glitchy undefined behaviour we have now?

@Gymnasiast
Copy link
Member

If it only changes behaviour in such cases, then it's good. I thought it would change behaviour in more cases.


SPR_G2_MINIATURE_RAILWAY_QUARTER_TURN_3_TILES_SW_SE_PART_3 = SPR_G2_BEGIN + 93,

SPR_G2_MINIATURE_RAILWAY_BEGIN = SPR_G2_BEGIN + 94,
Copy link
Member

Choose a reason for hiding this comment

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

Alignment looks a bit odd here. Same for the last entry.
Both forms of alignment are fine, but you must pick one for a block ;-)

@IntelOrca IntelOrca merged commit 271f153 into OpenRCT2:develop Nov 16, 2017
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.
@Gymnasiast Gymnasiast added paint-setup Related to how things get packaged before being drawn on screen. and removed testpaint change labels Jul 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
paint-setup Related to how things get packaged before being drawn on screen.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants