From ec4c2e21700867ed5dbd46ea616fc67565ed3b23 Mon Sep 17 00:00:00 2001 From: xenohedron Date: Sat, 24 Jun 2023 01:15:58 -0400 Subject: [PATCH] Cleanup: GainAbilityControlledSpellsEffect (#10446) * Hide reminder text on Zhulodok * Use logic from GainAbilitySpellsEffect, fix so that CastFromZonePredicate works * Text adjustments * Show cascade ability in hand for Abaddon the Despoiler * Remove redundant class * Simplify Cast Through Time * Don't add additional instances of redundant abilities * Remove redundant check * Add option to ignore mana validation when checking playable objects * Fix null errors * Fix GainAbilityControlledSpellsEffect to apply ability to playable cards rather than owned cards * Add unit test * Revert bad workaround code This reverts commit 17f5be6a79458f75094fe85608e4893ceb5e9d14. This reverts commit 7ebd2f1815bcf0e0ba90ad668fe88e790b5a804c. This reverts commit 00969d1fe726215952cce77760d6aaf916a14ae0. * Remove ownership check on exiled cards * Another test (currently failing) * ignore test * fix test: strict choose mode --- .../src/mage/cards/a/AbaddonTheDespoiler.java | 27 +++--- .../cards/c/CaetusSeaTyrantOfSegovia.java | 5 +- .../src/mage/cards/c/CastThroughTime.java | 72 ++-------------- Mage.Sets/src/mage/cards/c/ChiefEngineer.java | 2 +- .../src/mage/cards/f/FallajiWayfarer.java | 6 +- .../mage/cards/f/FiresongAndSunspeaker.java | 8 +- .../src/mage/cards/f/FlamekinHerald.java | 10 ++- .../src/mage/cards/h/HoardingBroodlord.java | 12 +-- .../mage/cards/i/ImotiCelebrantOfBounty.java | 9 +- .../src/mage/cards/i/InspiringStatuary.java | 2 +- .../src/mage/cards/m/MycosynthGolem.java | 2 +- .../src/mage/cards/p/PestilentSpirit.java | 10 +-- .../mage/cards/r/RadiantScrollwielder.java | 8 +- .../src/mage/cards/s/SoulfireGrandMaster.java | 7 +- .../cards/t/TezzeretMasterOfTheBridge.java | 2 +- .../src/mage/cards/t/TheFirstSliver.java | 2 +- .../src/mage/cards/t/ThrummingStone.java | 2 +- .../src/mage/cards/z/ZhulodokVoidGorger.java | 6 +- .../abilities/other/GainAbilitiesTest.java | 65 +++++++++++++++ .../effects/GainAbilitySpellsEffect.java | 83 ------------------- .../GainAbilityControlledSpellsEffect.java | 32 ++++--- Mage/src/main/java/mage/game/GameState.java | 8 +- 22 files changed, 154 insertions(+), 226 deletions(-) delete mode 100644 Mage/src/main/java/mage/abilities/effects/GainAbilitySpellsEffect.java diff --git a/Mage.Sets/src/mage/cards/a/AbaddonTheDespoiler.java b/Mage.Sets/src/mage/cards/a/AbaddonTheDespoiler.java index 20e74a9e3e7..131e01aec8c 100644 --- a/Mage.Sets/src/mage/cards/a/AbaddonTheDespoiler.java +++ b/Mage.Sets/src/mage/cards/a/AbaddonTheDespoiler.java @@ -6,23 +6,23 @@ import mage.abilities.common.SimpleStaticAbility; import mage.abilities.condition.common.MyTurnCondition; import mage.abilities.decorator.ConditionalContinuousEffect; import mage.abilities.dynamicvalue.common.OpponentsLostLifeCount; -import mage.abilities.effects.GainAbilitySpellsEffect; +import mage.abilities.effects.common.continuous.GainAbilityControlledSpellsEffect; import mage.abilities.hint.Hint; import mage.abilities.hint.ValueHint; import mage.abilities.keyword.CascadeAbility; import mage.abilities.keyword.TrampleAbility; +import mage.cards.Card; import mage.cards.CardImpl; import mage.cards.CardSetInfo; import mage.constants.CardType; import mage.constants.SubType; import mage.constants.SuperType; import mage.constants.Zone; -import mage.filter.FilterSpell; -import mage.filter.FilterStackObject; +import mage.filter.FilterCard; import mage.filter.predicate.Predicate; +import mage.filter.predicate.Predicates; import mage.filter.predicate.card.CastFromZonePredicate; import mage.game.Game; -import mage.game.stack.StackObject; import mage.watchers.common.PlayerLostLifeWatcher; import java.util.UUID; @@ -32,10 +32,11 @@ import java.util.UUID; */ public final class AbaddonTheDespoiler extends CardImpl { - private static final FilterStackObject filter = new FilterSpell(); + private static final FilterCard filter = new FilterCard(); static { filter.add(new CastFromZonePredicate(Zone.HAND)); + filter.add(Predicates.not(CardType.LAND.getPredicate())); filter.add(AbaddonTheDespoilerPredicate.instance); } @@ -57,7 +58,7 @@ public final class AbaddonTheDespoiler extends CardImpl { // Mark of Chaos Ascendant — During your turn, spells you cast from your hand with mana value X or less have cascade, where X is the total amount of life your opponents have lost this turn. this.addAbility(new SimpleStaticAbility(new ConditionalContinuousEffect( - new GainAbilitySpellsEffect(new CascadeAbility(false), filter), + new GainAbilityControlledSpellsEffect(new CascadeAbility(false), filter), MyTurnCondition.instance, "during your turn, spells you cast from " + "your hand with mana value X or less have cascade, where X is the " + "total amount of life your opponents have lost this turn" @@ -79,13 +80,13 @@ enum AbaddonTheDespoilerPredicate implements Predicate { @Override public boolean apply(MageObject input, Game game) { - if (!(input instanceof StackObject)) { - return false; + if (input instanceof Card) { + Card card = (Card) input; + return card.getManaValue() <= game + .getState() + .getWatcher(PlayerLostLifeWatcher.class) + .getAllOppLifeLost(card.getOwnerId(), game); } - StackObject stackObject = (StackObject) input; - return stackObject.getManaValue() <= game - .getState() - .getWatcher(PlayerLostLifeWatcher.class) - .getAllOppLifeLost(stackObject.getControllerId(), game); + return false; } } diff --git a/Mage.Sets/src/mage/cards/c/CaetusSeaTyrantOfSegovia.java b/Mage.Sets/src/mage/cards/c/CaetusSeaTyrantOfSegovia.java index 12c56c733b0..a83d1ba8917 100644 --- a/Mage.Sets/src/mage/cards/c/CaetusSeaTyrantOfSegovia.java +++ b/Mage.Sets/src/mage/cards/c/CaetusSeaTyrantOfSegovia.java @@ -4,11 +4,9 @@ import mage.MageInt; import mage.abilities.Ability; import mage.abilities.common.BeginningOfEndStepTriggeredAbility; import mage.abilities.common.SimpleStaticAbility; -import mage.abilities.effects.GainAbilitySpellsEffect; import mage.abilities.effects.common.UntapTargetEffect; import mage.abilities.effects.common.continuous.GainAbilityControlledSpellsEffect; import mage.abilities.keyword.ConvokeAbility; -import mage.abilities.keyword.ImproviseAbility; import mage.cards.CardImpl; import mage.cards.CardSetInfo; import mage.constants.CardType; @@ -16,7 +14,6 @@ import mage.constants.SubType; import mage.constants.SuperType; import mage.constants.TargetController; import mage.filter.FilterCard; -import mage.filter.FilterObject; import mage.filter.predicate.Predicates; import mage.filter.predicate.mageobject.AbilityPredicate; import mage.target.common.TargetCreaturePermanent; @@ -28,7 +25,7 @@ import java.util.UUID; */ public final class CaetusSeaTyrantOfSegovia extends CardImpl { - private static final FilterCard filter = new FilterCard("noncreature spells"); + private static final FilterCard filter = new FilterCard("noncreature spells you cast"); static { filter.add(Predicates.not(CardType.CREATURE.getPredicate())); diff --git a/Mage.Sets/src/mage/cards/c/CastThroughTime.java b/Mage.Sets/src/mage/cards/c/CastThroughTime.java index 718c36b76c5..0b600335f76 100644 --- a/Mage.Sets/src/mage/cards/c/CastThroughTime.java +++ b/Mage.Sets/src/mage/cards/c/CastThroughTime.java @@ -1,32 +1,27 @@ package mage.cards.c; -import java.util.Iterator; -import java.util.UUID; -import mage.abilities.Ability; import mage.abilities.common.SimpleStaticAbility; -import mage.abilities.effects.ContinuousEffectImpl; +import mage.abilities.effects.common.continuous.GainAbilityControlledSpellsEffect; import mage.abilities.keyword.ReboundAbility; -import mage.cards.Card; import mage.cards.CardImpl; import mage.cards.CardSetInfo; -import mage.constants.*; +import mage.constants.CardType; import mage.filter.FilterCard; import mage.filter.predicate.Predicates; -import mage.game.Game; -import mage.game.permanent.Permanent; -import mage.game.stack.Spell; -import mage.game.stack.StackObject; -import mage.players.Player; +import mage.filter.predicate.mageobject.AbilityPredicate; + +import java.util.UUID; /** * @author magenoxx_at_gmail.com */ public final class CastThroughTime extends CardImpl { - protected static final FilterCard filter = new FilterCard("Instant and sorcery spells you control"); + private static final FilterCard filter = new FilterCard("Instant and sorcery spells you control"); static { filter.add(Predicates.or(CardType.INSTANT.getPredicate(), CardType.SORCERY.getPredicate())); + filter.add(Predicates.not(new AbilityPredicate(ReboundAbility.class))); // So there are not redundant copies being added to each card } public CastThroughTime(UUID ownerId, CardSetInfo setInfo) { @@ -34,7 +29,7 @@ public final class CastThroughTime extends CardImpl { // Instant and sorcery spells you control have rebound. // (Exile the spell as it resolves if you cast it from your hand. At the beginning of your next upkeep, you may cast that card from exile without paying its mana cost.) - this.addAbility(new SimpleStaticAbility(Zone.BATTLEFIELD, new GainReboundEffect())); + this.addAbility(new SimpleStaticAbility(new GainAbilityControlledSpellsEffect(new ReboundAbility(), filter))); } private CastThroughTime(final CastThroughTime card) { @@ -46,54 +41,3 @@ public final class CastThroughTime extends CardImpl { return new CastThroughTime(this); } } - -class GainReboundEffect extends ContinuousEffectImpl { - - public GainReboundEffect() { - super(Duration.Custom, Layer.AbilityAddingRemovingEffects_6, SubLayer.NA, Outcome.AddAbility); - staticText = "Instant and sorcery spells you control have rebound (Exile the spell as it resolves if you cast it from your hand. At the beginning of your next upkeep, you may cast that card from exile without paying its mana cost.)"; - } - - public GainReboundEffect(final GainReboundEffect effect) { - super(effect); - } - - @Override - public GainReboundEffect copy() { - return new GainReboundEffect(this); - } - - @Override - public boolean apply(Game game, Ability source) { - Player player = game.getPlayer(source.getControllerId()); - Permanent permanent = game.getPermanent(source.getSourceId()); - if (player != null && permanent != null) { - for (Card card : player.getHand().getCards(CastThroughTime.filter, game)) { - addReboundAbility(card, game); - } - for (Iterator iterator = game.getStack().iterator(); iterator.hasNext();) { - StackObject stackObject = iterator.next(); - if (stackObject instanceof Spell && stackObject.isControlledBy(source.getControllerId())) { - Spell spell = (Spell) stackObject; - Card card = spell.getCard(); - if (card != null) { - addReboundAbility(card, game); - } - - } - } - return true; - } - return false; - } - - private void addReboundAbility(Card card, Game game) { - if (CastThroughTime.filter.match(card, game)) { - boolean found = card.getAbilities(game).containsClass(ReboundAbility.class); - if (!found) { - Ability ability = new ReboundAbility(); - game.getState().addOtherAbility(card, ability); - } - } - } -} diff --git a/Mage.Sets/src/mage/cards/c/ChiefEngineer.java b/Mage.Sets/src/mage/cards/c/ChiefEngineer.java index 155dc0c8607..a98d749a0ed 100644 --- a/Mage.Sets/src/mage/cards/c/ChiefEngineer.java +++ b/Mage.Sets/src/mage/cards/c/ChiefEngineer.java @@ -21,7 +21,7 @@ import java.util.UUID; */ public final class ChiefEngineer extends CardImpl { - private static final FilterCard filter = new FilterArtifactCard("artifact spells"); + private static final FilterCard filter = new FilterArtifactCard("artifact spells you cast"); static { filter.add(Predicates.not(CardType.LAND.getPredicate())); diff --git a/Mage.Sets/src/mage/cards/f/FallajiWayfarer.java b/Mage.Sets/src/mage/cards/f/FallajiWayfarer.java index 7d5ae19bae4..7df1a850de1 100644 --- a/Mage.Sets/src/mage/cards/f/FallajiWayfarer.java +++ b/Mage.Sets/src/mage/cards/f/FallajiWayfarer.java @@ -12,6 +12,8 @@ import mage.constants.SubType; import mage.constants.Zone; import mage.filter.FilterCard; import mage.filter.FilterMana; +import mage.filter.predicate.Predicates; +import mage.filter.predicate.mageobject.AbilityPredicate; import mage.filter.predicate.mageobject.MulticoloredPredicate; import java.util.UUID; @@ -21,10 +23,12 @@ import java.util.UUID; */ public final class FallajiWayfarer extends CardImpl { - private static final FilterCard filter = new FilterCard("multicolored spells"); + private static final FilterCard filter = new FilterCard("multicolored spells you cast"); static { filter.add(MulticoloredPredicate.instance); + filter.add(Predicates.not(CardType.LAND.getPredicate())); + filter.add(Predicates.not(new AbilityPredicate(ConvokeAbility.class))); // So there are not redundant copies being added to each card } public FallajiWayfarer(UUID ownerId, CardSetInfo setInfo) { diff --git a/Mage.Sets/src/mage/cards/f/FiresongAndSunspeaker.java b/Mage.Sets/src/mage/cards/f/FiresongAndSunspeaker.java index ec0163115fa..a58d0fc02ed 100644 --- a/Mage.Sets/src/mage/cards/f/FiresongAndSunspeaker.java +++ b/Mage.Sets/src/mage/cards/f/FiresongAndSunspeaker.java @@ -9,13 +9,13 @@ import mage.ObjectColor; import mage.abilities.TriggeredAbilityImpl; import mage.abilities.common.SimpleStaticAbility; import mage.abilities.effects.Effect; -import mage.abilities.effects.GainAbilitySpellsEffect; import mage.abilities.effects.common.DamageTargetEffect; +import mage.abilities.effects.common.continuous.GainAbilityControlledSpellsEffect; import mage.abilities.keyword.LifelinkAbility; import mage.constants.*; import mage.cards.CardImpl; import mage.cards.CardSetInfo; -import mage.filter.FilterObject; +import mage.filter.FilterCard; import mage.filter.predicate.Predicates; import mage.filter.predicate.mageobject.ColorPredicate; import mage.game.Game; @@ -28,7 +28,7 @@ import mage.target.common.TargetCreatureOrPlayer; */ public final class FiresongAndSunspeaker extends CardImpl { - private static final FilterObject filter = new FilterObject("Red instant and sorcery spells you control"); + private static final FilterCard filter = new FilterCard("red instant and sorcery spells you control"); static { filter.add(new ColorPredicate(ObjectColor.RED)); @@ -45,7 +45,7 @@ public final class FiresongAndSunspeaker extends CardImpl { this.toughness = new MageInt(6); // Red instant and sorcery spells you control have lifelink. - Effect effect = new GainAbilitySpellsEffect(LifelinkAbility.getInstance(), filter); + Effect effect = new GainAbilityControlledSpellsEffect(LifelinkAbility.getInstance(), filter); effect.setText("Red instant and sorcery spells you control have lifelink"); this.addAbility(new SimpleStaticAbility(Zone.BATTLEFIELD, effect)); // Whenever a white instant or sorcery spell causes you to gain life, Firesong and Sunspeaker deals 3 damage to target creature or player. diff --git a/Mage.Sets/src/mage/cards/f/FlamekinHerald.java b/Mage.Sets/src/mage/cards/f/FlamekinHerald.java index 256420b7072..07bb4023070 100644 --- a/Mage.Sets/src/mage/cards/f/FlamekinHerald.java +++ b/Mage.Sets/src/mage/cards/f/FlamekinHerald.java @@ -2,13 +2,14 @@ package mage.cards.f; import mage.MageInt; import mage.abilities.common.SimpleStaticAbility; -import mage.abilities.effects.GainAbilitySpellsEffect; +import mage.abilities.effects.common.continuous.GainAbilityControlledSpellsEffect; import mage.abilities.keyword.CascadeAbility; import mage.cards.CardImpl; import mage.cards.CardSetInfo; import mage.constants.CardType; import mage.constants.SubType; -import mage.filter.FilterSpell; +import mage.filter.FilterCard; +import mage.filter.predicate.Predicates; import mage.filter.predicate.mageobject.CommanderPredicate; import java.util.UUID; @@ -18,10 +19,11 @@ import java.util.UUID; */ public final class FlamekinHerald extends CardImpl { - private static final FilterSpell filter = new FilterSpell("Commander spells you cast"); + private static final FilterCard filter = new FilterCard("Commander spells you cast"); static { filter.add(CommanderPredicate.instance); + filter.add(Predicates.not(CardType.LAND.getPredicate())); } public FlamekinHerald(UUID ownerId, CardSetInfo setInfo) { @@ -33,7 +35,7 @@ public final class FlamekinHerald extends CardImpl { this.toughness = new MageInt(2); // Commander spells you cast have cascade. - this.addAbility(new SimpleStaticAbility(new GainAbilitySpellsEffect(new CascadeAbility(false), filter))); + this.addAbility(new SimpleStaticAbility(new GainAbilityControlledSpellsEffect(new CascadeAbility(false), filter))); } private FlamekinHerald(final FlamekinHerald card) { diff --git a/Mage.Sets/src/mage/cards/h/HoardingBroodlord.java b/Mage.Sets/src/mage/cards/h/HoardingBroodlord.java index 439fe5086ca..1c5e4a24c0b 100644 --- a/Mage.Sets/src/mage/cards/h/HoardingBroodlord.java +++ b/Mage.Sets/src/mage/cards/h/HoardingBroodlord.java @@ -4,17 +4,18 @@ import mage.MageInt; import mage.abilities.Ability; import mage.abilities.common.EntersBattlefieldTriggeredAbility; import mage.abilities.common.SimpleStaticAbility; -import mage.abilities.effects.GainAbilitySpellsEffect; import mage.abilities.effects.OneShotEffect; +import mage.abilities.effects.common.continuous.GainAbilityControlledSpellsEffect; import mage.abilities.keyword.ConvokeAbility; import mage.abilities.keyword.FlyingAbility; import mage.cards.Card; import mage.cards.CardImpl; import mage.cards.CardSetInfo; import mage.constants.*; -import mage.filter.FilterObject; -import mage.filter.FilterSpell; +import mage.filter.FilterCard; +import mage.filter.predicate.Predicates; import mage.filter.predicate.card.CastFromZonePredicate; +import mage.filter.predicate.mageobject.AbilityPredicate; import mage.game.Game; import mage.players.Player; import mage.target.common.TargetCardInLibrary; @@ -27,10 +28,11 @@ import java.util.UUID; */ public final class HoardingBroodlord extends CardImpl { - private static final FilterObject filter = new FilterSpell("spells you cast from exile"); + private static final FilterCard filter = new FilterCard("spells you cast from exile"); static { filter.add(new CastFromZonePredicate(Zone.EXILED)); + filter.add(Predicates.not(new AbilityPredicate(ConvokeAbility.class))); // So there are not redundant copies being added to each card } public HoardingBroodlord(UUID ownerId, CardSetInfo setInfo) { @@ -50,7 +52,7 @@ public final class HoardingBroodlord extends CardImpl { this.addAbility(new EntersBattlefieldTriggeredAbility(new HoardingBroodlordEffect())); // Spells you cast from exile have convoke. - this.addAbility(new SimpleStaticAbility(new GainAbilitySpellsEffect(new ConvokeAbility(), filter))); + this.addAbility(new SimpleStaticAbility(new GainAbilityControlledSpellsEffect(new ConvokeAbility(), filter))); } private HoardingBroodlord(final HoardingBroodlord card) { diff --git a/Mage.Sets/src/mage/cards/i/ImotiCelebrantOfBounty.java b/Mage.Sets/src/mage/cards/i/ImotiCelebrantOfBounty.java index 32a89d4b7bb..a9675a5bafa 100644 --- a/Mage.Sets/src/mage/cards/i/ImotiCelebrantOfBounty.java +++ b/Mage.Sets/src/mage/cards/i/ImotiCelebrantOfBounty.java @@ -2,7 +2,7 @@ package mage.cards.i; import mage.MageInt; import mage.abilities.common.SimpleStaticAbility; -import mage.abilities.effects.GainAbilitySpellsEffect; +import mage.abilities.effects.common.continuous.GainAbilityControlledSpellsEffect; import mage.abilities.keyword.CascadeAbility; import mage.cards.CardImpl; import mage.cards.CardSetInfo; @@ -10,7 +10,7 @@ import mage.constants.CardType; import mage.constants.ComparisonType; import mage.constants.SubType; import mage.constants.SuperType; -import mage.filter.FilterObject; +import mage.filter.FilterCard; import mage.filter.predicate.mageobject.ManaValuePredicate; import java.util.UUID; @@ -20,8 +20,7 @@ import java.util.UUID; */ public final class ImotiCelebrantOfBounty extends CardImpl { - private static final FilterObject filter - = new FilterObject("Spells you cast with mana value 6 or greater"); + private static final FilterCard filter = new FilterCard("spells you cast with mana value 6 or greater"); static { filter.add(new ManaValuePredicate(ComparisonType.MORE_THAN, 5)); @@ -41,7 +40,7 @@ public final class ImotiCelebrantOfBounty extends CardImpl { // Spells you cast with converted mana cost 6 or greater have cascade. this.addAbility(new SimpleStaticAbility( - new GainAbilitySpellsEffect(new CascadeAbility(false), filter) + new GainAbilityControlledSpellsEffect(new CascadeAbility(false), filter) )); } diff --git a/Mage.Sets/src/mage/cards/i/InspiringStatuary.java b/Mage.Sets/src/mage/cards/i/InspiringStatuary.java index a0939365400..6ece03d7daf 100644 --- a/Mage.Sets/src/mage/cards/i/InspiringStatuary.java +++ b/Mage.Sets/src/mage/cards/i/InspiringStatuary.java @@ -18,7 +18,7 @@ import java.util.UUID; */ public final class InspiringStatuary extends CardImpl { - private static final FilterCard filter = new FilterCard("nonartifact spells"); + private static final FilterCard filter = new FilterCard("nonartifact spells you cast"); static { filter.add(Predicates.not(CardType.ARTIFACT.getPredicate())); diff --git a/Mage.Sets/src/mage/cards/m/MycosynthGolem.java b/Mage.Sets/src/mage/cards/m/MycosynthGolem.java index a94cd570dcd..b663c77f676 100644 --- a/Mage.Sets/src/mage/cards/m/MycosynthGolem.java +++ b/Mage.Sets/src/mage/cards/m/MycosynthGolem.java @@ -18,7 +18,7 @@ import java.util.UUID; */ public final class MycosynthGolem extends CardImpl { - private static final FilterCard filter = new FilterCard("Artifact creature spells"); + private static final FilterCard filter = new FilterCard("Artifact creature spells you cast"); static { filter.add(CardType.ARTIFACT.getPredicate()); diff --git a/Mage.Sets/src/mage/cards/p/PestilentSpirit.java b/Mage.Sets/src/mage/cards/p/PestilentSpirit.java index c051388cc0e..d8842ffcd44 100644 --- a/Mage.Sets/src/mage/cards/p/PestilentSpirit.java +++ b/Mage.Sets/src/mage/cards/p/PestilentSpirit.java @@ -2,7 +2,7 @@ package mage.cards.p; import mage.MageInt; import mage.abilities.common.SimpleStaticAbility; -import mage.abilities.effects.GainAbilitySpellsEffect; +import mage.abilities.effects.common.continuous.GainAbilityControlledSpellsEffect; import mage.abilities.keyword.DeathtouchAbility; import mage.abilities.keyword.MenaceAbility; import mage.cards.CardImpl; @@ -10,7 +10,7 @@ import mage.cards.CardSetInfo; import mage.constants.CardType; import mage.constants.SubType; import mage.constants.Zone; -import mage.filter.FilterObject; +import mage.filter.FilterCard; import mage.filter.predicate.Predicates; import java.util.UUID; @@ -20,7 +20,7 @@ import java.util.UUID; */ public final class PestilentSpirit extends CardImpl { - private static final FilterObject filter = new FilterObject("instant and sorcery spells you control"); + private static final FilterCard filter = new FilterCard("instant and sorcery spells you control"); static { filter.add(Predicates.or( @@ -45,9 +45,7 @@ public final class PestilentSpirit extends CardImpl { // Instant and sorcery spells you control have deathtouch. this.addAbility(new SimpleStaticAbility( Zone.BATTLEFIELD, - new GainAbilitySpellsEffect( - DeathtouchAbility.getInstance(), filter - ).setText("Instant and sorcery spells you control have deathtouch") + new GainAbilityControlledSpellsEffect(DeathtouchAbility.getInstance(), filter) )); } diff --git a/Mage.Sets/src/mage/cards/r/RadiantScrollwielder.java b/Mage.Sets/src/mage/cards/r/RadiantScrollwielder.java index 7c9eae74af4..de0c34dc7d9 100644 --- a/Mage.Sets/src/mage/cards/r/RadiantScrollwielder.java +++ b/Mage.Sets/src/mage/cards/r/RadiantScrollwielder.java @@ -5,16 +5,16 @@ import mage.MageObjectReference; import mage.abilities.Ability; import mage.abilities.common.BeginningOfUpkeepTriggeredAbility; import mage.abilities.common.SimpleStaticAbility; -import mage.abilities.effects.GainAbilitySpellsEffect; import mage.abilities.effects.OneShotEffect; import mage.abilities.effects.ReplacementEffectImpl; import mage.abilities.effects.common.asthought.PlayFromNotOwnHandZoneTargetEffect; +import mage.abilities.effects.common.continuous.GainAbilityControlledSpellsEffect; import mage.abilities.keyword.LifelinkAbility; import mage.cards.Card; import mage.cards.CardImpl; import mage.cards.CardSetInfo; import mage.constants.*; -import mage.filter.FilterObject; +import mage.filter.FilterCard; import mage.filter.StaticFilters; import mage.filter.predicate.Predicates; import mage.game.Game; @@ -33,7 +33,7 @@ import java.util.UUID; */ public final class RadiantScrollwielder extends CardImpl { - private static final FilterObject filter = new FilterObject("instant and sorcery spells you control"); + private static final FilterCard filter = new FilterCard("instant and sorcery spells you control"); static { filter.add(Predicates.or( @@ -51,7 +51,7 @@ public final class RadiantScrollwielder extends CardImpl { this.toughness = new MageInt(4); // Instant and sorcery spells you control have lifelink. - this.addAbility(new SimpleStaticAbility(new GainAbilitySpellsEffect( + this.addAbility(new SimpleStaticAbility(new GainAbilityControlledSpellsEffect( LifelinkAbility.getInstance(), filter ).setText("instant and sorcery spells you control have lifelink"))); diff --git a/Mage.Sets/src/mage/cards/s/SoulfireGrandMaster.java b/Mage.Sets/src/mage/cards/s/SoulfireGrandMaster.java index d8df2ae8249..f43a8e82313 100644 --- a/Mage.Sets/src/mage/cards/s/SoulfireGrandMaster.java +++ b/Mage.Sets/src/mage/cards/s/SoulfireGrandMaster.java @@ -7,15 +7,14 @@ import mage.abilities.common.SimpleActivatedAbility; import mage.abilities.common.SimpleStaticAbility; import mage.abilities.costs.mana.ManaCostsImpl; import mage.abilities.effects.Effect; -import mage.abilities.effects.GainAbilitySpellsEffect; import mage.abilities.effects.ReplacementEffectImpl; +import mage.abilities.effects.common.continuous.GainAbilityControlledSpellsEffect; import mage.abilities.keyword.LifelinkAbility; import mage.cards.Card; import mage.cards.CardImpl; import mage.cards.CardSetInfo; import mage.constants.*; import mage.filter.FilterCard; -import mage.filter.FilterObject; import mage.filter.predicate.Predicates; import mage.game.Game; import mage.game.events.GameEvent; @@ -30,7 +29,7 @@ import java.util.UUID; */ public final class SoulfireGrandMaster extends CardImpl { - private static final FilterObject filter = new FilterObject("instant and sorcery spells you control"); + private static final FilterCard filter = new FilterCard("instant and sorcery spells you control"); static { filter.add(Predicates.or(CardType.INSTANT.getPredicate(), CardType.SORCERY.getPredicate())); @@ -47,7 +46,7 @@ public final class SoulfireGrandMaster extends CardImpl { this.addAbility(LifelinkAbility.getInstance()); // Instant and sorcery spells you control have lifelink. - Effect effect = new GainAbilitySpellsEffect(LifelinkAbility.getInstance(), filter); + Effect effect = new GainAbilityControlledSpellsEffect(LifelinkAbility.getInstance(), filter); effect.setText("Instant and sorcery spells you control have lifelink"); this.addAbility(new SimpleStaticAbility(Zone.BATTLEFIELD, effect)); diff --git a/Mage.Sets/src/mage/cards/t/TezzeretMasterOfTheBridge.java b/Mage.Sets/src/mage/cards/t/TezzeretMasterOfTheBridge.java index 9f7f47a11e8..a38b59f18d3 100644 --- a/Mage.Sets/src/mage/cards/t/TezzeretMasterOfTheBridge.java +++ b/Mage.Sets/src/mage/cards/t/TezzeretMasterOfTheBridge.java @@ -26,7 +26,7 @@ import java.util.UUID; */ public final class TezzeretMasterOfTheBridge extends CardImpl { - private static final FilterCard filter = new FilterCard("creature and planeswalker spells"); + private static final FilterCard filter = new FilterCard("creature and planeswalker spells you cast"); static { filter.add(Predicates.or( diff --git a/Mage.Sets/src/mage/cards/t/TheFirstSliver.java b/Mage.Sets/src/mage/cards/t/TheFirstSliver.java index e19ac7da362..ebd21c23252 100644 --- a/Mage.Sets/src/mage/cards/t/TheFirstSliver.java +++ b/Mage.Sets/src/mage/cards/t/TheFirstSliver.java @@ -18,7 +18,7 @@ import java.util.UUID; */ public final class TheFirstSliver extends CardImpl { - private static final FilterCard filter = new FilterCard("Sliver spells"); + private static final FilterCard filter = new FilterCard("Sliver spells you cast"); static { filter.add(SubType.SLIVER.getPredicate()); diff --git a/Mage.Sets/src/mage/cards/t/ThrummingStone.java b/Mage.Sets/src/mage/cards/t/ThrummingStone.java index 0f20a22da3b..e10791ba138 100644 --- a/Mage.Sets/src/mage/cards/t/ThrummingStone.java +++ b/Mage.Sets/src/mage/cards/t/ThrummingStone.java @@ -22,7 +22,7 @@ public final class ThrummingStone extends CardImpl { this.supertype.add(SuperType.LEGENDARY); // Spells you cast have Ripple 4 - this.addAbility(new SimpleStaticAbility(Zone.BATTLEFIELD, new GainAbilityControlledSpellsEffect(new RippleAbility(4), new FilterCard("Spells")))); + this.addAbility(new SimpleStaticAbility(Zone.BATTLEFIELD, new GainAbilityControlledSpellsEffect(new RippleAbility(4), new FilterCard("Spells you cast")))); } private ThrummingStone(final ThrummingStone card) { diff --git a/Mage.Sets/src/mage/cards/z/ZhulodokVoidGorger.java b/Mage.Sets/src/mage/cards/z/ZhulodokVoidGorger.java index 8d45a9e3e05..3e91d8eff35 100644 --- a/Mage.Sets/src/mage/cards/z/ZhulodokVoidGorger.java +++ b/Mage.Sets/src/mage/cards/z/ZhulodokVoidGorger.java @@ -20,7 +20,7 @@ import java.util.UUID; */ public final class ZhulodokVoidGorger extends CardImpl { - private static final FilterCard filter = new FilterCard(); + private static final FilterCard filter = new FilterCard("colorless spells you cast from your hand with mana value 7 or greater"); static { filter.add(ColorlessPredicate.instance); @@ -37,9 +37,9 @@ public final class ZhulodokVoidGorger extends CardImpl { this.toughness = new MageInt(4); // Colorless spells you cast from your hand with mana value 7 or greater have "Cascade, cascade." - Ability ability = new SimpleStaticAbility(new GainAbilityControlledSpellsEffect(new CascadeAbility(), filter) + Ability ability = new SimpleStaticAbility(new GainAbilityControlledSpellsEffect(new CascadeAbility(false), filter) .setText("colorless spells you cast from your hand with mana value 7")); - ability.addEffect(new GainAbilityControlledSpellsEffect(new CascadeAbility(), filter) + ability.addEffect(new GainAbilityControlledSpellsEffect(new CascadeAbility(false), filter) .setText("or greater have \"Cascade, cascade.\"")); this.addAbility(ability); } diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/abilities/other/GainAbilitiesTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/abilities/other/GainAbilitiesTest.java index f7a7cb6078d..89f063efd60 100644 --- a/Mage.Tests/src/test/java/org/mage/test/cards/abilities/other/GainAbilitiesTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/cards/abilities/other/GainAbilitiesTest.java @@ -66,6 +66,71 @@ public class GainAbilitiesTest extends CardTestPlayerBase { ).count()); } + @Test + public void testGainAbilityControlledSpells() { + removeAllCardsFromLibrary(playerA); + skipInitShuffling(); + + addCard(Zone.GRAVEYARD, playerA, "Hoarding Broodlord"); // gives Pestilent Spirit convoke + addCard(Zone.HAND, playerA, "Reanimate"); // to put Hoarding Broodlord in play + addCard(Zone.BATTLEFIELD, playerA, "Swamp", 1); // to cast Reanimate + addCard(Zone.BATTLEFIELD, playerA, "Firesong and Sunspeaker"); // gives Shock lifelink + addCard(Zone.LIBRARY, playerA, "Shock", 1); // to find with Hoarding Broodlord + addCard(Zone.HAND, playerA, "Covetous Urge"); // makes Pestilent Spirit castable from exile + addCard(Zone.BATTLEFIELD, playerA, "Island", 4); // to cast Covetous Urge + addCard(Zone.HAND, playerB, "Pestilent Spirit"); // gives Shock deathtouch + addCard(Zone.BATTLEFIELD, playerA, "Sol Ring"); // to cast Pestilent Spirit + + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Reanimate", "Hoarding Broodlord"); // tap Swamp, lose 8 life, find Shock + addTarget(playerA, "Shock"); + waitStackResolved(1, PhaseStep.PRECOMBAT_MAIN); + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Covetous Urge", playerB); // tap four Islands, find Pestilent Spirit + setChoice(playerA, "Pestilent Spirit"); + waitStackResolved(1, PhaseStep.PRECOMBAT_MAIN); + activateManaAbility(1, PhaseStep.PRECOMBAT_MAIN, playerA, "{T}: Add {C}{C}", 1); + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Pestilent Spirit"); // tap Sol Ring and Hoarding Broodlord + addTarget(playerA, "Hoarding Broodlord"); // convoke to pay B + waitStackResolved(1, PhaseStep.PRECOMBAT_MAIN); + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Shock", "Firesong and Sunspeaker"); // convoke, lethal, gain 2 life + addTarget(playerA, "Firesong and Sunspeaker"); // convoke to pay R + + setStrictChooseMode(true); + setStopAt(1, PhaseStep.BEGIN_COMBAT); + execute(); + + assertLife(playerA, 20 - 8 + 2); // confirms lifelink ability was added to Shock + assertGraveyardCount(playerA, "Firesong and Sunspeaker", 1); // must be lethal damage, confirms deathtouch ability added + assertTapped("Hoarding Broodlord", true); // confirms convoke ability added + + } + + @Ignore + // TODO: GainAbilityControlledSpellsEffect needs improvement to properly apply only to playable cards in non-hand zones + // TODO: Figure out how to make the ability apply to the reflexive trigger + @Test + public void testGainAbilityControlledSpellsOnly() { + + addCard(Zone.BATTLEFIELD, playerB, "Firesong and Sunspeaker"); // shouldn't give Searing Blood lifelink + addCard(Zone.BATTLEFIELD, playerA, "Walking Corpse"); // creature to target + addCard(Zone.HAND, playerA, "Covetous Urge"); // makes Searing Blood castable from exile + addCard(Zone.BATTLEFIELD, playerA, "Island", 4); // to cast Covetous Urge + addCard(Zone.HAND, playerB, "Searing Blood"); // to find with Covetous Urge + addCard(Zone.BATTLEFIELD, playerA, "Mountain", 2); // to cast Searing Blood + + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Covetous Urge", playerB); // tap four Islands, find Searing Blood + setChoice(playerA, "Searing Blood"); + waitStackResolved(1, PhaseStep.PRECOMBAT_MAIN); + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Searing Blood", "Walking Corpse"); + + setStrictChooseMode(true); + setStopAt(1, PhaseStep.BEGIN_COMBAT); + execute(); + + assertGraveyardCount(playerA, "Walking Corpse", 1); + assertLife(playerB, 20); // lifelink should not apply + assertLife(playerA, 20 - 3); + + } /** * Reported bug: https://github.com/magefree/mage/issues/9565 diff --git a/Mage/src/main/java/mage/abilities/effects/GainAbilitySpellsEffect.java b/Mage/src/main/java/mage/abilities/effects/GainAbilitySpellsEffect.java deleted file mode 100644 index 45e00330732..00000000000 --- a/Mage/src/main/java/mage/abilities/effects/GainAbilitySpellsEffect.java +++ /dev/null @@ -1,83 +0,0 @@ -package mage.abilities.effects; - -import mage.abilities.Ability; -import mage.cards.Card; -import mage.constants.*; -import mage.filter.FilterObject; -import mage.game.Game; -import mage.game.permanent.Permanent; -import mage.game.stack.StackObject; -import mage.players.Player; - -public class GainAbilitySpellsEffect extends ContinuousEffectImpl { - - private final Ability ability; - private final FilterObject filter; - - public GainAbilitySpellsEffect(Ability ability, FilterObject filter) { - super(Duration.WhileOnBattlefield, Layer.AbilityAddingRemovingEffects_6, SubLayer.NA, Outcome.AddAbility); - this.ability = ability; - this.filter = filter; - staticText = filter.getMessage() + " have " + ability.getRule(); - } - - private GainAbilitySpellsEffect(final GainAbilitySpellsEffect effect) { - super(effect); - this.ability = effect.ability; - this.filter = effect.filter; - } - - @Override - public GainAbilitySpellsEffect copy() { - return new GainAbilitySpellsEffect(this); - } - - @Override - public boolean apply(Game game, Ability source) { - Player player = game.getPlayer(source.getControllerId()); - Permanent permanent = game.getPermanent(source.getSourceId()); - if (player == null || permanent == null) { - return false; - } - for (Card card : game.getExile().getAllCards(game)) { - if (card.isOwnedBy(source.getControllerId()) && filter.match(card, game)) { - game.getState().addOtherAbility(card, ability); - } - } - for (Card card : player.getLibrary().getCards(game)) { - if (filter.match(card, game)) { - game.getState().addOtherAbility(card, ability); - } - } - for (Card card : player.getHand().getCards(game)) { - if (filter.match(card, game)) { - game.getState().addOtherAbility(card, ability); - } - } - for (Card card : player.getGraveyard().getCards(game)) { - if (filter.match(card, game)) { - game.getState().addOtherAbility(card, ability); - } - } - - // workaround to gain cost reduction abilities to commanders before cast (make it playable) - game.getCommanderCardsFromCommandZone(player, CommanderCardType.ANY) - .stream() - .filter(card -> filter.match(card, game)) - .forEach(card -> { - game.getState().addOtherAbility(card, ability); - }); - - for (StackObject stackObject : game.getStack()) { - if (!stackObject.isControlledBy(source.getControllerId())) { - continue; - } - Card card = game.getCard(stackObject.getSourceId()); - if (card == null || !filter.match(stackObject, 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 d51d26b7081..26918daff86 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 @@ -23,10 +23,10 @@ public class GainAbilityControlledSpellsEffect extends ContinuousEffectImpl { super(Duration.WhileOnBattlefield, Layer.AbilityAddingRemovingEffects_6, SubLayer.NA, Outcome.AddAbility); this.ability = ability; this.filter = filter; - staticText = filter.getMessage() + " you cast have " + ability.getRule() + '.'; + staticText = filter.getMessage() + " have " + ability.getRule(); } - public GainAbilityControlledSpellsEffect(final GainAbilityControlledSpellsEffect effect) { + private GainAbilityControlledSpellsEffect(final GainAbilityControlledSpellsEffect effect) { super(effect); this.ability = effect.ability; this.filter = effect.filter; @@ -45,9 +45,8 @@ public class GainAbilityControlledSpellsEffect extends ContinuousEffectImpl { return false; } - for (Card card : game.getExile().getAllCards(game)) { - if (card.isOwnedBy(source.getControllerId()) - && filter.match(card, game)) { + for (Card card : game.getExile().getAllCardsByRange(game, source.getControllerId())) { + if (filter.match(card, game)) { game.getState().addOtherAbility(card, ability); } } @@ -71,22 +70,19 @@ public class GainAbilityControlledSpellsEffect extends ContinuousEffectImpl { game.getCommanderCardsFromCommandZone(player, CommanderCardType.ANY) .stream() .filter(card -> filter.match(card, game)) - .forEach(card -> { - game.getState().addOtherAbility(card, ability); - }); + .forEach(card -> game.getState().addOtherAbility(card, ability)); for (StackObject stackObject : game.getStack()) { - // only spells cast, so no copies of spells - if ((stackObject instanceof Spell) - && !stackObject.isCopy() - && stackObject.isControlledBy(source.getControllerId())) { - Card card = game.getCard(stackObject.getSourceId()); - if (filter.match(card, game)) { - game.getState().addOtherAbility(card, ability); - return true; - } + if (!(stackObject instanceof Spell) || !stackObject.isControlledBy(source.getControllerId())) { + continue; } + // TODO: Distinguish "you cast" to exclude copies + Card card = game.getCard(stackObject.getSourceId()); + if (card == null || !filter.match((Spell) stackObject, game)) { + continue; + } + game.getState().addOtherAbility(card, ability); } - return false; // TODO: Why is this returning false? + return true; } } diff --git a/Mage/src/main/java/mage/game/GameState.java b/Mage/src/main/java/mage/game/GameState.java index 0352fa6aaa9..0b592049795 100644 --- a/Mage/src/main/java/mage/game/GameState.java +++ b/Mage/src/main/java/mage/game/GameState.java @@ -1183,8 +1183,6 @@ public class GameState implements Serializable, Copyable { * @param ability */ public void addOtherAbility(Card attachedTo, Ability ability) { - checkWrongDynamicAbilityUsage(attachedTo, ability); - addOtherAbility(attachedTo, ability, true); } @@ -1202,6 +1200,12 @@ public class GameState implements Serializable, Copyable { Ability newAbility; if (ability instanceof MageSingleton || !copyAbility) { + // Avoid adding another instance of an ability where multiple copies are redundant + if (attachedTo.getAbilities().contains(ability) + || (getAllOtherAbilities(attachedTo.getId()) != null + && getAllOtherAbilities(attachedTo.getId()).contains(ability))) { + return; + } newAbility = ability; } else { // must use new id, so you can add multiple instances of the same ability