From e5c1dfc4b7fe5cbf54f415fbbd25a7e8d7842258 Mon Sep 17 00:00:00 2001 From: Samuel Sandeen Date: Sun, 29 Jul 2018 08:16:07 -0400 Subject: [PATCH] Refactor ControlledCreaturesDealCombatDamagePlayerTriggeredAbility. (#5163) It now triggers once for each player damaged. Fixes https://github.com/magefree/mage/issues/5162 --- Mage.Sets/src/mage/cards/n/NaturesWill.java | 54 +++------------ Mage.Sets/src/mage/cards/s/StormTheVault.java | 7 ++ .../abilities/other/NaturesWillTest.java | 67 +++++++++++++++++++ .../abilities/other/StormTheVaultTest.java | 59 ++++++++++++++++ ...ealCombatDamagePlayerTriggeredAbility.java | 37 +++++----- 5 files changed, 160 insertions(+), 64 deletions(-) create mode 100644 Mage.Tests/src/test/java/org/mage/test/cards/abilities/other/NaturesWillTest.java create mode 100644 Mage.Tests/src/test/java/org/mage/test/cards/abilities/other/StormTheVaultTest.java diff --git a/Mage.Sets/src/mage/cards/n/NaturesWill.java b/Mage.Sets/src/mage/cards/n/NaturesWill.java index f96bc4b0065..4c771bf5fee 100644 --- a/Mage.Sets/src/mage/cards/n/NaturesWill.java +++ b/Mage.Sets/src/mage/cards/n/NaturesWill.java @@ -1,20 +1,18 @@ package mage.cards.n; -import java.util.HashSet; -import java.util.List; -import java.util.Set; import java.util.UUID; import mage.abilities.Ability; import mage.abilities.common.ControlledCreaturesDealCombatDamagePlayerTriggeredAbility; -import mage.abilities.effects.OneShotEffect; +import mage.abilities.effects.Effect; +import mage.abilities.effects.common.TapAllTargetPlayerControlsEffect; +import mage.abilities.effects.common.UntapAllEffect; import mage.cards.CardImpl; import mage.cards.CardSetInfo; import mage.constants.CardType; -import mage.constants.Outcome; -import mage.filter.StaticFilters; -import mage.game.Game; -import mage.game.permanent.Permanent; +import mage.constants.Zone; +import mage.filter.common.FilterControlledLandPermanent; +import mage.filter.common.FilterLandPermanent; /** * @@ -26,7 +24,11 @@ public final class NaturesWill extends CardImpl { super(ownerId, setInfo, new CardType[]{CardType.ENCHANTMENT}, "{2}{G}{G}"); // Whenever one or more creatures you control deal combat damage to a player, tap all lands that player controls and untap all lands you control. - this.addAbility(new ControlledCreaturesDealCombatDamagePlayerTriggeredAbility(new NaturesWillEffect())); + Effect tapAllEffect = new TapAllTargetPlayerControlsEffect(new FilterLandPermanent()); + tapAllEffect.setText("tap all lands that player controls"); + Ability ability = new ControlledCreaturesDealCombatDamagePlayerTriggeredAbility(Zone.BATTLEFIELD, tapAllEffect, true); + ability.addEffect(new UntapAllEffect(new FilterControlledLandPermanent())); + addAbility(ability); } public NaturesWill(final NaturesWill card) { @@ -38,37 +40,3 @@ public final class NaturesWill extends CardImpl { return new NaturesWill(this); } } - -class NaturesWillEffect extends OneShotEffect { - - public NaturesWillEffect() { - super(Outcome.Benefit); - this.staticText = "tap all lands that player controls and untap all lands you control"; - } - - public NaturesWillEffect(final NaturesWillEffect effect) { - super(effect); - } - - @Override - public NaturesWillEffect copy() { - return new NaturesWillEffect(this); - } - - @Override - public boolean apply(Game game, Ability source) { - Set damagedPlayers = (HashSet) this.getValue("damagedPlayers"); - if (damagedPlayers != null) { - List lands = game.getBattlefield().getActivePermanents(StaticFilters.FILTER_LAND, source.getControllerId(), source.getSourceId(), game); - for (Permanent land : lands) { - if (damagedPlayers.contains(land.getControllerId())) { - land.tap(game); - } else if (land.isControlledBy(source.getControllerId())) { - land.untap(game); - } - } - return true; - } - return false; - } -} diff --git a/Mage.Sets/src/mage/cards/s/StormTheVault.java b/Mage.Sets/src/mage/cards/s/StormTheVault.java index 41e3e483241..968f3b72d52 100644 --- a/Mage.Sets/src/mage/cards/s/StormTheVault.java +++ b/Mage.Sets/src/mage/cards/s/StormTheVault.java @@ -2,10 +2,14 @@ package mage.cards.s; import java.util.UUID; + +import mage.abilities.Ability; import mage.abilities.common.BeginningOfEndStepTriggeredAbility; import mage.abilities.common.ControlledCreaturesDealCombatDamagePlayerTriggeredAbility; import mage.abilities.condition.common.PermanentsOnTheBattlefieldCondition; import mage.abilities.decorator.ConditionalInterveningIfTriggeredAbility; +import mage.abilities.dynamicvalue.DynamicValue; +import mage.abilities.effects.Effect; import mage.abilities.effects.common.CreateTokenEffect; import mage.abilities.effects.common.TransformSourceEffect; import mage.abilities.keyword.TransformAbility; @@ -17,6 +21,7 @@ import mage.constants.ComparisonType; import mage.constants.SuperType; import mage.constants.TargetController; import mage.filter.StaticFilters; +import mage.game.Game; import mage.game.permanent.token.TreasureToken; /** @@ -45,6 +50,8 @@ public final class StormTheVault extends CardImpl { } + + public StormTheVault(final StormTheVault card) { super(card); } diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/abilities/other/NaturesWillTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/abilities/other/NaturesWillTest.java new file mode 100644 index 00000000000..2c8d49e50ae --- /dev/null +++ b/Mage.Tests/src/test/java/org/mage/test/cards/abilities/other/NaturesWillTest.java @@ -0,0 +1,67 @@ +package org.mage.test.cards.abilities.other; + +import mage.constants.MultiplayerAttackOption; +import mage.constants.PhaseStep; +import mage.constants.RangeOfInfluence; +import mage.constants.Zone; +import mage.game.FreeForAll; +import mage.game.Game; +import mage.game.GameException; +import org.junit.Test; +import org.mage.test.serverside.base.CardTestPlayerBase; + +import java.io.FileNotFoundException; + +public class NaturesWillTest extends CardTestPlayerBase { + @Override + protected Game createNewGameAndPlayers() throws GameException, FileNotFoundException { + Game game = new FreeForAll(MultiplayerAttackOption.MULTIPLE, RangeOfInfluence.ALL, 0, 20); + playerA = createPlayer(game, playerA, "PlayerA"); + playerB = createPlayer(game, playerB, "PlayerB"); + playerC = createPlayer(game, playerC, "PlayerC"); + return game; + } + + + @Test + public void testAttackMultiplePlayers() { + addCard(Zone.HAND, playerA, "Nature's Will"); + addCard(Zone.BATTLEFIELD, playerA, "Forest", 4); + addCard(Zone.BATTLEFIELD, playerA, "Grizzly Bears"); + addCard(Zone.BATTLEFIELD, playerA, "Suntail Hawk"); + + addCard(Zone.BATTLEFIELD, playerB, "Mountain", 4); + addCard(Zone.BATTLEFIELD, playerC, "Island", 4); + + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Nature's Will"); + attack(1, playerA, "Grizzly Bears", playerB); + attack(1, playerA, "Suntail Hawk", playerC); + + setStopAt(1, PhaseStep.END_TURN); + execute(); + + assertTappedCount("Forest", false, 4); + assertTappedCount("Mountain", true, 4); + assertTappedCount("Island", true, 4); + } + + @Test + public void testAttackOnePlayer() { + addCard(Zone.HAND, playerA, "Nature's Will"); + addCard(Zone.BATTLEFIELD, playerA, "Forest", 4); + addCard(Zone.BATTLEFIELD, playerA, "Grizzly Bears"); + + addCard(Zone.BATTLEFIELD, playerB, "Mountain", 4); + addCard(Zone.BATTLEFIELD, playerC, "Island", 4); + + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Nature's Will"); + attack(1, playerA, "Grizzly Bears", playerB); + + setStopAt(1, PhaseStep.END_TURN); + execute(); + + assertTappedCount("Forest", false, 4); + assertTappedCount("Mountain", true, 4); + assertTappedCount("Island", false, 4); + } +} diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/abilities/other/StormTheVaultTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/abilities/other/StormTheVaultTest.java new file mode 100644 index 00000000000..75f279af428 --- /dev/null +++ b/Mage.Tests/src/test/java/org/mage/test/cards/abilities/other/StormTheVaultTest.java @@ -0,0 +1,59 @@ +package org.mage.test.cards.abilities.other; + +import mage.constants.MultiplayerAttackOption; +import mage.constants.PhaseStep; +import mage.constants.RangeOfInfluence; +import mage.constants.Zone; +import mage.game.FreeForAll; +import mage.game.Game; +import mage.game.GameException; +import org.junit.Test; +import org.mage.test.serverside.base.CardTestPlayerBase; + +import java.io.FileNotFoundException; + +public class StormTheVaultTest extends CardTestPlayerBase { + @Override + protected Game createNewGameAndPlayers() throws GameException, FileNotFoundException { + Game game = new FreeForAll(MultiplayerAttackOption.MULTIPLE, RangeOfInfluence.ALL, 0, 20); + playerA = createPlayer(game, playerA, "PlayerA"); + playerB = createPlayer(game, playerB, "PlayerB"); + playerC = createPlayer(game, playerC, "PlayerC"); + return game; + } + + + @Test + public void testAttackMultiplePlayers() { + addCard(Zone.BATTLEFIELD, playerA, "Storm the Vault"); + addCard(Zone.BATTLEFIELD, playerA, "Grizzly Bears"); + addCard(Zone.BATTLEFIELD, playerA, "Nessian Courser"); + addCard(Zone.BATTLEFIELD, playerA, "Suntail Hawk"); + addCard(Zone.BATTLEFIELD, playerA, "Lantern Kami"); + + attack(1, playerA, "Grizzly Bears", playerB); + attack(1, playerA, "Nessian Courser", playerB); + attack(1, playerA, "Suntail Hawk", playerC); + attack(1, playerA, "Lantern Kami", playerC); + + setStopAt(1, PhaseStep.END_TURN); + execute(); + + assertPermanentCount(playerA, "Treasure", 2); + } + + @Test + public void testAttackOnePlayer() { + addCard(Zone.BATTLEFIELD, playerA, "Storm the Vault"); + addCard(Zone.BATTLEFIELD, playerA, "Grizzly Bears"); + addCard(Zone.BATTLEFIELD, playerA, "Suntail Hawk"); + + attack(1, playerA, "Grizzly Bears", playerB); + attack(1, playerA, "Suntail Hawk", playerB); + + setStopAt(1, PhaseStep.END_TURN); + execute(); + + assertPermanentCount(playerA, "Treasure", 1); + } +} diff --git a/Mage/src/main/java/mage/abilities/common/ControlledCreaturesDealCombatDamagePlayerTriggeredAbility.java b/Mage/src/main/java/mage/abilities/common/ControlledCreaturesDealCombatDamagePlayerTriggeredAbility.java index 71c0d50bde7..9aa3611a459 100644 --- a/Mage/src/main/java/mage/abilities/common/ControlledCreaturesDealCombatDamagePlayerTriggeredAbility.java +++ b/Mage/src/main/java/mage/abilities/common/ControlledCreaturesDealCombatDamagePlayerTriggeredAbility.java @@ -4,6 +4,7 @@ package mage.abilities.common; import java.util.HashSet; import java.util.Set; import java.util.UUID; + import mage.abilities.TriggeredAbilityImpl; import mage.abilities.effects.Effect; import mage.constants.Zone; @@ -11,8 +12,8 @@ import mage.game.Game; import mage.game.events.DamagedPlayerEvent; import mage.game.events.GameEvent; import mage.game.events.GameEvent.EventType; -import mage.game.events.ZoneChangeEvent; import mage.game.permanent.Permanent; +import mage.target.targetpointer.FixedTarget; /** * @@ -20,22 +21,25 @@ import mage.game.permanent.Permanent; */ public class ControlledCreaturesDealCombatDamagePlayerTriggeredAbility extends TriggeredAbilityImpl { - private boolean madeDamage = false; private Set damagedPlayerIds = new HashSet<>(); + private boolean setTargetPointer; public ControlledCreaturesDealCombatDamagePlayerTriggeredAbility(Effect effect) { this(Zone.BATTLEFIELD, effect); } public ControlledCreaturesDealCombatDamagePlayerTriggeredAbility(Zone zone, Effect effect) { + this(zone, effect, false); + } + + public ControlledCreaturesDealCombatDamagePlayerTriggeredAbility(Zone zone, Effect effect, boolean setTargetPointer) { super(zone, effect, false); + this.setTargetPointer = setTargetPointer; } public ControlledCreaturesDealCombatDamagePlayerTriggeredAbility(final ControlledCreaturesDealCombatDamagePlayerTriggeredAbility ability) { super(ability); - this.madeDamage = ability.madeDamage; this.damagedPlayerIds = new HashSet<>(); - this.damagedPlayerIds.addAll(ability.damagedPlayerIds); } @Override @@ -55,28 +59,19 @@ public class ControlledCreaturesDealCombatDamagePlayerTriggeredAbility extends T if (event.getType() == EventType.DAMAGED_PLAYER) { DamagedPlayerEvent damageEvent = (DamagedPlayerEvent) event; Permanent p = game.getPermanent(event.getSourceId()); - if (damageEvent.isCombatDamage() && p != null && p.isControlledBy(this.getControllerId())) { - madeDamage = true; + if (damageEvent.isCombatDamage() && p != null && p.isControlledBy(this.getControllerId()) && !damagedPlayerIds.contains(event.getPlayerId())) { damagedPlayerIds.add(event.getPlayerId()); - } - } - if (event.getType() == EventType.COMBAT_DAMAGE_STEP_PRIORITY) { - if (madeDamage) { - Set damagedPlayersCopy = new HashSet<>(); - damagedPlayersCopy.addAll(damagedPlayerIds); - for (Effect effect : this.getEffects()) { - effect.setValue("damagedPlayers", damagedPlayersCopy); + if (setTargetPointer) { + for (Effect effect : this.getEffects()) { + effect.setTargetPointer(new FixedTarget(event.getPlayerId())); + } } - damagedPlayerIds.clear(); - madeDamage = false; return true; } } - if (event.getType() == EventType.ZONE_CHANGE && event.getTargetId().equals(getSourceId())) { - ZoneChangeEvent zEvent = (ZoneChangeEvent) event; - if (zEvent.getFromZone() == Zone.GRAVEYARD) { - damagedPlayerIds.clear(); - } + if (event.getType() == EventType.COMBAT_DAMAGE_STEP_PRIORITY || + (event.getType() == EventType.ZONE_CHANGE && event.getTargetId().equals(getSourceId()))) { + damagedPlayerIds.clear(); } return false; }