Fix multiplayer turn checker potentially stopping turn checks for everyone that plays that game if a file was not found remotely #6901
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
…ryone that plays that game if a file was not found remotely
yairm210
approved these changes
May 22, 2022
GGGuenni
approved these changes
May 22, 2022
There was a problem hiding this comment.
The original idea of the GameInfoPreview variable was to use it as a toggle inside the multiplayer menu game settings to be able to disable turn checks per game. Since it never saw the light of day and I can't find the feature request anyway, it's better to clean it up. 👍
Azzurite
added a commit
to Azzurite-ServerAccount/Unciv
that referenced
this pull request
May 27, 2022
The fix in yairm210#6901 caused `arrayIndex` to go out of sync, because the `continue` happened before `arrayIndex` was incremented. This caused a later game preview to be saved to the previous game name, overwriting and duplicating it.
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.
This is a commit I already had on my feature branch but that I should PR separately.
Basically, with #6826 (so this problem is not released yet) I removed all usages of
updateCurrentTurn.updateCurrentTurnwas only used because the turn checker setGameInfoPreview.turnNotificationtofalseas a marker that this game should not be checked anymore because it was not found on the multiplayer server (dropbox).However, this likely happened because the user messed with the multiplayer server settings and set it to a wrong server. So when they fix those settings,
turnNotification=falsepotentially gets uploaded to the server if the game was up to date already and it was their turn, thus resulting in theturnNotificationbeingfalsefor everyone playing that game and no one getting turn notifications.The original fix for this behavior was using
updateCurrentTurninstead of simply replacing theGameInfoPreview(verified with the original author). I think using a simple field in the turn checker for this is a better solution, less code and no pollution of theGameInfoPreviewwith "useless" information. I just forgot to include this in the other PR where I removedupdateCurrentTurn.