diff --git a/forge-ai/src/main/java/forge/ai/AiController.java b/forge-ai/src/main/java/forge/ai/AiController.java index 9c1cc7cd8aa..4b7f89f3e88 100644 --- a/forge-ai/src/main/java/forge/ai/AiController.java +++ b/forge-ai/src/main/java/forge/ai/AiController.java @@ -903,13 +903,8 @@ public class AiController { if (sa instanceof SpellPermanent) { return canPlayFromEffectAI((SpellPermanent)sa, false, true); } - if (sa.usesTargeting()) { - if (!sa.isTargetNumberValid() && sa.getTargetRestrictions().getNumCandidates(sa, true) == 0) { - return AiPlayDecision.TargetingFailed; - } - if (!StaticAbilityMustTarget.meetsMustTargetRestriction(sa)) { - return AiPlayDecision.TargetingFailed; - } + if (sa.usesTargeting() && !sa.hasLegalTargets()) { + return AiPlayDecision.TargetingFailed; } if (sa instanceof Spell) { if (!player.cantLoseForZeroOrLessLife() && player.canLoseLife() && diff --git a/forge-ai/src/main/java/forge/ai/simulation/GameSimulator.java b/forge-ai/src/main/java/forge/ai/simulation/GameSimulator.java index 0cbd88374b5..847a69067f7 100644 --- a/forge-ai/src/main/java/forge/ai/simulation/GameSimulator.java +++ b/forge-ai/src/main/java/forge/ai/simulation/GameSimulator.java @@ -1,6 +1,9 @@ package forge.ai.simulation; +import forge.game.ability.ApiType; +import forge.game.ability.effects.CharmEffect; import forge.game.spellability.LandAbility; +import forge.game.zone.ZoneType; import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; @@ -198,13 +201,10 @@ public class GameSimulator { final SpellAbility playingSa = sa; simGame.copyLastState(); - boolean success = ComputerUtil.handlePlayingSpellAbility(aiPlayer, sa, simGame, new Runnable() { - @Override - public void run() { - if (interceptor != null) { - interceptor.announceX(playingSa); - interceptor.chooseTargets(playingSa, GameSimulator.this); - } + boolean success = ComputerUtil.handlePlayingSpellAbility(aiPlayer, sa, simGame, () -> { + if (interceptor != null) { + interceptor.announceX(playingSa); + interceptor.chooseTargets(playingSa, GameSimulator.this); } }); if (!success) { diff --git a/forge-ai/src/main/java/forge/ai/simulation/SpellAbilityChoicesIterator.java b/forge-ai/src/main/java/forge/ai/simulation/SpellAbilityChoicesIterator.java index b2294c0cf39..c1972564bf1 100644 --- a/forge-ai/src/main/java/forge/ai/simulation/SpellAbilityChoicesIterator.java +++ b/forge-ai/src/main/java/forge/ai/simulation/SpellAbilityChoicesIterator.java @@ -16,7 +16,7 @@ import forge.game.spellability.AbilitySub; import forge.game.spellability.SpellAbility; public class SpellAbilityChoicesIterator { - private SimulationController controller; + private final SimulationController controller; private Iterator modeIterator; private int[] selectedModes; @@ -34,11 +34,13 @@ public class SpellAbilityChoicesIterator { Card selectedChoice; Score bestScoreForChoice = new Score(Integer.MIN_VALUE); } - private ArrayList choicePoints = new ArrayList<>(); + private final ArrayList choicePoints = new ArrayList<>(); private int incrementedCpIndex = 0; private int cpIndex = -1; private int evalDepth; + // Maps from filtered mode indexes to original ones. + private List modesMap; public SpellAbilityChoicesIterator(SimulationController controller) { this.controller = controller; @@ -46,17 +48,26 @@ public class SpellAbilityChoicesIterator { public List chooseModesForAbility(List choices, int min, int num, boolean allowRepeat) { if (modeIterator == null) { - // TODO: Need to skip modes that are invalid (e.g. targets don't exist)! + // Skip modes that don't have legal targets. + modesMap = new ArrayList<>(); + int origIndex = -1; + for (AbilitySub sub : choices) { + origIndex++; + if (sub.usesTargeting() && !sub.hasLegalTargets()) { + continue; + } + modesMap.add(origIndex); + } // TODO: Do we need to do something special to support cards that have extra costs // when choosing more modes, like Blessed Alliance? if (!allowRepeat) { - modeIterator = CombinatoricsUtils.combinationsIterator(choices.size(), num); + modeIterator = CombinatoricsUtils.combinationsIterator(modesMap.size(), num); } else { // Note: When allowRepeat is true, it does result in many possibilities being tried. // We should ideally prune some of those at a higher level. - modeIterator = new AllowRepeatModesIterator(choices.size(), min, num); + modeIterator = new AllowRepeatModesIterator(modesMap.size(), min, num); } - selectedModes = modeIterator.next(); + selectedModes = remapModes(modeIterator.next()); advancedToNextMode = true; } // Note: If modeIterator already existed, selectedModes would have been updated in advance(). @@ -78,6 +89,13 @@ public class SpellAbilityChoicesIterator { return result; } + private int[] remapModes(int[] modes) { + for (int i = 0; i < modes.length; i++) { + modes[i] = modesMap.get(modes[i]); + } + return modes; + } + public Card chooseCard(CardCollection fetchList) { cpIndex++; if (cpIndex >= choicePoints.size()) { @@ -86,8 +104,7 @@ public class SpellAbilityChoicesIterator { ChoicePoint cp = choicePoints.get(cpIndex); // Prune duplicates. HashSet uniqueCards = new HashSet<>(); - for (int i = 0; i < fetchList.size(); i++) { - Card card = fetchList.get(i); + for (Card card : fetchList) { if (uniqueCards.add(card.getName()) && uniqueCards.size() == cp.nextChoice + 1) { cp.selectedChoice = card; } @@ -137,14 +154,9 @@ public class SpellAbilityChoicesIterator { evalDepth++; pushTarget = false; } - return; } } - public int[] getSelectModes() { - return selectedModes; - } - public boolean advance(Score lastScore) { cpIndex = -1; for (ChoicePoint cp : choicePoints) { @@ -195,7 +207,7 @@ public class SpellAbilityChoicesIterator { doneEvaluating(bestScoreForMode); bestScoreForMode = new Score(Integer.MIN_VALUE); if (modeIterator.hasNext()) { - selectedModes = modeIterator.next(); + selectedModes = remapModes(modeIterator.next()); advancedToNextMode = true; return true; } @@ -232,8 +244,8 @@ public class SpellAbilityChoicesIterator { } private static class AllowRepeatModesIterator implements Iterator { - private int numChoices; - private int max; + private final int numChoices; + private final int max; private int[] indexes; public AllowRepeatModesIterator(int numChoices, int min, int max) { diff --git a/forge-ai/src/main/java/forge/ai/simulation/SpellAbilityPicker.java b/forge-ai/src/main/java/forge/ai/simulation/SpellAbilityPicker.java index 03885c8fd4d..11ba01dc2c6 100644 --- a/forge-ai/src/main/java/forge/ai/simulation/SpellAbilityPicker.java +++ b/forge-ai/src/main/java/forge/ai/simulation/SpellAbilityPicker.java @@ -39,6 +39,7 @@ public class SpellAbilityPicker { private SpellAbilityChoicesIterator interceptor; private Plan plan; + private int numSimulations; public SpellAbilityPicker(Game game, Player player) { this.game = game; @@ -66,19 +67,7 @@ public class SpellAbilityPicker { private List getCandidateSpellsAndAbilities() { CardCollection cards = ComputerUtilAbility.getAvailableCards(game, player); List all = ComputerUtilAbility.getSpellAbilities(cards, player); - CardCollection landsToPlay = ComputerUtilAbility.getAvailableLandsToPlay(game, player); - if (landsToPlay != null) { - HashMap landsDeDupe = new HashMap<>(); - for (Card land : landsToPlay) { - Card previousLand = landsDeDupe.get(land.getName()); - // Skip identical lands. - if (previousLand != null && previousLand.getZone() == land.getZone() && previousLand.getOwner() == land.getOwner()) { - continue; - } - landsDeDupe.put(land.getName(), land); - all.add(new LandAbility(land)); - } - } + HashMap landsDeDupe = new HashMap<>(); List candidateSAs = ComputerUtilAbility.getOriginalAndAltCostAbilities(all, player); int writeIndex = 0; for (int i = 0; i < candidateSAs.size(); i++) { @@ -86,6 +75,16 @@ public class SpellAbilityPicker { if (sa.isManaAbility()) { continue; } + // Skip identical lands. + if (sa instanceof LandAbility) { + Card land = sa.getHostCard(); + Card previousLand = landsDeDupe.get(sa.getHostCard().getName()); + if (previousLand != null && previousLand.getZone() == land.getZone() && + previousLand.getOwner() == land.getOwner()) { + continue; + } + landsDeDupe.put(land.getName(), land); + } sa.setActivatingPlayer(player, true); AiPlayDecision opinion = canPlayAndPayForSim(sa); @@ -95,7 +94,7 @@ public class SpellAbilityPicker { if (opinion != AiPlayDecision.WillPlay) continue; - candidateSAs.set(writeIndex, sa); + candidateSAs.set(writeIndex, sa); writeIndex++; } candidateSAs.subList(writeIndex, candidateSAs.size()).clear(); @@ -353,7 +352,9 @@ public class SpellAbilityPicker { if (!ComputerUtilCost.canPayCost(sa, player, sa.isTrigger())) { return AiPlayDecision.CantAfford; } - + if (sa.usesTargeting() && !sa.hasLegalTargets()) { + return AiPlayDecision.TargetingFailed; + } if (shouldWaitForLater(sa)) { return AiPlayDecision.AnotherTime; } @@ -375,10 +376,13 @@ public class SpellAbilityPicker { final SpellAbilityChoicesIterator choicesIterator = new SpellAbilityChoicesIterator(controller); Score lastScore; do { + // TODO: MyRandom should be an instance on the game object, so that we could do + // simulations in parallel without messing up global state. MyRandom.setRandom(new Random(randomSeedToUse)); GameSimulator simulator = new GameSimulator(controller, game, player, phase); simulator.setInterceptor(choicesIterator); lastScore = simulator.simulateSpellAbility(sa); + numSimulations++; if (lastScore.value > bestScore.value) { bestScore = lastScore; } @@ -462,4 +466,8 @@ public class SpellAbilityPicker { } return ComputerUtil.chooseSacrificeType(player, type, ability, ability.getTargetCard(), effect, amount, exclude); } + + public int getNumSimulations() { + return numSimulations; + } } diff --git a/forge-game/src/main/java/forge/game/spellability/SpellAbility.java b/forge-game/src/main/java/forge/game/spellability/SpellAbility.java index d5d4b01d726..d71bf584e7d 100644 --- a/forge-game/src/main/java/forge/game/spellability/SpellAbility.java +++ b/forge-game/src/main/java/forge/game/spellability/SpellAbility.java @@ -1254,10 +1254,15 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit return false; } - final TargetRestrictions tr = getTargetRestrictions(); + final SpellAbility rootAbility = this.getRootAbility(); + // 115.5. A spell or ability on the stack is an illegal target for itself. + if (rootAbility.isSpell() && rootAbility.getHostCard() == entity) { + return false; + } // Restriction related to this ability if (usesTargeting()) { + final TargetRestrictions tr = getTargetRestrictions(); if (tr.isUniqueTargets() && getUniqueTargets().contains(entity)) return false; @@ -1649,6 +1654,16 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit return targetRestrictions != null; } + public boolean hasLegalTargets() { + if (!isTargetNumberValid() && getTargetRestrictions().getNumCandidates(this, true) == 0) { + return false; + } + if (!StaticAbilityMustTarget.meetsMustTargetRestriction(this)) { + return false; + } + return true; + } + public TargetRestrictions getTargetRestrictions() { return targetRestrictions; } 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 8aa56bc5053..29b8a3d3b01 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 @@ -483,4 +483,87 @@ public class SpellAbilityPickerSimulationTest extends SimulationTest { AssertJUnit.assertEquals("Chaos Warp", sa.getHostCard().getName()); AssertJUnit.assertEquals(expectedTarget, sa.getTargetCard()); } + + @Test + public void testNoSimulationsWhenNoTargets() { + Game game = initAndCreateGame(); + Player p = game.getPlayers().get(1); + + addCard("Island", p); + addCard("Island", p); + addCardToZone("Counterspell", p, ZoneType.Hand); + addCardToZone("Unsummon", p, ZoneType.Hand); + + game.getPhaseHandler().devModeSet(PhaseType.MAIN2, p); + game.getAction().checkStateEffects(true); + + SpellAbilityPicker picker = new SpellAbilityPicker(game, p); + SpellAbility sa = picker.chooseSpellAbilityToPlay(null); + AssertJUnit.assertNull(sa); + AssertJUnit.assertEquals(0, picker.getNumSimulations()); + } + + @Test + public void testLandDropPruning() { + Game game = initAndCreateGame(); + Player p = game.getPlayers().get(1); + + addCardToZone("Island", p, ZoneType.Hand); + addCardToZone("Island", p, ZoneType.Hand); + addCardToZone("Island", p, ZoneType.Hand); + + game.getPhaseHandler().devModeSet(PhaseType.MAIN2, p); + game.getAction().checkStateEffects(true); + + SpellAbilityPicker picker = new SpellAbilityPicker(game, p); + SpellAbility sa = picker.chooseSpellAbilityToPlay(null); + AssertJUnit.assertNotNull(sa); + // Only one land drop should be simulated, since the cards are identical. + AssertJUnit.assertEquals(1, picker.getNumSimulations()); + } + + @Test + public void testSpellCantTargetSelf() { + Game game = initAndCreateGame(); + Player p = game.getPlayers().get(1); + Player opponent = game.getPlayers().get(0); + + addCardToZone("Unsubstantiate", p, ZoneType.Hand); + addCard("Forest", p); + addCard("Island", p); + Card expectedTarget = 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.assertNotNull(sa); + AssertJUnit.assertEquals(expectedTarget, sa.getTargetCard()); + // Only a single simulation expected (no target self). + AssertJUnit.assertEquals(1, picker.getNumSimulations()); + } + + @Test + public void testModalSpellCantTargetSelf() { + Game game = initAndCreateGame(); + Player p = game.getPlayers().get(1); + Player opponent = game.getPlayers().get(0); + + addCardToZone("Decisive Denial", p, ZoneType.Hand); + addCard("Forest", p); + addCard("Island", p); + addCard("Runeclaw 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.assertNotNull(sa); + // Expected: Runeclaw Bear fights Flying Men + // Only a single simulation expected (no target self). + AssertJUnit.assertEquals(1, picker.getNumSimulations()); + } }