From 66cf6909680d4ab6546e53edda74134cdeb6d233 Mon Sep 17 00:00:00 2001 From: LevelX2 Date: Sun, 5 Apr 2015 11:13:26 +0200 Subject: [PATCH] Fixed some bugs that prevent to select shroud or hexproof targets by not targeted effects (e.g. Proliferate). --- .../sets/scarsofmirrodin/ContagionEngine.java | 12 +- .../cards/single/ContagionEngineTest.java | 108 ++++++++++++++++++ .../java/org/mage/test/player/TestPlayer.java | 46 +++++--- .../common/counter/ProliferateEffect.java | 102 ++++++++--------- .../predicate/permanent/CounterPredicate.java | 10 +- Mage/src/mage/target/TargetPermanent.java | 2 +- .../common/TargetPermanentOrPlayer.java | 24 +++- .../TargetPermanentOrPlayerWithCounter.java | 15 ++- 8 files changed, 232 insertions(+), 87 deletions(-) create mode 100644 Mage.Tests/src/test/java/org/mage/test/cards/single/ContagionEngineTest.java diff --git a/Mage.Sets/src/mage/sets/scarsofmirrodin/ContagionEngine.java b/Mage.Sets/src/mage/sets/scarsofmirrodin/ContagionEngine.java index b93cef49cb..f21304caff 100644 --- a/Mage.Sets/src/mage/sets/scarsofmirrodin/ContagionEngine.java +++ b/Mage.Sets/src/mage/sets/scarsofmirrodin/ContagionEngine.java @@ -58,9 +58,13 @@ public class ContagionEngine extends CardImpl { public ContagionEngine (UUID ownerId) { super(ownerId, 145, "Contagion Engine", Rarity.RARE, new CardType[]{CardType.ARTIFACT}, "{6}"); this.expansionSetCode = "SOM"; + + // When Contagion Engine enters the battlefield, put a -1/-1 counter on each creature target player controls. Ability ability = new EntersBattlefieldTriggeredAbility(new ContagionEngineEffect()); ability.addTarget(new TargetPlayer()); this.addAbility(ability); + + // {4}, {T}: Proliferate, then proliferate again. (You choose any number of permanents and/or players with counters on them, then give each another counter of a kind already there. Then do it again.) ability = new SimpleActivatedAbility(Zone.BATTLEFIELD, new ProliferateEffect(), new GenericManaCost(4)); ability.addCost(new TapSourceCost()); ability.addEffect(new ProliferateEffect()); @@ -90,10 +94,10 @@ class ContagionEngineEffect extends OneShotEffect { @Override public boolean apply(Game game, Ability source) { - Player target = game.getPlayer(source.getFirstTarget()); - if (target != null) { - for (Permanent p : game.getBattlefield().getAllActivePermanents(new FilterCreaturePermanent(), target.getId(), game)) { - p.addCounters(CounterType.M1M1.createInstance(), game); + Player targetPlayer = game.getPlayer(getTargetPointer().getFirst(game, source)); + if (targetPlayer != null) { + for (Permanent creature : game.getBattlefield().getAllActivePermanents(new FilterCreaturePermanent(), targetPlayer.getId(), game)) { + creature.addCounters(CounterType.M1M1.createInstance(), game); } return true; } diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/single/ContagionEngineTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/single/ContagionEngineTest.java new file mode 100644 index 0000000000..c82415ee09 --- /dev/null +++ b/Mage.Tests/src/test/java/org/mage/test/cards/single/ContagionEngineTest.java @@ -0,0 +1,108 @@ +/* + * Copyright 2010 BetaSteward_at_googlemail.com. All rights reserved. + * + * Redistribution and use in source and binary forms, with or without modification, are + * permitted provided that the following conditions are met: + * + * 1. Redistributions of source code must retain the above copyright notice, this list of + * conditions and the following disclaimer. + * + * 2. Redistributions in binary form must reproduce the above copyright notice, this list + * of conditions and the following disclaimer in the documentation and/or other materials + * provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY BetaSteward_at_googlemail.com ``AS IS'' AND ANY EXPRESS OR IMPLIED + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND + * FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL BetaSteward_at_googlemail.com OR + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR + * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON + * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING + * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF + * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + * The views and conclusions contained in the software and documentation are those of the + * authors and should not be interpreted as representing official policies, either expressed + * or implied, of BetaSteward_at_googlemail.com. + */ +package org.mage.test.cards.single; + +import mage.constants.PhaseStep; +import mage.constants.Zone; +import mage.counters.CounterType; +import org.junit.Test; +import org.mage.test.serverside.base.CardTestPlayerBase; + +/** + * + * @author LevelX2 + */ + +public class ContagionEngineTest extends CardTestPlayerBase { + + // When Contagion Engine enters the battlefield, put a -1/-1 counter on each creature target player controls. + // {4}, {T}: Proliferate, then proliferate again. (You choose any number of permanents and/or players with + // counters on them, then give each another counter of a kind already there. Then do it again.) + @Test + public void testCountersArePut() { + addCard(Zone.BATTLEFIELD, playerA, "Mountain",6); + addCard(Zone.HAND, playerA, "Contagion Engine"); + + addCard(Zone.BATTLEFIELD, playerB, "Silvercoat Lion"); + addCard(Zone.BATTLEFIELD, playerB, "Pincher Beetles"); // 3/1 + Shroud + addCard(Zone.BATTLEFIELD, playerB, "Sacred Wolf"); // 3/1 + Hexproof + addCard(Zone.BATTLEFIELD, playerB, "Beloved Chaplain"); // 3/1 + Protection from creatures + + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Contagion Engine"); + addTarget(playerA, playerB); + + setStopAt(1, PhaseStep.BEGIN_COMBAT); + execute(); + + assertLife(playerA, 20); + assertLife(playerB, 20); + + assertPowerToughness(playerB, "Silvercoat Lion", 1, 1); + + assertGraveyardCount(playerB, "Sacred Wolf",1); + assertGraveyardCount(playerB, "Beloved Chaplain",1); + assertGraveyardCount(playerB, "Pincher Beetles",1); + + } + + @Test + public void testCountersDoubledByProliferate() { + addCard(Zone.BATTLEFIELD, playerA, "Mountain",6); + addCard(Zone.HAND, playerA, "Contagion Engine"); + + addCard(Zone.BATTLEFIELD, playerA, "Ajani Goldmane"); + + addCard(Zone.BATTLEFIELD, playerB, "Wall of Frost"); // 0/7 + addCard(Zone.BATTLEFIELD, playerB, "Kalonian Behemoth"); // 9/9 + Shroud + addCard(Zone.BATTLEFIELD, playerB, "Plated Slagwurm"); // 8/8 + Hexproof + addCard(Zone.BATTLEFIELD, playerB, "Teysa, Envoy of Ghosts"); // 4/4 + Protection from creatures + + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Contagion Engine"); + addTarget(playerA, playerB); + + activateAbility(3, PhaseStep.PRECOMBAT_MAIN, playerA, "{4},{T}: Proliferate"); + setChoice(playerA, "Wall of Frost^Kalonian Behemoth^Plated Slagwurm^Teysa, Envoy of Ghosts^Ajani Goldmane"); + setChoice(playerA, "Wall of Frost^Kalonian Behemoth^Plated Slagwurm^Teysa, Envoy of Ghosts^Ajani Goldmane"); + + setStopAt(3, PhaseStep.BEGIN_COMBAT); + execute(); + + assertLife(playerA, 20); + assertLife(playerB, 20); + + assertCounterCount("Ajani Goldmane", CounterType.LOYALTY, 6); + + assertPowerToughness(playerB, "Kalonian Behemoth", 6, 6); + assertPowerToughness(playerB, "Plated Slagwurm", 5, 5); + assertPowerToughness(playerB, "Teysa, Envoy of Ghosts", 1, 1); + assertPowerToughness(playerB, "Wall of Frost", -3, 4); + + + } + +} \ No newline at end of file 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 1fb9cff865..1e9ba488b0 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 @@ -69,6 +69,7 @@ import mage.cards.Card; import mage.constants.Zone; import mage.target.TargetSource; import mage.target.common.TargetCardInHand; +import mage.target.common.TargetPermanentOrPlayer; /** * @@ -301,23 +302,40 @@ public class TestPlayer extends ComputerPlayer { @Override public boolean choose(Outcome outcome, Target target, UUID sourceId, Game game, Map options) { if (!choices.isEmpty()) { - if (target instanceof TargetPermanent) { - for (Permanent permanent : game.getBattlefield().getAllActivePermanents((FilterPermanent)target.getFilter(), game)) { - for (String choose2: choices) { - if (permanent.getName().equals(choose2)) { - if (((TargetPermanent)target).canTarget(playerId, permanent.getId(), null, game) && !target.getTargets().contains(permanent.getId())) { - target.add(permanent.getId(), game); - choices.remove(choose2); - return true; + if ((target instanceof TargetPermanent) || (target instanceof TargetPermanentOrPlayer)) { // player target not implemted yet + FilterPermanent filterPermanent; + if (target instanceof TargetPermanentOrPlayer) { + filterPermanent = ((TargetPermanentOrPlayer) target).getFilterPermanent(); + } else { + filterPermanent = ((TargetPermanent) target).getFilter(); + } + for (String choose2: choices) { + String[] targetList = choose2.split("\\^"); + boolean targetFound = false; + for (String targetName: targetList) { + for (Permanent permanent : game.getBattlefield().getAllActivePermanents(filterPermanent, game)) { + if (target.getTargets().contains(permanent.getId())) { + continue; } - } else if ((permanent.getName()+"-"+permanent.getExpansionSetCode()).equals(choose2)) { - if (((TargetPermanent)target).canTarget(playerId, permanent.getId(), null, game) && !target.getTargets().contains(permanent.getId())) { - target.add(permanent.getId(), game); - choices.remove(choose2); - return true; + if (permanent.getName().equals(targetName)) { + if (target.isNotTarget() || ((TargetPermanent)target).canTarget(playerId, permanent.getId(), null, game)) { + target.add(permanent.getId(), game); + targetFound = true; + break; + } + } else if ((permanent.getName()+"-"+permanent.getExpansionSetCode()).equals(targetName)) { + if (target.isNotTarget() || ((TargetPermanent)target).canTarget(playerId, permanent.getId(), null, game)) { + target.add(permanent.getId(), game); + targetFound = true; + break; + } } } } + if (targetFound) { + choices.remove(choose2); + return true; + } } } if (target instanceof TargetPlayer) { @@ -362,7 +380,7 @@ public class TestPlayer extends ComputerPlayer { @Override public boolean chooseTarget(Outcome outcome, Target target, Ability source, Game game) { if (!targets.isEmpty()) { - if (target instanceof TargetPermanent) { + if ((target instanceof TargetPermanent) || (target instanceof TargetPermanentOrPlayer)) { for (String targetDefinition: targets) { String[] targetList = targetDefinition.split("\\^"); boolean targetFound = false; diff --git a/Mage/src/mage/abilities/effects/common/counter/ProliferateEffect.java b/Mage/src/mage/abilities/effects/common/counter/ProliferateEffect.java index bd1a44e939..6d86478974 100644 --- a/Mage/src/mage/abilities/effects/common/counter/ProliferateEffect.java +++ b/Mage/src/mage/abilities/effects/common/counter/ProliferateEffect.java @@ -53,7 +53,7 @@ public class ProliferateEffect extends OneShotEffect { public ProliferateEffect() { super(Outcome.Benefit); - staticText = "Proliferate. (You choose any number of permanents and/or players with counters on them, then give each another counter of a kind already there.)"; + staticText = "Proliferate. (You choose any number of permanents and/or players with counters on them, then give each another counter of a kind already there.)"; } public ProliferateEffect(ProliferateEffect effect) { @@ -63,81 +63,71 @@ public class ProliferateEffect extends OneShotEffect { @Override public boolean apply(Game game, Ability source) { Player controller = game.getPlayer(source.getControllerId()); + if (controller == null) { + return false; + } Target target = new TargetPermanentOrPlayerWithCounter(0, Integer.MAX_VALUE, true); + Map options = new HashMap<>(); + options.put("UI.right.btn.text", "Done"); + controller.choose(Outcome.Benefit, target, source.getSourceId(), game, options); - //A spell or ability could have removed the only legal target this player - //had, if thats the case this ability should fizzle. - if (target.canChoose(controller.getId(), game)) { - boolean abilityApplied = false; - Map options = new HashMap<>(); - options.put("UI.right.btn.text", "Done"); - while (target.canChoose(controller.getId(), game)) { - if (controller.choose(Outcome.Benefit, target, source.getSourceId(), game, options)) { - break; - } - } - - for (int idx = 0; idx < target.getTargets().size(); idx++) { - UUID chosen = (UUID) target.getTargets().get(idx); - Permanent permanent = game.getPermanent(chosen); - if (permanent != null) { - if (permanent.getCounters().size() > 0) { - if (permanent.getCounters().size() == 1) { - for (Counter counter : permanent.getCounters().values()) { + for (int idx = 0; idx < target.getTargets().size(); idx++) { + UUID chosen = (UUID) target.getTargets().get(idx); + Permanent permanent = game.getPermanent(chosen); + if (permanent != null) { + if (permanent.getCounters().size() > 0) { + if (permanent.getCounters().size() == 1) { + for (Counter counter : permanent.getCounters().values()) { + permanent.addCounters(counter.getName(), 1, game); + } + } else { + Choice choice = new ChoiceImpl(true); + Set choices = new HashSet<>(); + for (Counter counter : permanent.getCounters().values()) { + choices.add(counter.getName()); + } + choice.setChoices(choices); + choice.setMessage("Choose a counter to proliferate (" + permanent.getName() + ")"); + controller.choose(Outcome.Benefit, choice, game); + for (Counter counter : permanent.getCounters().values()) { + if (counter.getName().equals(choice.getChoice())) { permanent.addCounters(counter.getName(), 1, game); + break; + } + } + } + } + } else { + Player player = game.getPlayer(chosen); + if (player != null) { + if (player.getCounters().size() > 0) { + if (player.getCounters().size() == 1) { + for (Counter counter : player.getCounters().values()) { + Counter newCounter = new Counter(counter.getName()); + player.addCounters(newCounter, game); } } else { Choice choice = new ChoiceImpl(true); Set choices = new HashSet<>(); - for (Counter counter : permanent.getCounters().values()) { + for (Counter counter : player.getCounters().values()) { choices.add(counter.getName()); } choice.setChoices(choices); - choice.setMessage("Choose a counter to proliferate (" + permanent.getName() + ")"); + choice.setMessage("Choose a counter to proliferate (" + player.getName() + ")"); controller.choose(Outcome.Benefit, choice, game); - for (Counter counter : permanent.getCounters().values()) { + for (Counter counter : player.getCounters().values()) { if (counter.getName().equals(choice.getChoice())) { - permanent.addCounters(counter.getName(), 1, game); + Counter newCounter = new Counter(counter.getName()); + player.addCounters(newCounter, game); break; } } } } - } else { - Player player = game.getPlayer(chosen); - if (player != null) { - if (player.getCounters().size() > 0) { - if (player.getCounters().size() == 1) { - for (Counter counter : player.getCounters().values()) { - Counter newCounter = new Counter(counter.getName()); - player.addCounters(newCounter, game); - } - } else { - Choice choice = new ChoiceImpl(true); - Set choices = new HashSet<>(); - for (Counter counter : player.getCounters().values()) { - choices.add(counter.getName()); - } - choice.setChoices(choices); - choice.setMessage("Choose a counter to proliferate (" + player.getName() + ")"); - controller.choose(Outcome.Benefit, choice, game); - for (Counter counter : player.getCounters().values()) { - if (counter.getName().equals(choice.getChoice())) { - Counter newCounter = new Counter(counter.getName()); - player.addCounters(newCounter, game); - break; - } - } - } - } - } } - } - - return abilityApplied; } - return false; + return true; } @Override diff --git a/Mage/src/mage/filter/predicate/permanent/CounterPredicate.java b/Mage/src/mage/filter/predicate/permanent/CounterPredicate.java index eb66dacc00..24b1d53669 100644 --- a/Mage/src/mage/filter/predicate/permanent/CounterPredicate.java +++ b/Mage/src/mage/filter/predicate/permanent/CounterPredicate.java @@ -40,13 +40,21 @@ public class CounterPredicate implements Predicate { private final CounterType counter; + /** + * + * @param counter if null any counter selects the permanent + */ public CounterPredicate(CounterType counter) { this.counter = counter; } @Override public boolean apply(Permanent input, Game game) { - return input.getCounters().containsKey(counter); + if (counter == null) { + return !input.getCounters().keySet().isEmpty(); + } else { + return input.getCounters().containsKey(counter); + } } @Override diff --git a/Mage/src/mage/target/TargetPermanent.java b/Mage/src/mage/target/TargetPermanent.java index baaac9edf3..222e08d058 100644 --- a/Mage/src/mage/target/TargetPermanent.java +++ b/Mage/src/mage/target/TargetPermanent.java @@ -89,7 +89,7 @@ public class TargetPermanent extends TargetObject { // second for protection from sources (e.g. protection from artifacts + equip ability) if (!isNotTarget()) { if (!permanent.canBeTargetedBy(game.getObject(source.getId()), controllerId, game) || - !permanent.canBeTargetedBy(game.getObject(source.getSourceId()), controllerId, game)) { + !permanent.canBeTargetedBy(game.getObject(source.getSourceId()), controllerId, game)) { return false; } } diff --git a/Mage/src/mage/target/common/TargetPermanentOrPlayer.java b/Mage/src/mage/target/common/TargetPermanentOrPlayer.java index 5923470bb1..66ddcb7a68 100644 --- a/Mage/src/mage/target/common/TargetPermanentOrPlayer.java +++ b/Mage/src/mage/target/common/TargetPermanentOrPlayer.java @@ -71,7 +71,7 @@ public class TargetPermanentOrPlayer extends TargetImpl { } public TargetPermanentOrPlayer(int minNumTargets, int maxNumTargets, boolean notTarget) { - this(minNumTargets, maxNumTargets); + this(minNumTargets, maxNumTargets); this.notTarget = notTarget; } @@ -116,10 +116,21 @@ public class TargetPermanentOrPlayer extends TargetImpl { if (source != null) { MageObject targetSource = game.getObject(source.getSourceId()); if (permanent != null) { - return permanent.canBeTargetedBy(targetSource, source.getControllerId(), game) && filter.match(permanent, source.getSourceId(), source.getControllerId(), game); + if (!isNotTarget()) { + if (!permanent.canBeTargetedBy(game.getObject(source.getId()), source.getControllerId(), game) || + !permanent.canBeTargetedBy(game.getObject(source.getSourceId()), source.getControllerId(), game)) { + return false; + } + } + return filter.match(permanent, source.getSourceId(), source.getControllerId(), game); } if (player != null) { - return player.canBeTargetedBy(targetSource, game) && filter.match(player, game); + if (!isNotTarget()) { + if (!player.canBeTargetedBy(targetSource, game)) { + return false; + } + } + return filter.match(player, game); } } @@ -202,12 +213,12 @@ public class TargetPermanentOrPlayer extends TargetImpl { MageObject targetSource = game.getObject(sourceId); for (UUID playerId: game.getPlayer(sourceControllerId).getInRange()) { Player player = game.getPlayer(playerId); - if (player != null && player.canBeTargetedBy(targetSource, game) && filter.match(player, game)) { + if (player != null && (notTarget || player.canBeTargetedBy(targetSource, game)) && filter.match(player, game)) { possibleTargets.add(playerId); } } for (Permanent permanent: game.getBattlefield().getActivePermanents(new FilterPermanent(), sourceControllerId, game)) { - if (permanent.canBeTargetedBy(targetSource, sourceControllerId, game) && filter.match(permanent, sourceId, sourceControllerId, game)) { + if ((notTarget || permanent.canBeTargetedBy(targetSource, sourceControllerId, game)) && filter.match(permanent, sourceId, sourceControllerId, game)) { possibleTargets.add(permanent.getId()); } } @@ -252,4 +263,7 @@ public class TargetPermanentOrPlayer extends TargetImpl { return new TargetPermanentOrPlayer(this); } + public FilterPermanent getFilterPermanent() { + return filterPermanent.copy(); + } } diff --git a/Mage/src/mage/target/common/TargetPermanentOrPlayerWithCounter.java b/Mage/src/mage/target/common/TargetPermanentOrPlayerWithCounter.java index 49ddc3449c..463df69230 100644 --- a/Mage/src/mage/target/common/TargetPermanentOrPlayerWithCounter.java +++ b/Mage/src/mage/target/common/TargetPermanentOrPlayerWithCounter.java @@ -34,6 +34,9 @@ import mage.game.Game; import mage.game.permanent.Permanent; import mage.players.Player; import java.util.UUID; +import mage.counters.CounterType; +import mage.filter.FilterPermanent; +import mage.filter.predicate.permanent.CounterPredicate; /** * @@ -52,15 +55,15 @@ public class TargetPermanentOrPlayerWithCounter extends TargetPermanentOrPlayer } public TargetPermanentOrPlayerWithCounter(int minNumTargets, int maxNumTargets) { - super(minNumTargets, maxNumTargets); - this.filter = new FilterPermanentOrPlayerWithCounter(); - this.targetName = filter.getMessage(); - super.setFilter(this.filter); + this(minNumTargets, maxNumTargets, false); } public TargetPermanentOrPlayerWithCounter(int minNumTargets, int maxNumTargets, boolean notTarget) { - this(minNumTargets, maxNumTargets); - this.notTarget = notTarget; + super(minNumTargets, maxNumTargets, notTarget); + this.filter = new FilterPermanentOrPlayerWithCounter(); + this.filterPermanent = new FilterPermanent(); + this.filterPermanent.add(new CounterPredicate(null)); + this.targetName = filter.getMessage(); } public TargetPermanentOrPlayerWithCounter(final TargetPermanentOrPlayerWithCounter target) {