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: Incorrect save/load array size of Town::cargo_accepted #8157

Merged
merged 2 commits into from Jun 28, 2020

Conversation

JGRennison
Copy link
Contributor

@JGRennison JGRennison commented May 17, 2020

In 11ab3c4 the number of cargo types was changed from 32 to 64.
The save/load of Town::cargo_accepted was not updated, such that
only half of the data structure is saved/loaded in savegame versions
199 to 218.
Discard and regenerate data from all savegame versions prior to 219.

@JGRennison JGRennison force-pushed the town-cargo-acceptance-savegame-fix branch from d61ee9e to 5cb9780 Compare May 17, 2020 21:44
In 11ab3c4 the number of cargo types was changed from 32 to 64.
The save/load of Town::cargo_accepted was not updated, such that
only half of the data structure is saved/loaded in savegame versions
199 to 218.
Discard and regenerate data from all savegame versions prior to 219.
@LordAro
Copy link
Member

LordAro commented May 17, 2020

Save game bump means it can't be backported, which is a shame. But if it's been broken since 1.9 anyway, I guess it's not a huge rush...

@ldpl
Copy link
Contributor

ldpl commented May 17, 2020

IMO Town::cargo_accepted should be just removed completely. It's not worth to waste so many resources and effort just to generate some subsidies.

@glx22
Copy link
Contributor

glx22 commented May 17, 2020

I think it's possible to backport the discard and regenerate part without the bump, and still save wrong data in 1.10 branch. While master will save correctly and with the bump.

@glx22
Copy link
Contributor

glx22 commented May 18, 2020

I have a simplified version of this PR. It should have the same effect while being easier to backport.

@JGRennison
Copy link
Contributor Author

IMO Town::cargo_accepted should be just removed completely. It's not worth to waste so many resources and effort just to generate some subsidies.

Having thought about more it I'm inclined to agree with you, though presumably this wouldn't be backportable to 1.10. Town::cargo_produced should also be binned for much the same reason.

@LordAro
Copy link
Member

LordAro commented May 18, 2020

I'm not opposed to having a "1.10-only commit", though it would be cleaner (easier to track, etc) to have a backportable fix first, then rewrite it to do it "properly"

@JGRennison
Copy link
Contributor Author

I think it's possible to backport the discard and regenerate part without the bump, and still save wrong data in 1.10 branch. While master will save correctly and with the bump.

The problem with this is that network clients would also regenerate it. Having stuff refreshed both monthly and on load is a bit of an anti-pattern which creates opportunities for desyncs.
Making it absolutely watertight so that there could be no desyncs would be non-trivial.

@nielsmh nielsmh merged commit 7a09413 into OpenTTD:master Jun 28, 2020
ldpl added a commit to ldpl/OpenTTD that referenced this pull request Jun 28, 2020
ldpl added a commit to ldpl/OpenTTD that referenced this pull request Jun 28, 2020
nielsmh pushed a commit that referenced this pull request Jun 28, 2020
@JGRennison JGRennison deleted the town-cargo-acceptance-savegame-fix branch January 9, 2024 19:17
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.

None yet

5 participants