From 78649c1a6256b03422805e79079b97daaa43de7f Mon Sep 17 00:00:00 2001 From: ssk97 Date: Mon, 22 Jul 2024 20:13:48 -0700 Subject: [PATCH] Remove KickerAbility.getSpellKickedCount (#12553) * Remove KickerAbility.getSpellKickedCount * Check spell LKI instead of only current spell object * Remove bad LKI storage under wrong ID --- .../src/mage/cards/b/BloodstoneGoblin.java | 54 ++++--------------- .../mage/cards/h/HallarTheFirefletcher.java | 50 +++++------------ .../src/mage/cards/r/RumblingAftershocks.java | 12 +++-- .../cards/abilities/keywords/KickerTest.java | 38 +++++++++++++ .../condition/common/KickedCondition.java | 3 +- .../condition/common/KickedCostCondition.java | 3 +- .../mage/abilities/keyword/KickerAbility.java | 23 ++------ Mage/src/main/java/mage/cards/CardImpl.java | 2 +- .../mageobject/KickedSpellPredicate.java | 9 ++-- Mage/src/main/java/mage/game/Game.java | 2 +- Mage/src/main/java/mage/game/GameImpl.java | 13 ++--- Mage/src/main/java/mage/util/CardUtil.java | 9 ++-- 12 files changed, 91 insertions(+), 127 deletions(-) diff --git a/Mage.Sets/src/mage/cards/b/BloodstoneGoblin.java b/Mage.Sets/src/mage/cards/b/BloodstoneGoblin.java index 7bcfe790598..aba623e9016 100644 --- a/Mage.Sets/src/mage/cards/b/BloodstoneGoblin.java +++ b/Mage.Sets/src/mage/cards/b/BloodstoneGoblin.java @@ -1,19 +1,17 @@ package mage.cards.b; import mage.MageInt; -import mage.abilities.TriggeredAbilityImpl; +import mage.abilities.TriggeredAbility; +import mage.abilities.common.SpellCastControllerTriggeredAbility; import mage.abilities.effects.common.continuous.BoostSourceEffect; import mage.abilities.effects.common.continuous.GainAbilitySourceEffect; -import mage.abilities.keyword.KickerAbility; import mage.abilities.keyword.MenaceAbility; import mage.cards.CardImpl; import mage.cards.CardSetInfo; import mage.constants.CardType; import mage.constants.Duration; import mage.constants.SubType; -import mage.constants.Zone; -import mage.game.Game; -import mage.game.events.GameEvent; +import mage.filter.StaticFilters; import java.util.UUID; @@ -32,7 +30,14 @@ public final class BloodstoneGoblin extends CardImpl { // Whenever you cast a spell, if that spell was kicked, // Bloodstone Goblin gets +1/+1 and gains menace until end of turn. - this.addAbility(new BloodstoneGoblinTriggeredAbility()); + TriggeredAbility ability = new SpellCastControllerTriggeredAbility( + new BoostSourceEffect(1, 1, Duration.EndOfTurn).setText("{this} gets +1/+1"), + StaticFilters.FILTER_SPELL_KICKED_A, false); + ability.addEffect(new GainAbilitySourceEffect( + new MenaceAbility(false), Duration.EndOfTurn) + .setText("and gains menace until end of turn. " + "(It can't be blocked except by two or more creatures.)")); + ability.setTriggerPhrase("Whenever you cast a spell, if that spell was kicked, "); + this.addAbility(ability); } private BloodstoneGoblin(final BloodstoneGoblin card) { @@ -44,40 +49,3 @@ public final class BloodstoneGoblin extends CardImpl { return new BloodstoneGoblin(this); } } - -class BloodstoneGoblinTriggeredAbility extends TriggeredAbilityImpl { - - BloodstoneGoblinTriggeredAbility() { - super( - Zone.BATTLEFIELD, - new BoostSourceEffect(1, 1, Duration.EndOfTurn).setText("{this} gets +1/+1"), - false); - this.addEffect( - new GainAbilitySourceEffect( - new MenaceAbility(false), - Duration.EndOfTurn - ).setText("and gains menace until end of turn. " - + "(It can't be blocked except by two or more creatures.)")); - setTriggerPhrase("Whenever you cast a spell, if that spell was kicked, "); - } - - private BloodstoneGoblinTriggeredAbility(final BloodstoneGoblinTriggeredAbility ability) { - super(ability); - } - - @Override - public BloodstoneGoblinTriggeredAbility copy() { - return new BloodstoneGoblinTriggeredAbility(this); - } - - @Override - public boolean checkEventType(GameEvent event, Game game) { - return event.getType() == GameEvent.EventType.SPELL_CAST - && event.getPlayerId() == this.controllerId; - } - - @Override - public boolean checkTrigger(GameEvent event, Game game) { - return KickerAbility.getSpellKickedCount(game, event.getTargetId()) > 0; - } -} diff --git a/Mage.Sets/src/mage/cards/h/HallarTheFirefletcher.java b/Mage.Sets/src/mage/cards/h/HallarTheFirefletcher.java index 2890dc3ded6..81c725e2501 100644 --- a/Mage.Sets/src/mage/cards/h/HallarTheFirefletcher.java +++ b/Mage.Sets/src/mage/cards/h/HallarTheFirefletcher.java @@ -1,18 +1,17 @@ package mage.cards.h; import mage.MageInt; -import mage.abilities.TriggeredAbilityImpl; +import mage.abilities.TriggeredAbility; +import mage.abilities.common.SpellCastControllerTriggeredAbility; import mage.abilities.dynamicvalue.common.CountersSourceCount; import mage.abilities.effects.common.DamagePlayersEffect; import mage.abilities.effects.common.counter.AddCountersSourceEffect; -import mage.abilities.keyword.KickerAbility; import mage.abilities.keyword.TrampleAbility; import mage.cards.CardImpl; import mage.cards.CardSetInfo; import mage.constants.*; import mage.counters.CounterType; -import mage.game.Game; -import mage.game.events.GameEvent; +import mage.filter.StaticFilters; import java.util.UUID; @@ -34,7 +33,15 @@ public final class HallarTheFirefletcher extends CardImpl { this.addAbility(TrampleAbility.getInstance()); // Whenever you cast a spell, if that spell was kicked, put a +1/+1 counter on Hallar, the Firefletcher, then Hallar deals damage equal to the number of +1/+1 counters on it to each opponent. - this.addAbility(new HallarTheFirefletcherTriggeredAbility()); + TriggeredAbility ability = new SpellCastControllerTriggeredAbility( + new AddCountersSourceEffect(CounterType.P1P1.createInstance()).setText("put a +1/+1 counter on {this}"), + StaticFilters.FILTER_SPELL_KICKED_A, false + ); + ability.addEffect(new DamagePlayersEffect(Outcome.Benefit, new CountersSourceCount(CounterType.P1P1), TargetController.OPPONENT) + .setText(", then {this} deals damage equal to the number of +1/+1 counters on it to each opponent") + ); + ability.setTriggerPhrase("Whenever you cast a spell, if that spell was kicked, "); + this.addAbility(ability); } private HallarTheFirefletcher(final HallarTheFirefletcher card) { @@ -46,36 +53,3 @@ public final class HallarTheFirefletcher extends CardImpl { return new HallarTheFirefletcher(this); } } - -class HallarTheFirefletcherTriggeredAbility extends TriggeredAbilityImpl { - - HallarTheFirefletcherTriggeredAbility() { - super(Zone.BATTLEFIELD, new AddCountersSourceEffect(CounterType.P1P1.createInstance()).setText("put a +1/+1 counter on {this}"), false); - this.addEffect(new DamagePlayersEffect(Outcome.Benefit, new CountersSourceCount(CounterType.P1P1), TargetController.OPPONENT) - .setText(", then {this} deals damage equal to the number of +1/+1 counters on it to each opponent") - ); - setTriggerPhrase("Whenever you cast a spell, if that spell was kicked, "); - } - - private HallarTheFirefletcherTriggeredAbility(final HallarTheFirefletcherTriggeredAbility ability) { - super(ability); - } - - @Override - public HallarTheFirefletcherTriggeredAbility copy() { - return new HallarTheFirefletcherTriggeredAbility(this); - } - - @Override - public boolean checkEventType(GameEvent event, Game game) { - return event.getType() == GameEvent.EventType.SPELL_CAST; - } - - @Override - public boolean checkTrigger(GameEvent event, Game game) { - if (!isControlledBy(event.getPlayerId())) { - return false; - } - return KickerAbility.getSpellKickedCount(game, event.getTargetId()) > 0; - } -} diff --git a/Mage.Sets/src/mage/cards/r/RumblingAftershocks.java b/Mage.Sets/src/mage/cards/r/RumblingAftershocks.java index 43006f460c6..e2749763d50 100644 --- a/Mage.Sets/src/mage/cards/r/RumblingAftershocks.java +++ b/Mage.Sets/src/mage/cards/r/RumblingAftershocks.java @@ -12,6 +12,7 @@ import mage.constants.Zone; import mage.game.Game; import mage.game.events.GameEvent; import mage.game.permanent.Permanent; +import mage.game.stack.Spell; import mage.players.Player; import mage.target.common.TargetAnyTarget; @@ -66,10 +67,13 @@ class RumblingAftershocksTriggeredAbility extends TriggeredAbilityImpl { @Override public boolean checkTrigger(GameEvent event, Game game) { - int kickedCount = KickerAbility.getSpellKickedCount(game, event.getTargetId()); - if (kickedCount > 0) { - this.getEffects().get(0).setValue("damageAmount", kickedCount); - return true; + Spell spell = game.getSpell(event.getTargetId()); + if (spell != null) { + int kickedCount = KickerAbility.getKickedCounter(game, spell.getSpellAbility()); + if (kickedCount > 0) { + this.getEffects().get(0).setValue("damageAmount", kickedCount); + return true; + } } return false; } diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/abilities/keywords/KickerTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/abilities/keywords/KickerTest.java index d2d29794d05..e2a3ee8ad0a 100644 --- a/Mage.Tests/src/test/java/org/mage/test/cards/abilities/keywords/KickerTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/cards/abilities/keywords/KickerTest.java @@ -814,4 +814,42 @@ public class KickerTest extends CardTestPlayerBase { assertTappedCount("Mountain", true, 5); } + @Test + public void testWastescapeBattlemage() { + String battlemage = "Wastescape Battlemage"; // 1C + kickers G (exile artifact/enchantment) + 1U (bounce creature) + + addCard(Zone.BATTLEFIELD, playerA, "Wastes", 5); + addCard(Zone.BATTLEFIELD, playerA, "Tropical Island", 3); + addCard(Zone.HAND, playerA, battlemage, 2); + addCard(Zone.BATTLEFIELD, playerB, "Darksteel Relic", 2); + addCard(Zone.BATTLEFIELD, playerB, "Squire", 2); + addCard(Zone.BATTLEFIELD, playerB, "Island", 2); + addCard(Zone.HAND, playerB, "Counterspell", 1); + + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, battlemage); + setChoice(playerA, true); + setChoice(playerA, false); + addTarget(playerA, "Darksteel Relic"); + + castSpell(1, PhaseStep.POSTCOMBAT_MAIN, playerA, battlemage); + setChoice(playerA, true); + setChoice(playerA, true); + setChoice(playerA, "When you cast this spell, if it was kicked with its {G} kicker, exile"); + addTarget(playerA, "Darksteel Relic"); + addTarget(playerA, "Squire"); + //Abilities have not resolved yet + checkStackSize("Spell+2 Triggers on Stack", 1, PhaseStep.POSTCOMBAT_MAIN, playerA, 3); + checkPermanentCount("Darksteel Relic count", 1, PhaseStep.POSTCOMBAT_MAIN, playerB, "Darksteel Relic", 1); + checkPermanentCount("Squire count", 1, PhaseStep.POSTCOMBAT_MAIN, playerB, "Squire", 2); + castSpell(1, PhaseStep.POSTCOMBAT_MAIN, playerB, "Counterspell", battlemage); + + setStrictChooseMode(true); + setStopAt(1, PhaseStep.END_TURN); + execute(); + assertPermanentCount(playerA, battlemage, 1); + assertPermanentCount(playerB, "Darksteel Relic", 0); + assertPermanentCount(playerB, "Squire", 1); + assertExileCount(playerB, 2); + assertHandCount(playerB, "Squire", 1); + } } diff --git a/Mage/src/main/java/mage/abilities/condition/common/KickedCondition.java b/Mage/src/main/java/mage/abilities/condition/common/KickedCondition.java index 740409f3a14..e0b40c02e34 100644 --- a/Mage/src/main/java/mage/abilities/condition/common/KickedCondition.java +++ b/Mage/src/main/java/mage/abilities/condition/common/KickedCondition.java @@ -24,8 +24,7 @@ public enum KickedCondition implements Condition { @Override public boolean apply(Game game, Ability source) { - return KickerAbility.getKickedCounter(game, source) >= kickedCount // for on battlefield - || KickerAbility.getSpellKickedCount(game, source.getSourceId()) >= kickedCount; // for on stack + return KickerAbility.getKickedCounter(game, source) >= kickedCount; } @Override diff --git a/Mage/src/main/java/mage/abilities/condition/common/KickedCostCondition.java b/Mage/src/main/java/mage/abilities/condition/common/KickedCostCondition.java index 4804c9700b1..b3dcf1ee4d4 100644 --- a/Mage/src/main/java/mage/abilities/condition/common/KickedCostCondition.java +++ b/Mage/src/main/java/mage/abilities/condition/common/KickedCostCondition.java @@ -20,7 +20,6 @@ public class KickedCostCondition implements Condition { @Override public boolean apply(Game game, Ability source) { - return KickerAbility.getKickedCounterStrict(game, source, kickerCostText) > 0 - || KickerAbility.getSpellKickedCountStrict(game, source.getSourceId(), kickerCostText) > 0; + return KickerAbility.getKickedCounterStrict(game, source, kickerCostText) > 0; } } diff --git a/Mage/src/main/java/mage/abilities/keyword/KickerAbility.java b/Mage/src/main/java/mage/abilities/keyword/KickerAbility.java index 557087ba13f..4f1cb0babe5 100644 --- a/Mage/src/main/java/mage/abilities/keyword/KickerAbility.java +++ b/Mage/src/main/java/mage/abilities/keyword/KickerAbility.java @@ -9,11 +9,13 @@ import mage.constants.Outcome; import mage.constants.Zone; import mage.game.Game; import mage.game.events.GameEvent; -import mage.game.stack.Spell; import mage.players.Player; import mage.util.CardUtil; -import java.util.*; +import java.util.Iterator; +import java.util.LinkedList; +import java.util.List; +import java.util.Map; import java.util.stream.Stream; /** @@ -269,21 +271,4 @@ public class KickerAbility extends StaticAbility implements OptionalAdditionalSo return sb.toString(); } - /** - * Find spell's kicked stats. Must be used on stack only, e.g. for SPELL_CAST events - */ - public static int getSpellKickedCount(Game game, UUID spellId) { - return getSpellKickedCountStrict(game, spellId, ""); - } - - /** - * Find spell's kicked stats. Must be used on stack only, e.g. for SPELL_CAST events - */ - public static int getSpellKickedCountStrict(Game game, UUID spellId, String needKickerCost) { - Spell spell = game.getSpellOrLKIStack(spellId); - if (spell != null) { - return KickerAbility.getKickedCounterStrict(game, spell.getSpellAbility(), needKickerCost); - } - return 0; - } } diff --git a/Mage/src/main/java/mage/cards/CardImpl.java b/Mage/src/main/java/mage/cards/CardImpl.java index dafc77981ea..10aad261a1d 100644 --- a/Mage/src/main/java/mage/cards/CardImpl.java +++ b/Mage/src/main/java/mage/cards/CardImpl.java @@ -562,7 +562,7 @@ public abstract class CardImpl extends MageObjectImpl implements Card { } if (removed) { if (fromZone != Zone.OUTSIDE) { - game.rememberLKI(lkiObject != null ? lkiObject.getId() : objectId, fromZone, lkiObject != null ? lkiObject : this); + game.rememberLKI(fromZone, lkiObject != null ? lkiObject : this); } } else { logger.warn("Couldn't find card in fromZone, card=" + getIdName() + ", fromZone=" + fromZone); diff --git a/Mage/src/main/java/mage/filter/predicate/mageobject/KickedSpellPredicate.java b/Mage/src/main/java/mage/filter/predicate/mageobject/KickedSpellPredicate.java index 4d91f8ef7b4..3b47000768b 100644 --- a/Mage/src/main/java/mage/filter/predicate/mageobject/KickedSpellPredicate.java +++ b/Mage/src/main/java/mage/filter/predicate/mageobject/KickedSpellPredicate.java @@ -4,13 +4,12 @@ import mage.MageObject; import mage.abilities.keyword.KickerAbility; import mage.filter.predicate.Predicate; import mage.game.Game; +import mage.game.stack.Spell; /** * Find spell's kicked stats. *

* Warning, must be used for SPELL_CAST events only - * (if you need kicked stats in ETB effects then search object's abilities instead spell, - * see MultikickerCount as example) * * @author TheElk801 */ @@ -19,7 +18,11 @@ public enum KickedSpellPredicate implements Predicate { @Override public boolean apply(MageObject input, Game game) { - return KickerAbility.getSpellKickedCount(game, input.getId()) > 0; + if (input instanceof Spell) { + return KickerAbility.getKickedCounter(game, ((Spell) input).getSpellAbility()) > 0; + } else { + return false; + } } @Override diff --git a/Mage/src/main/java/mage/game/Game.java b/Mage/src/main/java/mage/game/Game.java index f276fb45aaa..900f8f6f48b 100644 --- a/Mage/src/main/java/mage/game/Game.java +++ b/Mage/src/main/java/mage/game/Game.java @@ -305,7 +305,7 @@ public interface Game extends MageItem, Serializable, Copyable { */ boolean checkShortLivingLKI(UUID objectId, Zone zone); - void rememberLKI(UUID objectId, Zone zone, MageObject object); + void rememberLKI(Zone zone, MageObject object); void resetLKI(); diff --git a/Mage/src/main/java/mage/game/GameImpl.java b/Mage/src/main/java/mage/game/GameImpl.java index 6ea8dc969b1..2aee96a3875 100644 --- a/Mage/src/main/java/mage/game/GameImpl.java +++ b/Mage/src/main/java/mage/game/GameImpl.java @@ -714,12 +714,6 @@ public abstract class GameImpl implements Game { // SyrCarahTheBoldTest.java for an example of when this check is relevant. if (obj instanceof Spell) { spell = (Spell) obj; - } else if (obj != null) { - logger.error(String.format( - "getSpellOrLKIStack got non-spell id %s correlating to non-spell object %s.", - obj.getClass().getName(), obj.getName()), - new Throwable() - ); } } return spell; @@ -1801,7 +1795,6 @@ public abstract class GameImpl implements Game { } finally { if (top != null) { state.getStack().remove(top, this); // seems partly redundant because move card from stack to grave is already done and the stack removed - rememberLKI(top.getSourceId(), Zone.STACK, top); checkInfiniteLoop(top.getSourceId()); if (!getTurn().isEndTurnRequested()) { while (state.hasSimultaneousEvents()) { @@ -3320,7 +3313,7 @@ public abstract class GameImpl implements Game { } } for (Card card : toOutside) { - rememberLKI(card.getId(), Zone.BATTLEFIELD, card); + rememberLKI(Zone.BATTLEFIELD, card); } // needed to send event that permanent leaves the battlefield to allow non stack effects to execute player.moveCards(toOutside, Zone.OUTSIDE, null, this); @@ -3592,12 +3585,12 @@ public abstract class GameImpl implements Game { /** * Remembers object state to be used as Last Known Information. * - * @param objectId * @param zone * @param object */ @Override - public void rememberLKI(UUID objectId, Zone zone, MageObject object) { + public void rememberLKI(Zone zone, MageObject object) { + UUID objectId = object.getId(); if (object instanceof Permanent || object instanceof StackObject) { MageObject copy = object.copy(); diff --git a/Mage/src/main/java/mage/util/CardUtil.java b/Mage/src/main/java/mage/util/CardUtil.java index 7c6a877734f..9c850577269 100644 --- a/Mage/src/main/java/mage/util/CardUtil.java +++ b/Mage/src/main/java/mage/util/CardUtil.java @@ -1727,7 +1727,8 @@ public final class CardUtil { /** * Returns the entire cost tags map of either the source ability, or the permanent source of the ability. May be null. * Works in any moment (even before source ability activated) - * Usually you should use one of the single tag functions instead: getSourceCostsTag() or checkSourceCostsTagExists() + *

+ * Usually you should use one of the single tag functions instead: getSourceCostsTag() or checkSourceCostsTagExists(). * Use this function with caution, as it directly exposes the backing data structure. * * @param game @@ -1751,9 +1752,9 @@ public final class CardUtil { } // from any ability before resolve (on stack) - access by spell ability - MageObject sourceObject = source.getSourceObject(game); - if (sourceObject instanceof Spell) { - return ((Spell) sourceObject).getSpellAbility().getCostsTagMap(); + Spell sourceObject = game.getSpellOrLKIStack(source.getSourceId()); + if (sourceObject != null) { + return sourceObject.getSpellAbility().getCostsTagMap(); } return null;