Skip to content

Fix #18151: Park value and company value are out of date after loading a save - #27065

Open
sjoerddebruin wants to merge 1 commit into
OpenRCT2:developfrom
sjoerddebruin:sjoerddebruin/fix/18151-company-value-on-completion
Open

Fix #18151: Park value and company value are out of date after loading a save#27065
sjoerddebruin wants to merge 1 commit into
OpenRCT2:developfrom
sjoerddebruin:sjoerddebruin/fix/18151-company-value-on-completion

Conversation

@sjoerddebruin

@sjoerddebruin sjoerddebruin commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Description of changes

Fixes #18151. Recalculate the park value and the company value when a saved game is loaded, instead of using whatever the file stored.

Rationale behind changes

Both are only recalculated every 512 ticks. ScenarioReset refreshes them when a scenario begins, but loading a saved game never goes through it, so the park keeps the figures that came out of the file until the first periodic tick. For an RCT1 save that park value used the RCT1 formula, and a scenario completing in those few seconds records it as the score.

The park rating is left alone: RCT1 imports it as-is, and calculateGuestGenerationProbability starts from it and is compared against ScenarioRand, so refreshing it on load would change guest generation in every existing save.

Suggested testing steps

Load an RCT1 save and compare the company value in the finances window the moment it opens with the one it settles on seconds later. They should now match.

The save in #18151 no longer wins on its own, since Windstorm dropped just under the required 7.00 excitement when #18146 gave the Classic Wooden Roller Coaster its own ratings. Raising that locally makes it win on load again, and the score goes from roughly €41,500 to €386,626.

Did you use AI to help find, test, or implement this issue or feature?

Yes. Claude Opus 5 (high reasoning effort) assisted with investigating the reported issue, analysing the code, and drafting the accompanying text. Suggestions were not taken as-is: what to change, and how, was settled over several rounds of back and forth, and based on existing patterns in the code base plus my own knowledge. All changes have been verified by me in-game or with the project's own tooling, and I take responsibility for the diff.

@Gymnasiast Gymnasiast added this to the v0.5.6 milestone Sep 5, 2026
@sjoerddebruin
sjoerddebruin force-pushed the sjoerddebruin/fix/18151-company-value-on-completion branch from 6b64b6c to 5b9cb63 Compare September 6, 2026 17:59
@tupaschoal

Copy link
Copy Markdown
Member

Is company value not stored in the save file or does it get overwritten upon load?

@sjoerddebruin

Copy link
Copy Markdown
Contributor Author

Is company value not stored in the save file or does it get overwritten upon load?

See my comment here: #18151 (comment)

@tupaschoal

Copy link
Copy Markdown
Member

I see. I'm fine with scenarioSuccess calculating things that might be outdated due to wait for ticks/period such that completion has it right, nevertheless I think game init should make sure all periodic stats are properly initialized as to not depend on this

@sjoerddebruin
sjoerddebruin force-pushed the sjoerddebruin/fix/18151-company-value-on-completion branch from 5b9cb63 to 7bce843 Compare September 6, 2026 20:26
@sjoerddebruin sjoerddebruin changed the title Fix #18151: Company value is not up to date when a scenario is completed shortly after loading a save Fix #18151: Park value and company value are out of date after loading a save Sep 6, 2026
@sjoerddebruin

Copy link
Copy Markdown
Contributor Author

I see. I'm fine with scenarioSuccess calculating things that might be outdated due to wait for ticks/period such that completion has it right, nevertheless I think game init should make sure all periodic stats are properly initialized as to not depend on this

I've amended the commit and update the description with a new setup, does this address this?

@tupaschoal tupaschoal 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.

This is more in line with what I was thinking. If the reasoning for updating every 13s is right, parks can still finish with an outdated valuation, although I understand it will be less outdated now

@@ -87,8 +87,7 @@ void ScenarioReset(GameState_t& gameState)

auto& park = gameState.park;
park.rating = Park::CalculateParkRating(park, gameState);

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.

Is the rating not affected?

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.

See the PR description, this would otherwise influence ScenarioRand right?

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 know, but if you concluded that, probably worth leaving a comment to warn people to leave it alone

Comment thread src/openrct2/Context.cpp
Comment on lines +814 to +817

// Park::Update only recalculates these every ~13 seconds, so a save can hold a value that
// predates this park being loaded. Scenarios get the same treatment from ScenarioReset.
Park::updateValuations(gameState.park, gameState);

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 would have put this inside gameLoadInit, any reason why you didn't do it?

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.

I had a feeling it might cause desyncs in that case as the server and client would disagree? Would be a true edge-case though.

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.

Hmm, I don't wanddr enough into multiplayer to answer confidently. This is called just after init, wouldn't it happen anyway?

Comment thread src/openrct2/world/Park.h
int32_t CalculateParkRating(const ParkData& park, const GameState_t& gameState);
money64 CalculateParkValue(const ParkData& park, const GameState_t& gameState);
money64 CalculateCompanyValue(const ParkData& park);
void updateValuations(ParkData& park, const GameState_t& gameState);

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.

Suggested change
void updateValuations(ParkData& park, const GameState_t& gameState);
void UpdateValuations(ParkData& park, const GameState_t& gameState);

To follow naming convention of the file

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.

I was told before that new additions should follow the "new" naming conventions, no matter what pre-existed in the file. Is that not correct?

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.

You're probably right. I didn't keep up with all the renames, if this is the current style, feel free to mark the issue as resolved.

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.

Company value may not update before completing scenario loaded from RCT1 save

3 participants