From d56f6b991b77fb6799a367d5d4e098ad9680c1ef Mon Sep 17 00:00:00 2001 From: Samuel Sandeen Date: Mon, 10 Feb 2020 08:18:12 -0500 Subject: [PATCH] Fix Sevinne's Reclamation. (#6275) This also handles the rather unique case caused by Sevinne's Reclamation where the original spell resolves before the copy of it. Also fixes a couple typos. --- .../src/mage/cards/s/SevinnesReclamation.java | 15 +++++----- .../mage/test/cards/copy/CopySpellTest.java | 29 +++++++++++++++++++ .../main/java/mage/abilities/AbilityImpl.java | 4 +-- Mage/src/main/java/mage/cards/CardImpl.java | 12 ++++---- .../main/java/mage/game/stack/SpellStack.java | 8 ++++- 5 files changed, 51 insertions(+), 17 deletions(-) diff --git a/Mage.Sets/src/mage/cards/s/SevinnesReclamation.java b/Mage.Sets/src/mage/cards/s/SevinnesReclamation.java index c25def45091..22c8c465740 100644 --- a/Mage.Sets/src/mage/cards/s/SevinnesReclamation.java +++ b/Mage.Sets/src/mage/cards/s/SevinnesReclamation.java @@ -34,9 +34,11 @@ public final class SevinnesReclamation extends CardImpl { public SevinnesReclamation(UUID ownerId, CardSetInfo setInfo) { super(ownerId, setInfo, new CardType[]{CardType.SORCERY}, "{2}{W}"); - // Return target permanent card with converted mana cost 3 or less from your graveyard to the battlefield. If this spell was cast from a graveyard, you may copy this spell and may choose a new target for the copy. - this.getSpellAbility().addEffect(new SevinnesReclamationEffect()); + // Return target permanent card with converted mana cost 3 or less from your graveyard to the battlefield. + this.getSpellAbility().addEffect(new ReturnFromGraveyardToBattlefieldTargetEffect()); this.getSpellAbility().addTarget(new TargetCardInYourGraveyard(filter)); + // If this spell was cast from a graveyard, you may copy this spell and may choose a new target for the copy. + this.getSpellAbility().addEffect(new SevinnesReclamationEffect()); // Flashback {4}{W} this.addAbility(new FlashbackAbility(new ManaCostsImpl("{4}{W}"), TimingRule.SORCERY)); @@ -54,12 +56,9 @@ public final class SevinnesReclamation extends CardImpl { class SevinnesReclamationEffect extends OneShotEffect { - private static final Effect effect = new ReturnFromGraveyardToBattlefieldTargetEffect(); - SevinnesReclamationEffect() { super(Outcome.Benefit); - staticText = "Return target permanent card with converted mana cost 3 or less " + - "from your graveyard to the battlefield. If this spell was cast from a graveyard, " + + staticText = "If this spell was cast from a graveyard, " + "you may copy this spell and may choose a new target for the copy."; } @@ -74,12 +73,12 @@ class SevinnesReclamationEffect extends OneShotEffect { @Override public boolean apply(Game game, Ability source) { - Spell spell = (Spell) game.getStack().getStackObject(source.getSourceId()); + // If a spell is a copy it wasn't cast from the graveyard. + Spell spell = game.getStack().getSpell(source.getSourceId(), false); Player player = game.getPlayer(source.getControllerId()); if (spell == null || player == null) { return false; } - effect.apply(game, source); if (spell.getFromZone() == Zone.GRAVEYARD && player.chooseUse(outcome, "Copy this spell?", source, game)) { spell.createCopyOnStack(game, source, source.getControllerId(), true); diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/copy/CopySpellTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/copy/CopySpellTest.java index c780fbef4ca..643a3d5c978 100644 --- a/Mage.Tests/src/test/java/org/mage/test/cards/copy/CopySpellTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/cards/copy/CopySpellTest.java @@ -375,4 +375,33 @@ public class CopySpellTest extends CardTestPlayerBase { assertGraveyardCount(playerB, "Flame Slash", 1); } + + /** + * Sevinne's Reclamation is almost unique in that the original spell resolves before the copy. + * As a result when resolving the original the copy was being removed from the stack instead. + */ + @Test + public void testSevinnesReclamation() { + addCard(Zone.BATTLEFIELD, playerA, "Plains", 5); + // Return target permanent card with converted mana cost 3 or less from your graveyard to the battlefield. + // If this spell was cast from a graveyard, you may copy this spell and may choose a new target for the copy. + // Flashback 4W + addCard(Zone.GRAVEYARD, playerA, "Sevinne's Reclamation"); + addCard(Zone.GRAVEYARD, playerA, "Mountain"); + addCard(Zone.GRAVEYARD, playerA, "Island"); + + activateAbility(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Flashback"); + addTarget(playerA, "Mountain"); + setChoice(playerA, "Yes"); // Copy + setChoice(playerA, "Yes"); // Choose new target + addTarget(playerA, "Island"); + + setStopAt(1, PhaseStep.BEGIN_COMBAT); + setStrictChooseMode(true); + execute(); + assertAllCommandsUsed(); + + assertPermanentCount(playerA, "Mountain", 1); + assertPermanentCount(playerA, "Island", 1); + } } diff --git a/Mage/src/main/java/mage/abilities/AbilityImpl.java b/Mage/src/main/java/mage/abilities/AbilityImpl.java index 7fcb21e585b..361a6c4ec4e 100644 --- a/Mage/src/main/java/mage/abilities/AbilityImpl.java +++ b/Mage/src/main/java/mage/abilities/AbilityImpl.java @@ -198,7 +198,7 @@ public abstract class AbilityImpl implements Ability { /** * game.applyEffects() has to be done at least for every effect that * moves cards/permanent between zones, or changes control of - * objects so Static effects work as intened if dependant from the + * objects so Static effects work as intended if dependant from the * moved objects zone it is in Otherwise for example were static * abilities with replacement effects deactivated too late Example: * {@link org.mage.test.cards.replacement.DryadMilitantTest#testDiesByDestroy testDiesByDestroy} @@ -924,7 +924,7 @@ public abstract class AbilityImpl implements Ability { } else { parameterSourceId = getSourceId(); } - // check agains shortLKI for effects that move multiple object at the same time (e.g. destroy all) + // check against shortLKI for effects that move multiple object at the same time (e.g. destroy all) if (game.getShortLivingLKI(getSourceId(), getZone())) { return true; } diff --git a/Mage/src/main/java/mage/cards/CardImpl.java b/Mage/src/main/java/mage/cards/CardImpl.java index 8ebe60c3d79..d5daa56d42c 100644 --- a/Mage/src/main/java/mage/cards/CardImpl.java +++ b/Mage/src/main/java/mage/cards/CardImpl.java @@ -499,21 +499,21 @@ public abstract class CardImpl extends MageObjectImpl implements Card { case STACK: StackObject stackObject; if (getSpellAbility() != null) { - stackObject = game.getStack().getSpell(getSpellAbility().getId()); + stackObject = game.getStack().getSpell(getSpellAbility().getId(), false); } else { - stackObject = game.getStack().getSpell(this.getId()); + stackObject = game.getStack().getSpell(this.getId(), false); } if (stackObject == null && (this instanceof SplitCard)) { // handle if half of Split cast is on the stack - stackObject = game.getStack().getSpell(((SplitCard) this).getLeftHalfCard().getId()); + stackObject = game.getStack().getSpell(((SplitCard) this).getLeftHalfCard().getId(), false); if (stackObject == null) { - stackObject = game.getStack().getSpell(((SplitCard) this).getRightHalfCard().getId()); + stackObject = game.getStack().getSpell(((SplitCard) this).getRightHalfCard().getId(), false); } } if (stackObject == null && (this instanceof AdventureCard)) { - stackObject = game.getStack().getSpell(((AdventureCard) this).getSpellCard().getId()); + stackObject = game.getStack().getSpell(((AdventureCard) this).getSpellCard().getId(), false); } if (stackObject == null) { - stackObject = game.getStack().getSpell(getId()); + stackObject = game.getStack().getSpell(getId(), false); } if (stackObject != null) { removed = game.getStack().remove(stackObject, game); diff --git a/Mage/src/main/java/mage/game/stack/SpellStack.java b/Mage/src/main/java/mage/game/stack/SpellStack.java index 40b5c963883..fe58bfebeea 100644 --- a/Mage/src/main/java/mage/game/stack/SpellStack.java +++ b/Mage/src/main/java/mage/game/stack/SpellStack.java @@ -113,10 +113,16 @@ public class SpellStack extends ArrayDeque { } public Spell getSpell(UUID id) { + return getSpell(id, true); + } + + public Spell getSpell(UUID id, boolean allowCopies) { for (StackObject stackObject : this) { if (stackObject instanceof Spell) { if (stackObject.getId().equals(id) || stackObject.getSourceId().equals(id)) { - return (Spell) stackObject; + if (allowCopies || !stackObject.isCopy()) { + return (Spell) stackObject; + } } } }