Skip to content

Add support for setting a block brake speed. #7737

Closed
blackhand1001 wants to merge 1 commit into
OpenRCT2:developfrom
blackhand1001:patch-7
Closed

Add support for setting a block brake speed. #7737
blackhand1001 wants to merge 1 commit into
OpenRCT2:developfrom
blackhand1001:patch-7

Conversation

@blackhand1001

@blackhand1001 blackhand1001 commented Jun 26, 2018

Copy link
Copy Markdown
Contributor

Add support for setting a block brake speed rather than a hardcoded speed. It uses the existing brake/boost speed variable in the save format so saves are compatible with stock RCT2 and older versions of openrct2.

Comment thread src/openrct2-ui/windows/RideConstruction.cpp Outdated
@Broxzier Broxzier added the testing required PR needs to be tested before it is merged. label Jun 26, 2018
Comment thread src/openrct2/ride/Vehicle.cpp Outdated
{
// If the vehicle is below the speed limit
if (vehicle->velocity <= 0x20364)
if (vehicle->velocity <= speed)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think you needed to modify this block. If the velocity is between the minimum block brake speed and the set one, nothing needs to happen.

@blackhand1001 blackhand1001 Jun 27, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Its needed because it needs to go to the else if on line 6628 if the speed is above the set block brake speed. If this line was left unchanged it would always slow the train down to the fixed block release speed.

Comment thread src/openrct2/ride/Vehicle.cpp Outdated
Comment thread src/openrct2/ride/Vehicle.cpp Outdated
@wolfreak99

Copy link
Copy Markdown
Contributor

To preview what this has changed so far, here's a short recording of 2 block breaks with some regular breaks before (first pair including block breaks is set to 27mph, second pair is 13mph)

https://gfycat.com/AstonishingMatureClingfish

@manuelVo manuelVo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think when building a block brake the default speed shouldn't be 28km/h (as is the default for regular brakes) but instead it should be the speed that was hard coded before.

I also noticed that when loading an old save when editing a block brake the brake speed is displayed as 0km/h. This could be rather confusing to the user and I think the value of BLOCK_BRAKE_BASE_SPEED should be displayed instead.

The list of available block brake speeds should imo also be somewhat more limited. There is no way a block brake that breaks a train down to 50km/h if the block is free (which means the train arrives faster than that) is able to bring that car to a stop if the block isn't free. (This probably this is one of the reasons why this was hard coded in the first place)

There also is an issue with the rating of a track, since now the brake speed and the release speed are different. This brings up the question which of the two speeds will be used to rate the coaster (and probably how that will be calculated as well).

if (speed < BLOCK_BRAKE_BASE_SPEED)
{
//Fix blocks that were saved before block section speed settings were implemented.
speed = BLOCK_BRAKE_BASE_SPEED;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think if this would better be done when the map is loaded. This would then also fix the issue of displaying 0km/h when loading an old map.

@blackhand1001 blackhand1001 Jun 29, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

When the block is closed it behaves exactly the same as stock. There is no situation where the train could pass now that it couldn't already do in stock rct2.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ratings are calculated based on the first test run. Subsequent runs do not affect the rating. This is the case currently as well.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added a fix for the 0 speed issue. it will now display 4mph or 6km/h like it should.

@manuelVo

Copy link
Copy Markdown
Contributor

To clarify the rating issue: The way it was before the train left the block brake always with 6km/h. Either it was allowed to drive through in which case it was braked down to 6km/h (or accelerated to that speed if being slower) or the train was stopped completely. After the next block was free the train would be accelerated to 6km/h. That way the speed would always be 6km/h after the break and the stats would turn out the same, independently of whether the train had to stop during it's first run or not.

The way the patch is right now the train can leave the block brake at different speeds. The set target speed (let's say it's 24km/h) when the train is allowed to drive through or 6km/h if it had to stop to wait for the block to clear. The speed at which the train enters the block will change the resulting forces in the following block. Thus the stats of the coaster will vary depending on whether the train had to stop before entering the block on it's first run or not.

I think the best (and easiest) way to fix this is to make the block brakes accelerate the train up to the specified target speed when releasing it.

@blackhand1001

Copy link
Copy Markdown
Contributor Author

The reason it doesn't speed it up is because that is not how block brakes work in real life. Rides must be designed that they can make it through the course even after an emergency stop on the block. Also you should design your ride so that it shouldn't need to stop on any of the blocks in main part of your course under normal circumstances anyway.

If you want it to speed up on release just add boosters in your brake run right before the block. or after.

@manuelVo

Copy link
Copy Markdown
Contributor

The point I'm trying to make is from a game design perspective. Having the stats of a ride change depending on whether the train had to stop during it's first run or not can be frustrating to the player, as it might require them to close and re-open the ride multiple times just to get the right set of stats.

For example I tend to do the first tests of my coaster with the number of trains maxed out, so that each train has to stop at each block brake. This way I can verify the trains are able to make it through each block even though they have been stopped before. Afterwards I reduce the number of trains to the number I actually want to use. With the patch as it is right now if I were to use higher brake speeds the stats of that first ride where the train stops at each brake would make up the final result for my coaster. To get the actual stats (where the train doesn't need to stop until it arrives at the station) I'd have to close the coaster, edit it (without making any changes) to reset the stats and re-test it with fewer trains.

@Gymnasiast

Copy link
Copy Markdown
Member

I think the best (and easiest) way to fix this is to make the block brakes accelerate the train up to the specified target speed when releasing it.

That's utterly unrealistic. Also, having to stop for a block brake did affect stuff - it would cut out when the train reached 0 km/h, but that still meant it would differ from just passing through at 6 km/h.

@blackhand1001

blackhand1001 commented Jul 3, 2018

Copy link
Copy Markdown
Contributor Author

So there is a small issue with loading td6 files saved before this change. Some of them load with the speed set to 18mph for block sections. Others load at 4mph like they should. Strangely though saved games all load properly. Is there some kind of flag we can set thats not used in td6 to mark that this file supports block section speeds. I already know which block of code we can perform this check in.

@blackhand1001

Copy link
Copy Markdown
Contributor Author

@Gymnasiast @Broxzier Any ideas on using one of the unused flags to mark a track design as supporting block speeds?

@AaronVanGeffen AaronVanGeffen added the pending rebase PR needs to be rebased. label Aug 12, 2018
@blackhand1001

Copy link
Copy Markdown
Contributor Author

I will be taking a look at this in the near future again.

Comment thread src/openrct2/Cheats.cpp Outdated
@Gymnasiast

Copy link
Copy Markdown
Member

I want to postpone this until after we switched to the new save format. It adds yet another incompatibility with RCT2, and quite a heavy one at that.

@Gymnasiast Gymnasiast added requires new save format Cannot be done until the new save format is deployed. and removed requires new save format Cannot be done until the new save format is deployed. labels Oct 17, 2018
@Broxzier Broxzier added on hold A PR that should not be merged as is, waiting for a prerequisite. and removed pending improvement pending rebase PR needs to be rebased. testing required PR needs to be tested before it is merged. labels Nov 19, 2018
@AaronVanGeffen AaronVanGeffen mentioned this pull request Jan 7, 2019
@Gymnasiast Gymnasiast self-assigned this Feb 26, 2019

@Gymnasiast Gymnasiast left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Don't merge this yet. I still need to have a good think about how it works and I also want to postpone it until after the new save format.

@Generalcamo

Copy link
Copy Markdown
Contributor

I should note that the method proposed by @manuelVo is what RCT3 does to prevent the rating issue.

@Ruedii

Ruedii commented Dec 9, 2019

Copy link
Copy Markdown

It should be OK to merge this so long as the variable space is available in the current save file format and a file loaded with whatever is put there by default will have it's default changed to whatever the default is from RCT2.

@duncanspumpkin duncanspumpkin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just to also add this would break network games along with all the save issues

@tupaschoal tupaschoal added network version Network version needs updating - double check before merging! requires new save format Cannot be done until the new save format is deployed. labels Mar 22, 2020
@Gymnasiast

Copy link
Copy Markdown
Member

Since this PR was abandoned and not fully up to scratch, I'm closing it. I have created #11649 to track the feature, and I have backported the extraction of the constant to #11648.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

network version Network version needs updating - double check before merging! on hold A PR that should not be merged as is, waiting for a prerequisite. requires new save format Cannot be done until the new save format is deployed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.