hanabi-live icon indicating copy to clipboard operation
hanabi-live copied to clipboard

Cluing morphed cards breaks /copy replay

Open argothiel opened this issue 2 years ago • 4 comments

Description:

  1. When morphing a card and then cluing it, the "/copy" button creates an invalid game.
  2. Also, "/copy" button doesn't consider morphed cards (probably related to #1997).
  3. Also, an invalid JSON breaks the GUI (the scroll button behaves weirdly).

Example steps to reproduce:

  1. Click "Watch Replay".
  2. Select "Database ID", ID: 701785, Visibility: Shared -> Watch.
  3. Select "Enter Hypothetical".
  4. Select "Edit Cards" -> morph blue 2 into green 3.
  5. Clue green.
  6. Run "/copy".
  7. Open the copied link.

Expected behavior: Valid game replay.

Current behavior: No game loaded, a scroll button when clicked moves to the top left corner of the screen.

2653

argothiel avatar Aug 01 '22 11:08 argothiel

To add. Currently /copy copies the actions from the hypo and the cards from the original deck. So, if the replay calls for actions on cards that are not there (anymore), the replay won't move forward past that point. E.g., in an actual game (without empty clues), you cannot clue red to someone, who's not holding a red card.

I believe the scroll button thing happens, because a JSON without actions is invalid. But you effectively created one, when the first action is impossible.

jack67889 avatar Aug 01 '22 17:08 jack67889

Ah, the similar thing happens when I manually import a JSON with a single invalid action. The check if any actions are present works fine, but it doesn't validate the JSON that the number of valid actions is greater than 0. So, these are two independent issues:

  • missing morphing functionality in hypo export,
  • insufficient JSON validation when importing.

JSON reproduction.txt

Also, I believe we should be allowed to import a JSON with no actions, just to show the beginning state of the game - so it'd be just a GUI issue in that case, with a running scroll button.

argothiel avatar Aug 04 '22 13:08 argothiel

Let me know if you want me to split these issues into separate ones.

argothiel avatar Aug 04 '22 13:08 argothiel

Also, I believe we should be allowed to import a JSON with no actions, just to show the beginning state of the game - so it'd be just a GUI issue in that case, with a running scroll button.

You can import only an end-of-game action (type 4, for example value 10).

jack67889 avatar Aug 04 '22 14:08 jack67889

I started checking this, and wonder what an expected outcome should be, It's easy to let the code include the morphed cards instead of the original one, but that has consequences, due to the fact that the card might already have positive clues on it.

  • Alice clue red to Bob
  • We morph Bobs red card into a blue card.
  • Bob does something unrelated
  • Alice clues blue to Bob
  • Now the card have two positive colors marked and empathy don't know what it shall do with the card.
  • We use /copy
  • The card is exported as a blue card (the morphed card)
  • We import and run the replay
  • In the replay Alice clues red to Bob on a blue card, and the replay fails.

The question is, what is expected behavior. Do we want to add the "morph" action as something that also happened in the replay in the middle of the game? Should we validate morphs and say "you can't morph into this card, you will create an invalid state". Should we only exported morphed cards that don't have positive clues on them, before the morph was done?

Krycke avatar Nov 28 '22 13:11 Krycke

I think warning about the invalid state of the game/of the game history when morphed would be okay.

argothiel avatar Nov 28 '22 23:11 argothiel

The original problem is fixed now. You can still create invalid states which will break the replay, that now a valid state should work. I think we can close this issue for now.

Krycke avatar Nov 30 '22 07:11 Krycke

k

Zamiell avatar Nov 30 '22 09:11 Zamiell