From 73cad47c3c0f10ccedec1e72df210f056b9787ee Mon Sep 17 00:00:00 2001 From: Myrd Date: Mon, 1 Dec 2014 06:06:57 +0000 Subject: [PATCH] Fix bug with convoke where colored mana could end up being used as colorless. Before this patch, the code would try to pay the mana symbols as they appear - for example if you convoke with a white creature and click "white" in the dialog, but the cost for the spell is "1WW", it would actually use that white mana to pay for the "1" portion of the cost, leaving "WW" to be paid (which is problematic if you have a swamp and a plains untapped which you were planning to use in addition to the white creature). This change moves the logic for paying the cost from the UI code to the engine code (and re-uses existing code there) and also adds some unit tests for it. --- .gitattributes | 1 + forge-game/.classpath | 3 +- .../forge/game/mana/ManaCostBeingPaid.java | 20 +++++++++---- .../game/mana/ManaCostBeingPaidTest.java | 30 +++++++++++++++++++ .../input/InputSelectCardsForConvoke.java | 21 +++---------- 5 files changed, 52 insertions(+), 23 deletions(-) create mode 100644 forge-game/src/test/java/forge/game/mana/ManaCostBeingPaidTest.java diff --git a/.gitattributes b/.gitattributes index ffab5d906cc..16311cb022a 100644 --- a/.gitattributes +++ b/.gitattributes @@ -702,6 +702,7 @@ forge-game/src/main/java/forge/trackable/TrackableSerializer.java -text forge-game/src/main/java/forge/trackable/TrackableTypes.java -text forge-game/src/main/java/forge/util/Expressions.java -text forge-game/src/main/java/forge/util/MessageUtil.java -text +forge-game/src/test/java/forge/game/mana/ManaCostBeingPaidTest.java -text forge-gui-android/.classpath -text forge-gui-android/.project -text forge-gui-android/.settings/org.eclipse.core.resources.prefs -text diff --git a/forge-game/.classpath b/forge-game/.classpath index 51baab63605..4072c2cbeb3 100644 --- a/forge-game/.classpath +++ b/forge-game/.classpath @@ -5,5 +5,6 @@ + - \ No newline at end of file + diff --git a/forge-game/src/main/java/forge/game/mana/ManaCostBeingPaid.java b/forge-game/src/main/java/forge/game/mana/ManaCostBeingPaid.java index 3142ff73574..21488d424c7 100644 --- a/forge-game/src/main/java/forge/game/mana/ManaCostBeingPaid.java +++ b/forge-game/src/main/java/forge/game/mana/ManaCostBeingPaid.java @@ -321,7 +321,7 @@ public class ManaCostBeingPaid { } }; - return tryPayMana(colorMask, Iterables.filter(unpaidShards.keySet(), predCanBePaid), pool.getPossibleColorUses(colorMask)); + return tryPayMana(colorMask, Iterables.filter(unpaidShards.keySet(), predCanBePaid), pool.getPossibleColorUses(colorMask)) != null; } /** @@ -347,10 +347,20 @@ public class ManaCostBeingPaid { byte inColor = mana.getColor(); byte outColor = pool.getPossibleColorUses(inColor); - return tryPayMana(inColor, Iterables.filter(unpaidShards.keySet(), predCanBePaid), outColor); + return tryPayMana(inColor, Iterables.filter(unpaidShards.keySet(), predCanBePaid), outColor) != null; + } + + public final ManaCostShard payManaViaConvoke(final byte color) { + Predicate predCanBePaid = new Predicate() { + @Override + public boolean apply(ManaCostShard ms) { + return ms.canBePaidWithManaOfColor(color); + } + }; + return tryPayMana(color, Iterables.filter(unpaidShards.keySet(), predCanBePaid), (byte)0xFF); } - private boolean tryPayMana(final byte colorMask, Iterable payableShards, byte possibleUses) { + private ManaCostShard tryPayMana(final byte colorMask, Iterable payableShards, byte possibleUses) { Set choice = EnumSet.noneOf(ManaCostShard.class); int priority = Integer.MIN_VALUE; for (ManaCostShard toPay : payableShards) { @@ -365,7 +375,7 @@ public class ManaCostBeingPaid { } } if (choice.isEmpty()) { - return false; + return null; } ManaCostShard chosenShard = Iterables.getFirst(choice, null); @@ -391,7 +401,7 @@ public class ManaCostBeingPaid { } this.sunburstMap |= colorMask; - return true; + return chosenShard; } private int getPayPriority(ManaCostShard bill, byte paymentColor) { diff --git a/forge-game/src/test/java/forge/game/mana/ManaCostBeingPaidTest.java b/forge-game/src/test/java/forge/game/mana/ManaCostBeingPaidTest.java new file mode 100644 index 00000000000..90dc26e3e61 --- /dev/null +++ b/forge-game/src/test/java/forge/game/mana/ManaCostBeingPaidTest.java @@ -0,0 +1,30 @@ +package forge.game.mana; + +import junit.framework.TestCase; + +import static forge.card.MagicColor.*; +import forge.card.mana.ManaCost; +import forge.card.mana.ManaCostParser; + +public class ManaCostBeingPaidTest extends TestCase { + public void testPayManaViaConvoke() { + runConvokeTest("1 W W", new byte[] { WHITE, COLORLESS, WHITE }, new String[] {"{1}{W}{W}", "{1}{W}", "{W}"}); + runConvokeTest("1 W W", new byte[] { COLORLESS, WHITE, WHITE }, new String[] {"{1}{W}{W}", "{W}{W}", "{W}"}); + runConvokeTest("1 W W", new byte[] { GREEN, WHITE, WHITE }, new String[] {"{1}{W}{W}", "{W}{W}", "{W}"}); + runConvokeTest("1 W G", new byte[] { GREEN, RED, WHITE }, new String[] {"{1}{W}{G}", "{1}{W}", "{W}"}); + } + + private void runConvokeTest(String initialCost, byte[] colorsToPay, String[] expectedRemainder) { + ManaCostBeingPaid cost = createManaCostBeingPaid(initialCost); + for (int i = 0; i < colorsToPay.length; i++) { + assertEquals(expectedRemainder[i], cost.toString()); + cost.payManaViaConvoke(colorsToPay[i]); + } + assertEquals("0", cost.toString()); + } + + private ManaCostBeingPaid createManaCostBeingPaid(String cost) { + ManaCostParser parser = new ManaCostParser(cost); + return new ManaCostBeingPaid(new ManaCost(parser)); + } +} diff --git a/forge-gui/src/main/java/forge/match/input/InputSelectCardsForConvoke.java b/forge-gui/src/main/java/forge/match/input/InputSelectCardsForConvoke.java index fed6f93a2d9..61ca9997d25 100644 --- a/forge-gui/src/main/java/forge/match/input/InputSelectCardsForConvoke.java +++ b/forge-gui/src/main/java/forge/match/input/InputSelectCardsForConvoke.java @@ -52,18 +52,12 @@ public final class InputSelectCardsForConvoke extends InputSelectManyBase } else { byte chosenColor = player.getController().chooseColorAllowColorless("Convoke " + card.toString() + " for which color?", card, CardUtil.getColors(card)); - - if (remainingCost.getColorlessManaAmount() > 0 && (chosenColor == 0 || !remainingCost.needsColor(chosenColor, player.getManaPool()))) { - registerConvoked(card, ManaCostShard.COLORLESS, chosenColor); + ManaCostShard shard = remainingCost.payManaViaConvoke(chosenColor); + if (shard != null) { + chosenCards.put(card, ImmutablePair.of(chosenColor, shard)); + onSelectStateChanged(card, true); } else { - for (ManaCostShard shard : remainingCost.getDistinctShards()) { - if (shard.canBePaidWithManaOfColor(chosenColor)) { - registerConvoked(card, shard, chosenColor); - refresh(); - return true; - } - } showMessage("The colors provided by " + card.toString() + " you've chosen cannot be used to decrease the manacost of " + remainingCost.toString()); return false; } @@ -73,13 +67,6 @@ public final class InputSelectCardsForConvoke extends InputSelectManyBase return true; } - private void registerConvoked(Card card, ManaCostShard shard, byte chosenColor) { - remainingCost.decreaseShard(shard, 1); - chosenCards.put(card, ImmutablePair.of(chosenColor, shard)); - onSelectStateChanged(card, true); - } - - @Override protected final void onPlayerSelected(Player player, final ITriggerEvent triggerEvent) { }