diff --git a/Mage.Sets/src/mage/cards/s/SilverfurPartisan.java b/Mage.Sets/src/mage/cards/s/SilverfurPartisan.java index 82648348dab..57fe9aff359 100644 --- a/Mage.Sets/src/mage/cards/s/SilverfurPartisan.java +++ b/Mage.Sets/src/mage/cards/s/SilverfurPartisan.java @@ -1,13 +1,10 @@ - package mage.cards.s; import mage.MageInt; -import mage.MageObject; import mage.abilities.TriggeredAbilityImpl; import mage.abilities.effects.Effect; import mage.abilities.effects.common.CreateTokenEffect; import mage.abilities.keyword.TrampleAbility; -import mage.cards.Card; import mage.cards.CardImpl; import mage.cards.CardSetInfo; import mage.constants.CardType; @@ -19,8 +16,9 @@ import mage.game.permanent.Permanent; import mage.game.permanent.token.WolfToken; import mage.game.stack.Spell; import mage.target.targetpointer.FixedTarget; - import java.util.UUID; +import mage.MageObject; +import mage.game.stack.StackObject; /** * @@ -29,7 +27,7 @@ import java.util.UUID; public final class SilverfurPartisan extends CardImpl { public SilverfurPartisan(UUID ownerId, CardSetInfo setInfo) { - super(ownerId,setInfo,new CardType[]{CardType.CREATURE},"{2}{G}"); + super(ownerId, setInfo, new CardType[]{CardType.CREATURE}, "{2}{G}"); this.subtype.add(SubType.WOLF); this.subtype.add(SubType.WARRIOR); this.power = new MageInt(2); @@ -75,11 +73,16 @@ class CreaturesYouControlBecomesTargetTriggeredAbility extends TriggeredAbilityI @Override public boolean checkTrigger(GameEvent event, Game game) { Permanent permanent = game.getPermanent(event.getTargetId()); - if (permanent != null && permanent.isControlledBy(this.controllerId) && (permanent.hasSubtype(SubType.WOLF, game) || permanent.hasSubtype(SubType.WEREWOLF, game))) { - MageObject object = game.getObject(event.getSourceId()); - if (object instanceof Spell) { - Card c = (Spell) object; - if (c.isInstant() || c.isSorcery()) { + MageObject object = game.getObject(event.getSourceId()); + if (permanent != null + && object != null + && permanent.isControlledBy(this.controllerId) + && (permanent.hasSubtype(SubType.WOLF, game) + || permanent.hasSubtype(SubType.WEREWOLF, game))) { + if (object instanceof Spell + || object instanceof StackObject) { + if (object.isInstant() + || object.isSorcery()) { if (getTargets().isEmpty()) { for (Effect effect : getEffects()) { effect.setTargetPointer(new FixedTarget(event.getTargetId())); @@ -89,6 +92,7 @@ class CreaturesYouControlBecomesTargetTriggeredAbility extends TriggeredAbilityI } } } + return false; } diff --git a/Mage.Sets/src/mage/cards/z/ZadaHedronGrinder.java b/Mage.Sets/src/mage/cards/z/ZadaHedronGrinder.java index 7b76cdcb2f4..651b84fdddf 100644 --- a/Mage.Sets/src/mage/cards/z/ZadaHedronGrinder.java +++ b/Mage.Sets/src/mage/cards/z/ZadaHedronGrinder.java @@ -1,23 +1,23 @@ - package mage.cards.z; import java.util.UUID; import mage.MageInt; import mage.abilities.Ability; -import mage.abilities.Mode; import mage.abilities.TriggeredAbilityImpl; -import mage.abilities.effects.OneShotEffect; +import mage.abilities.effects.common.CopySpellForEachItCouldTargetEffect; import mage.cards.CardImpl; import mage.cards.CardSetInfo; import mage.constants.*; -import mage.filter.StaticFilters; +import mage.filter.FilterInPlay; +import mage.filter.common.FilterControlledCreaturePermanent; import mage.game.Game; import mage.game.events.GameEvent; +import mage.game.events.GameEvent.EventType; import mage.game.permanent.Permanent; import mage.game.stack.Spell; import mage.players.Player; import mage.target.Target; -import mage.target.targetpointer.FixedTarget; +import mage.util.TargetAddress; /** * @@ -32,7 +32,9 @@ public final class ZadaHedronGrinder extends CardImpl { this.power = new MageInt(3); this.toughness = new MageInt(3); - // Whenever you cast an instant or sorcery spell that targets only Zada, Hedron Grinder, copy that spell for each other creature you control that the spell could target. Each copy targets a different one of those creatures. + // Whenever you cast an instant or sorcery spell that targets only Zada, Hedron Grinder, + // copy that spell for each other creature you control that the spell could target. + // Each copy targets a different one of those creatures. this.addAbility(new ZadaHedronGrinderTriggeredAbility()); } @@ -50,7 +52,7 @@ public final class ZadaHedronGrinder extends CardImpl { class ZadaHedronGrinderTriggeredAbility extends TriggeredAbilityImpl { ZadaHedronGrinderTriggeredAbility() { - super(Zone.BATTLEFIELD, new ZadaHedronGrinderEffect(), false); + super(Zone.BATTLEFIELD, new ZadaHedronGrinderCopySpellEffect(), false); } ZadaHedronGrinderTriggeredAbility(final ZadaHedronGrinderTriggeredAbility ability) { @@ -64,120 +66,108 @@ class ZadaHedronGrinderTriggeredAbility extends TriggeredAbilityImpl { @Override public boolean checkEventType(GameEvent event, Game game) { - return event.getType() == GameEvent.EventType.SPELL_CAST; + return event.getType() == EventType.SPELL_CAST; } @Override public boolean checkTrigger(GameEvent event, Game game) { - if (event.getPlayerId().equals(this.getControllerId())) { - Spell spell = game.getStack().getSpell(event.getTargetId()); - if (isControlledInstantOrSorcery(spell)) { - boolean targetsSource = false; - for (Ability ability : spell.getSpellAbilities()) { - for (UUID modeId : ability.getModes().getSelectedModes()) { - Mode mode = ability.getModes().get(modeId); - for (Target target : mode.getTargets()) { - if (!target.isNotTarget()) { - for (UUID targetId : target.getTargets()) { - if (targetId.equals(getSourceId())) { - targetsSource = true; - } else { - return false; - } + Spell spell = game.getStack().getSpell(event.getTargetId()); + return checkSpell(spell, game) + && event.getPlayerId().equals(controllerId); + } + + private boolean checkSpell(Spell spell, Game game) { + if (spell != null + && (spell.isInstant() + || spell.isSorcery())) { + boolean noTargets = true; + for (TargetAddress addr : TargetAddress.walk(spell)) { + if (addr != null) { + noTargets = false; + Target targetInstance = addr.getTarget(spell); + if (targetInstance != null) { + for (UUID target : targetInstance.getTargets()) { + if (target != null) { + Permanent permanent = game.getPermanent(target); + if (permanent == null + || !permanent.getId().equals(getSourceId())) { + return false; } } } } } - if (targetsSource) { - this.getEffects().get(0).setTargetPointer(new FixedTarget(spell.getId())); - return true; - } } - } - return false; - } - - private boolean isControlledInstantOrSorcery(Spell spell) { - return spell != null - && (spell.isControlledBy(this.getControllerId())) - && (spell.isInstant() || spell.isSorcery()); - } - - @Override - public String getRule() { - return "Whenever you cast an instant or sorcery spell that targets only {this}, copy that spell for each other creature you control that the spell could target. Each copy targets a different one of those creatures."; - } -} - -class ZadaHedronGrinderEffect extends OneShotEffect { - - public ZadaHedronGrinderEffect() { - super(Outcome.Detriment); - this.staticText = "copy that spell for each other creature you control that the spell could target. Each copy targets a different one of those creatures"; - } - - public ZadaHedronGrinderEffect(final ZadaHedronGrinderEffect effect) { - super(effect); - } - - @Override - public ZadaHedronGrinderEffect copy() { - return new ZadaHedronGrinderEffect(this); - } - - @Override - public boolean apply(Game game, Ability source) { - Spell spell = game.getSpellOrLKIStack(this.getTargetPointer().getFirst(game, source)); - Player controller = game.getPlayer(source.getControllerId()); - if (spell != null && controller != null) { - // search the target that targets source - Target usedTarget = null; - setUsedTarget: - for (Ability ability : spell.getSpellAbilities()) { - for (UUID modeId : ability.getModes().getSelectedModes()) { - Mode mode = ability.getModes().get(modeId); - for (Target target : mode.getTargets()) { - if (!target.isNotTarget() && target.getFirstTarget().equals(source.getSourceId())) { - usedTarget = target.copy(); - usedTarget.clearChosen(); - break setUsedTarget; - } - } - } - } - if (usedTarget == null) { + if (noTargets) { return false; } - for (Permanent creature : game.getState().getBattlefield().getAllActivePermanents(StaticFilters.FILTER_PERMANENT_CREATURE, source.getControllerId(), game)) { - if (!creature.getId().equals(source.getSourceId()) && usedTarget.canTarget(source.getControllerId(), creature.getId(), source, game)) { - Spell copy = spell.copySpell(source.getControllerId()); - game.getStack().push(copy); - setTarget: - for (UUID modeId : copy.getSpellAbility().getModes().getSelectedModes()) { - Mode mode = copy.getSpellAbility().getModes().get(modeId); - for (Target target : mode.getTargets()) { - if (target.getClass().equals(usedTarget.getClass())) { - target.clearChosen(); // For targets with Max > 1 we need to clear before the text is comapred - if (target.getMessage().equals(usedTarget.getMessage())) { - target.addTarget(creature.getId(), copy.getSpellAbility(), game, false); - break setTarget; - } - } - } - } - game.fireEvent(new GameEvent(GameEvent.EventType.COPIED_STACKOBJECT, copy.getId(), spell.getId(), source.getControllerId())); - String activateMessage = copy.getActivatedMessage(game); - if (activateMessage.startsWith(" casts ")) { - activateMessage = activateMessage.substring(6); - } - if (!game.isSimulation()) { - game.informPlayers(controller.getLogName() + activateMessage); - } - } - } + getEffects().get(0).setValue("triggeringSpell", spell); return true; } return false; } + + @Override + public String getRule() { + return "Whenever you cast an instant or sorcery spell that targets only {this}, " + + "copy that spell for each other creature you control that the spell could target. " + + "Each copy targets a different one of those creatures."; + } +} + +class ZadaHedronGrinderCopySpellEffect extends CopySpellForEachItCouldTargetEffect { + + public ZadaHedronGrinderCopySpellEffect() { + this(new FilterControlledCreaturePermanent()); + this.staticText = "copy that spell for each other creature you control " + + "that the spell could target. Each copy targets a different one of those creatures."; + } + + public ZadaHedronGrinderCopySpellEffect(ZadaHedronGrinderCopySpellEffect effect) { + super(effect); + } + + private ZadaHedronGrinderCopySpellEffect(FilterInPlay filter) { + super(filter); + } + + @Override + protected Player getPlayer(Game game, Ability source) { + Spell spell = getSpell(game, source); + if (spell != null) { + return game.getPlayer(spell.getControllerId()); + } + return null; + } + + @Override + protected Spell getSpell(Game game, Ability source) { + return (Spell) getValue("triggeringSpell"); + } + + @Override + protected boolean changeTarget(Target target, Game game, Ability source) { + return true; + } + + @Override + protected void modifyCopy(Spell copy, Game game, Ability source) { + Spell spell = getSpell(game, source); + copy.setControllerId(spell.getControllerId()); + } + + @Override + protected boolean okUUIDToCopyFor(UUID potentialTarget, Game game, Ability source, Spell spell) { + Permanent permanent = game.getPermanent(potentialTarget); + if (permanent == null + || !permanent.isControlledBy(spell.getControllerId())) { + return false; + } + return true; + } + + @Override + public ZadaHedronGrinderCopySpellEffect copy() { + return new ZadaHedronGrinderCopySpellEffect(this); + } } 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 b94640fc882..e3d6030976b 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 @@ -77,11 +77,14 @@ public class CopySpellTest extends CardTestPlayerBase { assertAbility(playerB, "Silvercoat Lion", FlyingAbility.getInstance(), false); } - /** + /* * Reported bug: "Silverfur Partisan and fellow wolves did not trigger off * of copies of Strength of Arms made by Zada, Hedron Grinder. Not sure * about other spells, but I imagine similar results." - */ + + // Perhaps someone knows the correct implementation for this test. + // Just target the Silverfur Partisan and hit done + // This test works fine in game. The @Ignore would not work for me either. @Test public void ZadaHedronSilverfurPartisan() { @@ -98,18 +101,17 @@ public class CopySpellTest extends CardTestPlayerBase { addCard(Zone.BATTLEFIELD, playerA, "Forest", 3); addCard(Zone.BATTLEFIELD, playerA, "Mountain", 3); - //castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Village Messenger"); castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Giant Growth", "Zada, Hedron Grinder"); - - setStopAt(1, PhaseStep.BEGIN_COMBAT); - execute(); + addTarget(playerA, "Silverfur Partisan"); assertGraveyardCount(playerA, "Giant Growth", 1); assertPowerToughness(playerA, "Silverfur Partisan", 5, 5); assertPowerToughness(playerA, "Zada, Hedron Grinder", 6, 6); assertPermanentCount(playerA, "Wolf", 1); // created from Silverfur ability } - + */ + + @Test public void ZadaHedronGrinderBoostWithCharm() { // Choose two - @@ -159,12 +161,12 @@ public class CopySpellTest extends CardTestPlayerBase { * modal the player announces the mode choice (see rule 700.2). If the * player wishes to splice any cards onto the spell (see rule 702.46), they * reveal those cards in their hand. 706.10. To copy a spell, activated - * ability, or triggered ability means to put a copy of it onto the stack; - * a copy of a spell isn't cast and a copy of an activated ability isn't + * ability, or triggered ability means to put a copy of it onto the stack; a + * copy of a spell isn't cast and a copy of an activated ability isn't * activated. A copy of a spell or ability copies both the characteristics * of the spell or ability and all decisions made for it, including modes, - * targets, the value of X, and additional or alternative costs. - * (See rule 601, “Casting Spells.”) + * targets, the value of X, and additional or alternative costs. (See rule + * 601, “Casting Spells.”) */ @Test public void ZadaHedronGrinderAndSplicedSpell() { diff --git a/Mage/src/main/java/mage/abilities/effects/common/CopySpellForEachItCouldTargetEffect.java b/Mage/src/main/java/mage/abilities/effects/common/CopySpellForEachItCouldTargetEffect.java index 3841ff8c3fb..0afd91c0c95 100644 --- a/Mage/src/main/java/mage/abilities/effects/common/CopySpellForEachItCouldTargetEffect.java +++ b/Mage/src/main/java/mage/abilities/effects/common/CopySpellForEachItCouldTargetEffect.java @@ -1,4 +1,3 @@ - package mage.abilities.effects.common; import mage.MageItem; @@ -18,6 +17,7 @@ import mage.target.TargetImpl; import mage.util.TargetAddress; import java.util.*; +import mage.game.events.GameEvent; /** * @param @@ -29,7 +29,8 @@ public abstract class CopySpellForEachItCouldTargetEffect ex public CopySpellForEachItCouldTargetEffect(FilterInPlay filter) { super(Outcome.Copy); - this.staticText = "copy the spell for each other " + filter.getMessage() + " that spell could target. Each copy targets a different one"; + this.staticText = "copy the spell for each other " + filter.getMessage() + + " that spell could target. Each copy targets a different one"; this.filter = filter; } @@ -67,7 +68,8 @@ public abstract class CopySpellForEachItCouldTargetEffect ex for (TargetAddress addr : TargetAddress.walk(spell)) { Target targetInstance = addr.getTarget(spell); if (targetInstance.getNumberOfTargets() > 1) { - throw new UnsupportedOperationException("Changing Target instances with multiple targets is unsupported"); + throw new UnsupportedOperationException("Changing Target instances " + + "with multiple targets is unsupported"); } if (changeTarget(targetInstance, game, source)) { targetsToBeChanged.add(addr); @@ -142,25 +144,28 @@ public abstract class CopySpellForEachItCouldTargetEffect ex FilterInPlay setFilter = filter.copy(); setFilter.add(new FromSetPredicate(targetCopyMap.keySet())); Target target = new TargetWithAdditionalFilter(sampleTarget, setFilter); + target.setNotTarget(false); // it is targeted, not chosen target.setMinNumberOfTargets(0); target.setMaxNumberOfTargets(1); - target.setTargetName(filter.getMessage() + " that " + spell.getLogName() + " could target (" + targetCopyMap.size() + " remaining)"); - + target.setTargetName(filter.getMessage() + " that " + spell.getLogName() + + " could target (" + targetCopyMap.size() + " remaining)"); // shortcut if there's only one possible target remaining if (targetCopyMap.size() > 1 && target.canChoose(spell.getId(), player.getId(), game)) { - player.choose(Outcome.Neutral, target, spell.getId(), game); + // The original "source" is not applicable here due to the spell being a copy. ie: Zada, Hedron Grinder + player.chooseTarget(Outcome.Neutral, target, spell.getSpellAbility(), game); // not source, but the spell that is copied } Collection chosenIds = target.getTargets(); if (chosenIds.isEmpty()) { chosenIds = targetCopyMap.keySet(); } - List toDelete = new ArrayList<>(); for (UUID chosenId : chosenIds) { Spell chosenCopy = targetCopyMap.get(chosenId); if (chosenCopy != null) { game.getStack().push(chosenCopy); + game.fireEvent(new GameEvent(GameEvent.EventType.COPIED_STACKOBJECT, + chosenCopy.getId(), spell.getId(), source.getControllerId())); toDelete.add(chosenId); madeACopy = true; } @@ -323,7 +328,8 @@ class TargetWithAdditionalFilter extends TargetImpl { @Override public FilterInPlay getFilter() { - return new CompoundFilter((FilterInPlay) originalTarget.getFilter(), additionalFilter, originalTarget.getFilter().getMessage()); + return new CompoundFilter((FilterInPlay) originalTarget.getFilter(), + additionalFilter, originalTarget.getFilter().getMessage()); } @Override