From 23ef0e4269944cce2300b7247ce4d2bf68857602 Mon Sep 17 00:00:00 2001 From: Oleg Agafonov Date: Wed, 18 Dec 2019 17:46:46 +0400 Subject: [PATCH] * Spark Double - fixed that copy of spark contains legendary type (#6097) --- Mage.Sets/src/mage/cards/s/SparkDouble.java | 30 +++--- .../mage/test/cards/copy/SparkDoubleTest.java | 92 +++++++++++++++++++ .../java/org/mage/test/player/TestPlayer.java | 14 ++- .../common/CreateTokenCopyTargetEffect.java | 3 +- Mage/src/main/java/mage/game/GameImpl.java | 5 +- .../util/functions/ApplyToMageObject.java | 17 +++- .../mage/util/functions/ApplyToPermanent.java | 6 +- 7 files changed, 143 insertions(+), 24 deletions(-) diff --git a/Mage.Sets/src/mage/cards/s/SparkDouble.java b/Mage.Sets/src/mage/cards/s/SparkDouble.java index b036665e2f..f83a602620 100644 --- a/Mage.Sets/src/mage/cards/s/SparkDouble.java +++ b/Mage.Sets/src/mage/cards/s/SparkDouble.java @@ -6,6 +6,7 @@ import mage.abilities.Ability; import mage.abilities.common.EntersBattlefieldAbility; import mage.abilities.effects.Effect; import mage.abilities.effects.common.CopyPermanentEffect; +import mage.abilities.effects.common.counter.AddCountersSourceEffect; import mage.cards.CardImpl; import mage.cards.CardSetInfo; import mage.constants.CardType; @@ -71,12 +72,16 @@ class SparkDoubleExceptEffectsApplyerToPermanent extends ApplyToPermanent { @Override public boolean apply(Game game, MageObject copyFromBlueprint, Ability source, UUID copyToObjectId) { - Permanent destCard = game.getPermanentEntering(copyToObjectId); - if (destCard == null) { - return false; - } + // copyToObjectId can be new token outside from game, don't use it // it isn’t legendary if that permanent is legendary + // + // Spark Double isn’t legendary if it copies a legendary permanent, and this exception is copiable. If something + // else copies Spark Double later, that copy also won’t be legendary. If you control two or more permanents + // with the same name but only one is legendary, the “legend rule” doesn’t apply. + // (2019-05-03) + // + // So, it's must make changes in blueprint (for farther copyable) copyFromBlueprint.getSuperType().remove(SuperType.LEGENDARY); // TODO: Blood Moon problem, can't apply on type changing effects (same as TeferisTimeTwist) @@ -89,14 +94,17 @@ class SparkDoubleExceptEffectsApplyerToPermanent extends ApplyToPermanent { // doesn't get a +1/+1 counter. On the other hand, if Spark Double copies Gideon Blackblade during your turn, // Spark Double enters as a planeswalker creature and gets both kinds of counters. - // enters with an additional +1/+1 counter on it if it’s a creature - if (copyFromBlueprint.isCreature()) { - destCard.addCounters(CounterType.P1P1.createInstance(), source, game); - } + // counters only for original card, not copies + if (!isCopyOfCopy(source, copyToObjectId)) { + // enters with an additional +1/+1 counter on it if it’s a creature + if (copyFromBlueprint.isCreature()) { + new AddCountersSourceEffect(CounterType.P1P1.createInstance(), false).apply(game, source); + } - // enters with an additional loyalty counter on it if it’s a planeswalker - if (copyFromBlueprint.isPlaneswalker()) { - destCard.addCounters(CounterType.LOYALTY.createInstance(), source, game); + // enters with an additional loyalty counter on it if it’s a planeswalker + if (copyFromBlueprint.isPlaneswalker()) { + new AddCountersSourceEffect(CounterType.LOYALTY.createInstance(), false).apply(game, source); + } } return true; diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/copy/SparkDoubleTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/copy/SparkDoubleTest.java index b11ff19852..e7f908f57c 100644 --- a/Mage.Tests/src/test/java/org/mage/test/cards/copy/SparkDoubleTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/cards/copy/SparkDoubleTest.java @@ -1,5 +1,9 @@ package org.mage.test.cards.copy; +import mage.abilities.Ability; +import mage.abilities.common.SimpleActivatedAbility; +import mage.abilities.costs.mana.ManaCostsImpl; +import mage.abilities.effects.common.CreateTokenCopyTargetEffect; import mage.abilities.keyword.VigilanceAbility; import mage.constants.CardType; import mage.constants.PhaseStep; @@ -7,6 +11,7 @@ import mage.constants.Zone; import mage.counters.CounterType; import mage.game.Game; import mage.game.permanent.Permanent; +import mage.target.TargetPermanent; import org.junit.Assert; import org.junit.Ignore; import org.junit.Test; @@ -154,4 +159,91 @@ public class SparkDoubleTest extends CardTestPlayerBase { Assert.assertEquals("must add 1 loyalty", 4 + 1, spark.getCounters(currentGame).getCount(CounterType.LOYALTY)); Assert.assertEquals("must add 1 creature counter", 1, spark.getCounters(currentGame).getCount(CounterType.P1P1)); } + + @Test + public void test_CopyOfSparksCopy_BySpell() { + + /* + Spark Double isn’t legendary if it copies a legendary permanent, and this exception is copiable. + If something else copies Spark Double later, that copy also won’t be legendary. + If you control two or more permanents with the same name but only one is legendary, the “legend rule” doesn’t apply. (2019-05-03) + + it's applier copy check + */ + + // + addCard(Zone.HAND, playerA, "Spark Double"); // {3}{U} + addCard(Zone.BATTLEFIELD, playerA, "Island", 4); + // + addCard(Zone.BATTLEFIELD, playerA, "Akroma, Angel of Wrath", 1); // legendary + // + // Create a 1/1 white Bird creature token with flying, then populate. (Create a token that’s a copy of a creature token you control.) + addCard(Zone.HAND, playerA, "Eyes in the Skies"); // {3}{W} + addCard(Zone.BATTLEFIELD, playerA, "Plains", 4); + // + // Create a token that’s a copy of target creature you control. + addCard(Zone.HAND, playerA, "Quasiduplicate"); // {1}{U}{U} + addCard(Zone.BATTLEFIELD, playerA, "Island", 3); + + // make copy of legendary creature (it's not legendary now) + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Spark Double"); + setChoice(playerA, "Yes"); + setChoice(playerA, "Akroma, Angel of Wrath"); + waitStackResolved(1, PhaseStep.PRECOMBAT_MAIN, playerA); + checkPermanentCount("must have copy", 1, PhaseStep.PRECOMBAT_MAIN, playerA, "Akroma, Angel of Wrath", 2); + + // make copy of copy by CreateTokenCopyTargetEffect + showBattlefield("before last copy", 1, PhaseStep.PRECOMBAT_MAIN, playerA); + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Quasiduplicate"); + addTarget(playerA, "Akroma, Angel of Wrath[only copy]"); + waitStackResolved(1, PhaseStep.PRECOMBAT_MAIN, playerA); + checkPermanentCount("must have copy", 1, PhaseStep.PRECOMBAT_MAIN, playerA, "Akroma, Angel of Wrath", 3); + + showBattlefield("after all", 1, PhaseStep.BEGIN_COMBAT, playerA); + + setStrictChooseMode(true); + setStopAt(1, PhaseStep.END_COMBAT); + execute(); + assertAllCommandsUsed(); + } + + @Test + public void test_CopyOfSparksCopy_ByAbility() { + Ability ability = new SimpleActivatedAbility(new CreateTokenCopyTargetEffect(), new ManaCostsImpl("")); + ability.addTarget(new TargetPermanent()); + addCustomCardWithAbility("copy", playerA, ability); + + addCard(Zone.HAND, playerA, "Spark Double"); // {3}{U} + addCard(Zone.BATTLEFIELD, playerA, "Island", 4); + // + addCard(Zone.BATTLEFIELD, playerA, "Akroma, Angel of Wrath", 1); // legendary + // + // Create a 1/1 white Bird creature token with flying, then populate. (Create a token that’s a copy of a creature token you control.) + addCard(Zone.HAND, playerA, "Eyes in the Skies"); // {3}{W} + addCard(Zone.BATTLEFIELD, playerA, "Plains", 4); + + // make copy of legendary creature (it's not legendary now) + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Spark Double"); + setChoice(playerA, "Yes"); + setChoice(playerA, "Akroma, Angel of Wrath"); + waitStackResolved(1, PhaseStep.PRECOMBAT_MAIN, playerA); + checkPermanentCount("must have copy", 1, PhaseStep.PRECOMBAT_MAIN, playerA, "Akroma, Angel of Wrath", 2); + + // make copy of copy by CreateTokenCopyTargetEffect + showBattlefield("before last copy", 1, PhaseStep.PRECOMBAT_MAIN, playerA); + showAvaileableAbilities("before last copy", 1, PhaseStep.PRECOMBAT_MAIN, playerA); + activateAbility(1, PhaseStep.PRECOMBAT_MAIN, playerA, "create a token that"); + addTarget(playerA, "Akroma, Angel of Wrath[only copy]"); + waitStackResolved(1, PhaseStep.PRECOMBAT_MAIN, playerA); + checkPermanentCount("must have copy", 1, PhaseStep.PRECOMBAT_MAIN, playerA, "Akroma, Angel of Wrath", 3); + + showBattlefield("after all", 1, PhaseStep.BEGIN_COMBAT, playerA); + + setStrictChooseMode(true); + setStopAt(1, PhaseStep.END_COMBAT); + execute(); + assertAllCommandsUsed(); + } + + } diff --git a/Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java b/Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java index 14834dc934..5674010fb7 100644 --- a/Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java +++ b/Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java @@ -41,6 +41,7 @@ import mage.game.draft.Draft; import mage.game.match.Match; import mage.game.match.MatchPlayer; import mage.game.permanent.Permanent; +import mage.game.permanent.PermanentToken; import mage.game.stack.StackObject; import mage.game.tournament.Tournament; import mage.player.ai.ComputerPlayer; @@ -886,11 +887,14 @@ public class TestPlayer implements Player { System.out.println("Total permanents: " + cards.size()); List data = cards.stream() - .map(c -> (c.getIdName() - + " - " + c.getPower().getValue() + "/" + c.getToughness().getValue() - + (c.isPlaneswalker() ? " - L" + c.getCounters(game).getCount(CounterType.LOYALTY) : "") - + ", " + (c.isTapped() ? "Tapped" : "Untapped") - + (c.getAttachedTo() == null ? "" : ", attached to " + game.getPermanent(c.getAttachedTo()).getIdName()) + .map(c -> ( + ((c instanceof PermanentToken) ? "[T] " : "[C] ") + + c.getIdName() + + (c.isCopy() ? " [copy of " + c.getCopyFrom().getId().toString().substring(0, 3) + "]" : "") + + " - " + c.getPower().getValue() + "/" + c.getToughness().getValue() + + (c.isPlaneswalker() ? " - L" + c.getCounters(game).getCount(CounterType.LOYALTY) : "") + + ", " + (c.isTapped() ? "Tapped" : "Untapped") + + (c.getAttachedTo() == null ? "" : ", attached to " + game.getPermanent(c.getAttachedTo()).getIdName()) )) .sorted() .collect(Collectors.toList()); diff --git a/Mage/src/main/java/mage/abilities/effects/common/CreateTokenCopyTargetEffect.java b/Mage/src/main/java/mage/abilities/effects/common/CreateTokenCopyTargetEffect.java index 7618dbd63b..acc93941dc 100644 --- a/Mage/src/main/java/mage/abilities/effects/common/CreateTokenCopyTargetEffect.java +++ b/Mage/src/main/java/mage/abilities/effects/common/CreateTokenCopyTargetEffect.java @@ -7,7 +7,6 @@ import mage.abilities.Mode; import mage.abilities.common.delayed.AtTheBeginOfNextEndStepDelayedTriggeredAbility; import mage.abilities.common.delayed.AtTheEndOfCombatDelayedTriggeredAbility; import mage.abilities.effects.ContinuousEffect; -import mage.abilities.effects.Effect; import mage.abilities.effects.OneShotEffect; import mage.abilities.keyword.FlyingAbility; import mage.abilities.keyword.HasteAbility; @@ -136,6 +135,8 @@ public class CreateTokenCopyTargetEffect extends OneShotEffect { } else { permanent = game.getPermanentOrLKIBattlefield(targetId); } + + // can target card or permanent Card copyFrom; ApplyToPermanent applier = new EmptyApplyToPermanent(); if (permanent != null) { diff --git a/Mage/src/main/java/mage/game/GameImpl.java b/Mage/src/main/java/mage/game/GameImpl.java index 8f4eafff6b..5209de1f2b 100644 --- a/Mage/src/main/java/mage/game/GameImpl.java +++ b/Mage/src/main/java/mage/game/GameImpl.java @@ -45,6 +45,7 @@ import mage.game.permanent.Permanent; import mage.game.permanent.PermanentCard; import mage.game.stack.Spell; import mage.game.stack.SpellStack; +import mage.game.stack.StackAbility; import mage.game.stack.StackObject; import mage.game.turn.Phase; import mage.game.turn.Step; @@ -1568,7 +1569,7 @@ public abstract class GameImpl implements Game, Serializable { } newBluePrint.assignNewId(); if (copyFromPermanent.isTransformed()) { - TransformAbility.transform(newBluePrint, copyFromPermanent.getSecondCardFace(), this); + TransformAbility.transform(newBluePrint, newBluePrint.getSecondCardFace(), this); } } if (applier != null) { @@ -1584,7 +1585,7 @@ public abstract class GameImpl implements Game, Serializable { Ability newAbility = source.copy(); newEffect.init(newAbility, this); - // If there are already copy effects with dration = Custom to the same object, remove the existing effects because they no longer have any effect + // If there are already copy effects with duration = Custom to the same object, remove the existing effects because they no longer have any effect if (duration == Duration.Custom) { for (Effect effect : getState().getContinuousEffects().getLayeredEffects(this)) { if (effect instanceof CopyEffect) { diff --git a/Mage/src/main/java/mage/util/functions/ApplyToMageObject.java b/Mage/src/main/java/mage/util/functions/ApplyToMageObject.java index 65d09ff911..ff827c01bb 100644 --- a/Mage/src/main/java/mage/util/functions/ApplyToMageObject.java +++ b/Mage/src/main/java/mage/util/functions/ApplyToMageObject.java @@ -1,16 +1,27 @@ - package mage.util.functions; -import java.util.UUID; import mage.MageObject; import mage.abilities.Ability; import mage.game.Game; +import java.util.Objects; +import java.util.UUID; + /** - * * @author LevelX2 */ public abstract class ApplyToMageObject { + // WARNING: + // 1. Applier uses for copy effects only; + // 2. It's applies to blueprint, not real object (real object is targetObjectId and can be card or token, even outside from game like EmptyToken); + // 3. "source" is current copy ability and can be different from original copy ability (copy of copy); + // 4. Don't use "source" param at all; + // 5. Use isCopyOfCopy() to detect it (some effects can applies to copy of copy, but other can't -- ses Spark Double as example). + // TODO: check all aplliers implementations - remove source uses, add isCopyOfCopy processing public abstract boolean apply(Game game, MageObject mageObject, Ability source, UUID targetObjectId); + + public boolean isCopyOfCopy(Ability source, UUID targetObjectId) { + return !Objects.equals(targetObjectId, source.getSourceId()); + } } diff --git a/Mage/src/main/java/mage/util/functions/ApplyToPermanent.java b/Mage/src/main/java/mage/util/functions/ApplyToPermanent.java index fa6947ee24..ffde9aeca0 100644 --- a/Mage/src/main/java/mage/util/functions/ApplyToPermanent.java +++ b/Mage/src/main/java/mage/util/functions/ApplyToPermanent.java @@ -1,15 +1,17 @@ package mage.util.functions; -import java.io.Serializable; -import java.util.UUID; import mage.abilities.Ability; import mage.game.Game; import mage.game.permanent.Permanent; +import java.io.Serializable; +import java.util.UUID; + /** * @author noxx */ public abstract class ApplyToPermanent extends ApplyToMageObject implements Serializable { + // WARNING: see comments in ApplyToMageObject public abstract boolean apply(Game game, Permanent permanent, Ability source, UUID targetObjectId); }