From 712cf4576d4f594547779fc375bbb1ae914e22c7 Mon Sep 17 00:00:00 2001 From: Oleg Agafonov Date: Fri, 5 Mar 2021 16:22:46 +0400 Subject: [PATCH] * Gain abilities - fixed that objects can get only one instance of triggered ability instead multiple (example: 2+ cascades from copies of Imoti, Celebrant of Bounty, f52753ad61d10b731999f9e0acfa60415bfbf902); --- .../mage/test/cards/copy/CopySpellTest.java | 58 +++++++++++++++++++ .../main/java/mage/abilities/AbilityImpl.java | 4 ++ .../mage/abilities/TriggeredAbilities.java | 5 +- .../effects/GainAbilitySpellsEffect.java | 3 - .../GainAbilityControlledSpellsEffect.java | 12 ++-- .../abilities/keyword/CascadeAbility.java | 21 +++++++ .../abilities/keyword/SuspendAbility.java | 2 - Mage/src/main/java/mage/game/GameState.java | 6 +- 8 files changed, 97 insertions(+), 14 deletions(-) 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 643a3d5c97..774033a9c9 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 @@ -404,4 +404,62 @@ public class CopySpellTest extends CardTestPlayerBase { assertPermanentCount(playerA, "Mountain", 1); assertPermanentCount(playerA, "Island", 1); } + + @Test + public void test_AllowsMultipleInstancesOfGainedTriggers() { + // bug: multiple copies of Imoti, Celebrant of Bounty only giving cascade once + // reason: gained ability used same id, so only one trigger were possible (now it uses new ids) + removeAllCardsFromHand(playerA); + removeAllCardsFromLibrary(playerA); + skipInitShuffling(); + + // Spells you cast with converted mana cost 6 or greater have cascade. + // Cascade + // (When you cast this spell exile cards from the top of your library until you exile a + // nonland card whose converted mana cost is less than this spell's converted mana cost. You may cast + // that spell without paying its mana cost if its converted mana cost is less than this spell's + // converted mana cost. Then put all cards exiled this way that weren't cast on the bottom of + // your library in a random order.) + addCard(Zone.BATTLEFIELD, playerA, "Imoti, Celebrant of Bounty", 1); // {3}{G}{U} + // + addCard(Zone.LIBRARY, playerA, "Swamp", 1); + addCard(Zone.LIBRARY, playerA, "Lightning Bolt", 1); + addCard(Zone.LIBRARY, playerA, "Swamp", 1); + addCard(Zone.LIBRARY, playerA, "Lightning Bolt", 1); + addCard(Zone.LIBRARY, playerA, "Swamp", 1); + // + // You may have Spark Double enter the battlefield as a copy of a creature or planeswalker you control, + // except it enters with an additional +1/+1 counter on it if it’s a creature, it enters with an + // additional loyalty counter on it if it’s a planeswalker, and it isn’t legendary if that + // permanent is legendary. + addCard(Zone.HAND, playerA, "Spark Double", 1); // {3}{U} + addCard(Zone.BATTLEFIELD, playerA, "Island", 4); + // + addCard(Zone.HAND, playerA, "Alpha Tyrranax", 1); // {4}{G}{G} + addCard(Zone.BATTLEFIELD, playerA, "Forest", 6); + + // cast spark and make imoti's copy + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Spark Double"); + setChoice(playerA, "Yes"); // use copy + setChoice(playerA, "Imoti, Celebrant of Bounty"); // copy of imoti + waitStackResolved(1, PhaseStep.PRECOMBAT_MAIN); + checkPermanentCount("after copy", 1, PhaseStep.PRECOMBAT_MAIN, playerA, "Imoti, Celebrant of Bounty", 2); + + // cast big spell and catch cascade 2x times (from two copies) + // possible bug: cascade activates only 1x times + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Alpha Tyrranax"); + checkStackSize("afer big spell", 1, PhaseStep.PRECOMBAT_MAIN, playerA, 3); + setChoice(playerA, "cascade"); // choice between 2x gained cascades + setChoice(playerA, "Yes"); // cast first bolt by first cascade + addTarget(playerA, playerB); // target for first bolt + setChoice(playerA, "Yes"); // cast second bold by second cascade + addTarget(playerA, playerB); // target for second bolt + + setStopAt(1, PhaseStep.END_TURN); + setStrictChooseMode(true); + execute(); + assertAllCommandsUsed(); + + assertLife(playerB, 20 - 3 * 2); // 2x bolts from 2x cascades + } } diff --git a/Mage/src/main/java/mage/abilities/AbilityImpl.java b/Mage/src/main/java/mage/abilities/AbilityImpl.java index ccf8f89897..beeb6601ed 100644 --- a/Mage/src/main/java/mage/abilities/AbilityImpl.java +++ b/Mage/src/main/java/mage/abilities/AbilityImpl.java @@ -142,6 +142,10 @@ public abstract class AbilityImpl implements Ability { this.id = UUID.randomUUID(); } getEffects().newId(); + + for (Ability sub : getSubAbilities()) { + sub.newId(); + } } @Override diff --git a/Mage/src/main/java/mage/abilities/TriggeredAbilities.java b/Mage/src/main/java/mage/abilities/TriggeredAbilities.java index 39d20f9f0d..cd1517868f 100644 --- a/Mage/src/main/java/mage/abilities/TriggeredAbilities.java +++ b/Mage/src/main/java/mage/abilities/TriggeredAbilities.java @@ -11,6 +11,7 @@ import mage.game.events.GameEvent; import mage.game.events.NumberOfTriggersEvent; import mage.game.permanent.Permanent; import mage.game.stack.Spell; +import org.apache.log4j.Logger; /** * @author BetaSteward_at_googlemail.com @@ -21,6 +22,8 @@ import mage.game.stack.Spell; */ public class TriggeredAbilities extends ConcurrentHashMap { + private static final Logger logger = Logger.getLogger(TriggeredAbilities.class); + private final Map> sources = new HashMap<>(); public TriggeredAbilities() { @@ -115,7 +118,7 @@ public class TriggeredAbilities extends ConcurrentHashMap uuidList = new LinkedList<>(); uuidList.add(sourceId); - // if the object that gained the ability moves zone, also then the triggered ability must be removed + // if the object that gained the ability moves from zone then the triggered ability must be removed uuidList.add(attachedTo.getId()); sources.put(getKey(ability, attachedTo), uuidList); } diff --git a/Mage/src/main/java/mage/abilities/effects/GainAbilitySpellsEffect.java b/Mage/src/main/java/mage/abilities/effects/GainAbilitySpellsEffect.java index 1e91ab6e79..78cbab355a 100644 --- a/Mage/src/main/java/mage/abilities/effects/GainAbilitySpellsEffect.java +++ b/Mage/src/main/java/mage/abilities/effects/GainAbilitySpellsEffect.java @@ -77,9 +77,6 @@ public class GainAbilitySpellsEffect extends ContinuousEffectImpl { if (card == null || !filter.match(stackObject, game)) { continue; } - if (ability instanceof MageSingleton && card.hasAbility(ability, game)) { - continue; - } game.getState().addOtherAbility(card, ability); } return true; diff --git a/Mage/src/main/java/mage/abilities/effects/common/continuous/GainAbilityControlledSpellsEffect.java b/Mage/src/main/java/mage/abilities/effects/common/continuous/GainAbilityControlledSpellsEffect.java index f147c58137..179d8194ff 100644 --- a/Mage/src/main/java/mage/abilities/effects/common/continuous/GainAbilityControlledSpellsEffect.java +++ b/Mage/src/main/java/mage/abilities/effects/common/continuous/GainAbilityControlledSpellsEffect.java @@ -65,7 +65,8 @@ public class GainAbilityControlledSpellsEffect extends ContinuousEffectImpl { } // workaround to gain cost reduction abilities to commanders before cast (make it playable) - game.getCommanderCardsFromCommandZone(player, CommanderCardType.ANY).stream() + game.getCommanderCardsFromCommandZone(player, CommanderCardType.ANY) + .stream() .filter(card -> filter.match(card, game)) .forEach(card -> { game.getState().addOtherAbility(card, ability); @@ -77,12 +78,9 @@ public class GainAbilityControlledSpellsEffect extends ContinuousEffectImpl { && !stackObject.isCopy() && stackObject.isControlledBy(source.getControllerId())) { Card card = game.getCard(stackObject.getSourceId()); - if (card != null - && filter.match(card, game)) { - if (!card.hasAbility(ability, game)) { - game.getState().addOtherAbility(card, ability); - return true; - } + if (card != null && filter.match(card, game)) { + game.getState().addOtherAbility(card, ability); + return true; } } } diff --git a/Mage/src/main/java/mage/abilities/keyword/CascadeAbility.java b/Mage/src/main/java/mage/abilities/keyword/CascadeAbility.java index 98cd411a3a..7a9322d406 100644 --- a/Mage/src/main/java/mage/abilities/keyword/CascadeAbility.java +++ b/Mage/src/main/java/mage/abilities/keyword/CascadeAbility.java @@ -20,12 +20,33 @@ import java.util.List; import java.util.stream.Collectors; /** + * Cascade + * A keyword ability that may let a player cast a random extra spell for no cost. See rule 702.84, “Cascade.” + *

+ * 702.84. Cascade + *

+ * 702.84a Cascade is a triggered ability that functions only while the spell with cascade is on the stack. + * “Cascade” means “When you cast this spell, exile cards from the top of your library until you exile a + * nonland card whose converted mana cost is less than this spell’s converted mana cost. You may cast that + * card without paying its mana cost. Then put all cards exiled this way that weren’t cast on the bottom + * of your library in a random order.” + *

+ * 702.84b If an effect allows a player to take an action with one or more of the exiled cards “as you cascade,” + * the player may take that action after they have finished exiling cards due to the cascade ability. This action + * is taken before choosing whether to cast the last exiled card or, if no appropriate card was exiled, before + * putting the exiled cards on the bottom of their library in a random order. + *

+ * 702.84c If a spell has multiple instances of cascade, each triggers separately. + * * @author BetaSteward_at_googlemail.com */ public class CascadeAbility extends TriggeredAbilityImpl { //20091005 - 702.82 //20210215 - 702.84a - Updated Cascade rule + // can't use singletone due rules: + // 702.84c If a spell has multiple instances of cascade, each triggers separately. + private static final String REMINDERTEXT = " (When you cast this spell, " + "exile cards from the top of your library until you exile a " + "nonland card whose converted mana cost is less than this spell's converted mana cost. " diff --git a/Mage/src/main/java/mage/abilities/keyword/SuspendAbility.java b/Mage/src/main/java/mage/abilities/keyword/SuspendAbility.java index 37e762baf1..1bf640ed2f 100644 --- a/Mage/src/main/java/mage/abilities/keyword/SuspendAbility.java +++ b/Mage/src/main/java/mage/abilities/keyword/SuspendAbility.java @@ -180,13 +180,11 @@ public class SuspendAbility extends SpecialAction { ability1.setSourceId(card.getId()); ability1.setControllerId(card.getOwnerId()); game.getState().addOtherAbility(card, ability1); - game.getState().addAbility(ability1, source.getSourceId(), card); SuspendPlayCardAbility ability2 = new SuspendPlayCardAbility(); ability2.setSourceId(card.getId()); ability2.setControllerId(card.getOwnerId()); game.getState().addOtherAbility(card, ability2); - game.getState().addAbility(ability2, source.getSourceId(), card); } public static UUID getSuspendExileId(UUID controllerId, Game game) { diff --git a/Mage/src/main/java/mage/game/GameState.java b/Mage/src/main/java/mage/game/GameState.java index 85848a7825..62b4c047f1 100644 --- a/Mage/src/main/java/mage/game/GameState.java +++ b/Mage/src/main/java/mage/game/GameState.java @@ -1081,7 +1081,8 @@ public class GameState implements Serializable, Copyable { * @param attachedTo * @param ability * @param copyAbility copies non MageSingleton abilities before adding to - * state + * state (allows to have multiple instances in one object, + * e.g. false param will simulate keyword/singletone) */ public void addOtherAbility(Card attachedTo, Ability ability, boolean copyAbility) { checkWrongDynamicAbilityUsage(attachedTo, ability); @@ -1090,7 +1091,10 @@ public class GameState implements Serializable, Copyable { if (ability instanceof MageSingleton || !copyAbility) { newAbility = ability; } else { + // must use new id, so you can add multiple instances of the same ability + // (example: gained Cascade from multiple Imoti, Celebrant of Bounty) newAbility = ability.copy(); + newAbility.newId(); } newAbility.setSourceId(attachedTo.getId()); newAbility.setControllerId(attachedTo.getOwnerId());