diff --git a/forge-ai/src/main/java/forge/ai/simulation/MultiTargetSelector.java b/forge-ai/src/main/java/forge/ai/simulation/MultiTargetSelector.java index 00e5ad45e48..da65c4d5855 100644 --- a/forge-ai/src/main/java/forge/ai/simulation/MultiTargetSelector.java +++ b/forge-ai/src/main/java/forge/ai/simulation/MultiTargetSelector.java @@ -11,6 +11,10 @@ public class MultiTargetSelector { public static class Targets { private ArrayList targets; + public int size() { + return targets.size(); + } + @Override public String toString() { StringBuilder sb = new StringBuilder(); @@ -24,8 +28,8 @@ public class MultiTargetSelector { } } - private List selectors; - private List targetingSAs; + private final List selectors; + private final List targetingSAs; private int currentIndex; public MultiTargetSelector(SpellAbility sa, List plannedSubs) { @@ -52,8 +56,8 @@ public class MultiTargetSelector { public Targets getLastSelectedTargets() { Targets targets = new Targets(); targets.targets = new ArrayList<>(selectors.size()); - for (int i = 0; i < selectors.size(); i++) { - targets.targets.add(selectors.get(i).getLastSelectedTargets()); + for (PossibleTargetSelector selector : selectors) { + targets.targets.add(selector.getLastSelectedTargets()); } return targets; } @@ -78,34 +82,62 @@ public class MultiTargetSelector { currentIndex = -1; } - public void selectTargetsByIndex(int i) { + public boolean selectTargetsByIndex(int i) { + // The caller is telling us to select the i-th possible set of targets. if (i < currentIndex) { reset(); } while (currentIndex < i) { - selectNextTargets(); + if (!selectNextTargets()) { + return false; + } } + return true; } - public boolean selectNextTargets() { - if (currentIndex == -1) { - for (PossibleTargetSelector selector : selectors) { - if (!selector.selectNextTargets()) { + private boolean selectTargetsStartingFrom(int selectorIndex) { + // Don't reset the current selector, as it still has the correct list of targets set and has + // to remember its current/next target index. Subsequent selectors need a reset since their + // possible targets may change based on what was chosen for earlier ones. + if (selectors.get(selectorIndex).selectNextTargets()) { + for (int i = selectorIndex + 1; i < selectors.size(); i++) { + selectors.get(i).reset(); + if (!selectors.get(i).selectNextTargets()) { return false; } } - currentIndex = 0; return true; } - for (int i = selectors.size() - 1; i >= 0; i--) { - if (selectors.get(i).selectNextTargets()) { - currentIndex++; + return false; + } + + public boolean selectNextTargets() { + if (selectors.size() == 0) { + return false; + } + if (currentIndex == -1) { + // Select the first set of targets (calls selectNextTargets() on each selector). + if (selectTargetsStartingFrom(0)) { + currentIndex = 0; return true; } - selectors.get(i).reset(); - selectors.get(i).selectNextTargets(); + // No possible targets. + return false; } - return false; + // Subsequent call, first try selecting a new target for the last selector. If that doesn't + // work, backtrack (decrement selector index) and try selecting targets from there. + // This approach ensures that leaf selectors (end of list) are advanced first, before + // previous ones, so that we get an AA,AB,BA,BB ordering. + int selectorIndex = selectors.size() - 1; + while (!selectTargetsStartingFrom(selectorIndex)) { + if (selectorIndex == 0) { + // No more possible targets. + return false; + } + selectorIndex--; + } + currentIndex++; + return true; } private static boolean conditionsAreMet(SpellAbility saOrSubSa) { diff --git a/forge-ai/src/main/java/forge/ai/simulation/PossibleTargetSelector.java b/forge-ai/src/main/java/forge/ai/simulation/PossibleTargetSelector.java index 7ea91cb0ad2..eba03fc1b29 100644 --- a/forge-ai/src/main/java/forge/ai/simulation/PossibleTargetSelector.java +++ b/forge-ai/src/main/java/forge/ai/simulation/PossibleTargetSelector.java @@ -17,12 +17,11 @@ import forge.game.spellability.TargetRestrictions; public class PossibleTargetSelector { private final SpellAbility sa; - private SpellAbility targetingSa; - private int targetingSaIndex; + private final SpellAbility targetingSa; + private final int targetingSaIndex; private int maxTargets; - private TargetRestrictions tgt; - private int targetIndex; - private List validTargets; + private int nextTargetIndex; + private final List validTargets = new ArrayList<>(); public static class Targets { final int targetingSaIndex; @@ -36,7 +35,7 @@ public class PossibleTargetSelector { this.targetIndex = targetIndex; this.description = description; - if (targetIndex < 0 || targetIndex >= originalTargetCount) { + if (targetIndex != -1 && (targetIndex < 0 || targetIndex >= originalTargetCount)) { throw new IllegalArgumentException("Invalid targetIndex=" + targetIndex); } } @@ -51,12 +50,11 @@ public class PossibleTargetSelector { this.sa = sa; this.targetingSa = targetingSa; this.targetingSaIndex = targetingSaIndex; - this.validTargets = new ArrayList<>(); - generateValidTargets(sa.getHostCard().getController()); + reset(); } public void reset() { - targetIndex = 0; + nextTargetIndex = 0; validTargets.clear(); generateValidTargets(sa.getHostCard().getController()); } @@ -67,7 +65,7 @@ public class PossibleTargetSelector { } sa.setActivatingPlayer(player, true); targetingSa.resetTargets(); - tgt = targetingSa.getTargetRestrictions(); + TargetRestrictions tgt = targetingSa.getTargetRestrictions(); maxTargets = tgt.getMaxTargets(sa.getHostCard(), targetingSa); SimilarTargetSkipper skipper = new SimilarTargetSkipper(); @@ -80,8 +78,8 @@ public class PossibleTargetSelector { } private static class SimilarTargetSkipper { - private ArrayListMultimap validTargetsMap = ArrayListMultimap.create(); - private HashMap cardTypeStrings = new HashMap<>(); + private final ArrayListMultimap validTargetsMap = ArrayListMultimap.create(); + private final HashMap cardTypeStrings = new HashMap<>(); private HashMap creatureScores; private int getCreatureScore(Card c) { @@ -190,16 +188,7 @@ public class PossibleTargetSelector { } public Targets getLastSelectedTargets() { - return new Targets(targetingSaIndex, validTargets.size(), targetIndex - 1, targetingSa.getTargets().toString()); - } - - public boolean selectTargetsByIndex(int targetIndex) { - if (targetIndex >= validTargets.size()) { - return false; - } - selectTargetsByIndexImpl(targetIndex); - this.targetIndex = targetIndex + 1; - return true; + return new Targets(targetingSaIndex, validTargets.size(), nextTargetIndex - 1, targetingSa.getTargets().toString()); } public boolean selectTargets(Targets targets) { @@ -208,16 +197,16 @@ public class PossibleTargetSelector { return false; } selectTargetsByIndexImpl(targets.targetIndex); - this.targetIndex = targets.targetIndex + 1; + this.nextTargetIndex = targets.targetIndex + 1; return true; } public boolean selectNextTargets() { - if (targetIndex >= validTargets.size()) { + if (nextTargetIndex >= validTargets.size()) { return false; } - selectTargetsByIndexImpl(targetIndex); - targetIndex++; + selectTargetsByIndexImpl(nextTargetIndex); + nextTargetIndex++; return true; } } diff --git a/forge-gui-desktop/src/test/java/forge/ai/simulation/SimulationTest.java b/forge-gui-desktop/src/test/java/forge/ai/simulation/SimulationTest.java index aaa3bb7e46b..5829ca698cb 100644 --- a/forge-gui-desktop/src/test/java/forge/ai/simulation/SimulationTest.java +++ b/forge-gui-desktop/src/test/java/forge/ai/simulation/SimulationTest.java @@ -137,4 +137,10 @@ public class SimulationTest { protected Card addCard(String name, Player p) { return addCardToZone(name, p, ZoneType.Battlefield); } + + protected void addCards(String name, int count, Player p) { + for (int i = 0; i < count; i++) { + addCardToZone(name, p, ZoneType.Battlefield); + } + } } diff --git a/forge-gui-desktop/src/test/java/forge/ai/simulation/SpellAbilityPickerSimulationTest.java b/forge-gui-desktop/src/test/java/forge/ai/simulation/SpellAbilityPickerSimulationTest.java index 67780d024a3..45c889e2f98 100644 --- a/forge-gui-desktop/src/test/java/forge/ai/simulation/SpellAbilityPickerSimulationTest.java +++ b/forge-gui-desktop/src/test/java/forge/ai/simulation/SpellAbilityPickerSimulationTest.java @@ -95,11 +95,8 @@ public class SpellAbilityPickerSimulationTest extends SimulationTest { Game game = initAndCreateGame(); Player p = game.getPlayers().get(1); - addCard("Island", p); - addCard("Island", p); - addCard("Forest", p); - addCard("Forest", p); - addCard("Forest", p); + addCards("Island", 2, p); + addCards("Forest", 3, p); Card tatyova = addCardToZone("Tatyova, Benthic Druid", p, ZoneType.Hand); addCardToZone("Forest", p, ZoneType.Hand); @@ -169,10 +166,7 @@ public class SpellAbilityPickerSimulationTest extends SimulationTest { Game game = initAndCreateGame(); Player p = game.getPlayers().get(1); - addCard("Mountain", p); - addCard("Mountain", p); - addCard("Mountain", p); - addCard("Mountain", p); + addCards("Mountain", 4, p); Card spell = addCardToZone("Fiery Confluence", p, ZoneType.Hand); Player opponent = game.getPlayers().get(0); @@ -198,10 +192,7 @@ public class SpellAbilityPickerSimulationTest extends SimulationTest { Game game = initAndCreateGame(); Player p = game.getPlayers().get(1); - addCard("Mountain", p); - addCard("Mountain", p); - addCard("Mountain", p); - addCard("Mountain", p); + addCards("Mountain", 4, p); Card spell = addCardToZone("Fiery Confluence", p, ZoneType.Hand); Player opponent = game.getPlayers().get(0); @@ -226,8 +217,7 @@ public class SpellAbilityPickerSimulationTest extends SimulationTest { Game game = initAndCreateGame(); Player p = game.getPlayers().get(1); - addCard("Mountain", p); - addCard("Mountain", p); + addCards("Mountain", 2, p); Card spell = addCardToZone("Arc Trail", p, ZoneType.Hand); Player opponent = game.getPlayers().get(0); @@ -289,8 +279,7 @@ public class SpellAbilityPickerSimulationTest extends SimulationTest { Game game = initAndCreateGame(); Player p = game.getPlayers().get(1); - addCard("Mountain", p); - addCard("Mountain", p); + addCards("Mountain", 2, p); Card abbot = addCardToZone("Abbot of Keral Keep", p, ZoneType.Hand); addCardToZone("Lightning Bolt", p, ZoneType.Hand); // Note: This assumes the top of library is revealed. If the AI is made @@ -321,9 +310,7 @@ public class SpellAbilityPickerSimulationTest extends SimulationTest { Game game = initAndCreateGame(); Player p = game.getPlayers().get(1); - addCard("Mountain", p); - addCard("Mountain", p); - addCard("Mountain", p); + addCards("Mountain", 3, p); Card abbot = addCardToZone("Abbot of Keral Keep", p, ZoneType.Hand); // Note: This assumes the top of library is revealed. If the AI is made // smarter to not assume that, then this test can be updated to have @@ -426,8 +413,7 @@ public class SpellAbilityPickerSimulationTest extends SimulationTest { Card blocker = addCard("Fugitive Wizard", opponent); Card attacker1 = addCard("Dwarven Trader", p); attacker1.setSickness(false); - addCard("Swamp", p); - addCard("Swamp", p); + addCards("Swamp", 2, p); addCardToZone("Doom Blade", p, ZoneType.Hand); game.getPhaseHandler().devModeSet(PhaseType.MAIN1, p); @@ -455,9 +441,7 @@ public class SpellAbilityPickerSimulationTest extends SimulationTest { Player opponent = game.getPlayers().get(0); addCardToZone("Chaos Warp", p, ZoneType.Hand); - addCard("Mountain", p); - addCard("Mountain", p); - addCard("Mountain", p); + addCards("Mountain", 3, p); addCard("Plains", opponent); addCard("Mountain", opponent); @@ -489,8 +473,7 @@ public class SpellAbilityPickerSimulationTest extends SimulationTest { Game game = initAndCreateGame(); Player p = game.getPlayers().get(1); - addCard("Island", p); - addCard("Island", p); + addCards("Forest", 2, p); addCardToZone("Counterspell", p, ZoneType.Hand); addCardToZone("Unsummon", p, ZoneType.Hand); @@ -605,4 +588,74 @@ public class SpellAbilityPickerSimulationTest extends SimulationTest { // Still, this test case exercises the code path and ensures we don't crash in this case. AssertJUnit.assertEquals(1, picker.getNumSimulations()); } + + @Test + public void threeDistinctTargetSpell() { + Game game = initAndCreateGame(); + Player p = game.getPlayers().get(1); + Player opponent = game.getPlayers().get(0); + + addCardToZone("Incremental Growth", p, ZoneType.Hand); + addCards("Forest", 5, p); + addCard("Forest Bear", p); + addCard("Flying Men", opponent); + addCard("Runeclaw Bear", p); + addCard("Water Elemental", opponent); + addCard("Grizzly Bears", p); + + game.getPhaseHandler().devModeSet(PhaseType.MAIN2, p); + game.getAction().checkStateEffects(true); + SpellAbilityPicker picker = new SpellAbilityPicker(game, p); + SpellAbility sa = picker.chooseSpellAbilityToPlay(null); + AssertJUnit.assertNotNull(sa); + MultiTargetSelector.Targets targets = picker.getPlan().getSelectedDecision().targets; + AssertJUnit.assertEquals(3, targets.size()); + AssertJUnit.assertTrue(targets.toString().contains("Forest Bear")); + AssertJUnit.assertTrue(targets.toString().contains("Runeclaw Bear")); + AssertJUnit.assertTrue(targets.toString().contains("Grizzly Bear")); + // Expected 5*4*3=60 iterations (5 choices for first target, 4 for next, 3 for last.) + AssertJUnit.assertEquals(60, picker.getNumSimulations()); + } + + @Test + public void threeDistinctTargetSpellCantBeCast() { + Game game = initAndCreateGame(); + Player p = game.getPlayers().get(1); + Player opponent = game.getPlayers().get(0); + + addCardToZone("Incremental Growth", p, ZoneType.Hand); + addCards("Forest", 5, p); + addCard("Forest Bear", p); + addCard("Flying Men", opponent); + + game.getPhaseHandler().devModeSet(PhaseType.MAIN2, p); + game.getAction().checkStateEffects(true); + SpellAbilityPicker picker = new SpellAbilityPicker(game, p); + SpellAbility sa = picker.chooseSpellAbilityToPlay(null); + AssertJUnit.assertNull(sa); + } + + @Test + public void correctTargetChoicesWithTwoTargetSpell() { + Game game = initAndCreateGame(); + Player p = game.getPlayers().get(1); + Player opponent = game.getPlayers().get(0); + + addCardToZone("Rites of Reaping", p, ZoneType.Hand); + addCard("Swamp", p); + addCards("Forest", 5, p); + addCard("Flying Men", opponent); + addCard("Forest Bear", p); + addCard("Water Elemental", opponent); + + game.getPhaseHandler().devModeSet(PhaseType.MAIN2, p); + game.getAction().checkStateEffects(true); + SpellAbilityPicker picker = new SpellAbilityPicker(game, p); + SpellAbility sa = picker.chooseSpellAbilityToPlay(null); + AssertJUnit.assertNotNull(sa); + MultiTargetSelector.Targets targets = picker.getPlan().getSelectedDecision().targets; + AssertJUnit.assertEquals(2, targets.size()); + AssertJUnit.assertTrue(targets.toString().contains("Forest Bear")); + AssertJUnit.assertTrue(targets.toString().contains("Flying Men")); + } }