magefree / mage

Magic Another Game Engine
http://xmage.today
MIT License
1.85k stars 761 forks source link

The state of the random number generator used for shuffle effects is not rolled back when a rollback happens, which can be exploited to manipulate draws. #8058

Open T-a-n-d-E-m opened 3 years ago

T-a-n-d-E-m commented 3 years ago

This bug effects all players who rollback after a shuffle effect to a state before the shuffle.

For example, a cheating player could:

  1. Shuffle their library with a fetch land, spell etc.
  2. Draw cards with their draw step, spell etc.
  3. If the drawn cards are not desirable rollback to before the shuffle effect and shuffle again. The ordering of their library will be different from the previous shuffle as the state of the random number generator used to shuffle wasn't rolled back to it's previous state. Repeat until the desired cards are drawn.

Another less malicious example:

  1. You crack a fetch land in your first main step (triggering a shuffle), cast a draw spell, and find a game winning bomb you can cast next turn.
  2. During combat your opponent accidentally skips their chance to use a combat trick and being a good sport you accept their rollback request.
  3. You replay your first main as before (cracking the land and casting the draw spell) but this time your card draw spell has drawn something completely different!
JayDi85 commented 3 years ago

That's correct behaviour. Different result from every shuffle. Like real life. Just restrict the other player from rollback.

T-a-n-d-E-m commented 3 years ago

That's correct behaviour. Different result from every shuffle. Like real life. Just restrict the other player from rollback.

I disagree that this is, or should be, correct behaviour. A rollback should revert the state of the game to precisely the same state it was in at the time so the same plays result in exactly the same outcomes. This isn't paper Magic where (as far as I know!) rollbacks don't even exist.

Shuffle->rollback->shuffle is not two different shuffles - its the same shuffle and should result in the exact same library.

JayDi85 commented 3 years ago

Same results after rollback is cheating -- you can look ahead and find out the result.

BTW, xMage engine supports same random seed, but it uses for unit tests only.

sprangg commented 3 years ago

Afaik the point of a rollback is this:

  1. Someone makes a mistake (misclick etc) and requests a rollback.
  2. Rollback is accepted. The player who made the mistake, and everyone else, plays the turn exactly like they did before, until before that mistake happened.
  3. The player who made the mistake corrects the mistake.

In this scenario, it's a good thing that the shuffle result is the same before and after rollback, because the whole point is to play the turn the same way.

Rollback is only cheating if the players play the turn differently until the moment the game was rolled back, right? But that's not the purpose of rollback anyway.

As a concrete example: Let's say someone uses the card [[Ponder]] at the start of their turn, doesn't like the three cards on top, and shuffles. They draw a removal spell from Ponder and play a land, but that land is accidentally wrong color which causes them to be unable to cast the removal spell.

That player asks for a rollback to be able to play the correct land and the rollback is accepted. But now, when they shuffle their deck with Ponder, they draw a different card and not the removal spell!

This above example would be wrong behavior in my opinion, because the goal of the rollback would not be met.

github-actions[bot] commented 3 years ago

Ponder - (Gatherer) (Scryfall) (EDHREC)

{U} Sorcery Look at the top three cards of your library, then put them back in any order. You may shuffle. Draw a card.

duelcastermage commented 3 years ago

I agree with @T-and-Em and @sprangg - having a different shuffle on rollback is both more exploitable from a cheater's perspective and also results in an incorrect game state sometimes even if both players have good intentions and just need to rollback a misclick.

JayDi85 commented 2 months ago

It’s about additional rollback points in time (not only start of turn). Random seed will never be restored.

JayDi85 commented 2 months ago

@xenohedron why you re-opened it?

ssk97 commented 2 months ago

It’s about additional rollback points in time (not only start of turn). Random seed will never be restored.

Why could it not be restored? I agree with the others above stating that it would be more helpful if the random seed was also restored. It would be a good bit of work, requiring creating a separate RNG for each game and updating all related random calls, but that seems plausible to do.

JayDi85 commented 2 months ago

Why could it not be restored?

Cause it useless. Instead return to exact point it time — you force the players to do useless actions, hoping for their honesty and memory.

ssk97 commented 2 months ago

Why could it not be restored?

Cause it useless. Instead return to exact point it time — you force the players to do useless actions, hoping for their honesty and memory.

That seems much, much more difficult to implement. Most cases could be covered by "last spell cast", "last ability activated", and "last step/phase change", but not all. To go to any arbitrary (recent) point in time would be quite the UI challenge and require a lot more saved states which takes compute time to copy the state and memory to store them.

JayDi85 commented 2 months ago

There are priority time by mtg rules (after each action). It’s safe to save or restore. Current rollback system already use it.

By UX it’s only require to assign an additional info for each saved state for better choosing by users (“actions” that will be rollbacked). It’s technically already possible to implement (logs, game cycle to compare, limit by max amount of saved points, etc — maybe logs history required but it will be added anyway for logs restore on disconnection). That’s the real fix of the starting problem, not the users who trying to manually restore something with random seeds or users who trying to use cheats to directly change game state (from https://github.com/magefree/mage/issues/12505#issuecomment-2197854005).