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

Revert Crash caused by Religion Caching #11815

Merged
merged 1 commit into from
Jun 23, 2024

Conversation

tuvus
Copy link
Collaborator

@tuvus tuvus commented Jun 23, 2024

This PR reverts the Religion Caching commit which causes a bad crash on game load. See #11813.

Briefly looking into it, when the city-religion is initializing it is checking resources on all cities and not all cities have been initialized which causes a problem. Commenting out the two problem lines of code resulted in a different crash. Therefore the best option is to revert this for now.

Since this is a game-breaking level bug for any save with religion enabled I will be patching it immediately.

@tuvus
Copy link
Collaborator Author

tuvus commented Jun 23, 2024

Release Patch

@yairm210 yairm210 merged commit 5ab535b into yairm210:master Jun 23, 2024
3 checks passed
@tuvus
Copy link
Collaborator Author

tuvus commented Jun 23, 2024

Oops, I didn't realize the patch would merge before the checks passed.

@yairm210
Copy link
Owner

Thanks!!!

@SomeTroglodyte
Copy link
Collaborator

SomeTroglodyte commented Jun 23, 2024

Does this mean a unit test is lacking? Didn't we have one that just loads a game? In that case that game data is too simple.
I found the inverse - canSerializeGame - but wasn't there a big base64gzip const string value someplace in the tests?? Still looking. AH - has its own file. And - that game does indeed have Religion off.

Since we should expect that game to fail loading someday due to removal of backward compatibility stuff - and for that case I'd prefer to see it fail and have a test amendment commit in the same PR that removes the compat code - shall we simply duplicate the test and give the copy a fresh game with everything we can think of?

Let's collect which features (as in has distinct impact on a save json) a new "can load predefined game from string" test should not forget?

  • Smallest possible map size, maybe even below the normal limits
  • Religion
  • Escorting
  • Build road to
  • A war
  • 1 improvement queue

  • Some notification history (current test hasn't any)
  • Ongoing trades
  • Espionage

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

3 participants