GameCopier: fix three state-fidelity bugs in copied games

[Tyrathalis]

Summary

GameCopier produces copies that differ from the original game in three
ways: face-down exiled cards come back face up, players' field-managed
effect cards get duplicated, and all cards are renumbered with fresh ids —
which changes how the AI plays the copy. This PR fixes all three, with
regression tests in GameSimulationTest.

Background: I am building a neural-network player on top of Forge
(non-commercial, GPL-aligned) and use GameCopier for state forking. To
qualify it, I built a differential fork-fidelity harness: play seeded
AI-vs-AI Commander games, copy the live game at a quiescent point
(empty stack, priority in a main phase), replay both the original and the
copy to completion with the same RNG state, and compare per-turn state
digests. Runs are 500 games over ~110 competitive Duel Commander decks
(~1,700 distinct cards). These fixes are the result; I'd like to keep
contributing what the harness finds.

The three bugs

1. Face-down exiled cards are rebuilt face up (createCardCopy
rebuilds from the paper card, which defaults to face up). Foretold cards
and "exile face down" effects leak hidden information into the copy and
the copied state visibly mismatches the original. This was 45 of the 60
state-corruption repro seeds in a 500-game run. Fix: exile-zone copies
retain face-down and foretold state.

2. Player effect cards duplicate across copies. A player's
field-managed effect cards (keyword, monarch, initiative, blessing,
contraption sprocket, radiation, speed) are copied as command-zone
contents but never wired to the new Player's fields, so the lazy
getters re-create them on next use while the copied originals sit
orphaned in the command zone — duplicate "Keyword Effects" cards that
compound with each copy generation (the other 15/60 repro seeds). Fix:
new Player.copyEffectCardsToSnapshot, following the existing
copyCommandersToSnapshot idiom; GameCopier's setBlessing call
(the same duplication for the blessing card) is removed in its favor.

3. Copies renumber card ids, which changes AI behavior. Copied cards
get fresh ids in zone-traversal order, but id order is AI-visible:
Card.compareTo is id-based and several AI paths iterate id-keyed
collections. The practical effect is that a simulation running on the
copy deterministically diverges from how the same AI plays the original
game — in the harness, 249/500 games played out differently after a
mid-game copy, purely from renumbering. Fix: copies keep their original
ids via the id-taking factory overloads that already exist on all four
copy paths; a new Game.dangerouslySyncCardIdCounters aligns the
fresh-id counters so ids handed out after the copy can't collide. Also
copies gamePieceType in the non-paper path.

(Naming: dangerouslySyncCardIdCounters is deliberately alarming since
mis-use would mint duplicate ids; happy to rename if you prefer.)

Measured effect (500-game differential runs, same seeds/decks)

metric	before	after
copies with corrupted state (bugs 1+2)	12%	0%
trajectories diverging after a mid-game copy (bug 3)	49.8%	13.4%

The residual 13.4% reproduces deterministically and traces to the
documented this-turn copy TODOs in GameCopier
(creatureAttackedThisTurn, thisTurnCast, counters-added tracking) —
out of scope here; I have repro seeds and may follow up.

Tests

Three regression tests added to GameSimulationTest:

• testGameCopyPreservesFaceDownExileState
• testGameCopyWiresPlayerEffectCards
• testGameCopyPreservesCardIds

All three fail against the unfixed copier (verified by reverting the three
source files and re-running). One extra finding from that check: with a
monarch or blessing effect card in play, the unfixed copier can NPE
outright during the copy (Card.hasRemembered() on a null mapping —
the duplicate effect card created mid-copy has no original to map to),
so bug 2 was also a latent crash, not just state corruption.

All pre-existing forge-gui-desktop tests pass with the fix.

Per the contributing guide's AI-agents note: this contribution was
substantially coded with Claude Code (the commit carries the co-author
trailer); the diagnosis, measurements, and review are mine.

🤖 Generated with Claude Code

[tool4ever]

if we're determining that it's reasonable to assign the same ID for simulation game copies too the main difference compared to GameSnapshot would no longer be a problem?

this essentially means the EXPERIMENTAL_RESTORE_SNAPSHOT guarded path should cover both use cases and a lot of duplicated code can finally be cleaned up, ensuring future fixes for those features can more easily be done together 🤔

dangerouslySyncCardIdCounters still makes sense though

[Tyrathalis]

Thanks — agreed on the direction. Now that copies keep original ids, the map-vs-findById distinction is the only load-bearing difference between the two paths, and GameCopier already delegates to GameSnapshot behind EXPERIMENTAL_RESTORE_SNAPSHOT, so consolidating there looks right.

Two data points from this investigation that support it: the snapshot path has sibling versions of two of the bugs fixed here — setForetold/setForetoldCostByEffect are commented out in setCardInCopiedGame, and only commanders get re-wired, so the seven field-managed effect cards (keyword/monarch/initiative/blessing/sprocket/radiation/speed) re-create lazily while the copied originals sit orphaned. That's exactly the fixes-diverge-across-paths failure mode. (copyEffectCardsToSnapshot went on Player beside copyCommandersToSnapshot so both paths can share it.) Remaining deltas before snapshot can own simulation copies: the delegation ignores advanceToPhase (existing TODO), and PRUNE_HIDDEN_INFO has no snapshot equivalent.

Proposal: keep this PR as the bug-fix layer for the currently-default path (the three fixes are differential-tested and independently useful), and I'll take the consolidation as a follow-up PR — the differential fidelity harness that found these bugs (copy-vs-original state digests over hundreds of seeded games + twin determinism replays) can gate the switch the same way. Happy to also fold the two snapshot-path fixes (foretold + effect-card wiring) into this PR if you'd prefer.

[tool4ever]

acceptable

the Copier/Snapshot duplication is a longer technical debt (and getting rid of that should also help with your project)