Hanabi-Live / hanabi-live

A web server that allows people to play Hanab, a cooperative card game of logic and reasoning.
https://hanab.live
GNU General Public License v3.0
179 stars 118 forks source link

[Bug] JSON Replay: including seed ignores passed deck #2966

Open IceSelkie opened 2 months ago

IceSelkie commented 2 months ago

Describe the bug When loading a replay from JSON, if a deck is specified AND a seed is specified, the deck is thrown away and the seed is used to generate a new deck.

To Reproduce

  1. Get a valid game JSON: a. Open any game replay b. Go to the last turn in the game c. Use /copy and click the copy link to get the game as JSON
  2. Start a replay with the generated JSON: a. From the lobby, click "Create Replay" b. Select "JSON Data" and paste in the copied JSON c. Click "Watch" to start the replay. It should be fine to the end of the game.
  3. Wrong deck: a. Again from lobby, click "Create Replay" b. Select "JSON Data" and paste in the copied JSON c. Modify the seed entry to have any string as its value (instead of empty string) d. Click "Watch" to start the replay. It should terminate after a few turns since the action specified in the JSON does not match a valid move with the calculated deck.

Expected behavior When there is a conflict between the seed and the passed deck, the passed deck should be assumed correct rather than the seed. (Such as when using Hanab.live to review a game from another site/program which doesn't use the same seed/prng/shuffle algorithm as Hanab.live, but deck is still accurate)

IceSelkie commented 2 months ago

Here is an example game that includes a seed string, and terminates at turn 2 (but is a win if the seed is removed or set to "").

EDIT: I uploaded wrong file. This one is correct: example_hansim_game.json

Zamiell commented 2 months ago

Counterproposal: It should just reject a JSON with both seed and a deck, since it means that the end-user has a corrupt JSON and/or is confused.

IceSelkie commented 2 months ago

Yes that is indeed a better solution. It solve the problem and doesn't assume that either should take precedence over the other.

Zamiell commented 2 months ago

can you submit a pr?

IceSelkie commented 2 months ago

Upon further testing, it appears that the seed typically takes precedence over the passed deck, but sometimes the deck is required to meet certain requirements but is then ignored.

Cases:

  1. {players:["a","b","c"], seed:"", actions:[play 1, ...], deck:[ ... correct 50 card deck ... ]} Input: deck and no seed Result: Replay starts normally, using passed deck. Expected.
  2. {players:["a","b","c"], seed:"p3v0s123", actions:[play 1, ...], deck:[ ... correct 50 card deck ... ]} Input: deck and seed Result: Replay starts normally. Expected.
  3. {players:["a","b","c"], seed:"p3v0s123", actions:[play 1, ...], deck:[ ... incorrect 50 card deck ... ]} Input: deck and seed, seed matches actions but deck does not Result: Replay starts normally, ignoring passed deck, using seed only. Unexpected. Passed deck can also be comprised entirely of blue 5s or any other card and still be fine.
  4. {players:["a","b","c"], seed:"incorrect-seed", actions:[play 1, ...], deck:[ ... 50 card deck matching actions ... ]} Input: deck and seed, deck matches actions but seed does not Result: Replay starts normally, ignoring passed deck, generating new deck from seed; The game is truncated at first move that doesn't match the generated deck. Unexpected.
  5. {players:["a","b","c"], seed:"p3v0s123", actions:[play 1, ...], deck:[]} Input: seed and empty deck Result: "[...] invalid target (card order) of 1." Unexpected: it has the seed and no deck.
  6. {players:["a","b","c"], seed:"p3v0s123", actions:[play 1, ...]} Input: seed and no deck Result: "[...] invalid target (card order) of 1." Unexpected: it has the seed and no deck.
  7. {players:["a","b","c"], seed:"p3v0s123", actions:[play 1, ...], deck:[ ... any deck with length not exactly 0 or 50 ... ]} Input: seed and partial deck, or seed and too large of deck Result: "The deck must have 50 cards in it." Unexpected: it ignores the deck when the seed is there in any other case...