Card-Forge / forge

An unofficial rules engine for the world's greatest card game.
https://card-forge.github.io/forge/
GNU General Public License v3.0
935 stars 544 forks source link

Game.findByID cannot find components of merged or melded permanents, causing crashes when one is a commander #5925

Open Jetz72 opened 1 month ago

Jetz72 commented 1 month ago

Describe the bug Game.findById relies on forEachCardInGame to locate cards. This checks all the ordinary game zones, but does not check the ZoneType.Merged, which contains components of merged permanents beyond the original one in the battlefield. It also does not check PlayerZoneBattlefield.meldedCards (and in fact, nothing does), which is where secondary components of melded permanents are kept.

To Reproduce

Or:

In either case, the engine crashes in GameSnapshot.assignGameState with this stack trace:

Game-0 > java.lang.NullPointerException
    at forge.game.player.PlayerView.updateCommanderCast(PlayerView.java:399)
    at forge.game.player.Player.incCommanderCast(Player.java:2810)
    at forge.game.GameSnapshot.assignGameState(GameSnapshot.java:93)
    at forge.game.GameSnapshot.makeCopy(GameSnapshot.java:52)
    at forge.game.GameSnapshot.makeCopy(GameSnapshot.java:38)
    at forge.game.Game.stashGameState(Game.java:188)
    at forge.game.phase.PhaseHandler.startFirstTurn(PhaseHandler.java:1029)
    at forge.game.GameAction.startGame(GameAction.java:2150)
    at forge.game.Match.startGame(Match.java:90)
    at forge.gamemodes.match.HostedMatch.lambda$startGame$1(HostedMatch.java:269)

Version

Remarks Might make sense to have the secondary melded cards share the Merged zone as well. Also realized that forEachCardInGame is also missing the extra deck zones.

Hanmac commented 1 month ago

@Jetz72 should be fixed by this: https://github.com/Card-Forge/forge/pull/5921 merged yesterday

tehdiplomat commented 1 month ago

No. I didn't fix the merged/melded part yet

tehdiplomat commented 1 month ago

One important thing to note, this is only relevant for the experimental undo restore feature. So shouldn't really be blocking.

Jetz72 commented 1 month ago

Ah, that'd explain why this hasn't come up more. Could have sworn I turned that feature off but I guess not.

Tangential note, it might be worthwhile to add an issue tag for the snapshot/restore system.

github-actions[bot] commented 2 days ago

This issue has not been updated in a while and has now been marked as stale. Stale messages will be auto closed.