From eb0f426adcd14d1265771f7f68607b7967b170cc Mon Sep 17 00:00:00 2001 From: asvitkine Date: Fri, 23 Dec 2022 20:28:05 -0700 Subject: [PATCH] Use ComputerUtilAbility.isFullyTargetable() and add more tests. --- .../src/main/java/forge/ai/AiController.java | 10 ++++- .../SpellAbilityChoicesIterator.java | 3 +- .../ai/simulation/SpellAbilityPicker.java | 2 +- .../forge/game/spellability/SpellAbility.java | 10 ----- .../SpellAbilityPickerSimulationTest.java | 41 +++++++++++++++++++ 5 files changed, 52 insertions(+), 14 deletions(-) diff --git a/forge-ai/src/main/java/forge/ai/AiController.java b/forge-ai/src/main/java/forge/ai/AiController.java index 79a02420d0b..9c1cc7cd8aa 100644 --- a/forge-ai/src/main/java/forge/ai/AiController.java +++ b/forge-ai/src/main/java/forge/ai/AiController.java @@ -54,6 +54,7 @@ import forge.game.replacement.ReplacementLayer; import forge.game.replacement.ReplacementType; import forge.game.spellability.*; import forge.game.staticability.StaticAbility; +import forge.game.staticability.StaticAbilityMustTarget; import forge.game.trigger.Trigger; import forge.game.trigger.TriggerType; import forge.game.trigger.WrappedAbility; @@ -902,8 +903,13 @@ public class AiController { if (sa instanceof SpellPermanent) { return canPlayFromEffectAI((SpellPermanent)sa, false, true); } - if (sa.usesTargeting() && !sa.hasLegalTargets()) { - return AiPlayDecision.TargetingFailed; + if (sa.usesTargeting()) { + if (!sa.isTargetNumberValid() && sa.getTargetRestrictions().getNumCandidates(sa, true) == 0) { + return AiPlayDecision.TargetingFailed; + } + if (!StaticAbilityMustTarget.meetsMustTargetRestriction(sa)) { + return AiPlayDecision.TargetingFailed; + } } if (sa instanceof Spell) { if (!player.cantLoseForZeroOrLessLife() && player.canLoseLife() && 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 bbc2cead20f..fbf17632d44 100644 --- a/forge-ai/src/main/java/forge/ai/simulation/SpellAbilityChoicesIterator.java +++ b/forge-ai/src/main/java/forge/ai/simulation/SpellAbilityChoicesIterator.java @@ -1,5 +1,6 @@ package forge.ai.simulation; +import forge.ai.ComputerUtilAbility; import java.util.ArrayList; import java.util.HashSet; import java.util.Iterator; @@ -53,7 +54,7 @@ public class SpellAbilityChoicesIterator { int origIndex = -1; for (AbilitySub sub : choices) { origIndex++; - if (sub.usesTargeting() && !sub.hasLegalTargets()) { + if (!ComputerUtilAbility.isFullyTargetable(sub)) { continue; } modesMap.add(origIndex); 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 11ba01dc2c6..e02e90c6be6 100644 --- a/forge-ai/src/main/java/forge/ai/simulation/SpellAbilityPicker.java +++ b/forge-ai/src/main/java/forge/ai/simulation/SpellAbilityPicker.java @@ -352,7 +352,7 @@ public class SpellAbilityPicker { if (!ComputerUtilCost.canPayCost(sa, player, sa.isTrigger())) { return AiPlayDecision.CantAfford; } - if (sa.usesTargeting() && !sa.hasLegalTargets()) { + if (!ComputerUtilAbility.isFullyTargetable(sa)) { return AiPlayDecision.TargetingFailed; } if (shouldWaitForLater(sa)) { 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 3b398c05f53..a86cdcf21e5 100644 --- a/forge-game/src/main/java/forge/game/spellability/SpellAbility.java +++ b/forge-game/src/main/java/forge/game/spellability/SpellAbility.java @@ -1655,16 +1655,6 @@ 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 29b8a3d3b01..80dbba2f80b 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 @@ -566,4 +566,45 @@ public class SpellAbilityPickerSimulationTest extends SimulationTest { // Only a single simulation expected (no target self). AssertJUnit.assertEquals(1, picker.getNumSimulations()); } + + @Test + public void testModalSpellNoTargetsForModeWithSubAbility() { + Game game = initAndCreateGame(); + Player p = game.getPlayers().get(1); + Player opponent = game.getPlayers().get(0); + + addCardToZone("Temur Charm", p, ZoneType.Hand); + addCard("Forest", p); + addCard("Island", p); + addCard("Mountain", p); + + game.getPhaseHandler().devModeSet(PhaseType.MAIN2, p); + game.getAction().checkStateEffects(true); + + SpellAbilityPicker picker = new SpellAbilityPicker(game, p); + SpellAbility sa = picker.chooseSpellAbilityToPlay(null); + // Only mode "Creatures with power 3 or less can’t block this turn" should be simulated. + AssertJUnit.assertEquals(1, picker.getNumSimulations()); + } + + @Test + public void testModalSpellNoTargetsForAnyModes() { + Game game = initAndCreateGame(); + Player p = game.getPlayers().get(1); + Player opponent = game.getPlayers().get(0); + + addCardToZone("Drown in the Loch", p, ZoneType.Hand); + addCard("Swamp", p); + addCard("Island", p); + + game.getPhaseHandler().devModeSet(PhaseType.MAIN2, p); + game.getAction().checkStateEffects(true); + + SpellAbilityPicker picker = new SpellAbilityPicker(game, p); + SpellAbility sa = picker.chooseSpellAbilityToPlay(null); + // TODO: Ideally, this would be 0 simulations, but we currently only determine there are no + // valid in SpellAbilityChoicesIterator, which runs once we're already simulating the spell. + // Still, this test case exercises the code path and ensures we don't crash in this case. + AssertJUnit.assertEquals(1, picker.getNumSimulations()); + } }