From a377999f578dd7a99e95d3fd56932369679eb6e0 Mon Sep 17 00:00:00 2001 From: Oleg Agafonov Date: Mon, 22 Feb 2021 08:51:07 +0400 Subject: [PATCH] * Desertion - fixed rollback error on fizzled counter spell (#7613); --- Mage.Sets/src/mage/cards/d/Desertion.java | 16 ++++---- .../test/cards/single/vis/DesertionTest.java | 38 +++++++++++++++++++ .../main/java/mage/game/events/GameEvent.java | 2 +- Mage/src/main/java/mage/game/stack/Spell.java | 4 +- 4 files changed, 49 insertions(+), 11 deletions(-) create mode 100644 Mage.Tests/src/test/java/org/mage/test/cards/single/vis/DesertionTest.java diff --git a/Mage.Sets/src/mage/cards/d/Desertion.java b/Mage.Sets/src/mage/cards/d/Desertion.java index a13de6ba8ff..8fe5b50c07b 100644 --- a/Mage.Sets/src/mage/cards/d/Desertion.java +++ b/Mage.Sets/src/mage/cards/d/Desertion.java @@ -1,7 +1,5 @@ - package mage.cards.d; -import java.util.UUID; import mage.MageObject; import mage.abilities.Ability; import mage.abilities.common.SimpleStaticAbility; @@ -18,8 +16,10 @@ import mage.game.events.GameEvent; import mage.game.events.ZoneChangeEvent; import mage.target.TargetSpell; +import java.util.Objects; +import java.util.UUID; + /** - * * @author Quercitron */ public final class Desertion extends CardImpl { @@ -30,7 +30,7 @@ public final class Desertion extends CardImpl { // Counter target spell. this.getSpellAbility().addEffect(new CounterTargetEffect()); this.getSpellAbility().addTarget(new TargetSpell()); - + // If an artifact or creature spell is countered this way, put that card onto the battlefield under your control instead of into its owner's graveyard. this.addAbility(new SimpleStaticAbility(Zone.STACK, new DesertionReplacementEffect())); } @@ -80,13 +80,13 @@ class DesertionReplacementEffect extends ReplacementEffectImpl { } @Override - public boolean applies(GameEvent event, Ability source, Game game) { - if (!event.getSourceId().equals(source.getSourceId()) + public boolean applies(GameEvent event, Ability source, Game game) { + if (!Objects.equals(event.getSourceId(), source.getSourceId()) || !(((ZoneChangeEvent) event).getToZone() == Zone.GRAVEYARD)) { return false; } - MageObject mageObject = game.getObject(event.getTargetId()); - return mageObject != null + MageObject mageObject = game.getObject(event.getTargetId()); + return mageObject != null && (mageObject.isArtifact() || mageObject.isCreature()); } } diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/single/vis/DesertionTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/single/vis/DesertionTest.java new file mode 100644 index 00000000000..55f8ac92670 --- /dev/null +++ b/Mage.Tests/src/test/java/org/mage/test/cards/single/vis/DesertionTest.java @@ -0,0 +1,38 @@ +package org.mage.test.cards.single.vis; + +import mage.constants.PhaseStep; +import mage.constants.Zone; +import org.junit.Test; +import org.mage.test.serverside.base.CardTestPlayerBase; + +public class DesertionTest extends CardTestPlayerBase { + + @Test + public void test_MultipleCounter() { + // possible bug: error NPE if target spell already countered before resolve + + // Counter target spell. + // If an artifact or creature spell is countered this way, put that card onto the battlefield under your + // control instead of into its owner's graveyard. + addCard(Zone.HAND, playerA, "Desertion", 2); // {3}{U}{U} + addCard(Zone.BATTLEFIELD, playerA, "Island", 2 * 5); + // + addCard(Zone.HAND, playerA, "Grizzly Bears"); // {1}{G} + addCard(Zone.BATTLEFIELD, playerA, "Forest", 2); + + // counter same spell 2x times + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Grizzly Bears"); + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Desertion", "Grizzly Bears", "Cast Grizzly Bears"); + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Desertion", "Grizzly Bears", "Cast Grizzly Bears"); + checkStackObject("before resolve", 1, PhaseStep.PRECOMBAT_MAIN, playerA, "Cast Grizzly Bears", 1); + checkStackObject("before resolve", 1, PhaseStep.PRECOMBAT_MAIN, playerA, "Cast Desertion", 2); + + setStrictChooseMode(true); + setStopAt(1, PhaseStep.END_TURN); + execute(); + assertAllCommandsUsed(); + + assertPermanentCount(playerA, "Grizzly Bears", 1); + assertGraveyardCount(playerA, "Desertion", 2); + } +} diff --git a/Mage/src/main/java/mage/game/events/GameEvent.java b/Mage/src/main/java/mage/game/events/GameEvent.java index 7f3ea461911..3268df767de 100644 --- a/Mage/src/main/java/mage/game/events/GameEvent.java +++ b/Mage/src/main/java/mage/game/events/GameEvent.java @@ -65,7 +65,7 @@ public class GameEvent implements Serializable { //player events /* ZONE_CHANGE targetId id of the zone changing object - sourceId sourceId of the ability with the object moving effect + sourceId sourceId of the ability with the object moving effect (WARNING, can be null if it move of fizzled spells) playerId controller of the moved object amount not used for this event flag not used for this event diff --git a/Mage/src/main/java/mage/game/stack/Spell.java b/Mage/src/main/java/mage/game/stack/Spell.java index 9030c886ca6..a66b13fedfb 100644 --- a/Mage/src/main/java/mage/game/stack/Spell.java +++ b/Mage/src/main/java/mage/game/stack/Spell.java @@ -403,8 +403,8 @@ public class Spell extends StackObjImpl implements Card { @Override public void counter(Ability source, Game game, Zone zone, boolean owner, ZoneDetail zoneDetail) { - // source can be null for fizzled spells, found only one place with that usage -- Rebound Ability: - // event.getSourceId().equals(source.getSourceId()) + // source can be null for fizzled spells, don't use that code in your ZONE_CHANGE watchers/triggers: + // event.getSourceId().equals // TODO: so later it must be replaced to another technics with non null source UUID counteringSourceId = (source == null ? null : source.getSourceId()); this.countered = true;