Some cleanup

forge-ai/src/main/java/forge/ai/AiAbilityDecision.java

@@ -1,9 +1,46 @@
 package forge.ai;
 
-public record AiAbilityDecision(int rating, AiPlayDecision decision) {
+import forge.game.card.CardUtil;
+import forge.game.phase.PhaseHandler;
+import forge.game.phase.PhaseType;
+import forge.game.player.Player;
+import forge.game.spellability.SpellAbility;
+
+public record AiAbilityDecision(int rating, AiPlayDecision decision, SpellAbility sa) {
     private static int MIN_RATING = 30;
 
+    public AiAbilityDecision(int rating, AiPlayDecision decision) {
+        this(rating, decision, null);
+    }
+
     public boolean willingToPlay() {
-        return rating > MIN_RATING && decision.willingToPlay();
+        if (!decision.willingToPlay()) {
+            return false;
+        }
+        if (rating > MIN_RATING) {
+            return true;
+        }
+        if (sa == null) {
+            return false;
+        }
+        // passive turns don't win games
+        int boosted = rating;
+        Player ai = sa.getActivatingPlayer();
+        // TODO turn into proactive AI profile preference
+        int actionsThisTurn = CardUtil.getThisTurnActivated("Ability.YouCtrl", sa.getHostCard(), sa, ai).size()
+                + CardUtil.getThisTurnCast("Spell.YouCtrl", sa.getHostCard(), sa, ai).size();
+        if (actionsThisTurn == 0) {
+            PhaseHandler ph = sa.getHostCard().getGame().getPhaseHandler();
+            if (ph.getNextTurn() == ai) {
+                // try not to waste open mana
+                boosted += 10;
+            } else if (ph.getPhase() == PhaseType.MAIN2 && SpellAbilityAi.isSorcerySpeed(sa, ai)) {
+                boosted = 2;
+            }
+            if (ph.getPhase() == PhaseType.END_OF_TURN) {
+                boosted = 5;
+            }
+        }
+        return boosted > MIN_RATING;
     }
 }

forge-ai/src/main/java/forge/ai/AiController.java

@@ -188,29 +188,27 @@ private boolean checkCurseEffects(final SpellAbility sa) {
         CardCollectionView ccvGameBattlefield = CardLists.filter(game.getCardsIn(ZoneType.Battlefield), CardPredicates.hasSVar("AICurseEffect"));
         for (final Card c : ccvGameBattlefield) {
             final String curse = c.getSVar("AICurseEffect");
+            final Card host = sa.getHostCard();
             if ("NonActive".equals(curse) && !player.equals(game.getPhaseHandler().getPlayerTurn())) {
                 return true;
-            } else {
-                final Card host = sa.getHostCard();
-                if ("DestroyCreature".equals(curse) && sa.isSpell() && host.isCreature()
-                        && !host.hasKeyword(Keyword.INDESTRUCTIBLE)) {
-                    return true;
-                } else if ("CounterEnchantment".equals(curse) && sa.isSpell() && host.isEnchantment() && sa.isCounterableBy(null)) {
-                    return true;
-                } else if ("ChaliceOfTheVoid".equals(curse) && sa.isSpell() && sa.isCounterableBy(null)
-                        && host.getCMC() == c.getCounters(CounterEnumType.CHARGE)) {
-                    return true;
-                } else if ("BazaarOfWonders".equals(curse) && sa.isSpell() && sa.isCounterableBy(null)) {
-                    String hostName = host.getName();
-                    for (Card card : ccvGameBattlefield) {
-                        if (!card.isToken() && card.sharesNameWith(host)) {
-                            return true;
-                        }
-                    }
-                    if (game.getCardsIn(ZoneType.Graveyard).anyMatch(CardPredicates.nameEquals(hostName))) {
+            } else if ("DestroyCreature".equals(curse) && sa.isSpell() && host.isCreature()
+                    && !host.hasKeyword(Keyword.INDESTRUCTIBLE)) {
+                return true;
+            } else if ("CounterEnchantment".equals(curse) && sa.isSpell() && host.isEnchantment() && sa.isCounterableBy(null)) {
+                return true;
+            } else if ("ChaliceOfTheVoid".equals(curse) && sa.isSpell() && sa.isCounterableBy(null)
+                    && host.getCMC() == c.getCounters(CounterEnumType.CHARGE)) {
+                return true;
+            } else if ("BazaarOfWonders".equals(curse) && sa.isSpell() && sa.isCounterableBy(null)) {
+                String hostName = host.getName();
+                for (Card card : ccvGameBattlefield) {
+                    if (!card.isToken() && card.sharesNameWith(host)) {
                         return true;
                     }
                 }
+                if (game.getCardsIn(ZoneType.Graveyard).anyMatch(CardPredicates.nameEquals(hostName))) {
+                    return true;
+                }
             }
         }
         return false;
@@ -467,7 +465,7 @@ private CardCollection filterLandsToPlay(CardCollection landList) {
                 }
             }
             return c.getAllPossibleAbilities(player, true).stream().anyMatch(
-                    la -> la.isLandAbility() && canPlaySpellOrLandBasic(c, la) == AiPlayDecision.WillPlay
+                    la -> la.isLandAbility() && saSideEffects(c, la).willingToPlay()
             );
         });
         return landList;
@@ -850,7 +848,7 @@ private AiPlayDecision canPlayAndPayForFace(final SpellAbility sa) {
         final Card host = sa.getHostCard();
 
         if (sa.hasParam("AICheckSVar") && !aiShouldRun(sa, sa, host, null)) {
-            return AiPlayDecision.AnotherTime;
+            return AiPlayDecision.NeedsToPlayCriteriaNotMet;
         }
 
         // this is the "heaviest" check, which also sets up targets, defines X, etc.
@@ -885,14 +883,14 @@ public AiPlayDecision canPlaySa(SpellAbility sa) {
         }
 
         if (!sa.canCastTiming(player)) {
-            return AiPlayDecision.AnotherTime;
+            return AiPlayDecision.TimingRestrictions;
         }
 
         final Card card = sa.getHostCard();
 
         // Trying to play a card that has Buyback without a Buyback cost, look for possible additional considerations
         if (getBoolProperty(AiProps.TRY_TO_PRESERVE_BUYBACK_SPELLS) && card.hasKeyword(Keyword.BUYBACK)
-                && !sa.isBuyback() && !canPlaySpellWithoutBuyback(card, sa)) {
+                && !sa.isBuyback() && !canPlaySpellWithoutBuyback(sa)) {
             return AiPlayDecision.NeedsToPlayCriteriaNotMet;
         }
 
@@ -941,9 +939,6 @@ public AiPlayDecision canPlaySa(SpellAbility sa) {
                 return AiPlayDecision.WaitForMain2;
             }
         }
-        if (checkCurseEffects(sa)) {
-            return AiPlayDecision.CurseEffects;
-        }
         // TODO maybe other location for this?
         if (!sa.isLegalAfterStack()) {
             return AiPlayDecision.AnotherTime;
@@ -966,16 +961,25 @@ public AiPlayDecision canPlaySa(SpellAbility sa) {
                 return AiPlayDecision.TargetingFailed;
             }
         }
-        if (sa.isSpell()) {
-            return canPlaySpellOrLandBasic(card, sa);
-        }
 
-        return AiPlayDecision.WillPlay;
+        return saSideEffects(spellHost, sa);
     }
 
-    private AiPlayDecision canPlaySpellOrLandBasic(final Card card, final SpellAbility sa) {
+    private AiPlayDecision saSideEffects(final Card card, final SpellAbility sa) {
+        if (usesHybridSimulation()) {
+            return OnePlaySafetyChecker.isAcceptable(player, sa) ? AiPlayDecision.WillPlay : AiPlayDecision.CurseEffects;
+        }
+
+        if (!sa.isSpell() || usesFullSimulation()) {
+            return AiPlayDecision.WillPlay;
+        }
+
         if ("True".equals(card.getSVar("NonStackingEffect")) && ComputerUtilCard.isNonDisabledCardInPlay(player, card.getName())) {
-            return AiPlayDecision.NeedsToPlayCriteriaNotMet;
+            return AiPlayDecision.DoesntImpactGame;
+        }
+
+        if (checkCurseEffects(sa)) {
+            return AiPlayDecision.CurseEffects;
         }
 
         int damage = 0;
@@ -996,12 +1000,11 @@ private AiPlayDecision canPlaySpellOrLandBasic(final Card card, final SpellAbili
             }
         }
 
-        // add any other necessary logic to play a basic spell here
         return ComputerUtilCard.checkNeedsToPlayReqs(card, sa);
     }
 
-    private boolean canPlaySpellWithoutBuyback(Card card, SpellAbility sa) {
-        int copies = CardLists.count(player.getCardsIn(ZoneType.Hand), CardPredicates.nameEquals(card.getName()));
+    private boolean canPlaySpellWithoutBuyback(SpellAbility sa) {
+        int copies = CardLists.count(player.getCardsIn(ZoneType.Hand), CardPredicates.nameEquals(sa.getHostCard().getName()));
         // Have two copies : allow
         if (copies >= 2) {
             return true;
@@ -1282,23 +1285,18 @@ public AiPlayDecision canPlayFromEffectAI(Spell spell, boolean mandatory, boolea
             if (!chance) {
                 return AiPlayDecision.TargetingFailed;
             }
-
-            if (mandatory) {
-                return AiPlayDecision.WillPlay;
-            }
         }
 
-        AiPlayDecision basicDecision = canPlaySpellOrLandBasic(spell.getHostCard(), spell);
-        if (basicDecision != AiPlayDecision.WillPlay || mandatory) {
-            return basicDecision;
+        if (mandatory) {
+            return AiPlayDecision.WillPlay;
         }
 
         SpellAbility abilityToCheck = spell;
-        if (withoutPayingManaCost && !spell.hasParam("WithoutManaCost")) {
+        if (usesHybridSimulation() && withoutPayingManaCost && !spell.hasParam("WithoutManaCost")) {
             abilityToCheck = spell.copyWithNoManaCost(player);
         }
-        return isChosenPlayAcceptable(abilityToCheck)
-                ? AiPlayDecision.WillPlay : AiPlayDecision.CurseEffects;
+
+        return saSideEffects(spell.getHostCard(), abilityToCheck);
     }
 
     // declares blockers for given defender in a given combat
@@ -1356,13 +1354,6 @@ private List<SpellAbility> singleSpellAbilityList(SpellAbility sa) {
         return Lists.newArrayList(sa);
     }
 
-    private boolean isChosenPlayAcceptable(SpellAbility ability) {
-        if (usesFullSimulation() || !usesHybridSimulation()) {
-            return true;
-        }
-        return OnePlaySafetyChecker.isAcceptable(player, ability);
-    }
-
     public List<SpellAbility> chooseSpellAbilityToPlay() {
         AiCache.clear();
         // Reset cached predicted combat, as it may be stale. It will be
@@ -1403,9 +1394,7 @@ public List<SpellAbility> chooseSpellAbilityToPlay() {
 
                     if (!abilities.isEmpty()) {
                         // TODO extend this logic to evaluate MDFC with both sides land
-                        if (isChosenPlayAcceptable(abilities.get(0))) {
-                            return abilities;
-                        }
+                        return abilities;
                     }
                 }
             }
@@ -1690,10 +1679,7 @@ else if (!sa.getHostCard().isPermanent() && sa.canCastTiming(player)
                 // PhaseHandler ph = game.getPhaseHandler();
                 // System.out.printf("Ai thinks '%s' of %s -> %s @ %s %s >>> \n", opinion, sa.getHostCard(), sa, Lang.getInstance().getPossesive(ph.getPlayerTurn().getName()), ph.getPhase());
 
-                if (opinion != AiPlayDecision.WillPlay)
-                    continue;
-
-                if (!isChosenPlayAcceptable(sa)) {
+                if (opinion != AiPlayDecision.WillPlay) {
                     continue;
                 }
 
@@ -1820,8 +1806,7 @@ public final boolean aiShouldRun(final CardTraitBase effect, final SpellAbility
                 }
             }
 
-            int left = 0;
-
+            int left;
             if (sa == null) {
                 left = AbilityUtils.calculateAmount(host, svarToCheck, effect);
             } else {

forge-ai/src/main/java/forge/ai/AiPlayDecision.java

@@ -36,8 +36,8 @@ public enum AiPlayDecision {
     TargetingFailed,
     CostNotAcceptable,
     LifeInDanger,
-    WouldDestroyLegend,
     WouldBecomeZeroToughnessCreature,
+    WouldDestroyLegend,
     WouldDestroyWorldEnchantment,
     BadEtbEffects,
     CurseEffects;

forge-ai/src/main/java/forge/ai/SpellAbilityAi.java

@@ -112,7 +112,7 @@ private AiAbilityDecision canPlayWithoutRestrict(final Player ai, final SpellAbi
         if (!checkConditions(ai, sa)) {
             SpellAbility sub = sa.getSubAbility();
             if (sub == null || !checkConditions(ai, sub)) {
-                return new AiAbilityDecision(0, AiPlayDecision.NeedsToPlayCriteriaNotMet);
+                return new AiAbilityDecision(0, AiPlayDecision.ConditionsNotMet);
             }
         }
         return decision;

forge-ai/src/main/java/forge/ai/ability/ProtectAi.java

@@ -19,20 +19,18 @@
 import forge.game.spellability.TargetRestrictions;
 import forge.util.MyRandom;
 
-import java.util.ArrayList;
 import java.util.List;
 
 public class ProtectAi extends SpellAbilityAi {
     private static boolean hasProtectionFrom(final Card card, final String color) {
-        final List<String> onlyColors = new ArrayList<>(MagicColor.Constant.ONLY_COLORS);
+        final List<String> onlyColors = MagicColor.Constant.ONLY_COLORS;
 
         // make sure we have a valid color
         if (!onlyColors.contains(color)) {
             return false;
         }
 
         final String protection = "Protection from " + color;
-
         return card.hasKeyword(protection);
     }
 

forge-ai/src/main/java/forge/ai/simulation/OnePlaySafetyChecker.java

@@ -14,28 +14,20 @@ public static boolean isAcceptable(Player player, SpellAbility sa) {
         // during that resolution incrementally; priority responses need two full stack-resolution
         // branches and are not supported yet.
         if (sa == null || CHECKING.get()
-                || (!player.getGame().getStack().isEmpty()
-                && !player.getGame().getStack().isResolving())) {
+                || (!player.getGame().getStack().isEmpty() && !player.getGame().getStack().isResolving())) {
             return true;
         }
 
         CHECKING.set(true);
         try {
-            Score originalScore = new GameStateEvaluator().getScoreForGameState(player.getGame(), player);
-            SimulationController controller = new SimulationController(originalScore, 0);
+            SimulationController controller = new SimulationController(new Score(0), 0);
             GameSimulator simulator = new GameSimulator(controller, player.getGame(), player, null);
-            // TODO this doesn't respect heuristics shaping for the SA yet (targets etc.)
+            Score originalScore = simulator.getScoreForOrigGame();
             Score resultScore = simulator.simulateSpellAbility(sa);
-            Player simulatedPlayer = (Player) simulator.getGameCopier().find(player);
 
-            if (simulatedPlayer == null) {
-                return true;
-            }
-            if (simulatedPlayer.hasLost()) {
-                return false;
-            }
+            // desperate plays are ok if next combat was already likely to kill AI
             return resultScore.value == Integer.MIN_VALUE
-                    || (long) resultScore.value >= (long) originalScore.value - expectedCardScoreLoss(player, sa, simulator);
+                    || resultScore.value >= (long) originalScore.value - expectedCardScoreLoss(player, sa, simulator);
         } finally {
             CHECKING.remove();
         }

forge-core/src/main/java/forge/card/mana/ManaAtom.java

@@ -89,7 +89,7 @@ public static int getIndexOfFirstManaType(final byte color){
         return -1; // somehow the mana is not colored or colorless?
     }
 
-    public static int getIndexFromName(final String s){
+    public static int getIndexFromName(final String s) {
         return getIndexOfFirstManaType(fromName(s));
     }
 }

forge-game/src/main/java/forge/game/card/Card.java

@@ -3336,12 +3336,14 @@ private String formatSpellAbility(final SpellAbility sa) {
 
     public final boolean canProduceColorMana(final Set<String> colors) {
         for (final SpellAbility mana : getManaAbilities()) {
+            if (mana.getApi() == ApiType.ManaReflected) {
+                if (!Collections.disjoint(CardUtil.getReflectableManaColors(mana), colors)) {
+                    return true;
+                }
+                continue;
+            }
             for (String s : colors) {
-                if (mana.getApi() == ApiType.ManaReflected) {
-                    if (CardUtil.getReflectableManaColors(mana).contains(s)) {
-                        return true;
-                    }
-                } else if (mana.canProduce(MagicColor.toShortString(s))) {
+                if (mana.canProduce(MagicColor.toShortString(s))) {
                     return true;
                 }
             }

forge-game/src/main/java/forge/game/card/CardFactoryUtil.java

@@ -131,7 +131,7 @@ public static SpellAbility abilityTurnFaceUp(final CardState cardState, final Co
         if (!cost.isOnlyManaCost()) {
             sbCost.append(" — ");
         }
-        sbCost.append(cost.toString());
+        sbCost.append(cost);
 
         // Cost need to be set later
         StringBuilder sb = new StringBuilder();

forge-gui-mobile/src/forge/assets/FSkin.java

@@ -110,8 +110,7 @@ private static void checkThemeDir(FileHandle themeDir, String themeName) {
     }
     private static void useFallbackDir() {
         // iOS and Android both need to use internal() for bundled resources
-        boolean isMobile = GuiBase.isMobile();
-        preferredDir = isMobile ? Gdx.files.internal("fallback_skin") : Gdx.files.classpath("fallback_skin");
+        preferredDir = GuiBase.isMobile() ? Gdx.files.internal("fallback_skin") : Gdx.files.classpath("fallback_skin");
     }
     public static void loadLight(String skinName, final SplashScreen splashScreen,FileHandle prefDir) {
         preferredDir = prefDir;

forge-gui/res/cardsfolder/upcoming/moment_of_glory.txt

@@ -2,6 +2,6 @@ Name:Moment of Glory
 ManaCost:W
 Types:Sorcery
 A:SP$ PutCounter | ValidTgts$ Creature.YouCtrl | ValidTgtsDesc$ creature you control | CounterType$ P1P1 | CounterNum$ 1 | SubAbility$ DBPutCounterAll | SpellDescription$ Put a +1/+1 counter on target creature you control. If this spell was cast from a graveyard, also put a +1/+1 counter on each other creature you control.
-SVar:DBPutCounterAll:DB$ PutCounterAll | ValidCards$ Targeted.Other+YouCtrl+Creature | CounterType$ P1P1 | CounterNum$ 1 | ConditionDefined$ Self | ConditionPresent$ Card.wasCastFromGraveyard | ConditionCompare$ EQ1
+SVar:DBPutCounterAll:DB$ PutCounterAll | ValidCards$ Creature.YouCtrl+!targetedBy | CounterType$ P1P1 | CounterNum$ 1 | ConditionDefined$ Self | ConditionPresent$ Card.wasCastFromGraveyard | ConditionCompare$ EQ1
 K:Flashback:4 W
 Oracle:Put a +1/+1 counter on target creature you control. If this spell was cast from a graveyard, also put a +1/+1 counter on each other creature you control.\nFlashback {4}{W} (You may cast this card from your graveyard for its flashback cost. Then exile it.)