From 2c617a6aafc95b92b0b8978017c54b4ea923a015 Mon Sep 17 00:00:00 2001 From: poixen Date: Thu, 19 Nov 2015 17:09:40 +0100 Subject: [PATCH 1/2] Mana updates + calling subtraction will now throw an exception if you try and use more mana than is available. This is better than setting it to 0. Setting to 0 impose that you should still be allowed to perform the action. + updated subtraction test to check for exception + subtractionCost() will not allow using mana that is not available, same as subtract() --- .../java/org/mage/test/mana/ManaTest.java | 61 +++++++++++++++-- Mage/src/mage/Mana.java | 65 ++++++++++++++----- 2 files changed, 101 insertions(+), 25 deletions(-) diff --git a/Mage.Tests/src/test/java/org/mage/test/mana/ManaTest.java b/Mage.Tests/src/test/java/org/mage/test/mana/ManaTest.java index 7cfa5a88571..ba842ca32f1 100644 --- a/Mage.Tests/src/test/java/org/mage/test/mana/ManaTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/mana/ManaTest.java @@ -1,5 +1,6 @@ package org.mage.test.mana; +import junit.framework.Assert; import mage.Mana; import mage.constants.ColoredManaSymbol; import mage.constants.ManaType; @@ -434,6 +435,8 @@ public class ManaTest { @Test public void shouldNotSubtractLessThan0() { // given + expectedException.expect(ArithmeticException.class); + expectedException.expectMessage("You can not subtract below 0"); Mana thisMana = new Mana(2, 2, 2, 2, 2, 2, 2); Mana thatMana = new Mana(10, 1, 1, 1, 10, 1, 1); @@ -441,13 +444,57 @@ public class ManaTest { thisMana.subtract(thatMana); // then - assertEquals(-8, thisMana.getRed()); - assertEquals(1, thisMana.getGreen()); - assertEquals(1, thisMana.getBlue()); - assertEquals(1, thisMana.getWhite()); - assertEquals(-8, thisMana.getBlack()); - assertEquals(1, thisMana.getColorless()); - assertEquals(1, thisMana.getAny()); + } + + + @Test + public void shouldNotAllowMinusSubtractionCost() { + // given + expectedException.expect(ArithmeticException.class); + expectedException.expectMessage("You can not subtract below 0"); + Mana thisMana = new Mana(2, 2, 2, 2, 2, 2, 2); + Mana thatMana = new Mana(10, 1, 1, 1, 10, 1, 1); + + // when + thisMana.subtractCost(thatMana); + + // then + + } + + + @Test + public void shouldUseExistingManaToPayColorless() { + // given + Mana available = new Mana(); + available.setRed(7); + + Mana cost = new Mana(); + cost.setRed(4); + cost.setColorless(2); + + // when + available.subtractCost(cost); + + // then + assertEquals(1, available.getRed()); + } + + + @Test + public void shouldThrowExceptionOnUnavailableColorless() { + // given + expectedException.expect(ArithmeticException.class); + expectedException.expectMessage("Not enough mana to pay colorless"); + Mana available = new Mana(); + available.setRed(4); + + Mana cost = new Mana(); + cost.setRed(4); + cost.setColorless(2); + + // when + available.subtractCost(cost); } diff --git a/Mage/src/mage/Mana.java b/Mage/src/mage/Mana.java index 0038727309b..5e46cfb74a8 100644 --- a/Mage/src/mage/Mana.java +++ b/Mage/src/mage/Mana.java @@ -63,7 +63,8 @@ public class Mana implements Comparable, Serializable, Copyable { protected int any; protected boolean flag = false; - //todo unsafe and mutable + //TODO THIS IS UNSAFE AND MUTABLE + //TODO THIS SHOULD BE REMOVED public static final Mana RedMana = RedMana(1); public static final Mana GreenMana = GreenMana(1); public static final Mana BlueMana = BlueMana(1); @@ -268,24 +269,52 @@ public class Mana implements Comparable, Serializable, Copyable { * * @param mana mana values to subtract */ - public void subtract(final Mana mana) { - red -= mana.red; - green -= mana.green; - blue -= mana.blue; - white -= mana.white; - black -= mana.black; - colorless -= mana.colorless; - any -= mana.any; + public void subtract(final Mana mana) throws ArithmeticException { + red = validateSubtraction(red, mana.red); + green = validateSubtraction(green, mana.green); + blue = validateSubtraction(blue, mana.blue); + white = validateSubtraction(white, mana.white); + black = validateSubtraction(black, mana.black); + colorless = validateSubtraction(colorless, mana.colorless); + any = validateSubtraction(any, mana.any); } - public void subtractCost(Mana cost) { - red -= cost.red; - green -= cost.green; - blue -= cost.blue; - white -= cost.white; - black -= cost.black; - any -= cost.any; - colorless -= cost.colorless; + /** + * Ensures subtraction will not result in a negative number. + * + * @param lhs left hand side operand + * @param rhs right hand side operand + * @return returns the non-negative subtraction result + * @throws ArithmeticException thrown when the result of the subtraction + * is less than 0. + */ + private int validateSubtraction(final int lhs, final int rhs) throws ArithmeticException { + int result = lhs - rhs; + if (result < 0) { + throw new ArithmeticException("You can not subtract below 0"); + } + return result; + } + + + /** + * Subtracts the passed in mana values from this instance. Will not + * reduce this instances mana below 0. The difference between this and + * {@code subtract()} is that if we do not have the available colorlesss + * mana to pay, we take mana from our colored mana pools. + * + * @param mana mana values to subtract + * @throws ArithmeticException thrown if there is not enough available + * colored mana to make up the negative colorless cost + */ + public void subtractCost(final Mana mana) throws ArithmeticException { + red = validateSubtraction(red, mana.red); + green = validateSubtraction(green, mana.green); + blue = validateSubtraction(blue, mana.blue); + white = validateSubtraction(white, mana.white); + black = validateSubtraction(black, mana.black); + any = validateSubtraction(any, mana.any); + colorless -= mana.colorless; // can be minus, will use remaining mana to pay while (colorless < 0) { int oldColorless = colorless; @@ -318,7 +347,7 @@ public class Mana implements Comparable, Serializable, Copyable { colorless++; } if (oldColorless == colorless) { - break; // to prevent endless loop -> should not be possible, but who knows + throw new ArithmeticException("Not enough mana to pay colorless"); } } } From 05b841577a1f97162ff4511205624da1c64bd337 Mon Sep 17 00:00:00 2001 From: poixen Date: Thu, 19 Nov 2015 17:42:59 +0100 Subject: [PATCH 2/2] added subtraction logic to enough() + added subornation logic to enough(). We do not want to mix the public subtract() call with this method. As doing so would mean that either a) subtraction can go below 0 (this should not happen) or b) we break this function with exceptions. This is a work around for these scenarios. --- Mage.Tests/src/test/java/org/mage/test/mana/ManaTest.java | 1 - Mage/src/mage/Mana.java | 8 +++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/Mage.Tests/src/test/java/org/mage/test/mana/ManaTest.java b/Mage.Tests/src/test/java/org/mage/test/mana/ManaTest.java index ba842ca32f1..6d31cb52bc7 100644 --- a/Mage.Tests/src/test/java/org/mage/test/mana/ManaTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/mana/ManaTest.java @@ -1,6 +1,5 @@ package org.mage.test.mana; -import junit.framework.Assert; import mage.Mana; import mage.constants.ColoredManaSymbol; import mage.constants.ManaType; diff --git a/Mage/src/mage/Mana.java b/Mage/src/mage/Mana.java index 5e46cfb74a8..bccbdd7f846 100644 --- a/Mage/src/mage/Mana.java +++ b/Mage/src/mage/Mana.java @@ -507,7 +507,13 @@ public class Mana implements Comparable, Serializable, Copyable { */ public boolean enough(final Mana avail) { Mana compare = avail.copy(); - compare.subtract(this); + red -= avail.red; + green -= avail.green; + blue -= avail.blue; + white -= avail.white; + black -= avail.black; + colorless -= avail.colorless; + any -= avail.any; if (compare.getRed() < 0) { compare.setAny(compare.getAny() + compare.getRed()); if (compare.getAny() < 0) {