Cleanup & Fixes

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

@@ -969,21 +969,16 @@ public class AiController {
     }
 
     private boolean canPlaySpellWithoutBuyback(Card card, SpellAbility sa) {
-        boolean wasteBuybackAllowed = false;
-
-        // About to lose game : allow
-        if (ComputerUtil.aiLifeInDanger(player, true, 0)) {
-            wasteBuybackAllowed = true;
-        }
-
         int copies = CardLists.count(player.getCardsIn(ZoneType.Hand), CardPredicates.nameEquals(card.getName()));
         // Have two copies : allow
         if (copies >= 2) {
-            wasteBuybackAllowed = true;
+            return true;
         }
 
-        int neededMana = 0;
-        boolean dangerousRecurringCost = false;
+        // About to lose game : allow
+        if (ComputerUtil.aiLifeInDanger(player, true, 0)) {
+            return true;
+        }
 
         Cost costWithBuyback = sa.getPayCosts().copy();
         for (OptionalCostValue opt : GameActionUtil.getOptionalCostValues(sa)) {
@@ -991,22 +986,19 @@ public class AiController {
                 costWithBuyback.add(opt.getCost());
             }
         }
-        CostAdjustment.adjust(costWithBuyback, sa);
-        if (costWithBuyback.getCostMana() != null) {
-            neededMana = costWithBuyback.getCostMana().getMana().getCMC();
-        }
+        costWithBuyback = CostAdjustment.adjust(costWithBuyback, sa);
         if (costWithBuyback.hasSpecificCostType(CostPayLife.class)
                 || costWithBuyback.hasSpecificCostType(CostDiscard.class)
                 || costWithBuyback.hasSpecificCostType(CostSacrifice.class)) {
-            dangerousRecurringCost = true;
+            // won't be able to afford buyback any time soon
+            // if Buyback cost includes sacrifice, life, discard
+            return true;
         }
 
-        // won't be able to afford buyback any time soon
-        // if Buyback cost includes sacrifice, life, discard
-        if (dangerousRecurringCost) {
-            wasteBuybackAllowed = true;
+        int neededMana = 0;
+        if (costWithBuyback.getCostMana() != null) {
+            neededMana = costWithBuyback.getCostMana().getMana().getCMC();
         }
-
         // Memory Crystal-like effects need special handling
         for (Card c : game.getCardsIn(ZoneType.Battlefield)) {
             for (StaticAbility s : c.getStaticAbilities()) {
@@ -1022,10 +1014,10 @@ public class AiController {
 
         int hasMana = ComputerUtilMana.getAvailableManaEstimate(player, false);
         if (hasMana < neededMana - 1) {
-            wasteBuybackAllowed = true;
+            return true;
         }
 
-        return wasteBuybackAllowed;
+        return false;
     }
 
     // not sure "playing biggest spell" matters?

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

@@ -1871,10 +1871,9 @@ public class ComputerUtilCard {
 
     public static int getMaxSAEnergyCostOnBattlefield(final Player ai) {
         // returns the maximum energy cost of an ability that permanents on the battlefield under AI's control have
-        CardCollectionView otb = ai.getCardsIn(ZoneType.Battlefield);
         int maxEnergyCost = 0;
 
-        for (Card c : otb) {
+        for (Card c : ai.getCardsIn(ZoneType.Battlefield)) {
             for (SpellAbility sa : c.getSpellAbilities()) {
                 CostPayEnergy energyCost = sa.getPayCosts().getCostEnergy();
                 if (energyCost != null) {

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

@@ -669,8 +669,7 @@ public class AttachAi extends SpellAbilityAi {
             // Prefer "tap to deal damage"
             // TODO : Skip this one if triggers on combat damage only?
             for (SpellAbility sa2 : card.getSpellAbilities()) {
-                if (ApiType.DealDamage.equals(sa2.getApi())
-                        && (sa2.getTargetRestrictions().canTgtPlayer())) {
+                if (ApiType.DealDamage.equals(sa2.getApi()) && sa2.usesTargeting() && sa2.getTargetRestrictions().canTgtPlayer()) {
                     cardPriority += 300;
                 }
             }

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

@@ -71,6 +71,9 @@ public class CopySpellAbilityAi extends SpellAbilityAi {
             } else if (top.getApi() == ApiType.CopySpellAbility) {
                 // Don't try to copy a copy ability, too complex for the AI to handle
                 return false;
+            } else if (top.getApi() == ApiType.Mana) {
+                // would lead to Stack Overflow by trying to play this again
+                return false;
             } else if (top.getApi() == ApiType.DestroyAll || top.getApi() == ApiType.SacrificeAll || top.getApi() == ApiType.ChangeZoneAll || top.getApi() == ApiType.TapAll || top.getApi() == ApiType.UnattachAll) {
                 if (!top.usesTargeting() || top.getActivatingPlayer().equals(aiPlayer)) {
                     // If we activated a mass removal / mass tap / mass bounce / etc. spell, or if the opponent activated it but

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

@@ -64,8 +64,7 @@ public class PoisonAi extends SpellAbilityAi {
             return true;
         } else {
             // currently there are no optional Trigger
-            final PlayerCollection players = AbilityUtils.getDefinedPlayers(sa.getHostCard(), sa.getParam("Defined"),
-                    sa);
+            final PlayerCollection players = AbilityUtils.getDefinedPlayers(sa.getHostCard(), sa.getParam("Defined"), sa);
             if (players.isEmpty()) {
                 return false;
             }