[PATCH] Clean up endGameManager code

[sanderd17]

The EndGameManager has some dirty code that gets executed once a second via timers.

Instead, I update it every turn, but with cached values to save work. And only check the entire state if something changed to the cache (which doesn't happen often).

I tried to not change any functionality, but it should be tested if it still works in all cases (diplomacy, saved games, resign ...)

[sanderd17]

Hmm, the current patch seems to break saved games because of the cmpPlayer instances it saves. Instead, it should just save the player IDs.

[sanderd17]

Alternative version of the patch, it's a lot cleaner IMO.

[leper]

I'm quite sure that line 70 (after the patch) shouldn't be return;. Probably the else clause shouldn't be there at all.

Line 76 could be inverted to an if (...) return;.

[sanderd17]

You're right about line 76.

About line 70, I do think there needs to be a return. The point of that code is to see if you need to mark players as "won". So you store all allies. The moment you have more than solely allies, you don't need to mark a player as "won", so you can simply return.

There is a problem in that loop though, the allies array will contain duplicates of the player ids, they should only appear once. So it should break earlier and not continue the entire for loop.

[leper]

What about making the for loops (line 58-74) into two non-nested for loops? That would make the logic easier to understand and be a lot less convoluted.

[sanderd17]

leper, better?

[sanderd17]

In 14087:

Clean up EndGameManager. Fixes #2130