Skip to content

Clean up the ability when a cast is rolled back via snapshot - #11418

Open
cuinhellcat wants to merge 1 commit into
Card-Forge:masterfrom
cuinhellcat:rollback-ability-cleanup
Open

Clean up the ability when a cast is rolled back via snapshot#11418
cuinhellcat wants to merge 1 commit into
Card-Forge:masterfrom
cuinhellcat:rollback-ability-cleanup

Conversation

@cuinhellcat

Copy link
Copy Markdown

Addresses #10049 and, most likely, #8762.

Problem

GameActionUtil.rollbackAbility returns as soon as the snapshot has been restored:

if (game.restoreGameState()) {
    // If we're able to restore the whole game state when rolling back an ability don't try to manually roll back
    System.out.println("Restored state from snapshot! ...");
    return;
}

That is right for the cards, their zones and the mana pool — copyManaPool already puts the mana back, so payment.refundPayment() on top of it would hand out the mana twice. It is not right for everything else the early return skips:

  • ability.setXManaCostPaid(null), setSpendPhyrexianMana(false), clearPipsToReduce(), setPaidLife(0) and the Announce SVars
  • ability.clearTargets(), the Charm mode chain, resetOnceResolved()
  • oldCard.setCastSA(null) and setCastFrom(null)
  • game.getStack().clearFrozen() and getTriggerHandler().clearWaitingTriggers()

A SpellAbility is not part of a snapshot, and neither is the frozen stack. So after cancelling a cast with undo restore enabled, the card keeps the announced value and targets from the attempt that was called off, still believes it is being cast, and the stack stays frozen.

That is what the two reports look like from the outside:

  • Known Rollback issues #10049 — March of the Otherworldly Light can no longer be cast after cancelling with an invalid number. Its additional cost is announced, and the announced value is exactly what survives.
  • Canceling casts sometimes freezes the game #8762 — cancelling a cast sometimes freezes the game. A stack left frozen with waiting triggers does that, and it explains why it only happens sometimes: Hanmac already pointed at GameSnapshot in that thread, and the reporter was told to turn undo restore off.

Fix

Do the ability-level cleanup in both paths — still no refund, still no zone handling. The values reset in the snapshot path are the same ones the manual path resets, pulled into two small helpers so the paths cannot drift apart.

Reproduction

At engine level: enable snapshots, stash, then set xManaCostPaid, castSA, castFrom and freeze the stack the way a cast does, and call rollbackAbility. Before the change the announced X, the cast markers and the frozen stack all survive; after it they are gone. I could not drive the actual GUI cancel headlessly, so I have not confirmed the two reports end to end — the mechanism matches what they describe.

Notes

  • No test added, per the note in CONTRIBUTING about agent-added tests. The reproduction above exists as one (fails before, passes after) if you want it.
  • Forge's desktop suite (278 tests) passes with the change.

AI disclosure

Written with Claude Code (Claude Opus 5), as requested in CONTRIBUTING; the agent is also recorded as co-author on the commit.

…rge#10049, Card-Forge#8762)

rollbackAbility returns as soon as the snapshot has been restored, on the
grounds that the manual rollback would double up on it. That holds for
the cards, their zones and the mana pool — restoring those twice would
refund mana the snapshot already put back — but not for everything the
early return skips.

The ability object is not part of a snapshot. Its announced values, the
X it was cast for, chosen modes and targets, and the once-resolved flag
all survive the rollback, as do castSA and castFrom on the card and the
frozen stack with its waiting triggers.

That leaves the card in a state where the next cast uses stale values or
is refused outright, which is what Card-Forge#10049 describes (March of the
Otherworldly Light can no longer be cast after cancelling with an
invalid number), and it leaves the stack frozen, which matches the
freeze on cancel in Card-Forge#8762.

Do that part of the cleanup in both paths, still without the refund and
without touching zones. The values reset here are the ones the manual
path already resets; they are pulled into two helpers so both paths stay
in step.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tool4ever

Copy link
Copy Markdown
Contributor

Hmn?
SpellAbilities live on Cards, so they should be fresh either way...

@cuinhellcat

Copy link
Copy Markdown
Author

Different directions of the snapshot.

Taking one builds new cards, so their abilities are fresh. Restoring one builds nothing: copyGameState looks each card up by id and keeps the object it finds. Same card, same SpellAbility instances, including whatever the cancelled cast wrote onto them.

Unmodified master — stash, set xManaCostPaid, restore, look the card up again:

same card object?    true
same ability object? true
X on that ability:   3

The manual path already resets exactly this, a few lines below the early return: setXManaCostPaid(null), clearTargets(), resetOnceResolved(), clearFrozen(), clearWaitingTriggers(). If abilities came back fresh, those resets would be pointless there too.

Concretely: announce X=4 on Fireball, cancel at the mana prompt. With undo restore on, the card is back in hand still carrying X=4 and its target, and the stack stays frozen — #10049 and #8762 from the outside.

Refund and zone handling stay skipped, since the snapshot covers those.

@Jetz72 Jetz72 added the Experimental Restore Relating to the state-based Undo/Restore feature. label Jul 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Experimental Restore Relating to the state-based Undo/Restore feature.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants