Several mistakes with ending-year and extreme values #8512
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…00001 and beyond MAX_YEAR is set to 5000000, but having an ending-year set to the same meant you could bypass this, and play till the uint32 wrapped. The game can either show highscore or wrap year, not both. When you would do both, every year you get the highscore dialog. By changing the maximum value of ending-year to 4999999 we prevent this issue.
… you start in year zero Using zero as "never" value can have its drawbacks ;)
Despite what it looked like, you could never really change the ending-year (it was always reset to 2050 on start-up). See commit 683b65e for details. As a side-effect, the variable that was suppose to store the ending-year was just zero, never containing a real ending-year.
TrueBrain
force-pushed
the
end-year-mixup
branch
from
6e5f665
to
007964a
Compare
LordAro
approved these changes
Jan 7, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #8050.
Motivation / Problem
While 2fd871e tried to re-add a removed feature, it was not sufficiently tested in edge-cases, causing some weird problems if people tried to push their luck:
-1MAX_YEARmade sure you could play on for-ever (well, till the uint32 wraps)Neverand starting in year 0 caused a nice "highscore" screen to be shown at the end of year 0Description
Most of these problems were created because the code originally showed the high-score when you started the ending-year (2051); the new code makes this when you end the ending-year (2050), which, from a user perspective, makes much more sense. Sadly, this means there needs to be done
+ 1and- 1in some places, causing these weird quirks.For the ending-year is
MAX_YEARproblem, I decided to simply not allow ending-year to becomeMAX_YEAR, but at mostMAX_YEAR - 1. Alternatively, we could record a variable that states if highscore has been done. But this would have to be stored in the savegame too. And as this is such an edge-case, that is troubling 99.999999% of our players with 4 more bytes in the savegame, for having a silly number line up. Alternatively, I considered bumpingMAX_YEARby 1 year, realising it was equally silly.Regarding the pre 0.7 savegames, this was simply misunderstood in 2fd871e. The reason 683b65e removes ending-year, is because it was impossible to configure it to anything else than 2051. When opening the game, the ending-year was always reset to this value. Basically, it was a fixed value. In the savegames, it always stored 0.
My suspicion is that the code was written, but never really tested. At least, I do not see a way this could ever work, or someone happened to have a savegame where ending-year was written to the savegame (but I do not see how). I used the title-screen of pre 0.7 as example, where the value is 0.
Edit: seems I am not completely correct; there are even older savegames that did have this variable written correctly, as in, pre-0.5. It is difficult to confirm exactly when it broke. Either way, the code, from the moment of introduction of ending-year, was always resetting the setting to 2051 every time you started the game; I believe this was because the newspaper always showed 2050, and we didn't know how to fix this :D. Either way, I would consider it highly unlikely anyone managed to get a savegame with any other value anyway. Given this is also over 10 years ago, I still think this solution is more robust.
Limitations
Checklist for review
Some things are not automated, and forgotten often. This list is a reminder for the reviewers.