Diagonal brakes and block brakes

[spacek531]

Speedy block brakes is merged 🎉 and this is the follow-up PR - implementing diagonal brakes and block brakes.

For paint code reasons, track elements with multiple sequence blocks now have all their sequence blocks updated when modifying brakes or brake speed.

[spacek531]

The build should be in a playable state now. There are a few outstanding things to do like fixing looping and LIM sprites. Waiting on #19922 and potentially another PR I have yet to make before this can be merged.

[github-actions]

This pull request has been marked as stale and will be closed in 14 days if no action is taken. To keep it open, leave a comment or remove the stale-pr label. If you're awaiting feedback from a developer, please send us a reminder (either here or on Discord).

[karst]

Keep this open please.
Any progress on this?

[github-actions]

This pull request has been marked as stale and will be closed in 14 days if no action is taken. To keep it open, leave a comment or remove the stale-pr label. If you're awaiting feedback from a developer, please send us a reminder (either here or on Discord).

[SpartanFrederic104]

Keep this open please

[spacek531]

I have given X123M3-256 write access, he has the motivation to finish the PR in a timely manner.

[X123M3-256]

Does anyone know why the CI is failing?

[duncanspumpkin]

its a replay test failure so something has diverged from how it was before. Are we expecting existing logic to have changed at all?

[X123M3-256]

I don't think so but I didn't create the PR so I'm not sure exactly what was changed (@spacek531?)

[fidwell]

I see this error message a lot in the build output: Replay network version mismatch: '0.3.5.1-15', expected: '0.4.5-13'. So maybe it just needs rebased / main merged in, since the original PR is way behind the current replay.

[X123M3-256]

I see this error message a lot in the build output: Replay network version mismatch: '0.3.5.1-15', expected: '0.4.5-13'. So maybe it just needs rebased / main merged in, since the original PR is way behind the current replay.

I did rebase it, but I rebased it on top of my local develop which is a little out of date, so I can try rebasing again. The network version hasn't increased since I last pulled develop, though, it's still 0.4.5-12, so I think 0.4.5-13 is the correct network version because this PR requires it to be increased?

[X123M3-256]

I rebased it again on top of the latest develop, seems it's still failing.

[ZehMatt]

I see this error message a lot in the build output: Replay network version mismatch: '0.3.5.1-15', expected: '0.4.5-13'. So maybe it just needs rebased / main merged in, since the original PR is way behind the current replay.

This is just the network version the replays were recorded with, this has nothing to do with the actual replay failing. The current one fails because something has changed that influences the outcome of the simulation.

[X123M3-256]

What is a replay? What do you mean recorded? It builds and works on my machine, I find it odd that some of the Linux builds succeed and others are failing?

[spacek531]

its a replay test failure so something has diverged from how it was before. Are we expecting existing logic to have changed at all?

Off the top of my head, all tiles of multi-tile block brake tracks (diagonal chain up-25/60-to-flat, cable lift hill) will have their brake flag set instead of only tile 0. I don't remember if there are other changes.

[duncanspumpkin]

What is a replay? What do you mean recorded? It builds and works on my machine, I find it odd that some of the Linux builds succeed and others are failing?

We record a 1000 ticks of a bpb save file and record the exact properties of every single entity in a replay. If you make a change to the logic that changes how an entity performs anything (assuming that action happens in bpb during the 1000ticks) then it will show up has a replay failure. It takes a little bit of time to run these replays so we don't run it on every ci just Windows 32/64, Mac, Linux gcc/clang. The idea is that it will catch accidental changes to logic that we weren't expecting. If we have on purpose made a change then i just rerecord the replay. I can also get the replay to generate a difference report that will say exactly which entity fields have changed but its not very reliable doing that and it often goes wrong (and it can only tell you the difference after 1000 ticks but the divergence might have happened much earlier and the butterfly effect might mess up lots).

[X123M3-256]

I can also get the replay to generate a difference report that will say exactly which entity fields have changed

How do I do this? I'd quite like to get this PR finished off because it's been almost 2 years since it was first implemented, but I've not encountered this issue before.

[spacek531]

I can also get the replay to generate a difference report that will say exactly which entity fields have changed but its not very reliable doing that and it often goes wrong (and it can only tell you the difference after 1000 ticks but the divergence might have happened much earlier and the butterfly effect might mess up lots).

You are referring to the desync report, correct? When I used it previously it would identify the first frame(s) of the desync instead of the last.

My expectations for a good outcome on the desync report are that the desync consists of only TrackElement brake flags being set, and peep rand is the same on left and right.

[spacek531]

I just ran bpb1000 ticks against the branch and against OpenRCT2, v0.4.5-222-gccba517 (ccba517 on develop) provided by GitHub.

The desync report is as follows:

tick left = 00014BD7, tick right = 00014BD7
srand0 left = C5AB4C49, srand0 right = C5AB4C49
Sprite modifications (Misc: Money effect), index: 511
  MoneyEffect::OffsetX, len = 2, offset = 56, left = 0x000000000000FFEC, right = 0x000000000000FFED
Sprite modifications (Vehicle), index: 575
  Vehicle::BlockBrakeSpeed, len = 1, offset = 252, left = 0x0000000000000002, right = 0x0000000000000008
Sprite modifications (Misc: Money effect), index: 578
  MoneyEffect::OffsetX, len = 2, offset = 56, left = 0x000000000000FFEC, right = 0x000000000000FFED
Sprite modifications (Misc: Money effect), index: 583
  MoneyEffect::OffsetX, len = 2, offset = 56, left = 0x000000000000FFEC, right = 0x000000000000FFED
Sprite modifications (Vehicle), index: 743
  Vehicle::BlockBrakeSpeed, len = 1, offset = 252, left = 0x0000000000000002, right = 0x0000000000000008
Sprite modifications (Vehicle), index: 1687
  Vehicle::BlockBrakeSpeed, len = 1, offset = 252, left = 0x0000000000000002, right = 0x0000000000000008
Sprite modifications (Vehicle), index: 1861
  Vehicle::BlockBrakeSpeed, len = 1, offset = 252, left = 0x0000000000000002, right = 0x0000000000000008
Sprite modifications (Vehicle), index: 1918
  Vehicle::BlockBrakeSpeed, len = 1, offset = 252, left = 0x0000000000000002, right = 0x0000000000000008
Sprite modifications (Vehicle), index: 2011
  Vehicle::BlockBrakeSpeed, len = 1, offset = 252, left = 0x0000000000000002, right = 0x0000000000000008
Sprite modifications (Vehicle), index: 2212
  Vehicle::BlockBrakeSpeed, len = 1, offset = 252, left = 0x0000000000000002, right = 0x0000000000000008
Sprite modifications (Vehicle), index: 3042
  Vehicle::BlockBrakeSpeed, len = 1, offset = 252, left = 0x0000000000000002, right = 0x0000000000000008
Sprite modifications (Vehicle), index: 3416
  Vehicle::BlockBrakeSpeed, len = 1, offset = 252, left = 0x0000000000000002, right = 0x0000000000000008
Sprite modifications (Vehicle), index: 3908
  Vehicle::BlockBrakeSpeed, len = 1, offset = 252, left = 0x0000000000000002, right = 0x0000000000000008

Ignore the money desync, that occurs even on the prebuilt version.

The desync is telling me that block brake speed is getting loaded by the vehicle incorrectly. The affected field was added in speedy block brakes. If I knew which vehicles the entity indices referred to, I could better troubleshoot.

[karst]

it seems the include ../../paint/Boundbox.h is not located where it used to be.

[karst]

@duncanspumpkin Can you take a look at thisone one more time after all these small fixes?
I believe the only thing that has changed is the addition of a few more sprites + paintcode. @spacek531 can you verify?

[spacek531]

yes

[karst]

I have done one more playtest checking the functionality and it seems identical to its normal counter part. Very happy with this implementation, thx for the hard work :)

[Realsteel89]

A krimi this branch but clean work @spacek I will test it again tomorrow.

[spacek531]

I don't have any more planned changes for this branch. The ride types that do not have brakes will be covered in a folllow-up PR.

[duncanspumpkin]

Minor issue with park version but other than that good to go.

[karst]

Thisone doesn't line up with the json file. Switching them around here.