kestasjk / webDiplomacy

Play Diplomacy online
http://webdiplomacy.net/
GNU Affero General Public License v3.0
183 stars 114 forks source link

Fix last phase #816

Closed lightvector closed 2 years ago

lightvector commented 2 years ago

Ported from https://github.com/adamlerer/webDiplomacy/pull/138

This PR fixes the API for the last turn of finished games.

First bug: Webdip will duplicate the orders of the last phase of games that did not end by solo, creating an entire new set of phases (creating a whole new set of Diplomacy + Retreat + Build where the orders are the exact same as on the previous Diplomacy + Retreat + Build) . There is specifically code to do this, which makes me think it's deliberate and I have some suspicisons why, but anyways the API didn't handle it well.

Since the duplicated orders are actually in the database, we need a way to detect this and filter them out. We can't simply check whether the game was "Won" or not, because some games can be won by resignation/concession rather than by solo, and those games will have the duplication. We can't simply filter out the last turn unconditionally, because solo games don't have this duplication. We could try to count supply centers, but that's awkward. So I just added a heuristic check - if the game is over and the last two turn's orders look like duplicates, we just get rid of one of the copies.

Second bug: The API unconditionally avoided populating the units array for the last phase from the historical tables, expecting them to be populated by the slightly different database tables that were used during a live game. But if the game is finished, the live game database tables are empty, so we have to populate the last phase units array from the historical tables.

Third detail: I made the API append an extra dummy phase with the phase name "Finished" for finished games, with the final unit positions. This makes things a bit nicer on the API-caller side. (I suspect the reason why webdip duplicates the final phase orders was a hacky way of doing this for the old UI, except of course it leaves a permanent bit of ugliness in the database in that the final phase orders are now in there twice whereas all other orders are in there only once).

Lastly, I stripped out all of the draw phase idx hackery we had in the UI side. I think we don't need it any more.

Screenshots of second to last phase and last phase after this change:

image image

kestasjk commented 2 years ago

Good stuff thanks. @jmo1121109 do you know why the last phase/orders get duplicated? I don't recall the system doing that

kestasjk commented 2 years ago

Looks like it's working fine on test.webdiplomacy.net, are you happy to push it to production? @adamlerer @lightvector

lightvector commented 2 years ago

@kestasjk @jmo1121109 - If it helps, here is the code that seems to be duplicating the last phase: https://github.com/kestasjk/webDiplomacy/blob/master/gamemaster/game.php#L1256-L1262 https://github.com/kestasjk/webDiplomacy/blob/master/gamemaster/game.php#L1308-L1314

I'm not sure if these are the only places, but they are definitely two of the relevant spots. I'm inferring that the duplication is intentional from nothing here looking like a typo, the code is really SELECTing from wD_MovesArchive on ($this->turn-1), and inserting an exact copy of that data that it just selected right back into wD_MovesArchive for that turn +1. But although it looks intentional, of course I'm not 100% sure what the intention was. :)

lightvector commented 2 years ago

Looks like it's working fine on test.webdiplomacy.net, are you happy to push it to production? @adamlerer @lightvector

If it all seems to be working on test, then sure, I'm okay with it.