From 0c7965a72572432e9f96626645f248293236bd01 Mon Sep 17 00:00:00 2001 From: Susucre <34709007+Susucre@users.noreply.github.com> Date: Tue, 18 Jul 2023 02:02:49 +0200 Subject: [PATCH] Fix Pyrrhic Revival not adding -1/-1 counters on returned creatures. (#10626) * Add (failing) unit test on Pyrrhic Revival * fix PyrrhicRevival reworked ReturnFromGraveyardToBattlefieldWithCounterTargetEffect to support having multiple cards in its targetPointer. added test for Persist (the card from mh2). * refactor & cleanup * add myself as author, the effect was remade. --- .../src/mage/cards/p/PyrrhicRevival.java | 56 +++++++++---------- .../cards/single/eve/PyrrhicRevivalTest.java | 45 +++++++++++++++ .../test/cards/single/mh2/PersistTest.java | 40 +++++++++++++ ...romGraveyardToBattlefieldTargetEffect.java | 19 ++++++- ...dToBattlefieldWithCounterTargetEffect.java | 22 ++++++-- 5 files changed, 147 insertions(+), 35 deletions(-) create mode 100644 Mage.Tests/src/test/java/org/mage/test/cards/single/eve/PyrrhicRevivalTest.java create mode 100644 Mage.Tests/src/test/java/org/mage/test/cards/single/mh2/PersistTest.java diff --git a/Mage.Sets/src/mage/cards/p/PyrrhicRevival.java b/Mage.Sets/src/mage/cards/p/PyrrhicRevival.java index 178886270df..590119fbd00 100644 --- a/Mage.Sets/src/mage/cards/p/PyrrhicRevival.java +++ b/Mage.Sets/src/mage/cards/p/PyrrhicRevival.java @@ -1,30 +1,27 @@ package mage.cards.p; -import java.util.HashSet; -import java.util.Set; -import java.util.UUID; import mage.abilities.Ability; -import mage.abilities.effects.ContinuousEffect; -import mage.abilities.effects.EntersBattlefieldEffect; +import mage.abilities.effects.Effect; import mage.abilities.effects.OneShotEffect; -import mage.abilities.effects.common.counter.AddCountersTargetEffect; +import mage.abilities.effects.common.ReturnFromGraveyardToBattlefieldWithCounterTargetEffect; import mage.cards.Card; import mage.cards.CardImpl; import mage.cards.CardSetInfo; import mage.constants.CardType; -import mage.constants.Duration; import mage.constants.Outcome; -import mage.constants.Zone; import mage.counters.CounterType; import mage.game.Game; import mage.players.Player; -import mage.target.targetpointer.FixedTarget; +import mage.target.targetpointer.FixedTargets; + +import java.util.Objects; +import java.util.Set; +import java.util.UUID; +import java.util.stream.Collectors; /** - * - * @author jeffwadsworth - * + * @author jeffwadsworth, Susucr */ public final class PyrrhicRevival extends CardImpl { @@ -68,22 +65,25 @@ class PyrrhicRevivalEffect extends OneShotEffect { if (controller == null) { return false; } - Set toBattlefield = new HashSet<>(); - for (UUID playerId : game.getState().getPlayersInRange(source.getControllerId(), game)) { - Player player = game.getPlayer(playerId); - if (player != null) { - for (Card card : player.getGraveyard().getCards(game)) { - if (card != null && card.isCreature(game)) { - toBattlefield.add(card); - ContinuousEffect effect = new EntersBattlefieldEffect(new AddCountersTargetEffect(CounterType.M1M1.createInstance())); - effect.setDuration(Duration.OneUse); - effect.setTargetPointer(new FixedTarget(card.getId())); - game.addEffect(effect, source); - } - } - } - } - controller.moveCards(toBattlefield, Zone.BATTLEFIELD, source, game, false, false, true, null); + + Set toBattlefield = + game.getState().getPlayersInRange(source.getControllerId(), game) + .stream() + .map(game::getPlayer) + .filter(Objects::nonNull) + .flatMap(p -> p.getGraveyard().getCards(game).stream()) + .filter(c -> c != null && c.isCreature(game)) + .collect(Collectors.toSet()); + + Effect returnEffect = + new ReturnFromGraveyardToBattlefieldWithCounterTargetEffect( + CounterType.M1M1.createInstance(), + true, + true); + + returnEffect.setTargetPointer(new FixedTargets(toBattlefield, game)); + returnEffect.apply(game, source); + return true; } } diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/single/eve/PyrrhicRevivalTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/single/eve/PyrrhicRevivalTest.java new file mode 100644 index 00000000000..ea86dff6337 --- /dev/null +++ b/Mage.Tests/src/test/java/org/mage/test/cards/single/eve/PyrrhicRevivalTest.java @@ -0,0 +1,45 @@ +package org.mage.test.cards.single.eve; + +import mage.constants.PhaseStep; +import mage.constants.Zone; +import org.junit.Test; +import org.mage.test.serverside.base.CardTestPlayerBase; + +/** + * @author Susucr + */ +public class PyrrhicRevivalTest extends CardTestPlayerBase { + + // Pyrrhic Revival {3}{W/B}{W/B}{W/B} + // Sorcery + // Each player returns each creature card from their graveyard to the battlefield with an additional -1/-1 counter on it. + private static final String revival = "Pyrrhic Revival"; + // Cathedral Sanctifier {W} + // Creature — Human Cleric + // + // When Cathedral Sanctifier enters the battlefield, you gain 3 life. + // 1/1 + private static final String sanctifier = "Cathedral Sanctifier"; + + @Test + public void test_PyrrhicRevival() { + addCard(Zone.GRAVEYARD, playerA, sanctifier, 2); + addCard(Zone.GRAVEYARD, playerB, sanctifier, 1); + addCard(Zone.BATTLEFIELD, playerA, "plains", 6); + addCard(Zone.HAND, playerA, revival); + + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, revival); + setChoice(playerA, "When {this} enters the battlefield, you gain 3 life."); + + setStrictChooseMode(true); + setStopAt(1, PhaseStep.END_TURN); + execute(); + + assertPermanentCount(playerA, sanctifier, 0); + assertPermanentCount(playerB, sanctifier, 0); + assertGraveyardCount(playerA, sanctifier, 2); + assertGraveyardCount(playerB, sanctifier, 1); + assertLife(playerA, 20 + 6); + assertLife(playerB, 20 + 3); + } +} diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/single/mh2/PersistTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/single/mh2/PersistTest.java new file mode 100644 index 00000000000..aa5a10dbe87 --- /dev/null +++ b/Mage.Tests/src/test/java/org/mage/test/cards/single/mh2/PersistTest.java @@ -0,0 +1,40 @@ +package org.mage.test.cards.single.mh2; + +import mage.constants.PhaseStep; +import mage.constants.Zone; +import org.junit.Test; +import org.mage.test.serverside.base.CardTestPlayerBase; + +/** + * @author Susucr + */ +public class PersistTest extends CardTestPlayerBase { + + // Persist {1}{B} + // Sorcery + // Return target nonlegendary creature card from your graveyard to the battlefield with a -1/-1 counter on it. + private static final String persist = "Persist"; + // Cathedral Sanctifier {W} + // Creature — Human Cleric + // + // When Cathedral Sanctifier enters the battlefield, you gain 3 life. + // 1/1 + private static final String sanctifier = "Cathedral Sanctifier"; + + @Test + public void test_Persist_Card() { + addCard(Zone.GRAVEYARD, playerA, sanctifier, 1); + addCard(Zone.BATTLEFIELD, playerA, "swamp", 2); + addCard(Zone.HAND, playerA, persist); + + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, persist, sanctifier); + + setStrictChooseMode(true); + setStopAt(1, PhaseStep.END_TURN); + execute(); + + assertPermanentCount(playerA, sanctifier, 0); + assertGraveyardCount(playerA, sanctifier, 1); + assertLife(playerA, 20 + 3); + } +} diff --git a/Mage/src/main/java/mage/abilities/effects/common/ReturnFromGraveyardToBattlefieldTargetEffect.java b/Mage/src/main/java/mage/abilities/effects/common/ReturnFromGraveyardToBattlefieldTargetEffect.java index 3070d28eb50..9bb538495ea 100644 --- a/Mage/src/main/java/mage/abilities/effects/common/ReturnFromGraveyardToBattlefieldTargetEffect.java +++ b/Mage/src/main/java/mage/abilities/effects/common/ReturnFromGraveyardToBattlefieldTargetEffect.java @@ -23,6 +23,9 @@ public class ReturnFromGraveyardToBattlefieldTargetEffect extends OneShotEffect private final boolean tapped; private final boolean attacking; + // If true, creatures are returned to their owner's control. + // If false, creatures are returned under the effect's controller control. + private final boolean underOwnerControl; public ReturnFromGraveyardToBattlefieldTargetEffect() { this(false); @@ -31,17 +34,22 @@ public class ReturnFromGraveyardToBattlefieldTargetEffect extends OneShotEffect public ReturnFromGraveyardToBattlefieldTargetEffect(boolean tapped) { this(tapped, false); } - public ReturnFromGraveyardToBattlefieldTargetEffect(boolean tapped, boolean attacking) { + this(tapped, attacking, false); + } + + public ReturnFromGraveyardToBattlefieldTargetEffect(boolean tapped, boolean attacking, boolean underOwnerControl) { super(Outcome.PutCreatureInPlay); this.tapped = tapped; this.attacking = attacking; + this.underOwnerControl = underOwnerControl; } protected ReturnFromGraveyardToBattlefieldTargetEffect(final ReturnFromGraveyardToBattlefieldTargetEffect effect) { super(effect); this.tapped = effect.tapped; this.attacking = effect.attacking; + this.underOwnerControl = effect.underOwnerControl; } @Override @@ -60,7 +68,7 @@ public class ReturnFromGraveyardToBattlefieldTargetEffect extends OneShotEffect cardsToMove.add(card); } } - controller.moveCards(cardsToMove, Zone.BATTLEFIELD, source, game, tapped, false, false, null); + controller.moveCards(cardsToMove, Zone.BATTLEFIELD, source, game, tapped, false, underOwnerControl, null); if (attacking) { for (Card card : cardsToMove) { game.getCombat().addAttackingCreature(card.getId(), game); @@ -111,7 +119,12 @@ public class ReturnFromGraveyardToBattlefieldTargetEffect extends OneShotEffect sb.append(" attacking"); } if (!yourGrave) { - sb.append(" under your control"); + if (underOwnerControl) { + sb.append("under their owner's control"); + } + else { + sb.append(" under your control"); + } } return sb.toString(); } diff --git a/Mage/src/main/java/mage/abilities/effects/common/ReturnFromGraveyardToBattlefieldWithCounterTargetEffect.java b/Mage/src/main/java/mage/abilities/effects/common/ReturnFromGraveyardToBattlefieldWithCounterTargetEffect.java index 5c4e0d66b52..dfd63d9b657 100644 --- a/Mage/src/main/java/mage/abilities/effects/common/ReturnFromGraveyardToBattlefieldWithCounterTargetEffect.java +++ b/Mage/src/main/java/mage/abilities/effects/common/ReturnFromGraveyardToBattlefieldWithCounterTargetEffect.java @@ -10,6 +10,9 @@ import mage.game.Game; import mage.game.events.EntersTheBattlefieldEvent; import mage.game.events.GameEvent; import mage.game.permanent.Permanent; +import mage.target.targetpointer.FixedTarget; + +import java.util.UUID; /** * @author weirddan455 @@ -24,7 +27,11 @@ public class ReturnFromGraveyardToBattlefieldWithCounterTargetEffect extends Ret } public ReturnFromGraveyardToBattlefieldWithCounterTargetEffect(Counter counter, boolean additional) { - super(); + this(counter, additional, false); + } + + public ReturnFromGraveyardToBattlefieldWithCounterTargetEffect(Counter counter, boolean additional, boolean underOwnerControl) { + super(false, false, underOwnerControl); this.counter = counter; this.additional = additional; } @@ -42,8 +49,11 @@ public class ReturnFromGraveyardToBattlefieldWithCounterTargetEffect extends Ret @Override public boolean apply(Game game, Ability source) { - AddCounterTargetReplacementEffect counterEffect = new AddCounterTargetReplacementEffect(counter); - game.addEffect(counterEffect, source); + for (UUID targetId : getTargetPointer().getTargets(game, source)) { + AddCounterTargetReplacementEffect counterEffect = new AddCounterTargetReplacementEffect(counter); + counterEffect.setTargetPointer(new FixedTarget(targetId, game)); + game.addEffect(counterEffect, source); + } return super.apply(game, source); } @@ -69,7 +79,11 @@ public class ReturnFromGraveyardToBattlefieldWithCounterTargetEffect extends Ret if (counter.getCount() != 1) { sb.append('s'); } - sb.append(" on it"); + if (targetPointer.isPlural(mode.getTargets())) { + sb.append(" on them"); + } else { + sb.append(" on it"); + } return sb.toString(); } }