Refactor flat ride track pieces #14021
Conversation
Gymnasiast
force-pushed
the
refactor/flat-ride-pieces
branch
2 times, most recently
from
16bb9bf
to
81d18e0
Compare
|
Replays are only different due to the change in track id. All okay to rerecord. |
src/openrct2/rct2/S6Importer.cpp
Outdated
| if (src.proximity_track_type == TrackElemType::RotationControlToggleAlias | ||
| && !RCT2TrackTypeIsBooster(_s6.rides[src.current_ride].type, src.proximity_track_type)) | ||
| dst.ProximityTrackType = TrackElemType::RotationControlToggle; | ||
| dst.ProximityTrackType = RCT2TrackTypeToOpenRCT2(src.proximity_track_type, _s6.rides[src.current_ride].type); |
There was a problem hiding this comment.
tests are crashing on this if the current ride is null then this will throw. Also proximity_track_type could be 0xFF to imply null this is a subtle bug from #13977.
There was a problem hiding this comment.
Hm, but 255 was also the quarter loop of the multi-dim. What if the proximity_track_type was that piece?
There was a problem hiding this comment.
Yeah it's a bit of a mess. I think though you can get away with just checking for null rides and just pretend that the track null issue doesn't exist.
There was a problem hiding this comment.
Hm, before I saw this I already pushed a commit that assumed that 255 was null (considering the special checks in RideRating, that seems safest to me). Is it okay to keep that?
src/openrct2/rct2/S6Importer.cpp
Outdated
| @@ -803,7 +803,12 @@ class S6Importer final : public IParkImporter | |||
| dst.ProximityStart = { src.proximity_start_x, src.proximity_start_y, src.proximity_start_z }; | |||
| dst.CurrentRide = src.current_ride; | |||
| dst.State = src.state; | |||
| dst.ProximityTrackType = RCT2TrackTypeToOpenRCT2(src.proximity_track_type, _s6.rides[src.current_ride].type); | |||
| if (src.proximity_track_type == 0xFF) | |||
There was a problem hiding this comment.
I'm not sure this will work. I think the first thing to check would be if the current ride is invalid.
There was a problem hiding this comment.
Should I set it to null if src.proximity_track_type == 0xFF || src.current_ride >= RCT12_MAX_RIDES_IN_PARK?
There was a problem hiding this comment.
So I've looked into it and its impossible to get the information we need. We have lost too much information (really we never had the information) due to the change in track types! We are just going to have to do something hacky and modify the ride rating code to get out of this.
Change the if to the following:
if (src.current_ride < RCT12_MAX_RIDES_IN_PARK && _s6.rides[src.current_ride].type != RIDE_TYPE_NULL)
dst.ProximityTrackType = RCT2TrackTypeToOpenRCT2(src.proximity_track_type, _s6.rides[src.current_ride].type);
else
dst.ProximityTrackType = 0xFF;
We will then need to modify the ride rating checks L335 and L229 to add an additional hack:
// TODO: Hack to be removed with new save format trackType 0xFF should not be here
if (trackType == 0xFF || trackType == TrackElemType::None || ...
There was a problem hiding this comment.
I just thought of something: do we even need to import these at all? Can't we just reset the ratings calc data upon import?
The only downside I see is that it may take a bit longer for the data to come up if you save and reload during calculation, since it would have to start from scratch.
There was a problem hiding this comment.
Yes we do need it. Peep decisions are changed depending on rating. Changes to when the rating is available will break the simulation
Gymnasiast
force-pushed
the
refactor/flat-ride-pieces
branch
from
0c17e6f
to
8968dd2
Compare
Regression from OpenRCT2#14021
No description provided.