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 81062163: for (really) old games, station bus/truck station cache was not updated #9366

Merged
merged 1 commit into from Jun 13, 2021

Conversation

@TrueBrain
Copy link
Member

@TrueBrain TrueBrain commented Jun 13, 2021

Motivation / Problem

So I was doing the following:

For a huge set of savegames, ranging from 0.3 up til 1.11.2, do this:

  • Save the old savegame, call it "main".
  • Load the old savegame (call it "original"), run it for 256 ticks. Save the game, export as JSON.
  • Load the "main", run it for 256 ticks. Save the game, export as JSON.
  • Compare the JSON files. There should be no difference, in theory.

I did this to validate "Afterload", that we are doing the right thing in all cases.

Turns out .. 12 years ago we made a boo-boo, and we do not do the right thing in all cases :)

Description

In 8106216 we introduced a station cache, where we know the area the bus/truck-stations are within. This cache is generated in AfterloadStations. Unaware to the writer of that change, AfterloadStations is not -always- loaded when loading old games. Specifically, if the version is pre-27, it is not executed. So for those savegames, the cache value was never correctly calculated.

This means that in the above example, you see, sometimes even after a few ticks, that road vehicles start to make different decisions. One thinks he reached the station, the other thinks he did not.

As it turns out, AfterloadStations is called in many other places too, mostly when there is any change to the NewGRF configuration. So clearly it is fine to call this function unconditionally, and indeed, by removing the version-check, the whole test-set validates a lot better. (still some issues, but not for this PR).

Version 27 is the 0.4.X series, for context.

Limitations

  • Ran the full test-suite again, and this solved 90% of the issues it found. The remaining ones are not for this PR.

Checklist for review

Some things are not automated, and forgotten often. This list is a reminder for the reviewers.

  • The bug fix is important enough to be backported? (label: 'backport requested')
  • This PR affects the save game format? (label 'savegame upgrade')
  • This PR affects the GS/AI API? (label 'needs review: Script API')
    • ai_changelog.hpp, gs_changelog.hpp need updating.
    • The compatibility wrappers (compat_*.nut) need updating.
  • This PR affects the NewGRF API? (label 'needs review: NewGRF')
Copy link
Contributor

@rubidium42 rubidium42 left a comment

AfterLoadGame is also called for the really old (TTD/TTO) save games, so those get the same "fix" for free.

Loading

@TrueBrain TrueBrain merged commit 1e432fb into OpenTTD:master Jun 13, 2021
14 checks passed
Loading
@TrueBrain TrueBrain deleted the fix-station-cache-for-old-games branch Jun 13, 2021
@James103
Copy link
Contributor

@James103 James103 commented Jun 14, 2021

  • Save the game, export as JSON.

How exactly did you "export as JSON"?

Loading

@TrueBrain
Copy link
Member Author

@TrueBrain TrueBrain commented Jun 14, 2021

  • Save the game, export as JSON.

How exactly did you "export as JSON"?

There are other external tools that read our savegame format and can export it, but I am currently working on #9322 which makes our savegames self-descriptive. That together with https://github.com/TrueBrain/OpenTTD-savegame-reader allows me to (mostly) export a savegame as JSON. That was what I was referring to.

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants