diff --git a/Mage.Server.Plugins/Mage.Player.AI/src/main/java/mage/player/ai/ComputerPlayer.java b/Mage.Server.Plugins/Mage.Player.AI/src/main/java/mage/player/ai/ComputerPlayer.java index 617bc182858..f7e0c0a3aa8 100644 --- a/Mage.Server.Plugins/Mage.Player.AI/src/main/java/mage/player/ai/ComputerPlayer.java +++ b/Mage.Server.Plugins/Mage.Player.AI/src/main/java/mage/player/ai/ComputerPlayer.java @@ -1536,7 +1536,7 @@ public class ComputerPlayer extends PlayerImpl implements Player { if (!unpaid.getMana().includesMana(mana)) { continue ManaAbility; } - colored += mana.countColored(); + colored += CardUtil.overflowInc(colored, mana.countColored()); } if (colored > 1 && (cost instanceof ColoredManaCost)) { for (Mana netMana : manaAbility.getNetMana(game)) { @@ -1773,7 +1773,7 @@ public class ComputerPlayer extends PlayerImpl implements Player { a2Max = netMana.count(); } } - return a2Max - a1Max; + return CardUtil.overflowDec(a2Max, a1Max); } }); } diff --git a/Mage.Sets/src/mage/cards/c/CavernOfSouls.java b/Mage.Sets/src/mage/cards/c/CavernOfSouls.java index 3acd3b26d27..48ea307c61d 100644 --- a/Mage.Sets/src/mage/cards/c/CavernOfSouls.java +++ b/Mage.Sets/src/mage/cards/c/CavernOfSouls.java @@ -84,6 +84,20 @@ class CavernOfSoulsManaBuilder extends ConditionalManaBuilder { public String getRule() { return "Spend this mana only to cast a creature spell of the chosen type, and that spell can't be countered"; } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), this.creatureType); + } + + @Override + public boolean equals(Object obj) { + if (!super.equals(obj)) { + return false; + } + + return this.creatureType == ((CavernOfSoulsManaBuilder) obj).creatureType; + } } class CavernOfSoulsConditionalMana extends ConditionalMana { diff --git a/Mage.Sets/src/mage/cards/k/KolvoriGodOfKinship.java b/Mage.Sets/src/mage/cards/k/KolvoriGodOfKinship.java index 5ced6c1550b..2a1cc5831b6 100644 --- a/Mage.Sets/src/mage/cards/k/KolvoriGodOfKinship.java +++ b/Mage.Sets/src/mage/cards/k/KolvoriGodOfKinship.java @@ -1,5 +1,6 @@ package mage.cards.k; +import java.util.Objects; import java.util.UUID; import mage.ConditionalMana; @@ -125,6 +126,20 @@ class TheRinghartCrestManaBuilder extends ConditionalManaBuilder { public String getRule() { return "Spend this mana only to cast a creature spell of the chosen type or a legendary creature spell"; } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), this.creatureType); + } + + @Override + public boolean equals(Object obj) { + if (!super.equals(obj)) { + return false; + } + + return this.creatureType == ((TheRinghartCrestManaBuilder) obj).creatureType; + } } class TheRinghartCrestConditionalMana extends ConditionalMana { diff --git a/Mage.Sets/src/mage/cards/p/PillarOfOrigins.java b/Mage.Sets/src/mage/cards/p/PillarOfOrigins.java index 28cee80e143..f8c5acec9c1 100644 --- a/Mage.Sets/src/mage/cards/p/PillarOfOrigins.java +++ b/Mage.Sets/src/mage/cards/p/PillarOfOrigins.java @@ -1,6 +1,7 @@ package mage.cards.p; +import java.util.Objects; import java.util.UUID; import mage.ConditionalMana; import mage.MageObject; @@ -72,6 +73,21 @@ class PillarOfOriginsManaBuilder extends ConditionalManaBuilder { public String getRule() { return "Spend this mana only to cast a creature spell of the chosen type"; } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), this.creatureType); + } + + @Override + public boolean equals(Object obj) { + if (!super.equals(obj)) { + return false; + } + + return this.creatureType == ((PillarOfOriginsManaBuilder) obj).creatureType; + } + } class PillarOfOriginsConditionalMana extends ConditionalMana { diff --git a/Mage.Sets/src/mage/cards/s/SecludedCourtyard.java b/Mage.Sets/src/mage/cards/s/SecludedCourtyard.java index 90041f9719f..73873fa894f 100644 --- a/Mage.Sets/src/mage/cards/s/SecludedCourtyard.java +++ b/Mage.Sets/src/mage/cards/s/SecludedCourtyard.java @@ -5,6 +5,7 @@ */ package mage.cards.s; +import java.util.Objects; import java.util.UUID; import mage.ConditionalMana; import mage.MageObject; @@ -83,12 +84,24 @@ class SecludedCourtyardManaBuilder extends ConditionalManaBuilder { public String getRule() { return "Spend this mana only to cast a creature spell of the chosen type or activate an ability of a creature or creature card of the chosen type"; } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), this.creatureType); + } + + @Override + public boolean equals(Object obj) { + if (!super.equals(obj)) { + return false; + } + + return this.creatureType == ((SecludedCourtyardManaBuilder) obj).creatureType; + } } class SecludedCourtyardConditionalMana extends ConditionalMana { - SubType creatureType; - public SecludedCourtyardConditionalMana(Mana mana, SubType creatureType) { super(mana); staticText = "Spend this mana only to cast a creature spell of the chosen type or activate an ability of a creature or creature card of the chosen type"; diff --git a/Mage.Sets/src/mage/cards/s/SquanderedResources.java b/Mage.Sets/src/mage/cards/s/SquanderedResources.java index 8396e8a9129..d27039222d4 100644 --- a/Mage.Sets/src/mage/cards/s/SquanderedResources.java +++ b/Mage.Sets/src/mage/cards/s/SquanderedResources.java @@ -1,9 +1,6 @@ package mage.cards.s; -import java.util.HashSet; -import java.util.List; -import java.util.Set; -import java.util.UUID; +import java.util.*; import java.util.stream.Collectors; import mage.Mana; import mage.abilities.Ability; @@ -82,8 +79,8 @@ class SquanderedResourcesEffect extends ManaEffect { } allPossibleMana.addMana(currentPossibleMana); } - allPossibleMana.removeDuplicated(); - return allPossibleMana.stream().collect(Collectors.toList()); + allPossibleMana.removeFullyIncludedVariations(); + return new ArrayList<>(allPossibleMana); } return ManaType.getManaListFromManaTypes(getManaTypesFromSacrificedPermanent(game, source), false); } diff --git a/Mage.Sets/src/mage/cards/u/UnclaimedTerritory.java b/Mage.Sets/src/mage/cards/u/UnclaimedTerritory.java index c2f84fb4a31..104a9823bea 100644 --- a/Mage.Sets/src/mage/cards/u/UnclaimedTerritory.java +++ b/Mage.Sets/src/mage/cards/u/UnclaimedTerritory.java @@ -1,6 +1,7 @@ package mage.cards.u; +import java.util.Objects; import java.util.UUID; import mage.ConditionalMana; import mage.MageObject; @@ -79,6 +80,20 @@ class UnclaimedTerritoryManaBuilder extends ConditionalManaBuilder { public String getRule() { return "Spend this mana only to cast a creature spell of the chosen type"; } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), this.creatureType); + } + + @Override + public boolean equals(Object obj) { + if (!super.equals(obj)) { + return false; + } + + return this.creatureType == ((UnclaimedTerritoryManaBuilder) obj).creatureType; + } } class UnclaimedTerritoryConditionalMana extends ConditionalMana { diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/abilities/keywords/BestowTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/abilities/keywords/BestowTest.java index 2e1dd8bffa4..b6bf97604df 100644 --- a/Mage.Tests/src/test/java/org/mage/test/cards/abilities/keywords/BestowTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/cards/abilities/keywords/BestowTest.java @@ -496,7 +496,7 @@ public class BestowTest extends CardTestPlayerBase { assertPermanentCount(playerB, "Song of the Dryads", 1); ManaOptions options = playerA.getAvailableManaTest(currentGame); - Assert.assertEquals("Player should be able to create 1 green mana", "{G}", options.get(0).toString()); + Assert.assertEquals("Player should be able to create 1 green mana", "{G}", options.getAtIndex(0).toString()); assertPermanentCount(playerA, "Nighthowler", 1); assertPowerToughness(playerA, "Nighthowler", 2, 2); diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/flip/SasayaOrochiAscendantTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/flip/SasayaOrochiAscendantTest.java index 2ef951e3ca6..07dd3f2344d 100644 --- a/Mage.Tests/src/test/java/org/mage/test/cards/flip/SasayaOrochiAscendantTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/cards/flip/SasayaOrochiAscendantTest.java @@ -8,7 +8,6 @@ import org.junit.Assert; import org.junit.Test; import org.mage.test.serverside.base.CardTestPlayerBase; -import static org.mage.test.utils.ManaOptionsTestUtils.assertDuplicatedManaOptions; import static org.mage.test.utils.ManaOptionsTestUtils.assertManaOptions; /** @@ -45,7 +44,6 @@ public class SasayaOrochiAscendantTest extends CardTestPlayerBase { assertManaPool(playerA, ManaType.GREEN, 2); ManaOptions manaOptions = playerA.getAvailableManaTest(currentGame); - assertDuplicatedManaOptions(manaOptions); Assert.assertEquals("mana variations don't fit", 1, manaOptions.size()); assertManaOptions("{G}{G}{G}{G}{G}", manaOptions); @@ -81,7 +79,6 @@ public class SasayaOrochiAscendantTest extends CardTestPlayerBase { assertLife(playerA, 18); ManaOptions manaOptions = playerA.getAvailableManaTest(currentGame); - assertDuplicatedManaOptions(manaOptions); Assert.assertEquals("mana variations don't fit", 3, manaOptions.size()); assertManaOptions("{C}{C}{C}{G}{G}", manaOptions); @@ -121,7 +118,6 @@ public class SasayaOrochiAscendantTest extends CardTestPlayerBase { assertLife(playerA, 20); ManaOptions manaOptions = playerA.getAvailableManaTest(currentGame); - assertDuplicatedManaOptions(manaOptions); Assert.assertEquals("mana variations don't fit", 4, manaOptions.size()); assertManaOptions("{R}{R}{R}{R}{G}{G}{G}{G}{G}", manaOptions); diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/mana/HarvesterDruidTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/mana/HarvesterDruidTest.java index cf083a2fbd6..2465a983a75 100644 --- a/Mage.Tests/src/test/java/org/mage/test/cards/mana/HarvesterDruidTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/cards/mana/HarvesterDruidTest.java @@ -7,7 +7,6 @@ import org.junit.Assert; import org.junit.Test; import org.mage.test.serverside.base.CardTestPlayerBase; -import static org.mage.test.utils.ManaOptionsTestUtils.assertDuplicatedManaOptions; import static org.mage.test.utils.ManaOptionsTestUtils.assertManaOptions; /** @@ -27,7 +26,6 @@ public class HarvesterDruidTest extends CardTestPlayerBase { execute(); ManaOptions options = playerA.getAvailableManaTest(currentGame); - assertDuplicatedManaOptions(options); Assert.assertEquals(2, options.size()); assertManaOptions("{U}{R}{R}", options); assertManaOptions("{U}{U}{R}", options); @@ -45,7 +43,6 @@ public class HarvesterDruidTest extends CardTestPlayerBase { execute(); ManaOptions options = playerA.getAvailableManaTest(currentGame); - assertDuplicatedManaOptions(options); Assert.assertEquals(3, options.size()); assertManaOptions("{U}{R}{R}{R}", options); assertManaOptions("{U}{U}{R}{R}", options); diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/mana/NagaVitalistTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/mana/NagaVitalistTest.java index 0bec8c2ea56..ab3fbb4b1b0 100644 --- a/Mage.Tests/src/test/java/org/mage/test/cards/mana/NagaVitalistTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/cards/mana/NagaVitalistTest.java @@ -8,7 +8,6 @@ import org.junit.Assert; import org.junit.Test; import org.mage.test.serverside.base.CardTestPlayerBase; -import static org.mage.test.utils.ManaOptionsTestUtils.assertDuplicatedManaOptions; import static org.mage.test.utils.ManaOptionsTestUtils.assertManaOptions; /** @@ -50,9 +49,7 @@ public class NagaVitalistTest extends CardTestPlayerBase { execute(); ManaOptions manaOptions = playerA.getAvailableManaTest(currentGame); - assertDuplicatedManaOptions(manaOptions); - Assert.assertEquals("mana variations don't fit", 2, manaOptions.size()); - assertManaOptions("{B}", manaOptions); + Assert.assertEquals("mana variations don't fit", 1, manaOptions.size()); assertManaOptions("{Any}{Any}", manaOptions); } diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/mana/ReflectingPoolTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/mana/ReflectingPoolTest.java index 654be542d9d..451cd190cac 100644 --- a/Mage.Tests/src/test/java/org/mage/test/cards/mana/ReflectingPoolTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/cards/mana/ReflectingPoolTest.java @@ -63,7 +63,7 @@ public class ReflectingPoolTest extends CardTestPlayerBase { execute(); ManaOptions options = playerA.getAvailableManaTest(currentGame); - Assert.assertEquals("Player should be able to create 2 red mana", "{R}{R}", options.get(0).toString()); + Assert.assertEquals("Player should be able to create 2 red mana", "{R}{R}", options.getAtIndex(0).toString()); } @@ -83,7 +83,7 @@ public class ReflectingPoolTest extends CardTestPlayerBase { execute(); ManaOptions options = playerA.getAvailableManaTest(currentGame); - Assert.assertEquals("Player should be able to create 3 red mana", "{R}{R}{R}", options.get(0).toString()); + Assert.assertEquals("Player should be able to create 3 red mana", "{R}{R}{R}", options.getAtIndex(0).toString()); } @@ -103,7 +103,7 @@ public class ReflectingPoolTest extends CardTestPlayerBase { execute(); ManaOptions options = playerA.getAvailableManaTest(currentGame); - Assert.assertEquals("Player should be able to create 3 red mana", "{R}{R}{R}", options.get(0).toString()); + Assert.assertEquals("Player should be able to create 3 red mana", "{R}{R}{R}", options.getAtIndex(0).toString()); } @@ -124,7 +124,7 @@ public class ReflectingPoolTest extends CardTestPlayerBase { execute(); ManaOptions options = playerA.getAvailableManaTest(currentGame); - Assert.assertEquals("Player should be able to create 2 red mana", "{G}{G}", options.get(0).toString()); + Assert.assertEquals("Player should be able to create 2 green mana", "{G}{G}", options.getAtIndex(0).toString()); } diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/mana/SylvokExplorerTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/mana/SylvokExplorerTest.java index 8fb9d485b7a..050e36798e7 100644 --- a/Mage.Tests/src/test/java/org/mage/test/cards/mana/SylvokExplorerTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/cards/mana/SylvokExplorerTest.java @@ -26,8 +26,8 @@ public class SylvokExplorerTest extends CardTestPlayerBase { execute(); ManaOptions options = playerA.getAvailableManaTest(currentGame); - Assert.assertEquals("Player should be able to create 1 red and 1 white mana", "{R}{W}", options.get(0).toString()); - Assert.assertEquals("Player should be able to create 1 blue and 1 white mana", "{W}{U}", options.get(1).toString()); + Assert.assertEquals("Player should be able to create 1 red and 1 white mana", "{R}{W}", options.getAtIndex(0).toString()); + Assert.assertEquals("Player should be able to create 1 blue and 1 white mana", "{W}{U}", options.getAtIndex(1).toString()); } @Test @@ -45,6 +45,6 @@ public class SylvokExplorerTest extends CardTestPlayerBase { execute(); ManaOptions options = playerA.getAvailableManaTest(currentGame); - Assert.assertEquals("Player should be able to create 3 white mana", "{W}{W}{W}", options.get(0).toString()); + Assert.assertEquals("Player should be able to create 3 white mana", "{W}{W}{W}", options.getAtIndex(0).toString()); } } diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/mana/conditional/ConditionalManaTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/mana/conditional/ConditionalManaTest.java index 402f2e4ba0a..56498dcbbd7 100644 --- a/Mage.Tests/src/test/java/org/mage/test/cards/mana/conditional/ConditionalManaTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/cards/mana/conditional/ConditionalManaTest.java @@ -1,5 +1,8 @@ package org.mage.test.cards.mana.conditional; +import mage.ConditionalMana; +import mage.Mana; +import mage.abilities.condition.common.AdamantCondition; import mage.abilities.keyword.FlyingAbility; import mage.abilities.mana.ManaOptions; import mage.constants.PhaseStep; @@ -9,7 +12,6 @@ import org.junit.Assert; import org.junit.Test; import org.mage.test.serverside.base.CardTestPlayerBase; -import static org.mage.test.utils.ManaOptionsTestUtils.assertDuplicatedManaOptions; import static org.mage.test.utils.ManaOptionsTestUtils.assertManaOptions; /** @@ -330,7 +332,6 @@ public class ConditionalManaTest extends CardTestPlayerBase { execute(); ManaOptions manaOptions = playerA.getAvailableManaTest(currentGame); - assertDuplicatedManaOptions(manaOptions); Assert.assertEquals("mana variations don't fit", 1, manaOptions.size()); assertManaOptions("{R}{R}", manaOptions); } @@ -386,4 +387,34 @@ public class ConditionalManaTest extends CardTestPlayerBase { assertManaOptions("{C}{C}{C}{C}{C}{C}{R}[{RosheenMeandererManaCondition}{TitansNestManaCondition}]", manaOptions); } + @Test + public void testConditionalManaDeduplication() { + ManaOptions manaOptions = new ManaOptions(); + + ConditionalMana originalMana = new ConditionalMana(Mana.GreenMana(1)); + + ConditionalMana mana2 = originalMana.copy(); + mana2.addCondition(AdamantCondition.WHITE); + + ConditionalMana mana3 = originalMana.copy(); + mana3.addCondition(AdamantCondition.BLUE); + ConditionalMana mana3Copy = originalMana.copy(); + mana3Copy.addCondition(AdamantCondition.BLUE); + mana3Copy.add(Mana.GreenMana(1)); + + ConditionalMana mana4 = originalMana.copy(); + mana4.addCondition(AdamantCondition.BLACK); + ConditionalMana mana4Copy = originalMana.copy(); + mana4Copy.addCondition(AdamantCondition.BLACK); + + manaOptions.add(originalMana); + manaOptions.add(mana2); + manaOptions.add(mana3); + manaOptions.add(mana3Copy); // Added, and should remain since different amount of Green mana + manaOptions.add(mana4); + manaOptions.add(mana4Copy); // Adding it to make sure it gets removed + + Assert.assertEquals("Incorrect number of mana", 5, manaOptions.size()); + } + } diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/replacement/ManaReflectionTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/replacement/ManaReflectionTest.java index c1a2e946e38..5fff04c51fc 100644 --- a/Mage.Tests/src/test/java/org/mage/test/cards/replacement/ManaReflectionTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/cards/replacement/ManaReflectionTest.java @@ -9,7 +9,6 @@ import org.junit.Assert; import org.junit.Test; import org.mage.test.serverside.base.CardTestPlayerBase; -import static org.mage.test.utils.ManaOptionsTestUtils.assertDuplicatedManaOptions; import static org.mage.test.utils.ManaOptionsTestUtils.assertManaOptions; public class ManaReflectionTest extends CardTestPlayerBase { @@ -72,7 +71,6 @@ public class ManaReflectionTest extends CardTestPlayerBase { execute(); ManaOptions manaOptions = playerA.getAvailableManaTest(currentGame); - assertDuplicatedManaOptions(manaOptions); Assert.assertEquals("mana variations don't fit", 4, manaOptions.size()); assertManaOptions("{R}{R}{R}{R}{R}{R}{G}{G}", manaOptions); diff --git a/Mage.Tests/src/test/java/org/mage/test/commander/piper/ThePrismaticPiperTest2.java b/Mage.Tests/src/test/java/org/mage/test/commander/piper/ThePrismaticPiperTest2.java index 8d10e22d6b4..b671f3eebf4 100644 --- a/Mage.Tests/src/test/java/org/mage/test/commander/piper/ThePrismaticPiperTest2.java +++ b/Mage.Tests/src/test/java/org/mage/test/commander/piper/ThePrismaticPiperTest2.java @@ -7,8 +7,6 @@ import org.junit.Assert; import org.junit.Test; import org.mage.test.utils.ManaOptionsTestUtils; -import static org.mage.test.utils.ManaOptionsTestUtils.assertDuplicatedManaOptions; - /** * @author TheElk801 */ @@ -44,7 +42,7 @@ public class ThePrismaticPiperTest2 extends ThePrismaticPiperBaseTest { execute(); ManaOptions manaOptions = playerA.getAvailableManaTest(currentGame); - assertDuplicatedManaOptions(manaOptions); + Assert.assertEquals("mana variations don't fit", 2, manaOptions.size()); ManaOptionsTestUtils.assertManaOptions("{U}", manaOptions); ManaOptionsTestUtils.assertManaOptions("{R}", manaOptions); diff --git a/Mage.Tests/src/test/java/org/mage/test/commander/piper/ThePrismaticPiperTest7.java b/Mage.Tests/src/test/java/org/mage/test/commander/piper/ThePrismaticPiperTest7.java index f8457448530..2b0ac7979a0 100644 --- a/Mage.Tests/src/test/java/org/mage/test/commander/piper/ThePrismaticPiperTest7.java +++ b/Mage.Tests/src/test/java/org/mage/test/commander/piper/ThePrismaticPiperTest7.java @@ -13,8 +13,6 @@ import org.mage.test.utils.ManaOptionsTestUtils; import java.util.ArrayList; import java.util.List; -import static org.mage.test.utils.ManaOptionsTestUtils.assertDuplicatedManaOptions; - /** * @author TheElk801 */ @@ -54,7 +52,7 @@ public class ThePrismaticPiperTest7 extends ThePrismaticPiperBaseTest { execute(); ManaOptions manaOptions = playerA.getAvailableManaTest(currentGame); - assertDuplicatedManaOptions(manaOptions); + Assert.assertEquals("mana variations don't fit", 2, manaOptions.size()); ManaOptionsTestUtils.assertManaOptions("{R}", manaOptions); ManaOptionsTestUtils.assertManaOptions("{G}", manaOptions); diff --git a/Mage.Tests/src/test/java/org/mage/test/commander/piper/ThePrismaticPiperTest8.java b/Mage.Tests/src/test/java/org/mage/test/commander/piper/ThePrismaticPiperTest8.java index cd2c4f83748..eeee2aa5612 100644 --- a/Mage.Tests/src/test/java/org/mage/test/commander/piper/ThePrismaticPiperTest8.java +++ b/Mage.Tests/src/test/java/org/mage/test/commander/piper/ThePrismaticPiperTest8.java @@ -7,8 +7,6 @@ import org.junit.Assert; import org.junit.Test; import org.mage.test.utils.ManaOptionsTestUtils; -import static org.mage.test.utils.ManaOptionsTestUtils.assertDuplicatedManaOptions; - /** * @author TheElk801 */ @@ -50,7 +48,7 @@ public class ThePrismaticPiperTest8 extends ThePrismaticPiperBaseTest { execute(); ManaOptions manaOptions = playerA.getAvailableManaTest(currentGame); - assertDuplicatedManaOptions(manaOptions); + Assert.assertEquals("mana variations don't fit", 3, manaOptions.size()); ManaOptionsTestUtils.assertManaOptions("{U}", manaOptions); ManaOptionsTestUtils.assertManaOptions("{R}", manaOptions); diff --git a/Mage.Tests/src/test/java/org/mage/test/utils/ManaOptionsTest.java b/Mage.Tests/src/test/java/org/mage/test/utils/ManaOptionsTest.java index 1aa01b342e5..911c89f653d 100644 --- a/Mage.Tests/src/test/java/org/mage/test/utils/ManaOptionsTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/utils/ManaOptionsTest.java @@ -5,10 +5,10 @@ import mage.constants.PhaseStep; import mage.constants.Zone; import mage.counters.CounterType; import org.junit.Assert; +import org.junit.Ignore; import org.junit.Test; import org.mage.test.serverside.base.CardTestPlayerBase; -import static org.mage.test.utils.ManaOptionsTestUtils.assertDuplicatedManaOptions; import static org.mage.test.utils.ManaOptionsTestUtils.assertManaOptions; /** @@ -27,7 +27,6 @@ public class ManaOptionsTest extends CardTestPlayerBase { execute(); ManaOptions manaOptions = playerA.getAvailableManaTest(currentGame); - assertDuplicatedManaOptions(manaOptions); Assert.assertEquals("mana variations don't fit", 1, manaOptions.size()); assertManaOptions("{G}{G}{G}", manaOptions); @@ -45,7 +44,6 @@ public class ManaOptionsTest extends CardTestPlayerBase { execute(); ManaOptions manaOptions = playerA.getAvailableManaTest(currentGame); - assertDuplicatedManaOptions(manaOptions); Assert.assertEquals("mana variations don't fit", 4, manaOptions.size()); assertManaOptions("{G}{G}{G}", manaOptions); @@ -66,7 +64,6 @@ public class ManaOptionsTest extends CardTestPlayerBase { execute(); ManaOptions manaOptions = playerA.getAvailableManaTest(currentGame); - assertDuplicatedManaOptions(manaOptions); Assert.assertEquals("mana variations don't fit", 10, manaOptions.size()); assertManaOptions("{C}{C}{C}", manaOptions); @@ -92,7 +89,6 @@ public class ManaOptionsTest extends CardTestPlayerBase { execute(); ManaOptions manaOptions = playerA.getAvailableManaTest(currentGame); - assertDuplicatedManaOptions(manaOptions); Assert.assertEquals("mana variations don't fit", 1, manaOptions.size()); assertManaOptions("{Any}{Any}", manaOptions); @@ -110,7 +106,6 @@ public class ManaOptionsTest extends CardTestPlayerBase { execute(); ManaOptions manaOptions = playerA.getAvailableManaTest(currentGame); - assertDuplicatedManaOptions(manaOptions); Assert.assertEquals("mana variations don't fit", 1, manaOptions.size()); assertManaOptions("{W}{W}{Any}{Any}", manaOptions); @@ -129,7 +124,6 @@ public class ManaOptionsTest extends CardTestPlayerBase { execute(); ManaOptions manaOptions = playerA.getAvailableManaTest(currentGame); - assertDuplicatedManaOptions(manaOptions); Assert.assertEquals("mana variations don't fit", 1, manaOptions.size()); assertManaOptions("{C}{G}{G}{W}{W}", manaOptions); @@ -148,7 +142,6 @@ public class ManaOptionsTest extends CardTestPlayerBase { execute(); ManaOptions manaOptions = playerA.getAvailableManaTest(currentGame); - assertDuplicatedManaOptions(manaOptions); Assert.assertEquals("mana variations don't fit", 2, manaOptions.size()); assertManaOptions("{C}{G}{G}{G}{W}{W}", manaOptions); @@ -169,7 +162,6 @@ public class ManaOptionsTest extends CardTestPlayerBase { execute(); ManaOptions manaOptions = playerA.getAvailableManaTest(currentGame); - assertDuplicatedManaOptions(manaOptions); Assert.assertEquals("mana variations don't fit", 3, manaOptions.size()); assertManaOptions("{C}{G}{G}{G}", manaOptions); @@ -188,7 +180,6 @@ public class ManaOptionsTest extends CardTestPlayerBase { execute(); ManaOptions manaOptions = playerA.getAvailableManaTest(currentGame); - assertDuplicatedManaOptions(manaOptions); Assert.assertEquals("mana variations don't fit", 3, manaOptions.size()); assertManaOptions("{C}{G}{G}{G}", manaOptions); @@ -206,7 +197,6 @@ public class ManaOptionsTest extends CardTestPlayerBase { execute(); ManaOptions manaOptions = playerA.getAvailableManaTest(currentGame); - assertDuplicatedManaOptions(manaOptions); Assert.assertEquals("mana variations don't fit", 1, manaOptions.size()); assertManaOptions("{C}{G}{Any}", manaOptions); @@ -225,7 +215,6 @@ public class ManaOptionsTest extends CardTestPlayerBase { execute(); ManaOptions manaOptions = playerA.getAvailableManaTest(currentGame); - assertDuplicatedManaOptions(manaOptions); Assert.assertEquals("mana variations don't fit", 2, manaOptions.size()); assertManaOptions("{C}{G}{G}{G}", manaOptions); @@ -249,7 +238,6 @@ public class ManaOptionsTest extends CardTestPlayerBase { execute(); ManaOptions manaOptions = playerA.getAvailableManaTest(currentGame); - assertDuplicatedManaOptions(manaOptions); Assert.assertEquals("mana variations don't fit", 1, manaOptions.size()); assertManaOptions("{C}{G}{G}{G}", manaOptions); @@ -267,7 +255,6 @@ public class ManaOptionsTest extends CardTestPlayerBase { execute(); ManaOptions manaOptions = playerA.getAvailableManaTest(currentGame); - assertDuplicatedManaOptions(manaOptions); Assert.assertEquals("mana variations don't fit", 3, manaOptions.size()); assertManaOptions("{G}{W}{W}", manaOptions); @@ -296,7 +283,6 @@ public class ManaOptionsTest extends CardTestPlayerBase { execute(); ManaOptions manaOptions = playerA.getAvailableManaTest(currentGame); - assertDuplicatedManaOptions(manaOptions); Assert.assertEquals("mana variations don't fit", 6, manaOptions.size()); assertManaOptions("{C}{G}{G}{G}", manaOptions); @@ -316,7 +302,6 @@ public class ManaOptionsTest extends CardTestPlayerBase { execute(); ManaOptions manaOptions = playerA.getAvailableManaTest(currentGame); - assertDuplicatedManaOptions(manaOptions); } @Test @@ -328,7 +313,6 @@ public class ManaOptionsTest extends CardTestPlayerBase { execute(); ManaOptions manaOptions = playerA.getAvailableManaTest(currentGame); - assertDuplicatedManaOptions(manaOptions); } @Test @@ -357,7 +341,6 @@ public class ManaOptionsTest extends CardTestPlayerBase { ManaOptions manaOptions = playerA.getAvailableManaTest(currentGame); Assert.assertEquals("mana variations don't fit", 3, manaOptions.size()); - assertDuplicatedManaOptions(manaOptions); assertManaOptions("{C}{C}", manaOptions); assertManaOptions("{Any}{Any}", manaOptions); assertManaOptions("{C}{Any}", manaOptions); @@ -374,7 +357,6 @@ public class ManaOptionsTest extends CardTestPlayerBase { execute(); ManaOptions manaOptions = playerA.getAvailableManaTest(currentGame); - assertDuplicatedManaOptions(manaOptions); Assert.assertEquals("mana variations don't fit", 4, manaOptions.size()); assertManaOptions("{C}{W}", manaOptions); @@ -396,9 +378,8 @@ public class ManaOptionsTest extends CardTestPlayerBase { execute(); ManaOptions manaOptions = playerA.getAvailableManaTest(currentGame); - assertDuplicatedManaOptions(manaOptions); - Assert.assertEquals("mana variations don't fit", 1, manaOptions.size()); + Assert.assertEquals("mana variations don't fit", 1, manaOptions.size()); // TODO: Why one, should there be {B} and {B}{U}? assertManaOptions("{W}{B}", manaOptions); } @@ -412,7 +393,6 @@ public class ManaOptionsTest extends CardTestPlayerBase { execute(); ManaOptions manaOptions = playerA.getAvailableManaTest(currentGame); - assertDuplicatedManaOptions(manaOptions); Assert.assertEquals("mana variations don't fit", 2, manaOptions.size()); assertManaOptions("{W}{B}{B}", manaOptions); @@ -432,7 +412,6 @@ public class ManaOptionsTest extends CardTestPlayerBase { execute(); ManaOptions manaOptions = playerA.getAvailableManaTest(currentGame); - assertDuplicatedManaOptions(manaOptions); Assert.assertEquals("mana variations don't fit", 1, manaOptions.size()); assertManaOptions("{C}{W}{B}", manaOptions); @@ -452,7 +431,6 @@ public class ManaOptionsTest extends CardTestPlayerBase { execute(); ManaOptions manaOptions = playerA.getAvailableManaTest(currentGame); - assertDuplicatedManaOptions(manaOptions); Assert.assertEquals("mana variations don't fit", 1, manaOptions.size()); assertManaOptions("{C}{C}{C}{C}{W}{B}", manaOptions); @@ -470,7 +448,6 @@ public class ManaOptionsTest extends CardTestPlayerBase { execute(); ManaOptions manaOptions = playerA.getAvailableManaTest(currentGame); - assertDuplicatedManaOptions(manaOptions); Assert.assertEquals("mana variations don't fit", 1, manaOptions.size()); assertManaOptions("{B}{B}", manaOptions); @@ -489,7 +466,6 @@ public class ManaOptionsTest extends CardTestPlayerBase { execute(); ManaOptions manaOptions = playerA.getAvailableManaTest(currentGame); - assertDuplicatedManaOptions(manaOptions); Assert.assertEquals("mana variations don't fit", 1, manaOptions.size()); assertManaOptions("{C}", manaOptions); @@ -508,7 +484,6 @@ public class ManaOptionsTest extends CardTestPlayerBase { execute(); ManaOptions manaOptions = playerA.getAvailableManaTest(currentGame); - assertDuplicatedManaOptions(manaOptions); Assert.assertEquals("mana variations don't fit", 1, manaOptions.size()); assertManaOptions("{C}{C}", manaOptions); @@ -529,7 +504,6 @@ public class ManaOptionsTest extends CardTestPlayerBase { execute(); ManaOptions manaOptions = playerA.getAvailableManaTest(currentGame); - assertDuplicatedManaOptions(manaOptions); Assert.assertEquals("mana variations don't fit", 16, manaOptions.size()); assertManaOptions("{C}{C}{C}{C}{W}{W}{W}", manaOptions); @@ -561,7 +535,6 @@ public class ManaOptionsTest extends CardTestPlayerBase { execute(); ManaOptions manaOptions = playerA.getAvailableManaTest(currentGame); - assertDuplicatedManaOptions(manaOptions); Assert.assertEquals("mana variations don't fit", 2, manaOptions.size()); @@ -582,7 +555,6 @@ public class ManaOptionsTest extends CardTestPlayerBase { execute(); ManaOptions manaOptions = playerA.getAvailableManaTest(currentGame); - assertDuplicatedManaOptions(manaOptions); Assert.assertEquals("mana variations don't fit", 88, manaOptions.size()); @@ -603,7 +575,6 @@ public class ManaOptionsTest extends CardTestPlayerBase { execute(); ManaOptions manaOptions = playerA.getAvailableManaTest(currentGame); - assertDuplicatedManaOptions(manaOptions); Assert.assertEquals("mana variations don't fit", 4, manaOptions.size()); assertManaOptions("{U}{U}", manaOptions); @@ -612,4 +583,47 @@ public class ManaOptionsTest extends CardTestPlayerBase { assertManaOptions("{G}{W}{W}", manaOptions); } + /** + * This is a stress test for the implementation of ManaUtils. + * Based on the bug from: https://github.com/magefree/mage/issues/7710 + */ + @Test + public void testCascadingCataracts() { + addCard(Zone.BATTLEFIELD, playerA, "Plains", 5); + addCard(Zone.BATTLEFIELD, playerA, "Island", 5); + addCard(Zone.BATTLEFIELD, playerA, "Swamp", 5); + addCard(Zone.BATTLEFIELD, playerA, "Mountain", 5); + addCard(Zone.BATTLEFIELD, playerA, "Forest", 5); + addCard(Zone.BATTLEFIELD, playerA, "Desert", 5); + addCard(Zone.BATTLEFIELD, playerA, "Cascading Cataracts", 3); + + setStopAt(1, PhaseStep.END_TURN); + execute(); + + ManaOptions manaOptions = playerA.getAvailableManaTest(currentGame); + // The size below is based on no optimization to remove any mana possibilities beyond exact duplicates. + Assert.assertEquals("mana variations don't match", 6902, manaOptions.size()); + } + + /** + * Similar to above, except without a hardcoded expected result, and used to check scaling. + * Leave the @Ignore added when pushing commits. + */ + @Test + @Ignore + public void testCascadingCataractsN() { + int n = 5; + addCard(Zone.BATTLEFIELD, playerA, "Plains", n); + addCard(Zone.BATTLEFIELD, playerA, "Island", n); + addCard(Zone.BATTLEFIELD, playerA, "Swamp", n); + addCard(Zone.BATTLEFIELD, playerA, "Mountain", n); + addCard(Zone.BATTLEFIELD, playerA, "Forest", n); + addCard(Zone.BATTLEFIELD, playerA, "Desert", n); + addCard(Zone.BATTLEFIELD, playerA, "Cascading Cataracts", n); + + setStopAt(1, PhaseStep.END_TURN); + execute(); + + ManaOptions manaOptions = playerA.getAvailableManaTest(currentGame); + } } diff --git a/Mage.Tests/src/test/java/org/mage/test/utils/ManaOptionsTestUtils.java b/Mage.Tests/src/test/java/org/mage/test/utils/ManaOptionsTestUtils.java index 4d76d17143d..d1cb71b3661 100644 --- a/Mage.Tests/src/test/java/org/mage/test/utils/ManaOptionsTestUtils.java +++ b/Mage.Tests/src/test/java/org/mage/test/utils/ManaOptionsTestUtils.java @@ -1,7 +1,11 @@ package org.mage.test.utils; import mage.Mana; +import mage.abilities.costs.mana.ManaCost; +import mage.abilities.costs.mana.ManaCosts; +import mage.abilities.costs.mana.ManaCostsImpl; import mage.abilities.mana.ManaOptions; +import mage.util.ManaUtil; import org.junit.Assert; import java.util.HashSet; @@ -19,8 +23,8 @@ public class ManaOptionsTestUtils { //mana info //logger.info(playerA.getManaPool().getMana().toString()); //logger.info(playerA.getManaAvailable(currentGame).toString()); - public static boolean manaOptionsContain(ManaOptions list, String searchMana) { - for (Mana mana : list) { + public static boolean manaOptionsContain(String searchMana, ManaOptions manaOptions) { + for (Mana mana : manaOptions) { if (mana instanceof ConditionalMana) { if ((mana.toString() + ((ConditionalMana)mana).getConditionString()).equals(searchMana)) { return true; @@ -34,21 +38,9 @@ public class ManaOptionsTestUtils { return false; } - public static void assertManaOptions(String searchMana, ManaOptions manaList) { - if (!manaOptionsContain(manaList, searchMana)) { - Assert.fail("Can't find " + searchMana + " in " + manaList.toString()); - } - } - - public static void assertDuplicatedManaOptions(ManaOptions manaList) { - Set list = new HashSet<>(); - for (Mana mana : manaList) { - String s = mana.toString(); - if (list.contains(s)) { - Assert.fail("Found duplicated mana option " + s + " in " + manaList.toString()); - } else { - list.add(s); - } + public static void assertManaOptions(String searchMana, ManaOptions manaOptions) { + if (!manaOptionsContain(searchMana, manaOptions)) { + Assert.fail("Can't find " + searchMana + " in " + manaOptions.toString()); } } } diff --git a/Mage/src/main/java/mage/ConditionalMana.java b/Mage/src/main/java/mage/ConditionalMana.java index 1d153d90961..476cc5307ef 100644 --- a/Mage/src/main/java/mage/ConditionalMana.java +++ b/Mage/src/main/java/mage/ConditionalMana.java @@ -12,9 +12,12 @@ import mage.game.Game; import java.io.Serializable; import java.util.ArrayList; import java.util.List; +import java.util.Objects; import java.util.UUID; /** + * If subclassing and adding extra field, you must be sure to override equals() and hashCode to include the new fields. + * * @author nantuko */ public class ConditionalMana extends Mana implements Serializable, Emptiable { @@ -188,11 +191,17 @@ public class ConditionalMana extends Mana implements Serializable, Emptiable { } public String getConditionString() { - String condStr = "["; + StringBuilder sb = new StringBuilder(); + + sb.append('['); for (Condition condition : conditions) { - condStr += condition.getManaText(); + sb.append('{'); + sb.append(condition.getManaText()); + sb.append('}'); } - return condStr + "]"; + sb.append(']'); + + return sb.toString(); } public void add(ManaType manaType, int amount) { @@ -220,4 +229,42 @@ public class ConditionalMana extends Mana implements Serializable, Emptiable { break; } } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), conditions, staticText, scope, manaProducerId, manaProducerOriginalId); + } + + @Override + public boolean equals(Object o) { + // Check Mana.equals(). If that's isn't equal no need to check further. + if (!super.equals(o)) { + return false; + } + ConditionalMana that = (ConditionalMana) o; + + if (!Objects.equals(this.staticText, that.staticText)) { + return false; + } + if (!Objects.equals(this.manaProducerId, that.manaProducerId)) { + return false; + } + if (!Objects.equals(this.manaProducerOriginalId, that.manaProducerOriginalId)) { + return false; + } + if (!Objects.equals(this.scope, that.scope)) { + return false; + } + if (this.conditions == null || that.conditions == null + || this.conditions.size() != that.conditions.size()) { + return false; + } + for (int i = 0; i < this.conditions.size(); i++) { + if (!(Objects.equals(this.conditions.get(i), that.conditions.get(i)))) { + return false; + } + } + + return true; + } } diff --git a/Mage/src/main/java/mage/Mana.java b/Mage/src/main/java/mage/Mana.java index 74b6f02b939..5d3f4ad3d87 100644 --- a/Mage/src/main/java/mage/Mana.java +++ b/Mage/src/main/java/mage/Mana.java @@ -1,5 +1,6 @@ package mage; +import mage.abilities.condition.Condition; import mage.constants.ColoredManaSymbol; import mage.constants.ManaType; import mage.filter.FilterMana; @@ -8,10 +9,8 @@ import mage.util.Copyable; import org.apache.log4j.Logger; import java.io.Serializable; -import java.util.HashMap; -import java.util.LinkedList; -import java.util.Map; -import java.util.Objects; +import java.util.*; +import java.util.function.BiFunction; /** * WARNING, all mana operations must use overflow check, see usage of CardUtil.addWithOverflowCheck and same methods @@ -320,32 +319,51 @@ public class Mana implements Comparable, Serializable, Copyable { } /** - * Increases the given mana by one. + * Increases this mana by one of the passed in ManaType. * * @param manaType the type of mana to increase by one. */ public void increase(ManaType manaType) { + increaseOrDecrease(manaType, true); + } + + /** + * Decreases this mana by onw of the passed in ManaType. + * + * @param manaType the type of mana to increase by one. + */ + public void decrease(ManaType manaType) { + increaseOrDecrease(manaType, false); + } + + /** + * Helper function for increase and decrease to not have the code duplicated. + * @param manaType + * @param increase + */ + private void increaseOrDecrease(ManaType manaType, boolean increase) { + BiFunction overflowIncOrDec = increase ? CardUtil::overflowInc : CardUtil::overflowDec; switch (manaType) { case WHITE: - white = CardUtil.overflowInc(white, 1); + white = overflowIncOrDec.apply(white, 1); break; case BLUE: - blue = CardUtil.overflowInc(blue, 1); + blue = overflowIncOrDec.apply(blue, 1); break; case BLACK: - black = CardUtil.overflowInc(black, 1); + black = overflowIncOrDec.apply(black, 1); break; case RED: - red = CardUtil.overflowInc(red, 1); + red = overflowIncOrDec.apply(red, 1); break; case GREEN: - green = CardUtil.overflowInc(green, 1); + green = overflowIncOrDec.apply(green, 1); break; case COLORLESS: - colorless = CardUtil.overflowInc(colorless, 1); + colorless = overflowIncOrDec.apply(colorless, 1); break; case GENERIC: - generic = CardUtil.overflowInc(generic, 1); + generic = overflowIncOrDec.apply(generic, 1); break; } } @@ -399,6 +417,13 @@ public class Mana implements Comparable, Serializable, Copyable { colorless = CardUtil.overflowInc(colorless, 1); } + public void increaseAny() { + any = CardUtil.overflowInc(any, 1); + } + public void decreaseAny() { + any = CardUtil.overflowDec(any, 1); + } + /** * Subtracts the passed in mana values from this instance. * @@ -477,14 +502,11 @@ public class Mana implements Comparable, Serializable, Copyable { * @return the total count of all combined mana. */ public int count() { - return white - + blue - + black - + red - + green - + generic - + colorless - + any; + int sum = countColored(); + sum = CardUtil.overflowInc(sum, generic); + sum = CardUtil.overflowInc(sum, colorless); + + return sum; } /** @@ -493,12 +515,13 @@ public class Mana implements Comparable, Serializable, Copyable { * @return the total count of all colored mana. */ public int countColored() { - return white - + blue - + black - + red - + green - + any; + int sum = CardUtil.overflowInc(white, blue); + sum = CardUtil.overflowInc(sum, black); + sum = CardUtil.overflowInc(sum, red); + sum = CardUtil.overflowInc(sum, green); + sum = CardUtil.overflowInc(sum, any); + + return sum; } /** @@ -1103,7 +1126,7 @@ public class Mana implements Comparable, Serializable, Copyable { case GREEN: return green; case COLORLESS: - return CardUtil.overflowInc(generic, colorless); + return CardUtil.overflowInc(generic, colorless); // TODO: This seems like a mistake } return 0; } @@ -1206,77 +1229,114 @@ public class Mana implements Comparable, Serializable, Copyable { } + public boolean isMoreValuableThan(Mana that) { + // Use of == is intentional since getMoreValuableMana returns one of its inputs. + return this == Mana.getMoreValuableMana(this, that); + } + /** * Returns the mana that is more colored or has a greater amount but does - * not contain one less mana in any color but generic. + * not contain one less mana in any type but generic. + *

+ * See tests ManaTest.moreValuableManaTest for several examples * * Examples: + * {1} and {R} -> {R} + * {2} and {1}{W} -> {1}{W} + * {3} and {1}{W} -> {1}{W} * {1}{W}{R} and {G}{W}{R} -> {G}{W}{R} - * {G}{W}{R} and {G}{W}{R} -> {G}{W}{R} + * {G}{W}{R} and {G}{W}{R} -> null * {G}{W}{B} and {G}{W}{R} -> null + * {C} and {ANY} -> null * * @param mana1 The 1st mana to compare. * @param mana2 The 2nd mana to compare. - * @return The greater of the two manas, or null if they're the same + * @return The greater of the two manas, or null if they're the same OR they cannot be compared */ public static Mana getMoreValuableMana(final Mana mana1, final Mana mana2) { - String conditionString1 = ""; - String conditionString2 = ""; - if (mana1 instanceof ConditionalMana) { - conditionString1 = ((ConditionalMana) mana1).getConditionString(); - } - if (mana2 instanceof ConditionalMana) { - conditionString2 = ((ConditionalMana) mana2).getConditionString(); - } - if (!conditionString1.equals(conditionString2)) { + if (mana1.equals(mana2)) { return null; } + + boolean mana1IsConditional = mana1 instanceof ConditionalMana; + boolean mana2IsConditional = mana2 instanceof ConditionalMana; + if (mana1IsConditional != mana2IsConditional) { + return null; + } + + if (mana1IsConditional) { + List conditions1 = ((ConditionalMana) mana1).getConditions(); + List conditions2 = ((ConditionalMana) mana2).getConditions(); + if (!Objects.equals(conditions1, conditions2)) { + return null; + } + } + + // Set one mana as moreMana and one as lessMana. Mana moreMana; Mana lessMana; - if (mana2.countColored() > mana1.countColored() || mana2.getAny() > mana1.getAny() || mana2.count() > mana1.count()) { + if (mana2.any > mana1.any + || mana2.colorless > mana1.colorless + || mana2.countColored() > mana1.countColored() + || (mana2.countColored() == mana1.countColored() + && mana2.colorless == mana1.colorless + && mana2.count() > mana1.count())) { moreMana = mana2; lessMana = mana1; } else { moreMana = mana1; lessMana = mana2; } - int anyDiff = CardUtil.overflowDec(mana2.getAny(), mana1.getAny()); - if (lessMana.getWhite() > moreMana.getWhite()) { - anyDiff = CardUtil.overflowDec(anyDiff, CardUtil.overflowDec(lessMana.getWhite(), moreMana.getWhite())); - if (anyDiff < 0) { - return null; - } - } - if (lessMana.getRed() > moreMana.getRed()) { - anyDiff = CardUtil.overflowDec(anyDiff, CardUtil.overflowDec(lessMana.getRed(), moreMana.getRed())); - if (anyDiff < 0) { - return null; - } - } - if (lessMana.getGreen() > moreMana.getGreen()) { - anyDiff = CardUtil.overflowDec(anyDiff, CardUtil.overflowDec(lessMana.getGreen(), moreMana.getGreen())); - if (anyDiff < 0) { - return null; - } - } - if (lessMana.getBlue() > moreMana.getBlue()) { - anyDiff = CardUtil.overflowDec(anyDiff, CardUtil.overflowDec(lessMana.getBlue(), moreMana.getBlue())); - if (anyDiff < 0) { - return null; - } - } - if (lessMana.getBlack() > moreMana.getBlack()) { - anyDiff = CardUtil.overflowDec(anyDiff, CardUtil.overflowDec(lessMana.getBlack(), moreMana.getBlack())); - if (anyDiff < 0) { - return null; - } - } - if (lessMana.getColorless() > moreMana.getColorless()) { - return null; // Any (color) can't produce colorless mana - } - if (lessMana.getAny() > moreMana.getAny()) { + + if (lessMana.any > moreMana.any) { return null; } + if (lessMana.colorless > moreMana.colorless) { + return null; // Any (color) can't produce colorless mana + } + + int anyDiff = CardUtil.overflowDec(moreMana.any, lessMana.any); + + int whiteDiff = CardUtil.overflowDec(lessMana.white, moreMana.white); + if (whiteDiff > 0) { + anyDiff = CardUtil.overflowDec(anyDiff, whiteDiff); + if (anyDiff < 0) { + return null; + } + } + + int redDiff = CardUtil.overflowDec(lessMana.red, moreMana.red); + if (redDiff > 0) { + anyDiff = CardUtil.overflowDec(anyDiff, redDiff); + if (anyDiff < 0) { + return null; + } + } + + int greenDiff = CardUtil.overflowDec(lessMana.green, moreMana.green); + if (greenDiff > 0) { + anyDiff = CardUtil.overflowDec(anyDiff, greenDiff); + if (anyDiff < 0) { + return null; + } + } + + int blueDiff = CardUtil.overflowDec(lessMana.blue, moreMana.blue); + if (blueDiff > 0) { + anyDiff = CardUtil.overflowDec(anyDiff, blueDiff); + if (anyDiff < 0) { + return null; + } + } + + int blackDiff = CardUtil.overflowDec(lessMana.black, moreMana.black); + if (blackDiff > 0) { + anyDiff = CardUtil.overflowDec(anyDiff, blackDiff); + if (anyDiff < 0) { + return null; + } + } + return moreMana; } @@ -1323,9 +1383,27 @@ public class Mana implements Comparable, Serializable, Copyable { && any == mana.any; } + /** + * Hardcoding here versus using Objects.hash in order to increase performance since this is + * called thousands of times by {@link mage.abilities.mana.ManaOptions#addManaWithCost(List, Game)} + * + * @return + */ @Override public int hashCode() { - return Objects.hash(white, blue, black, red, green, generic, colorless, any, flag); + long result = 1; + + result = 31 * result + white; + result = 31 * result + blue; + result = 31 * result + black; + result = 31 * result + red; + result = 31 * result + green; + result = 31 * result + generic; + result = 31 * result + colorless; + result = 31 * result + any; + result = 31 * result + (flag ? 1 : 0); + + return Long.hashCode(result); } /** diff --git a/Mage/src/main/java/mage/abilities/condition/Condition.java b/Mage/src/main/java/mage/abilities/condition/Condition.java index b5a03c083f1..9be2cf5dfa0 100644 --- a/Mage/src/main/java/mage/abilities/condition/Condition.java +++ b/Mage/src/main/java/mage/abilities/condition/Condition.java @@ -23,7 +23,7 @@ public interface Condition extends Serializable { boolean apply(Game game, Ability source); default String getManaText() { - return "{" + this.getClass().getSimpleName() + "}"; + return this.getClass().getSimpleName(); } default boolean caresAboutManaColor() { diff --git a/Mage/src/main/java/mage/abilities/effects/mana/AddManaInAnyCombinationEffect.java b/Mage/src/main/java/mage/abilities/effects/mana/AddManaInAnyCombinationEffect.java index 600f6a23bf7..3df03241496 100644 --- a/Mage/src/main/java/mage/abilities/effects/mana/AddManaInAnyCombinationEffect.java +++ b/Mage/src/main/java/mage/abilities/effects/mana/AddManaInAnyCombinationEffect.java @@ -104,7 +104,7 @@ public class AddManaInAnyCombinationEffect extends ManaEffect { allPossibleMana.addMana(currentPossibleMana); } - allPossibleMana.removeDuplicated(); + allPossibleMana.removeFullyIncludedVariations(); return new ArrayList<>(allPossibleMana); } else { diff --git a/Mage/src/main/java/mage/abilities/keyword/ConvokeAbility.java b/Mage/src/main/java/mage/abilities/keyword/ConvokeAbility.java index 48170510ef8..bfde10f3c23 100644 --- a/Mage/src/main/java/mage/abilities/keyword/ConvokeAbility.java +++ b/Mage/src/main/java/mage/abilities/keyword/ConvokeAbility.java @@ -164,7 +164,7 @@ public class ConvokeAbility extends SimpleStaticAbility implements AlternateMana options.addMana(permMana); }); - options.removeDuplicated(); + options.removeFullyIncludedVariations(); return options; } } diff --git a/Mage/src/main/java/mage/abilities/keyword/OfferingAbility.java b/Mage/src/main/java/mage/abilities/keyword/OfferingAbility.java index ad13d488752..63f05309f1c 100644 --- a/Mage/src/main/java/mage/abilities/keyword/OfferingAbility.java +++ b/Mage/src/main/java/mage/abilities/keyword/OfferingAbility.java @@ -115,7 +115,7 @@ public class OfferingAbility extends StaticAbility implements AlternateManaPayme } ); - additionalManaOptionsForThisAbility.removeDuplicated(); + additionalManaOptionsForThisAbility.removeFullyIncludedVariations(); return additionalManaOptionsForThisAbility; } } diff --git a/Mage/src/main/java/mage/abilities/mana/ManaOptions.java b/Mage/src/main/java/mage/abilities/mana/ManaOptions.java index 122b9a6296a..178a8ec51f9 100644 --- a/Mage/src/main/java/mage/abilities/mana/ManaOptions.java +++ b/Mage/src/main/java/mage/abilities/mana/ManaOptions.java @@ -3,11 +3,13 @@ package mage.abilities.mana; import mage.ConditionalMana; import mage.Mana; import mage.abilities.Ability; +import mage.constants.ManaType; import mage.game.Game; import mage.game.events.GameEvent; import mage.game.events.ManaEvent; import mage.game.events.TappedForManaEvent; import mage.players.Player; +import mage.util.TreeNode; import org.apache.log4j.Logger; import java.util.*; @@ -19,10 +21,14 @@ import java.util.*; * be used to find all the ways to pay a mana cost or all the different mana * combinations available to a player *

- * TODO: Conditional Mana is not supported yet. The mana adding removes the - * condition of conditional mana + * TODO: Conditional Mana is not supported yet. + * The mana adding removes the condition of conditional mana + *

+ * A LinkedHashSet is used to get the performance benefits of automatic de-duplication of the Mana + * to avoid performance issues related with manual de-duplication (see https://github.com/magefree/mage/issues/7710). + * */ -public class ManaOptions extends ArrayList { +public class ManaOptions extends LinkedHashSet { private static final Logger logger = Logger.getLogger(ManaOptions.class); @@ -39,69 +45,68 @@ public class ManaOptions extends ArrayList { if (isEmpty()) { this.add(new Mana()); } - if (!abilities.isEmpty()) { - if (abilities.size() == 1) { - //if there is only one mana option available add it to all the existing options - List netManas = abilities.get(0).getNetMana(game); - if (netManas.size() == 1) { - checkManaReplacementAndTriggeredMana(abilities.get(0), game, netManas.get(0)); - addMana(netManas.get(0)); - addTriggeredMana(game, abilities.get(0)); - } else if (netManas.size() > 1) { - addManaVariation(netManas, abilities.get(0), game); - } + if (abilities.isEmpty()) { + return; // Do nothing + } - } else { // mana source has more than 1 ability - //perform a union of all existing options and the new options - List copy = copy(); - this.clear(); - for (ActivatedManaAbilityImpl ability : abilities) { - for (Mana netMana : ability.getNetMana(game)) { - checkManaReplacementAndTriggeredMana(ability, game, netMana); - for (Mana triggeredManaVariation : getTriggeredManaVariations(game, ability, netMana)) { - SkipAddMana: - for (Mana mana : copy) { - Mana newMana = new Mana(); - newMana.add(mana); - newMana.add(triggeredManaVariation); - for (Mana existingMana : this) { - if (existingMana.equalManaValue(newMana)) { - continue SkipAddMana; - } - Mana moreValuable = Mana.getMoreValuableMana(newMana, existingMana); - if (moreValuable != null) { - // only keep the more valuable mana - existingMana.setToMana(moreValuable); - continue SkipAddMana; - } + if (abilities.size() == 1) { + //if there is only one mana option available add it to all the existing options + List netManas = abilities.get(0).getNetMana(game); + if (netManas.size() == 1) { + checkManaReplacementAndTriggeredMana(abilities.get(0), game, netManas.get(0)); + addMana(netManas.get(0)); + addTriggeredMana(game, abilities.get(0)); + } else if (netManas.size() > 1) { + addManaVariation(netManas, abilities.get(0), game); + } + + } else { // mana source has more than 1 ability + //perform a union of all existing options and the new options + List copy = new ArrayList<>(this); + this.clear(); + for (ActivatedManaAbilityImpl ability : abilities) { + for (Mana netMana : ability.getNetMana(game)) { + checkManaReplacementAndTriggeredMana(ability, game, netMana); + for (Mana triggeredManaVariation : getTriggeredManaVariations(game, ability, netMana)) { + SkipAddMana: + for (Mana mana : copy) { + Mana newMana = new Mana(); + newMana.add(mana); + newMana.add(triggeredManaVariation); + for (Mana existingMana : this) { + if (existingMana.equalManaValue(newMana)) { + continue SkipAddMana; + } + Mana moreValuable = Mana.getMoreValuableMana(newMana, existingMana); + if (moreValuable != null) { + // only keep the more valuable mana + existingMana.setToMana(moreValuable); + continue SkipAddMana; } - this.add(newMana); } + this.add(newMana); } - } + } } } - - forceManaDeduplication(); } private void addManaVariation(List netManas, ActivatedManaAbilityImpl ability, Game game) { - List copy = copy(); + Mana newMana; + + List copy = new ArrayList<>(this); this.clear(); for (Mana netMana : netManas) { for (Mana mana : copy) { if (!ability.hasTapCost() || checkManaReplacementAndTriggeredMana(ability, game, netMana)) { - Mana newMana = new Mana(); - newMana.add(mana); + newMana = mana.copy(); newMana.add(netMana); this.add(newMana); } } } - - forceManaDeduplication(); } private static List> getSimulatedTriggeredManaFromPlayer(Game game, Ability ability) { @@ -138,8 +143,7 @@ public class ManaOptions extends ArrayList { } /** - * This adds the mana the abilities can produce to the possible mana - * variabtion. + * This adds the mana the abilities can produce to the possible mana variation. * * @param abilities * @param game @@ -147,14 +151,13 @@ public class ManaOptions extends ArrayList { */ public boolean addManaWithCost(List abilities, Game game) { boolean wasUsable = false; - int replaces = 0; if (isEmpty()) { this.add(new Mana()); // needed if this is the first available mana, otherwise looping over existing options woold not loop } if (!abilities.isEmpty()) { if (abilities.size() == 1) { List netManas = abilities.get(0).getNetMana(game); - if (netManas.size() > 0) { // ability can produce mana + if (!netManas.isEmpty()) { // ability can produce mana ActivatedManaAbilityImpl ability = abilities.get(0); // The ability has no mana costs if (ability.getManaCosts().isEmpty()) { // No mana costs, so no mana to subtract from available @@ -163,14 +166,13 @@ public class ManaOptions extends ArrayList { addMana(netManas.get(0)); addTriggeredMana(game, ability); } else { - List copy = copy(); + List copy = new ArrayList<>(this); this.clear(); for (Mana netMana : netManas) { checkManaReplacementAndTriggeredMana(ability, game, netMana); for (Mana triggeredManaVariation : getTriggeredManaVariations(game, ability, netMana)) { for (Mana mana : copy) { - Mana newMana = new Mana(); - newMana.add(mana); + Mana newMana = new Mana(mana); newMana.add(triggeredManaVariation); this.add(newMana); wasUsable = true; @@ -179,23 +181,22 @@ public class ManaOptions extends ArrayList { } } } else {// The ability has mana costs - List copy = copy(); + List copy = new ArrayList<>(this); this.clear(); + Mana manaCosts = ability.getManaCosts().getMana(); for (Mana netMana : netManas) { checkManaReplacementAndTriggeredMana(ability, game, netMana); for (Mana triggeredManaVariation : getTriggeredManaVariations(game, ability, netMana)) { - for (Mana prevMana : copy) { - Mana startingMana = prevMana.copy(); - Mana manaCosts = ability.getManaCosts().getMana(); + for (Mana startingMana : copy) { if (startingMana.includesMana(manaCosts)) { // can pay the mana costs to use the ability if (!subtractCostAddMana(manaCosts, triggeredManaVariation, ability.getCosts().isEmpty(), startingMana, ability, game)) { // the starting mana includes mana parts that the increased mana does not include, so add starting mana also as an option - add(prevMana); + add(startingMana); } wasUsable = true; } else { // mana costs can't be paid so keep starting mana - add(prevMana); + add(startingMana); } } } @@ -204,7 +205,7 @@ public class ManaOptions extends ArrayList { } } else { //perform a union of all existing options and the new options - List copy = copy(); + List copy = new ArrayList<>(this); this.clear(); for (ActivatedManaAbilityImpl ability : abilities) { List netManas = ability.getNetMana(game); @@ -213,8 +214,7 @@ public class ManaOptions extends ArrayList { checkManaReplacementAndTriggeredMana(ability, game, netMana); for (Mana triggeredManaVariation : getTriggeredManaVariations(game, ability, netMana)) { for (Mana mana : copy) { - Mana newMana = new Mana(); - newMana.add(mana); + Mana newMana = new Mana(mana); newMana.add(triggeredManaVariation); this.add(newMana); wasUsable = true; @@ -226,9 +226,9 @@ public class ManaOptions extends ArrayList { checkManaReplacementAndTriggeredMana(ability, game, netMana); for (Mana triggeredManaVariation : getTriggeredManaVariations(game, ability, netMana)) { for (Mana previousMana : copy) { - CombineWithExisting: for (Mana manaOption : ability.getManaCosts().getManaOptions()) { if (previousMana.includesMana(manaOption)) { // costs can be paid + // subtractCostAddMana has side effects on {this}. Do not add wasUsable to the if-statement above wasUsable |= subtractCostAddMana(manaOption, triggeredManaVariation, ability.getCosts().isEmpty(), previousMana, ability, game); } } @@ -241,41 +241,41 @@ public class ManaOptions extends ArrayList { } } - if (this.size() > 30 || replaces > 30) { - logger.trace("ManaOptionsCosts " + this.size() + " Ign:" + replaces + " => " + this.toString()); + if (logger.isTraceEnabled() && this.size() > 30) { + logger.trace("ManaOptionsCosts " + this.size()); logger.trace("Abilities: " + abilities.toString()); } - forceManaDeduplication(); return wasUsable; } public boolean addManaPoolDependant(List abilities, Game game) { + if (abilities.isEmpty() || abilities.size() != 1) { + return false; + } + boolean wasUsable = false; - if (!abilities.isEmpty()) { - if (abilities.size() == 1) { - ActivatedManaAbilityImpl ability = (ActivatedManaAbilityImpl) abilities.get(0); - List copy = copy(); - this.clear(); - for (Mana previousMana : copy) { - Mana startingMana = previousMana.copy(); - Mana manaCosts = ability.getManaCosts().getMana(); - if (startingMana.includesMana(manaCosts)) { // can pay the mana costs to use the ability - for (Mana manaOption : ability.getManaCosts().getManaOptions()) { - if (!subtractCostAddMana(manaOption, null, ability.getCosts().isEmpty(), startingMana, ability, game)) { - // the starting mana includes mana parts that the increased mana does not include, so add starting mana also as an option - add(previousMana); - } - } - wasUsable = true; - } else { - // mana costs can't be paid so keep starting mana + ActivatedManaAbilityImpl ability = (ActivatedManaAbilityImpl) abilities.get(0); + Mana manaCosts = ability.getManaCosts().getMana(); + + Set copy = copy(); + this.clear(); + + for (Mana previousMana : copy) { + if (previousMana.includesMana(manaCosts)) { // can pay the mana costs to use the ability + for (Mana manaOption : ability.getManaCosts().getManaOptions()) { + if (!subtractCostAddMana(manaOption, null, ability.getCosts().isEmpty(), previousMana, ability, game)) { + // the starting mana includes mana parts that the increased mana does not include, so add starting mana also as an option add(previousMana); } } - + wasUsable = true; + } else { + // mana costs can't be paid so keep starting mana + add(previousMana); } } + return wasUsable; } @@ -311,20 +311,17 @@ public class ManaOptions extends ArrayList { addMana(triggeredNetMana.get(0)); } else if (triggeredNetMana.size() > 1) { // Add variations - List copy = copy(); + List copy = new ArrayList<>(this); this.clear(); for (Mana triggeredMana : triggeredNetMana) { for (Mana mana : copy) { - Mana newMana = new Mana(); - newMana.add(mana); + Mana newMana = new Mana(mana); newMana.add(triggeredMana); this.add(newMana); } } } } - - forceManaDeduplication(); } /** @@ -337,7 +334,7 @@ public class ManaOptions extends ArrayList { this.add(new Mana()); } if (addMana instanceof ConditionalMana) { - ManaOptions copy = this.copy(); + List copy = new ArrayList<>(this); this.clear(); for (Mana mana : copy) { ConditionalMana condMana = ((ConditionalMana) addMana).copy(); @@ -351,8 +348,6 @@ public class ManaOptions extends ArrayList { mana.add(addMana); } } - - forceManaDeduplication(); } public void addMana(ManaOptions options) { @@ -362,32 +357,20 @@ public class ManaOptions extends ArrayList { if (!options.isEmpty()) { if (options.size() == 1) { //if there is only one mana option available add it to all the existing options - addMana(options.get(0)); + addMana(options.getAtIndex(0)); } else { //perform a union of all existing options and the new options - List copy = copy(); + List copy = new ArrayList<>(this); this.clear(); for (Mana addMana : options) { for (Mana mana : copy) { - Mana newMana = new Mana(); - newMana.add(mana); + Mana newMana = new Mana(mana); newMana.add(addMana); this.add(newMana); } } } } - - forceManaDeduplication(); - } - - private void forceManaDeduplication() { - // memory overflow protection - force de-duplication on too much mana sources - // bug example: https://github.com/magefree/mage/issues/6938 - // use it after new mana adding - if (this.size() > 1000) { - this.removeDuplicated(); - } } public ManaOptions copy() { @@ -406,62 +389,46 @@ public class ManaOptions extends ArrayList { * @param oldManaWasReplaced returns the info if the new complete mana does * replace the current mana completely */ - private boolean subtractCostAddMana(Mana cost, Mana manaToAdd, boolean onlyManaCosts, Mana currentMana, ManaAbility manaAbility, Game game) { - boolean oldManaWasReplaced = false; // true if the newly created mana includes all mana possibilities of the old - boolean repeatable = false; - if (manaToAdd != null && (manaToAdd.countColored() > 0 || manaToAdd.getAny() > 0) && manaToAdd.count() > 0 && onlyManaCosts) { - repeatable = true; // only replace to any with mana costs only will be repeated if able - } + private boolean subtractCostAddMana(Mana cost, Mana manaToAdd, boolean onlyManaCosts, final Mana currentMana, ManaAbility manaAbility, Game game) { + boolean oldManaWasReplaced = false; // True if the newly created mana includes all mana possibilities of the old + boolean repeatable = manaToAdd != null // TODO: re-write "only replace to any with mana costs only will be repeated if able" + && onlyManaCosts + && (manaToAdd.getAny() > 0 || manaToAdd.countColored() > 0) + && manaToAdd.count() > 0; + boolean newCombinations; + + Mana newMana = new Mana(); + Mana currentManaCopy = new Mana(); for (Mana payCombination : ManaOptions.getPossiblePayCombinations(cost, currentMana)) { - Mana currentManaCopy = currentMana.copy(); // copy start mana because in iteration it will be updated - while (currentManaCopy.includesMana(payCombination)) { // loop for multiple usage if possible - boolean newCombinations = false; + currentManaCopy.setToMana(currentMana); // copy start mana because in iteration it will be updated + do { // loop for multiple usage if possible + newCombinations = false; - if (manaToAdd == null) { - Mana newMana = currentManaCopy.copy(); - newMana.subtract(payCombination); - for (Mana mana : manaAbility.getNetMana(game, newMana)) { // get the mana to add from the ability related to the currently generated possible mana pool - newMana.add(mana); - if (!isExistingManaCombination(newMana)) { - this.add(newMana); // add the new combination - newCombinations = true; // repeat the while as long there are new combinations and usage is repeatable + newMana.setToMana(currentManaCopy); + newMana.subtract(payCombination); + // Get the mana to iterate over. + // If manaToAdd is specified add it, otherwise add the mana produced by the mana ability + List manasToAdd = (manaToAdd != null) ? Collections.singletonList(manaToAdd) : manaAbility.getNetMana(game, newMana); + for (Mana mana : manasToAdd) { + newMana.add(mana); + if (this.contains(newMana)) { + continue; + } - Mana moreValuable = Mana.getMoreValuableMana(currentManaCopy, newMana); - if (newMana.equals(moreValuable)) { - oldManaWasReplaced = true; // the new mana includes all possibilities of the old one, so no need to add it after return - if (!currentMana.equalManaValue(currentManaCopy)) { - this.removeEqualMana(currentManaCopy); - } - } - currentManaCopy = newMana.copy(); + this.add(newMana.copy()); // add the new combination + newCombinations = true; // repeat the while as long there are new combinations and usage is repeatable + + if (newMana.isMoreValuableThan(currentManaCopy)) { + oldManaWasReplaced = true; // the new mana includes all possible mana of the old one, so no need to add it after return + if (!currentMana.equalManaValue(currentManaCopy)) { + this.removeEqualMana(currentManaCopy); } } - } else { - Mana newMana = currentManaCopy.copy(); - newMana.subtract(payCombination); - newMana.add(manaToAdd); - if (!isExistingManaCombination(newMana)) { - this.add(newMana); // add the new combination - newCombinations = true; // repeat the while as long there are new combinations and usage is repeatable - Mana moreValuable = Mana.getMoreValuableMana(currentManaCopy, newMana); - if (newMana.equals(moreValuable)) { - oldManaWasReplaced = true; // the new mana includes all possible mana of the old one, so no need to add it after return - if (!currentMana.equalManaValue(currentManaCopy)) { - this.removeEqualMana(currentManaCopy); - } - } - currentManaCopy = newMana.copy(); - } + currentManaCopy.setToMana(newMana); } - if (!newCombinations || !repeatable) { - break; - } - } - + } while (repeatable && newCombinations && currentManaCopy.includesMana(payCombination)); } - forceManaDeduplication(); - return oldManaWasReplaced; } @@ -470,86 +437,72 @@ public class ManaOptions extends ArrayList { * @param manaAvailable * @return */ - public static List getPossiblePayCombinations(Mana manaCost, Mana manaAvailable) { - List payCombinations = new ArrayList<>(); - List payCombinationsStrings = new ArrayList<>(); - // handle fixed mana costs + public static Set getPossiblePayCombinations(Mana manaCost, Mana manaAvailable) { + Set payCombinations = new HashSet<>(); + Mana fixedMana = manaCost.copy(); + + // If there are no generic costs, then there is only one combination of colors available to pay for it. + // That combination is itself (fixedMana) if (manaCost.getGeneric() == 0) { payCombinations.add(fixedMana); return payCombinations; } + + // Get the available mana left to pay for the cost after subtracting the non-generic parts of the cost from it fixedMana.setGeneric(0); Mana manaAfterFixedPayment = manaAvailable.copy(); manaAfterFixedPayment.subtract(fixedMana); // handle generic mana costs - if (manaAvailable.countColored() > 0) { - + if (manaAvailable.countColored() == 0) { + payCombinations.add(Mana.ColorlessMana(manaCost.getGeneric())); + } else { + ManaType[] manaTypes = ManaType.values(); // Do not move, here for optimization reasons. + Mana manaToPayFrom = new Mana(); for (int i = 0; i < manaCost.getGeneric(); i++) { - List existingManas = new ArrayList<>(); + List existingManas = new ArrayList<>(payCombinations.size()); if (i > 0) { existingManas.addAll(payCombinations); payCombinations.clear(); - payCombinationsStrings.clear(); } else { existingManas.add(new Mana()); } - for (Mana existingMana : existingManas) { - Mana manaToPayFrom = manaAfterFixedPayment.copy(); - manaToPayFrom.subtract(existingMana); - String manaString = existingMana.toString(); - if (manaToPayFrom.getBlack() > 0 && !payCombinationsStrings.contains(manaString + Mana.BlackMana(1))) { - manaToPayFrom.subtract(Mana.BlackMana(1)); - ManaOptions.addManaCombination(Mana.BlackMana(1), existingMana, payCombinations, payCombinationsStrings); - } - if (manaToPayFrom.getBlue() > 0 && !payCombinationsStrings.contains(manaString + Mana.BlueMana(1).toString())) { - manaToPayFrom.subtract(Mana.BlueMana(1)); - ManaOptions.addManaCombination(Mana.BlueMana(1), existingMana, payCombinations, payCombinationsStrings); - } - if (manaToPayFrom.getGreen() > 0 && !payCombinationsStrings.contains(manaString + Mana.GreenMana(1).toString())) { - manaToPayFrom.subtract(Mana.GreenMana(1)); - ManaOptions.addManaCombination(Mana.GreenMana(1), existingMana, payCombinations, payCombinationsStrings); - } - if (manaToPayFrom.getRed() > 0 && !payCombinationsStrings.contains(manaString + Mana.RedMana(1).toString())) { - manaToPayFrom.subtract(Mana.RedMana(1)); - ManaOptions.addManaCombination(Mana.RedMana(1), existingMana, payCombinations, payCombinationsStrings); - } - if (manaToPayFrom.getWhite() > 0 && !payCombinationsStrings.contains(manaString + Mana.WhiteMana(1).toString())) { - manaToPayFrom.subtract(Mana.WhiteMana(1)); - ManaOptions.addManaCombination(Mana.WhiteMana(1), existingMana, payCombinations, payCombinationsStrings); - } - if (manaToPayFrom.getColorless() > 0 && !payCombinationsStrings.contains(manaString + Mana.ColorlessMana(1).toString())) { - manaToPayFrom.subtract(Mana.ColorlessMana(1)); - ManaOptions.addManaCombination(Mana.ColorlessMana(1), existingMana, payCombinations, payCombinationsStrings); + for (Mana existingMana : existingManas) { + manaToPayFrom.setToMana(manaAfterFixedPayment); + manaToPayFrom.subtract(existingMana); + + for (ManaType manaType : manaTypes) { + existingMana.increase(manaType); + if (manaToPayFrom.get(manaType) > 0 && !payCombinations.contains(existingMana)) { + payCombinations.add(existingMana.copy()); + + manaToPayFrom.decrease(manaType); + } + existingMana.decrease(manaType); } + // Pay with any only needed if colored payment was not possible - if (payCombinations.isEmpty() && manaToPayFrom.getAny() > 0 && !payCombinationsStrings.contains(manaString + Mana.AnyMana(1).toString())) { - manaToPayFrom.subtract(Mana.AnyMana(1)); - ManaOptions.addManaCombination(Mana.AnyMana(1), existingMana, payCombinations, payCombinationsStrings); + // NOTE: This isn't in the for loop since ManaType doesn't include ANY. + existingMana.increaseAny(); + if (payCombinations.isEmpty() && manaToPayFrom.getAny() > 0) { + payCombinations.add(existingMana.copy()); + + manaToPayFrom.decreaseAny(); } + existingMana.decreaseAny(); } } - } else { - payCombinations.add(Mana.ColorlessMana(manaCost.getGeneric())); } + for (Mana mana : payCombinations) { mana.add(fixedMana); } + // All mana values in here are of length 5 return payCombinations; } - private boolean isExistingManaCombination(Mana newMana) { - for (Mana mana : this) { - Mana moreValuable = Mana.getMoreValuableMana(mana, newMana); - if (mana.equals(moreValuable)) { - return true; - } - } - return false; - } - public boolean removeEqualMana(Mana manaToRemove) { boolean result = false; for (Iterator iterator = this.iterator(); iterator.hasNext(); ) { @@ -562,22 +515,20 @@ public class ManaOptions extends ArrayList { return result; } - public static void addManaCombination(Mana mana, Mana existingMana, List payCombinations, List payCombinationsStrings) { - Mana newMana = existingMana.copy(); - newMana.add(mana); - payCombinations.add(newMana); - payCombinationsStrings.add(newMana.toString()); - } - - public void removeDuplicated() { + /** + * Remove fully included variations. + * E.g. If both {R} and {R}{W} are in this, then {R} will be removed. + */ + public void removeFullyIncludedVariations() { + List that = new ArrayList<>(this); Set list = new HashSet<>(); for (int i = this.size() - 1; i >= 0; i--) { String s; - if (this.get(i) instanceof ConditionalMana) { - s = this.get(i).toString() + ((ConditionalMana) this.get(i)).getConditionString(); + if (that.get(i) instanceof ConditionalMana) { + s = that.get(i).toString() + ((ConditionalMana) that.get(i)).getConditionString(); } else { - s = this.get(i).toString(); + s = that.get(i).toString(); } if (s.isEmpty()) { this.remove(i); @@ -590,18 +541,19 @@ public class ManaOptions extends ArrayList { } // Remove fully included variations - // TODO: research too many manas and freeze (put 1 card to slow down, put 3 cards to freeze here) - // battlefield:Human:Cascading Cataracts:1 for (int i = this.size() - 1; i >= 0; i--) { for (int ii = 0; ii < i; ii++) { - Mana moreValuable = Mana.getMoreValuableMana(this.get(i), this.get(ii)); + Mana moreValuable = Mana.getMoreValuableMana(that.get(i), that.get(ii)); if (moreValuable != null) { - this.get(ii).setToMana(moreValuable); - this.remove(i); + that.get(ii).setToMana(moreValuable); + that.remove(i); break; } } } + + this.clear(); + this.addAll(that); } /** @@ -648,4 +600,81 @@ public class ManaOptions extends ArrayList { sb.append(',').append(' '); } } + + /** + * Utility function to get a Mana from ManaOptions at the specified position. + * Since the implementation uses a LinkedHashSet the ordering of the items is preserved. + * + * NOTE: Do not use in tight loops as performance of the lookup is much worse than + * for ArrayList (the previous superclass of ManaOptions). + */ + public Mana getAtIndex(int i) { + if (i < 0 || i >= this.size()) { + throw new IndexOutOfBoundsException(); + } + Iterator itr = this.iterator(); + while(itr.hasNext()) { + if (i == 0) { + return itr.next(); + } else { + itr.next(); // Ignore the value + i--; + } + } + return null; // Not sure how we'd ever get here, but leave just in case since IDE complains. + } } + +/** + * from: https://stackoverflow.com/a/35000727/7983747 + * @author Gili Tzabari + */ +final class Comparators +{ + /** + * Verify that a comparator is transitive. + * + * @param the type being compared + * @param comparator the comparator to test + * @param elements the elements to test against + * @throws AssertionError if the comparator is not transitive + */ + public static void verifyTransitivity(Comparator comparator, Collection elements) { + for (T first: elements) { + for (T second: elements) { + int result1 = comparator.compare(first, second); + int result2 = comparator.compare(second, first); + if (result1 != -result2 && !(result1 == 0 && result1 == result2)) { + // Uncomment the following line to step through the failed case + comparator.compare(first, second); + comparator.compare(second, first); + throw new AssertionError("compare(" + first + ", " + second + ") == " + result1 + + " but swapping the parameters returns " + result2); + } + } + } + for (T first: elements) { + for (T second: elements) { + int firstGreaterThanSecond = comparator.compare(first, second); + if (firstGreaterThanSecond <= 0) + continue; + for (T third: elements) { + int secondGreaterThanThird = comparator.compare(second, third); + if (secondGreaterThanThird <= 0) + continue; + int firstGreaterThanThird = comparator.compare(first, third); + if (firstGreaterThanThird <= 0) { + // Uncomment the following line to step through the failed case + comparator.compare(first, second); + comparator.compare(second, third); + comparator.compare(first, third); + throw new AssertionError("compare(" + first + ", " + second + ") > 0, " + + "compare(" + second + ", " + third + ") > 0, but compare(" + first + ", " + third + ") == " + + firstGreaterThanThird); + } + } + } + } + } + private Comparators() {} +} \ No newline at end of file diff --git a/Mage/src/main/java/mage/abilities/mana/builder/ConditionalManaBuilder.java b/Mage/src/main/java/mage/abilities/mana/builder/ConditionalManaBuilder.java index 0ff2e7eed7b..a79b981a0c7 100644 --- a/Mage/src/main/java/mage/abilities/mana/builder/ConditionalManaBuilder.java +++ b/Mage/src/main/java/mage/abilities/mana/builder/ConditionalManaBuilder.java @@ -6,6 +6,8 @@ import mage.Mana; import mage.abilities.Ability; import mage.game.Game; +import java.util.Objects; + /** * @author noxx */ @@ -19,4 +21,22 @@ public abstract class ConditionalManaBuilder implements Builder } public abstract String getRule(); + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + + if (obj == null || this.getClass() != obj.getClass()) { + return false; + } + + return Objects.equals(this.mana, ((ConditionalManaBuilder) obj).mana); + } + + @Override + public int hashCode() { + return Objects.hash(mana); + } } diff --git a/Mage/src/main/java/mage/abilities/mana/conditional/ConditionalSpellManaBuilder.java b/Mage/src/main/java/mage/abilities/mana/conditional/ConditionalSpellManaBuilder.java index b9d2ad51193..2e606209850 100644 --- a/Mage/src/main/java/mage/abilities/mana/conditional/ConditionalSpellManaBuilder.java +++ b/Mage/src/main/java/mage/abilities/mana/conditional/ConditionalSpellManaBuilder.java @@ -14,7 +14,9 @@ import mage.game.Game; import mage.game.command.Commander; import mage.game.stack.Spell; import mage.game.stack.StackObject; +import sun.font.CreatedFontTracker; +import java.util.Objects; import java.util.UUID; /** @@ -38,6 +40,20 @@ public class ConditionalSpellManaBuilder extends ConditionalManaBuilder { public String getRule() { return "Spend this mana only to cast " + filter.getMessage() + '.'; } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), this.filter); + } + + @Override + public boolean equals(Object obj) { + if (!super.equals(obj)) { + return false; + } + + return Objects.equals(this.filter, ((ConditionalSpellManaBuilder) obj).filter); + } } class SpellCastConditionalMana extends ConditionalMana { diff --git a/Mage/src/main/java/mage/players/PlayerImpl.java b/Mage/src/main/java/mage/players/PlayerImpl.java index 46894c09496..627217e4771 100644 --- a/Mage/src/main/java/mage/players/PlayerImpl.java +++ b/Mage/src/main/java/mage/players/PlayerImpl.java @@ -3352,7 +3352,6 @@ public abstract class PlayerImpl implements Player, Serializable { } if (used) { iterator.remove(); - availableMana.removeDuplicated(); anAbilityWasUsed = true; } } @@ -3363,9 +3362,8 @@ public abstract class PlayerImpl implements Player, Serializable { } } - // remove duplicated variants (see ManaOptionsTest for info - when that rises) - availableMana.removeDuplicated(); - + availableMana.removeFullyIncludedVariations(); + availableMana.remove(new Mana()); // Remove any empty mana that was left over from the way the code is written game.setCheckPlayableState(oldState); return availableMana; } diff --git a/Mage/src/main/java/mage/util/CardUtil.java b/Mage/src/main/java/mage/util/CardUtil.java index f2d20c99657..f0cb437add3 100644 --- a/Mage/src/main/java/mage/util/CardUtil.java +++ b/Mage/src/main/java/mage/util/CardUtil.java @@ -176,7 +176,7 @@ public final class CardUtil { } // ignore unknown mana - if (manaCost.getOptions().size() == 0) { + if (manaCost.getOptions().isEmpty()) { continue; } @@ -186,7 +186,7 @@ public final class CardUtil { } // generic mana reduce - Mana mana = manaCost.getOptions().get(0); + Mana mana = manaCost.getOptions().getAtIndex(0); int colorless = mana != null ? mana.getGeneric() : 0; if (restToReduce != 0 && colorless > 0) { if ((colorless - restToReduce) > 0) { @@ -219,7 +219,7 @@ public final class CardUtil { if (manaCost instanceof MonoHybridManaCost) { // current implemention supports reduce from left to right hybrid cost without cost parts announce MonoHybridManaCost mono = (MonoHybridManaCost) manaCost; - int colorless = mono.getOptions().get(1).getGeneric(); + int colorless = mono.getOptions().getAtIndex(1).getGeneric(); if (restToReduce != 0 && colorless > 0) { if ((colorless - restToReduce) > 0) { // partly reduce @@ -254,7 +254,7 @@ public final class CardUtil { // add to existing cost if (reduceCount != 0 && manaCost instanceof GenericManaCost) { GenericManaCost gen = (GenericManaCost) manaCost; - changedCost.put(manaCost, new GenericManaCost(gen.getOptions().get(0).getGeneric() + -reduceCount)); + changedCost.put(manaCost, new GenericManaCost(gen.getOptions().getAtIndex(0).getGeneric() + -reduceCount)); reduceCount = 0; added = true; } else { @@ -708,9 +708,9 @@ public final class CardUtil { } private static int overflowResult(long value) { - if (value > Integer.MAX_VALUE) { + if (value >= Integer.MAX_VALUE) { return Integer.MAX_VALUE; - } else if (value < Integer.MIN_VALUE) { + } else if (value <= Integer.MIN_VALUE) { return Integer.MIN_VALUE; } else { return (int) value; diff --git a/Mage/src/test/java/mage/ManaTest.java b/Mage/src/test/java/mage/ManaTest.java index a3b61dd1b2f..7404f52fd82 100644 --- a/Mage/src/test/java/mage/ManaTest.java +++ b/Mage/src/test/java/mage/ManaTest.java @@ -2,6 +2,7 @@ package mage; import mage.abilities.SpellAbility; import mage.abilities.costs.mana.ManaCost; +import mage.abilities.costs.mana.ManaCostImpl; import mage.abilities.costs.mana.ManaCostsImpl; import mage.constants.ColoredManaSymbol; import mage.constants.ManaType; @@ -12,6 +13,9 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; +import java.util.Arrays; +import java.util.List; + import static org.junit.Assert.*; @@ -767,11 +771,11 @@ public class ManaTest { /** * Mana.needed is used by the AI to know how much mana it needs in order to be able to play a card. + * + * TODO: How should generic and any be handled? */ @Test - public void should() { - // TODO: How does it handle generic and any. - // How *should* it handle them? + public void manaNeededWorks() { testManaNeeded( new Mana(ManaType.COLORLESS, 1), // Available new Mana(ManaType.COLORLESS, 2), // Cost @@ -794,6 +798,99 @@ public class ManaTest { ); } + /** + * Test that {@link Mana#getMoreValuableMana(Mana, Mana)} works as intended. + * All calls to getMoreValuableMana are run twice, with the second time having the inputs flipped to make sure the same result is given. + */ + @Test + public void moreValuableManaTest() { + final Mana anyMana = Mana.AnyMana(1); + final Mana genericMana = Mana.GenericMana(1); + final Mana colorlessMana = Mana.ColorlessMana(1); + + final Mana whiteMana = Mana.WhiteMana(1); + final Mana blueMana = Mana.BlueMana(1); + final Mana blackMana = Mana.BlackMana(1); + final Mana redMana = Mana.RedMana(1); + final Mana greenMana = Mana.GreenMana(1); + + final List coloredManas = Arrays.asList(whiteMana, blueMana, blackMana, redMana, greenMana); + + // 1. A color of WUBURG is not more valuable than any other + for (Mana coloredMana1 : coloredManas) { + for (Mana coloredMana2 : coloredManas) { + assertNull(coloredMana1 + " and " + coloredMana2 + " should not be comparable.", Mana.getMoreValuableMana(coloredMana1, coloredMana2)); + assertNull(coloredMana1 + " and " + coloredMana2 + " should not be comparable.", Mana.getMoreValuableMana(coloredMana2, coloredMana1)); + } + } + + // 2. Generic is less valuable than any other type of mana + final List nonGenericManas = Arrays.asList(whiteMana, blueMana, blackMana, redMana, greenMana, colorlessMana); + for (Mana coloredMana : nonGenericManas) { + assertEquals(coloredMana, Mana.getMoreValuableMana(coloredMana, genericMana)); + assertEquals(coloredMana, Mana.getMoreValuableMana(genericMana, coloredMana)); + } + assertEquals(anyMana, Mana.getMoreValuableMana(genericMana, anyMana)); + assertEquals(anyMana, Mana.getMoreValuableMana(anyMana, genericMana)); + + // 3. ANY mana is more valuable than generic or a specific color + for (Mana coloredMana : coloredManas) { + assertEquals(anyMana, Mana.getMoreValuableMana(coloredMana, anyMana)); + assertEquals(anyMana, Mana.getMoreValuableMana(anyMana, coloredMana)); + } + + // 4. Colorless mana is not comparable with colored mana or ANY mana + for (Mana coloredMana : coloredManas) { + assertNull(Mana.getMoreValuableMana(colorlessMana, coloredMana)); + assertNull(Mana.getMoreValuableMana(coloredMana, colorlessMana)); + } + assertNull(Mana.getMoreValuableMana(anyMana, colorlessMana)); + assertNull(Mana.getMoreValuableMana(colorlessMana, anyMana)); + + // 5. Mana is more valuable if it has more of any type of mana but not less of any type (other than generic) + final List allManas = Arrays.asList(whiteMana, blueMana, blackMana, redMana, greenMana, colorlessMana, genericMana, anyMana); + for (Mana specificMana : nonGenericManas) { + for (Mana toAddMana : allManas) { + Mana manaToCompare = specificMana.copy(); + manaToCompare.add(toAddMana); + manaToCompare.add(toAddMana); + + assertEquals(manaToCompare, Mana.getMoreValuableMana(specificMana, manaToCompare)); + assertEquals(manaToCompare, Mana.getMoreValuableMana(manaToCompare, specificMana)); + } + } + + // 6. Greater amount of mana but no less of any kind other than generic + final List nonAnyManas = Arrays.asList(whiteMana, blueMana, blackMana, redMana, greenMana, colorlessMana, genericMana); + Mana manaBase = new ManaCostsImpl<>("{1}{W}{U}{B}{R}{G}{C}").getMana(); + Mana manaToCompare = manaBase.copy(); + + // To avoid the copying that goes with it, manaToCompare is edited in place and always + // reset back to its base state at the end of each outer loop. + for (Mana manaToAddTwice : nonAnyManas) { + manaToCompare.add(manaToAddTwice); + manaToCompare.add(manaToAddTwice); + for (Mana manaToSubtract : nonAnyManas) { + manaToCompare.subtract(manaToSubtract); + + if (manaToSubtract == genericMana || manaToSubtract == manaToAddTwice) { + assertEquals(manaToCompare, Mana.getMoreValuableMana(manaBase, manaToCompare)); + assertEquals(manaToCompare, Mana.getMoreValuableMana(manaToCompare, manaBase)); + } else if (manaToAddTwice == genericMana ){ + assertEquals(manaBase, Mana.getMoreValuableMana(manaBase, manaToCompare)); + assertEquals(manaBase, Mana.getMoreValuableMana(manaToCompare, manaBase)); + } else { + assertNull(Mana.getMoreValuableMana(manaBase, manaToCompare)); + assertNull(Mana.getMoreValuableMana(manaToCompare, manaBase)); + } + + manaToCompare.add(manaToSubtract); + } + manaToCompare.subtract(manaToAddTwice); + manaToCompare.subtract(manaToAddTwice); + } + } + /** * Checks if the mana needed calculations produces the expected needed mana amount. *