From 41af3e9d4a381e09035e850344c053bd45fcaec3 Mon Sep 17 00:00:00 2001 From: LevelX2 Date: Sat, 5 Mar 2016 10:33:27 +0100 Subject: [PATCH] * Disrupting Shoal - Fixed that the converted mana costs of split cards were not handled correctly. --- .../betrayersofkamigawa/DisruptingShoal.java | 5 +- .../counterspell/DisruptingShoalTest.java | 162 +++++++++--------- .../ExileFromHandCostCardConvertedMana.java | 33 ++++ 3 files changed, 118 insertions(+), 82 deletions(-) diff --git a/Mage.Sets/src/mage/sets/betrayersofkamigawa/DisruptingShoal.java b/Mage.Sets/src/mage/sets/betrayersofkamigawa/DisruptingShoal.java index 4266767d1c1..6474d3c57a9 100644 --- a/Mage.Sets/src/mage/sets/betrayersofkamigawa/DisruptingShoal.java +++ b/Mage.Sets/src/mage/sets/betrayersofkamigawa/DisruptingShoal.java @@ -33,7 +33,6 @@ import mage.abilities.Ability; import mage.abilities.Mode; import mage.abilities.costs.AlternativeCostSourceAbility; import mage.abilities.costs.common.ExileFromHandCost; -import mage.abilities.dynamicvalue.DynamicValue; import mage.abilities.dynamicvalue.common.ExileFromHandCostCardConvertedMana; import mage.abilities.effects.OneShotEffect; import mage.cards.CardImpl; @@ -60,7 +59,6 @@ public class DisruptingShoal extends CardImpl { this.expansionSetCode = "BOK"; this.subtype.add("Arcane"); - // You may exile a blue card with converted mana cost X from your hand rather than pay Disrupting Shoal's mana cost. FilterOwnedCard filter = new FilterOwnedCard("a blue card with converted mana cost X from your hand"); filter.add(new ColorPredicate(ObjectColor.BLUE)); @@ -100,9 +98,8 @@ class DisruptingShoalCounterTargetEffect extends OneShotEffect { @Override public boolean apply(Game game, Ability source) { - DynamicValue amount = new ExileFromHandCostCardConvertedMana(); Spell spell = game.getStack().getSpell(targetPointer.getFirst(game, source)); - if (spell != null && spell.getConvertedManaCost() == amount.calculate(game, source, this)) { + if (spell != null && new ExileFromHandCostCardConvertedMana().isConvertedManaCostEqual(game, source, this, spell.getConvertedManaCost())) { return game.getStack().counter(source.getFirstTarget(), source.getSourceId(), game); } return false; diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/abilities/oneshot/counterspell/DisruptingShoalTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/abilities/oneshot/counterspell/DisruptingShoalTest.java index 5900a917886..2a61230e91c 100644 --- a/Mage.Tests/src/test/java/org/mage/test/cards/abilities/oneshot/counterspell/DisruptingShoalTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/cards/abilities/oneshot/counterspell/DisruptingShoalTest.java @@ -36,13 +36,12 @@ import org.mage.test.serverside.base.CardTestPlayerBase; * * @author LevelX2 */ - public class DisruptingShoalTest extends CardTestPlayerBase { /** - * Test that Disrupting Shoal can be played with alternate casting costs - * And the X Value is equal to the CMC of the exiled blue card - * + * Test that Disrupting Shoal can be played with alternate casting costs And + * the X Value is equal to the CMC of the exiled blue card + * */ @Test public void testWithBlueCardsInHand() { @@ -50,137 +49,144 @@ public class DisruptingShoalTest extends CardTestPlayerBase { addCard(Zone.HAND, playerA, "Spell Snare"); addCard(Zone.BATTLEFIELD, playerA, "Plains", 3); addCard(Zone.BATTLEFIELD, playerA, "Island", 4); - + + // You may exile a blue card with converted mana cost X from your hand rather than pay Disrupting Shoal's mana cost. + // Counter target spell if its converted mana cost is X. addCard(Zone.HAND, playerB, "Disrupting Shoal"); addCard(Zone.HAND, playerB, "Mistfire Adept", 2); // blue cards with 4 CMC to pay Disrupting Shoal addCard(Zone.BATTLEFIELD, playerB, "Island", 2); - + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Pillarfield Ox"); - + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerB, "Disrupting Shoal", "Pillarfield Ox", "Pillarfield Ox"); playerB.addChoice("Yes"); // use alternate costs = Mistfire Adept = CMC = 4 castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Spell Snare", "Disrupting Shoal", "Disrupting Shoal"); - - + setStopAt(1, PhaseStep.CLEANUP); execute(); - assertLife(playerA, 20); - assertLife(playerB, 20); + assertLife(playerA, 20); + assertLife(playerB, 20); - assertGraveyardCount(playerB,"Disrupting Shoal", 1); + assertGraveyardCount(playerB, "Disrupting Shoal", 1); assertExileCount(playerB, 1); // Mistfire Adept assertHandCount(playerB, "Mistfire Adept", 1); // One Left - + assertHandCount(playerA, "Spell Snare", 1); // Can't be cast -> no valid target - + assertGraveyardCount(playerA, "Pillarfield Ox", 1); } - + /** - * Test that Disrupting Shoal can be played with alternate casting costs - * And the X Value can be equal to either half of a fuse card. - * - * Reported bug: "Casting Disrupting Shoal pitching Far // Away does not counter spells with converted mana cost 2 or 3, - * which it should. Instead it does counter spells with converted mana cost 5, which it shouldn't". + * Test that Disrupting Shoal can be played with alternate casting costs And + * the X Value can be equal to either half of a fuse card. + * + * Reported bug: "Casting Disrupting Shoal pitching Far // Away does not + * counter spells with converted mana cost 2 or 3, which it should. Instead + * it does counter spells with converted mana cost 5, which it shouldn't". */ @Test public void testWithFuseCardCounterCMCTwo() { - + // CMC 2 and CMC 3 addCard(Zone.HAND, playerA, "Grizzly Bears"); // 2/2 {1}{G} - + addCard(Zone.BATTLEFIELD, playerA, "Forest", 2); - + + // You may exile a blue card with converted mana cost X from your hand rather than pay Disrupting Shoal's mana cost. + // Counter target spell if its converted mana cost is X. + addCard(Zone.HAND, playerB, "Disrupting Shoal", 1); /** * Far {1}{U} Instant Return target creature to its owner's hand. Away * {2}{B} Instant Target player sacrifices a creature. Fuse (You may * cast one or both halves of this card from your hand.) - */ - addCard(Zone.HAND, playerB, "Disrupting Shoal", 1); - addCard(Zone.HAND, playerB, "Far // Away", 1); - + */ + addCard(Zone.HAND, playerB, "Far // Away", 1); + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Grizzly Bears"); - - castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerB, "Disrupting Shoal", "Grizzly Bears", "Grizzly Bears"); + + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerB, "Disrupting Shoal", "Grizzly Bears"); playerB.addChoice("Yes"); // use alternate costs = 2 CMC = Far - - setStopAt(1, PhaseStep.CLEANUP); - execute(); - - assertExileCount(playerB, 1); // Far // Away should be exiled as part of Disrupting alternative cost - assertGraveyardCount(playerB,"Disrupting Shoal", 1); + + setStopAt(1, PhaseStep.BEGIN_COMBAT); + execute(); + + assertExileCount(playerB, 1); // Far // Away should be exiled as part of Disrupting alternative cost + assertGraveyardCount(playerB, "Disrupting Shoal", 1); assertPermanentCount(playerA, "Grizzly Bears", 0); // should have been countered by Shoal - assertGraveyardCount(playerA, "Grizzly Bears", 1); - } - + assertGraveyardCount(playerA, "Grizzly Bears", 1); + } + /** - * Test that Disrupting Shoal can be played with alternate casting costs - * And the X Value can be equal to either half of a fuse card. - * - * Reported bug: "Casting Disrupting Shoal pitching Far // Away does not counter spells with converted mana cost 2 or 3, - * which it should. Instead it does counter spells with converted mana cost 5, which it shouldn't". + * Test that Disrupting Shoal can be played with alternate casting costs And + * the X Value can be equal to either half of a fuse card. + * + * Reported bug: "Casting Disrupting Shoal pitching Far // Away does not + * counter spells with converted mana cost 2 or 3, which it should. Instead + * it does counter spells with converted mana cost 5, which it shouldn't". */ @Test public void testWithFuseCardCounterCMCThree() { - - addCard(Zone.HAND, playerA, "Centaur Courser"); // 3/3 {2}{G} + + addCard(Zone.HAND, playerA, "Centaur Courser"); // 3/3 {2}{G} addCard(Zone.BATTLEFIELD, playerA, "Forest", 3); - + /** * Far {1}{U} Instant Return target creature to its owner's hand. Away * {2}{B} Instant Target player sacrifices a creature. Fuse (You may * cast one or both halves of this card from your hand.) - */ + */ addCard(Zone.HAND, playerB, "Disrupting Shoal", 1); - addCard(Zone.HAND, playerB, "Far // Away", 1); - + addCard(Zone.HAND, playerB, "Far // Away", 1); + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Centaur Courser"); - + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerB, "Disrupting Shoal", "Centaur Courser", "Centaur Courser"); playerB.addChoice("Yes"); // use alternate costs = 3 CMC = Away - + setStopAt(1, PhaseStep.CLEANUP); - execute(); - - assertExileCount(playerB, 1); // Far // Away should be exiled as part of Disrupting alternative cost - assertGraveyardCount(playerB,"Disrupting Shoal", 1); + execute(); + + assertExileCount(playerB, 1); // Far // Away should be exiled as part of Disrupting alternative cost + assertGraveyardCount(playerB, "Disrupting Shoal", 1); assertPermanentCount(playerA, "Centaur Courser", 0); // should have been countered by Shoal - assertGraveyardCount(playerA, "Centaur Courser", 1); - } - + assertGraveyardCount(playerA, "Centaur Courser", 1); + } + /** - * Test that Disrupting Shoal can be played with alternate casting costs - * And the X Value can be equal to either half of a fuse card. Not the combined cost of both. - * - * Reported bug: "Casting Disrupting Shoal pitching Far // Away does not counter spells with converted mana cost 2 or 3, - * which it should. Instead it does counter spells with converted mana cost 5, which it shouldn't". + * Test that Disrupting Shoal can be played with alternate casting costs And + * the X Value can be equal to either half of a fuse card. Not the combined + * cost of both. + * + * Reported bug: "Casting Disrupting Shoal pitching Far // Away does not + * counter spells with converted mana cost 2 or 3, which it should. Instead + * it does counter spells with converted mana cost 5, which it shouldn't". */ @Test public void testWithFuseCardShouldNotCounterCMCFive() { - - addCard(Zone.HAND, playerA, "Air Elemental"); // 4/4 Flying {3}{U}{U} + + addCard(Zone.HAND, playerA, "Air Elemental"); // 4/4 Flying {3}{U}{U} addCard(Zone.BATTLEFIELD, playerA, "Island", 5); - + /** * Far {1}{U} Instant Return target creature to its owner's hand. Away * {2}{B} Instant Target player sacrifices a creature. Fuse (You may * cast one or both halves of this card from your hand.) - */ + */ addCard(Zone.HAND, playerB, "Disrupting Shoal", 1); - addCard(Zone.HAND, playerB, "Far // Away", 1); - + addCard(Zone.HAND, playerB, "Far // Away", 1); + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Air Elemental"); - + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerB, "Disrupting Shoal", "Air Elemental", "Air Elemental"); playerB.addChoice("Yes"); // use alternate costs = 2 or 3 CMC = Far // Away, not the combined cost! - + setStopAt(1, PhaseStep.CLEANUP); - execute(); - - assertExileCount(playerB, 1); // Far // Away should be exiled as part of Disrupting alternative cost - assertGraveyardCount(playerB,"Disrupting Shoal", 1); + execute(); + + assertExileCount(playerB, 1); // Far // Away should be exiled as part of Disrupting alternative cost + assertGraveyardCount(playerB, "Disrupting Shoal", 1); assertPermanentCount(playerA, "Air Elemental", 1); // should NOT have been countered by Shoal - assertGraveyardCount(playerA, "Air Elemental", 0); - } -} \ No newline at end of file + assertGraveyardCount(playerA, "Air Elemental", 0); + } +} diff --git a/Mage/src/main/java/mage/abilities/dynamicvalue/common/ExileFromHandCostCardConvertedMana.java b/Mage/src/main/java/mage/abilities/dynamicvalue/common/ExileFromHandCostCardConvertedMana.java index 29adcaf1eca..d0d850f808f 100644 --- a/Mage/src/main/java/mage/abilities/dynamicvalue/common/ExileFromHandCostCardConvertedMana.java +++ b/Mage/src/main/java/mage/abilities/dynamicvalue/common/ExileFromHandCostCardConvertedMana.java @@ -33,6 +33,7 @@ import mage.abilities.costs.common.ExileFromHandCost; import mage.abilities.dynamicvalue.DynamicValue; import mage.abilities.effects.Effect; import mage.cards.Card; +import mage.cards.SplitCard; import mage.game.Game; /** @@ -59,6 +60,38 @@ public class ExileFromHandCostCardConvertedMana implements DynamicValue { return sourceAbility.getManaCostsToPay().getX(); } + /** + * This method does only work to compare the cmc for one (or the first card) + * exiled as a cost + * + * @param game + * @param sourceAbility + * @param effect + * @param amount cmc to compare against + * @return + */ + public boolean isConvertedManaCostEqual(Game game, Ability sourceAbility, Effect effect, int amount) { + for (Cost cost : sourceAbility.getCosts()) { + if (cost.isPaid() && cost instanceof ExileFromHandCost) { + for (Card card : ((ExileFromHandCost) cost).getCards()) { + if (card instanceof SplitCard) { + if (((SplitCard) card).getLeftHalfCard().getManaCost().convertedManaCost() == amount) { + return true; + } + if (((SplitCard) card).getRightHalfCard().getManaCost().convertedManaCost() == amount) { + return true; + } + } else if (card.getManaCost().convertedManaCost() == amount) { + return true; + } + return false; + } + + } + } + return false; + } + @Override public ExileFromHandCostCardConvertedMana copy() { return new ExileFromHandCostCardConvertedMana();