Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Jetz72 opened this issue Aug 14, 2024 · 5 comments
Assignees
Labels

Comments

@Jetz72
Copy link
Contributor

Jetz72 commented Aug 14, 2024

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

  • Experimental Undo/Restore enabled.
  • Illuna, Apex of Wishes as your commander, any nonhuman creature in the battlefield.
  • Mutate Illuna onto a creature. Placement at top or bottom does not matter.
  • Resolve the stack.

Or:

  • Experimental Undo/Restore enabled.
  • Bruna, the Fading Light as your commander, Gisela, the Broken Blade in the battlefield.
  • Cast Bruna, the Fading Light.
  • Proceed to End Step.
  • Resolve the Meld trigger.

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

  • Tested on Master in Adventure UI as of e93b130a3631c5f4642330a65f31efa9f3a0e1ca. Few days old, but should still be there unless I overlooked a relevant commit in the last couple days.

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 Hanmac added bug Something isn't working Game Mechanics labels Aug 14, 2024
@Hanmac
Copy link
Contributor

Hanmac commented Aug 14, 2024

@Jetz72 should be fixed by this: #5921 merged yesterday

@tehdiplomat
Copy link
Contributor

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

@tehdiplomat
Copy link
Contributor

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

@Jetz72
Copy link
Contributor Author

Jetz72 commented Aug 14, 2024

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.

Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants