Merge pull request #2898 from tool4ever/cleanup_12

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;
+        // About to lose game : allow
-        boolean dangerousRecurringCost = false;
+        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);
+        costWithBuyback = CostAdjustment.adjust(costWithBuyback, sa);
-        if (costWithBuyback.getCostMana() != null) {
-            neededMana = costWithBuyback.getCostMana().getMana().getCMC();
-        }
         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
+        int neededMana = 0;
-        // if Buyback cost includes sacrifice, life, discard
+        if (costWithBuyback.getCostMana() != null) {
-        if (dangerousRecurringCost) {
+            neededMana = costWithBuyback.getCostMana().getMana().getCMC();
-            wasteBuybackAllowed = true;
         }
-
         // 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())
+                if (ApiType.DealDamage.equals(sa2.getApi()) && sa2.usesTargeting() && sa2.getTargetRestrictions().canTgtPlayer()) {
-                        && (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"),
+            final PlayerCollection players = AbilityUtils.getDefinedPlayers(sa.getHostCard(), sa.getParam("Defined"), sa);
-                    sa);
             if (players.isEmpty()) {
                 return false;
             }

forge-game/src/main/java/forge/game/cost/CostAdjustment.java

@@ -553,25 +553,6 @@ public class CostAdjustment {
                 return false;
             }
         }
-        if (st.hasParam("ValidSpellTarget")) {
-            SpellAbility curSa = sa;
-            boolean targetValid = false;
-            outer: while (curSa != null) {
-                if (!curSa.usesTargeting()) {
-                    curSa = curSa.getSubAbility();
-                    continue;
-                }
-                for (SpellAbility target : curSa.getTargets().getTargetSpells()) {
-                    Card targetCard = target.getHostCard();
-                    if (targetCard.isValid(st.getParam("ValidSpellTarget").split(","), controller, hostCard, curSa)) {
-                        targetValid = true;
-                        break outer;
-                    }
-                }
-                curSa = curSa.getSubAbility();
-            }
-            return targetValid;
-        }
         return true;
     }
 }

forge-game/src/main/java/forge/game/trigger/TriggerHandler.java

@@ -207,7 +207,6 @@ public class TriggerHandler {
     public final void resetActiveTriggers() {
         resetActiveTriggers(true);
     }
-
     public final void resetActiveTriggers(boolean collect) {
         if (collect) {
             collectTriggerForWaiting();
@@ -281,11 +280,11 @@ public class TriggerHandler {
     }
 
     public final boolean runWaitingTriggers() {
-        final List<TriggerWaiting> waiting = new ArrayList<>(waitingTriggers);
+        if (waitingTriggers.isEmpty()) {
-        waitingTriggers.clear();
-        if (waiting.isEmpty()) {
             return false;
         }
+        final List<TriggerWaiting> waiting = new ArrayList<>(waitingTriggers);
+        waitingTriggers.clear();
 
         boolean haveWaiting = false;
         for (final TriggerWaiting wt : waiting) {

forge-gui/res/cardsfolder/k/kaerveks_torch.txt

@@ -2,6 +2,6 @@ Name:Kaervek's Torch
 ManaCost:X R
 Types:Sorcery
 A:SP$ DealDamage | Cost$ X R | ValidTgts$ Creature,Player,Planeswalker | TgtPrompt$ Select any target | NumDmg$ X | SpellDescription$ CARDNAME deals X damage to any target.
-S:Mode$ RaiseCost | ValidSpellTarget$ Card.Self | Activator$ Player | Type$ Spell | Amount$ 2 | EffectZone$ Stack | Description$ As long as CARDNAME is on the stack, spells that target it cost {2} more to cast.
+S:Mode$ RaiseCost | ValidTarget$ Spell.Self | Activator$ Player | Type$ Spell | Amount$ 2 | EffectZone$ Stack | Description$ As long as CARDNAME is on the stack, spells that target it cost {2} more to cast.
 SVar:X:Count$xPaid
 Oracle:As long as Kaervek's Torch is on the stack, spells that target it cost {2} more to cast.\nKaervek's Torch deals X damage to any target.

forge-gui/res/cardsfolder/r/rousing_refrain.txt

@@ -2,7 +2,7 @@ Name:Rousing Refrain
 ManaCost:3 R R
 Types:Sorcery
 K:Suspend:3:1 R
-A:SP$ Mana | Cost$ 3 R R | ValidTgts$ Opponent | TgtPrompt$ Select target opponent | SubAbility$ DBMana | AILogic$ ManaRitual | Produced$ R | Amount$ Z | PersistentMana$ True | Defined$ You | SubAbility$ DBChange | StackDescription$ SpellDescription | SpellDescription$ Until end of turn, you don't lose this mana as steps and phases end.		
+A:SP$ Mana | Cost$ 3 R R | ValidTgts$ Opponent | TgtPrompt$ Select target opponent | SubAbility$ DBMana | AILogic$ ManaRitual | Produced$ R | Amount$ Z | PersistentMana$ True | Defined$ You | SubAbility$ DBChange | StackDescription$ SpellDescription | SpellDescription$ Until end of turn, you don't lose this mana as steps and phases end.
 SVar:Z:TargetedPlayer$CardsInHand
 SVar:DBChange:DB$ ChangeZone | Origin$ Stack | Destination$ Exile | WithCountersType$ TIME | WithCountersAmount$ 3 | SpellDescription$ Exile CARDNAME with three time counters on it.
 Oracle:Add {R} for each card in target opponent's hand. Until end of turn, you don't lose this mana as steps and phases end. Exile Rousing Refrain with three time counters on it.\nSuspend 3—{1}{R} (Rather than cast this card from your hand, you may pay {1}{R} and exile it with three time counters on it. At the beginning of your upkeep, remove a time counter. When the last is removed, cast it without paying its mana cost.)

forge-gui/res/cardsfolder/t/the_royal_scions.txt

@@ -6,8 +6,9 @@ A:AB$ Draw | Cost$ AddCounter<1/LOYALTY> | Planeswalker$ True | NumCards$ 1 | Sp
 SVar:DBDiscard:DB$ Discard | Defined$ You | NumCards$ 1 | Mode$ TgtChoose
 AI:RemoveDeck:All
 A:AB$ Pump | Cost$ AddCounter<1/LOYALTY> | Planeswalker$ True | ValidTgts$ Creature | TgtPrompt$ Select target creature | NumAtt$ +2 | KW$ First Strike & Trample | SpellDescription$ Target creature gets +2/+0 and gains first strike and trample until end of turn.
-A:AB$ Draw | Cost$ SubCounter<8/LOYALTY> | Planeswalker$ True | Ultimate$ True | NumCards$ 4 | SubAbility$ DBTrigger | SpellDescription$ Draw four cards. When you do, CARDNAME deals damage to any target equal to the number of cards in your hand.
+A:AB$ Draw | Cost$ SubCounter<8/LOYALTY> | Planeswalker$ True | Ultimate$ True | NumCards$ 4 | RememberDrawn$ True | SubAbility$ DBTrigger | SpellDescription$ Draw four cards. When you do, CARDNAME deals damage to any target equal to the number of cards in your hand.
-SVar:DBTrigger:DB$ ImmediateTrigger | Execute$ DBDamage | TriggerDescription$ When you do, CARDNAME deals damage to any target equal to the number of cards in your hand.
+SVar:DBTrigger:DB$ ImmediateTrigger | ConditionDefined$ Remembered | ConditionPresent$ Card | ConditionCompare$ GE4 | Execute$ DBDamage | TriggerDescription$ When you do, CARDNAME deals damage to any target equal to the number of cards in your hand. | SubAbility$ DBCleanup
 SVar:DBDamage:DB$ DealDamage | ValidTgts$ Creature,Player,Planeswalker | TgtPrompt$ Select any target | NumDmg$ X
+SVar:DBCleanup:DB$ Cleanup | ClearRemembered$ True
 SVar:X:Count$InYourHand
 Oracle:[+1]: Draw a card, then discard a card.\n[+1]: Target creature gets +2/+0 and gains first strike and trample until end of turn.\n[-8]: Draw four cards. When you do, The Royal Scions deals damage to any target equal to the number of cards in your hand.