From 84f7f341ec46e36fb53fd006b034a9e27ba5df8d Mon Sep 17 00:00:00 2001 From: Madwand99 Date: Thu, 30 Apr 2026 07:13:30 -0700 Subject: [PATCH 1/2] Improve AI mode selection for modal spells with optional extra costs --- .../main/java/forge/ai/ability/CharmAi.java | 126 +++++++++++++++- .../java/forge/ai/AIIntegrationTests.java | 138 ++++++++++++++++++ 2 files changed, 261 insertions(+), 3 deletions(-) diff --git a/forge-ai/src/main/java/forge/ai/ability/CharmAi.java b/forge-ai/src/main/java/forge/ai/ability/CharmAi.java index dc9558e414b..fb823550ea7 100644 --- a/forge-ai/src/main/java/forge/ai/ability/CharmAi.java +++ b/forge-ai/src/main/java/forge/ai/ability/CharmAi.java @@ -2,11 +2,16 @@ import com.google.common.collect.Lists; import forge.ai.*; +import forge.game.GameActionUtil; import forge.game.ability.AbilityUtils; import forge.game.ability.effects.CharmEffect; import forge.game.card.Card; +import forge.game.cost.Cost; +import forge.game.keyword.KeywordInterface; import forge.game.player.Player; import forge.game.spellability.AbilitySub; +import forge.game.spellability.OptionalCost; +import forge.game.spellability.OptionalCostValue; import forge.game.spellability.SpellAbility; import forge.util.Aggregates; import forge.util.collect.FCollection; @@ -94,8 +99,6 @@ protected AiAbilityDecision checkApiLogic(Player ai, SpellAbility sa) { private List chooseOptionsAi(SpellAbility sa, List choices, final Player ai, boolean isTrigger, int num, int min) { List chosen = Lists.newArrayList(); AiController aic = ((PlayerControllerAi) ai.getController()).getAi(); - // TODO unused for now, the AI doesn't know how to effectively handle repeated choices - boolean allowRepeat = sa.hasParam("CanRepeatModes"); final int pawprintLimit = sa.hasParam("Pawprint") ? AbilityUtils.calculateAmount(sa.getHostCard(), sa.getParam("Pawprint"), sa) : 0; if (pawprintLimit > 0) { @@ -109,7 +112,7 @@ private List chooseOptionsAi(SpellAbility sa, List choic handleDependentModes(sa, chosen, sub); sub.setActivatingPlayer(ai); // TODO refactor to obtain the AiAbilityDecision instead, then we can check all to sort by value - if (AiPlayDecision.WillPlay == aic.canPlaySa(sub)) { + if (AiPlayDecision.WillPlay == aic.canPlaySa(sub) && canPayForAdditionalMode(sa, chosen, sub, ai)) { if (pawprintLimit > 0) { int curPawprintAmount = AbilityUtils.calculateAmount(sub.getHostCard(), sub.getParamOrDefault("Pawprint", "0"), sub); if (pawprintAmount + curPawprintAmount > pawprintLimit) { @@ -150,6 +153,18 @@ private List chooseOptionsAi(SpellAbility sa, List choic } } } + if (!isTrigger && chosen.size() < num && min < num) { + choices.removeAll(chosen); + for (AbilitySub sub : choices) { + handleDependentModes(sa, chosen, sub); + if (isModeWorthAdding(aic, ai, sub) && canPayForAdditionalMode(sa, chosen, sub, ai)) { + chosen.add(sub); + if (chosen.size() == num) { + break; + } + } + } + } if (chosen.size() < min) { // not enough choices chosen.clear(); @@ -158,6 +173,64 @@ private List chooseOptionsAi(SpellAbility sa, List choic return chosen; } + private boolean canPayForAdditionalMode(SpellAbility sa, List chosen, AbilitySub sub, Player ai) { + Card source = sa.getHostCard(); + if (source.hasStartOfKeyword("Spree") || source.hasStartOfKeyword("Tiered")) { + Cost fullCost = sa.getPayCosts().copy(); + for (AbilitySub mode : chosen) { + if (!mode.hasParam("ModeCost")) { + return false; + } + fullCost.add(new Cost(mode.getParam("ModeCost"), false)); + } + if (!sub.hasParam("ModeCost")) { + return false; + } + fullCost.add(new Cost(sub.getParam("ModeCost"), false)); + return ComputerUtilCost.canPayCost(sa.copyWithDefinedCost(fullCost), ai, false); + } + + if (!source.hasStartOfKeyword("Escalate")) { + return true; + } + String escalateCost = getEscalateCost(source); + if (escalateCost == null) { + return false; + } + + Cost fullCost = sa.getPayCosts().copy(); + for (int i = 0; i < chosen.size(); i++) { + fullCost.add(new Cost(escalateCost, false)); + } + return ComputerUtilCost.canPayCost(sa.copyWithDefinedCost(fullCost), ai, false); + } + + private String getEscalateCost(Card source) { + for (KeywordInterface inst : source.getKeywords()) { + String kw = inst.getOriginal(); + if (kw.startsWith("Escalate:")) { + return kw.substring("Escalate:".length()); + } + } + return null; + } + + private boolean isModeWorthAdding(AiController aic, Player ai, AbilitySub sub) { + sub.setActivatingPlayer(ai); + try { + if (AiPlayDecision.WillPlay == aic.canPlaySa(sub)) { + return true; + } + } catch (RuntimeException ex) { + // Some sub-mode AI assumes full spell timing context; fall back to trigger heuristics. + } + try { + return aic.doTrigger(sub, false); + } catch (RuntimeException ex) { + return false; + } + } + private List chooseTriskaidekaphobia(List choices, final Player ai) { List chosenList = Lists.newArrayList(); if (choices == null || choices.isEmpty()) { return chosenList; } @@ -288,6 +361,53 @@ public Player chooseSinglePlayer(Player ai, SpellAbility sa, Iterable op return Aggregates.random(opponents); } + @Override + public List chooseOptionalCosts(Player payer, SpellAbility chosen, List optionalCostValues) { + OptionalCostValue entwine = null; + List otherCosts = Lists.newArrayList(); + for (OptionalCostValue opt : optionalCostValues) { + if (opt.getType() == OptionalCost.Entwine) { + entwine = opt; + } else { + otherCosts.add(opt); + } + } + + List chosenCosts = super.chooseOptionalCosts(payer, chosen, otherCosts); + if (entwine == null || !shouldPayEntwine(payer, chosen, chosenCosts, entwine)) { + return chosenCosts; + } + + chosenCosts.add(entwine); + return chosenCosts; + } + + private boolean shouldPayEntwine(Player payer, SpellAbility chosen, List chosenCosts, OptionalCostValue entwine) { + List costsWithEntwine = Lists.newArrayList(chosenCosts); + costsWithEntwine.add(entwine); + + SpellAbility entwined = GameActionUtil.addOptionalCosts(chosen, costsWithEntwine); + if (!ComputerUtilCost.canPayCost(entwined, payer, false)) { + return false; + } + + List choices = CharmEffect.makePossibleOptions(entwined); + if (choices.size() < 2) { + return false; + } + + AiController aic = ((PlayerControllerAi) payer.getController()).getAi(); + for (AbilitySub sub : choices) { + if (!isModeWorthAdding(aic, payer, sub)) { + entwined.setSubAbility(null); + return false; + } + } + + entwined.setSubAbility(null); + return true; + } + @Override public AiAbilityDecision chkDrawbackWithSubs(Player aiPlayer, AbilitySub ab) { // choices were already targeted diff --git a/forge-gui-desktop/src/test/java/forge/ai/AIIntegrationTests.java b/forge-gui-desktop/src/test/java/forge/ai/AIIntegrationTests.java index 2489034c588..6c409746ee4 100644 --- a/forge-gui-desktop/src/test/java/forge/ai/AIIntegrationTests.java +++ b/forge-gui-desktop/src/test/java/forge/ai/AIIntegrationTests.java @@ -1,16 +1,22 @@ package forge.ai; import forge.card.mana.ManaAtom; +import forge.game.GameActionUtil; import forge.game.Game; import forge.game.card.Card; import forge.game.mana.Mana; import forge.game.phase.PhaseType; import forge.game.player.Player; +import forge.game.spellability.OptionalCost; +import forge.game.spellability.OptionalCostValue; +import forge.game.spellability.SpellAbility; import forge.game.zone.Zone; import forge.game.zone.ZoneType; import org.testng.AssertJUnit; import org.testng.annotations.Test; +import java.util.List; + public class AIIntegrationTests extends AITest { @Test public void testSwingForLethal() { @@ -113,4 +119,136 @@ public void testDoesNotCastRepopulateWhenNoCreaturesInOpponentGraveyard() { AssertJUnit.assertTrue("Repopulate must still be in hand", hand.contains(repopulate)); } + + @Test + public void testCrushContrabandChoosesBothModes() { + Game game = initAndCreateGame(); + Player p = game.getPlayers().get(1); + p.setTeam(0); + + Card crushContraband = addCardToZone("Crush Contraband", p, ZoneType.Hand); + SpellAbility sa = crushContraband.getSpellAbilities().get(0); + sa.setActivatingPlayer(p); + + Player opponent = game.getPlayers().get(0); + opponent.setTeam(1); + addCard("Sol Ring", opponent); + addCard("Honor of the Pure", opponent); + + AiPlayDecision decision = ((PlayerControllerAi) p.getController()).getAi().canPlaySa(sa); + + AssertJUnit.assertEquals(AiPlayDecision.WillPlay, decision); + AssertJUnit.assertEquals("AI should choose both available modes", 2, sa.getChosenList().size()); + } + + @Test + public void testEscalateDoesNotChooseUnaffordableExtraMode() { + Game game = initAndCreateGame(); + Player p = game.getPlayers().get(1); + p.setTeam(0); + addCard("Forest", p); + addCard("Wastes", p); + + Card collectiveResistance = addCardToZone("Collective Resistance", p, ZoneType.Hand); + SpellAbility sa = collectiveResistance.getSpellAbilities().get(0); + sa.setActivatingPlayer(p); + + Player opponent = game.getPlayers().get(0); + opponent.setTeam(1); + addCard("Sol Ring", opponent); + addCard("Honor of the Pure", opponent); + + AiPlayDecision decision = ((PlayerControllerAi) p.getController()).getAi().canPlaySa(sa); + + AssertJUnit.assertEquals(AiPlayDecision.WillPlay, decision); + AssertJUnit.assertEquals("AI should not choose an extra Escalate mode it cannot pay for", 1, sa.getChosenList().size()); + } + + @Test + public void testEscalateChoosesAffordableExtraMode() { + Game game = initAndCreateGame(); + Player p = game.getPlayers().get(1); + p.setTeam(0); + addCards("Forest", 3, p); + + Card collectiveResistance = addCardToZone("Collective Resistance", p, ZoneType.Hand); + SpellAbility sa = collectiveResistance.getSpellAbilities().get(0); + sa.setActivatingPlayer(p); + + Player opponent = game.getPlayers().get(0); + opponent.setTeam(1); + addCard("Sol Ring", opponent); + addCard("Honor of the Pure", opponent); + + AiPlayDecision decision = ((PlayerControllerAi) p.getController()).getAi().canPlaySa(sa); + + AssertJUnit.assertEquals(AiPlayDecision.WillPlay, decision); + AssertJUnit.assertEquals("AI should choose an extra Escalate mode when it is useful and payable", 2, sa.getChosenList().size()); + } + + @Test + public void testEntwineChoosesCostWhenAllModesAreUseful() { + Game game = initAndCreateGame(); + Player p = game.getPlayers().get(1); + p.setTeam(0); + addCards("Forest", 6, p); + addCardToZone("Forest", p, ZoneType.Library); + addCardToZone("Plains", p, ZoneType.Library); + + Card journey = addCardToZone("Journey of Discovery", p, ZoneType.Hand); + SpellAbility sa = journey.getSpellAbilities().get(0); + sa.setActivatingPlayer(p); + + List optionalCosts = GameActionUtil.getOptionalCostValues(sa); + List chosenCosts = p.getController().chooseOptionalCosts(sa, optionalCosts); + + AssertJUnit.assertTrue("AI should pay Entwine when both modes are useful", + chosenCosts.stream().anyMatch(cost -> cost.getType() == OptionalCost.Entwine)); + } + + @Test + public void testSpreeDoesNotChooseUnaffordableExtraMode() { + Game game = initAndCreateGame(); + Player p = game.getPlayers().get(1); + p.setTeam(0); + addCard("Plains", p); + addCard("Wastes", p); + + Card requisitionRaid = addCardToZone("Requisition Raid", p, ZoneType.Hand); + SpellAbility sa = requisitionRaid.getSpellAbilities().get(0); + sa.setActivatingPlayer(p); + + Player opponent = game.getPlayers().get(0); + opponent.setTeam(1); + addCard("Sol Ring", opponent); + addCard("Honor of the Pure", opponent); + + AiPlayDecision decision = ((PlayerControllerAi) p.getController()).getAi().canPlaySa(sa); + + AssertJUnit.assertEquals(AiPlayDecision.WillPlay, decision); + AssertJUnit.assertEquals("AI should not choose a Spree mode it cannot pay for", 1, sa.getChosenList().size()); + } + + @Test + public void testSpreeChoosesAffordableExtraMode() { + Game game = initAndCreateGame(); + Player p = game.getPlayers().get(1); + p.setTeam(0); + addCard("Plains", p); + addCards("Wastes", 2, p); + + Card requisitionRaid = addCardToZone("Requisition Raid", p, ZoneType.Hand); + SpellAbility sa = requisitionRaid.getSpellAbilities().get(0); + sa.setActivatingPlayer(p); + + Player opponent = game.getPlayers().get(0); + opponent.setTeam(1); + addCard("Sol Ring", opponent); + addCard("Honor of the Pure", opponent); + + AiPlayDecision decision = ((PlayerControllerAi) p.getController()).getAi().canPlaySa(sa); + + AssertJUnit.assertEquals(AiPlayDecision.WillPlay, decision); + AssertJUnit.assertEquals("AI should choose an extra Spree mode when it is useful and payable", 2, sa.getChosenList().size()); + } } From 80a31848afd80a010677b32bb290ae62b4818eb6 Mon Sep 17 00:00:00 2001 From: Madwand99 Date: Thu, 30 Apr 2026 15:15:51 -0700 Subject: [PATCH 2/2] Remove isModeWorthAdding --- .../main/java/forge/ai/ability/CharmAi.java | 23 ++++--------------- 1 file changed, 5 insertions(+), 18 deletions(-) diff --git a/forge-ai/src/main/java/forge/ai/ability/CharmAi.java b/forge-ai/src/main/java/forge/ai/ability/CharmAi.java index fb823550ea7..af0b6c79f16 100644 --- a/forge-ai/src/main/java/forge/ai/ability/CharmAi.java +++ b/forge-ai/src/main/java/forge/ai/ability/CharmAi.java @@ -100,6 +100,7 @@ private List chooseOptionsAi(SpellAbility sa, List choic List chosen = Lists.newArrayList(); AiController aic = ((PlayerControllerAi) ai.getController()).getAi(); + // TODO the AI doesn't know how to effectively handle repeated choices from CanRepeatModes yet. final int pawprintLimit = sa.hasParam("Pawprint") ? AbilityUtils.calculateAmount(sa.getHostCard(), sa.getParam("Pawprint"), sa) : 0; if (pawprintLimit > 0) { // try to pay for the more expensive subs first @@ -154,10 +155,11 @@ private List chooseOptionsAi(SpellAbility sa, List choic } } if (!isTrigger && chosen.size() < num && min < num) { + // Optional extra modes can be worth adding even when canPlaySa() is too strict for a standalone mode. choices.removeAll(chosen); for (AbilitySub sub : choices) { handleDependentModes(sa, chosen, sub); - if (isModeWorthAdding(aic, ai, sub) && canPayForAdditionalMode(sa, chosen, sub, ai)) { + if (aic.doTrigger(sub, false) && canPayForAdditionalMode(sa, chosen, sub, ai)) { chosen.add(sub); if (chosen.size() == num) { break; @@ -215,22 +217,6 @@ private String getEscalateCost(Card source) { return null; } - private boolean isModeWorthAdding(AiController aic, Player ai, AbilitySub sub) { - sub.setActivatingPlayer(ai); - try { - if (AiPlayDecision.WillPlay == aic.canPlaySa(sub)) { - return true; - } - } catch (RuntimeException ex) { - // Some sub-mode AI assumes full spell timing context; fall back to trigger heuristics. - } - try { - return aic.doTrigger(sub, false); - } catch (RuntimeException ex) { - return false; - } - } - private List chooseTriskaidekaphobia(List choices, final Player ai) { List chosenList = Lists.newArrayList(); if (choices == null || choices.isEmpty()) { return chosenList; } @@ -398,7 +384,8 @@ private boolean shouldPayEntwine(Player payer, SpellAbility chosen, List