Merge branch 'aifix' into 'master'

Fix missing triggers

See merge request core-developers/forge!5876

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

@@ -1871,7 +1871,7 @@ public class ComputerUtilCard {
             return oppCards;
         }
 
-        CardCollection aiCreats = CardLists.filter(ai.getCardsIn(ZoneType.Battlefield), CardPredicates.Presets.CREATURES);
+        CardCollection aiCreats = ai.getCreaturesInPlay();
         if (temporary) {
             // Pump effects that add "CARDNAME can't attack" and similar things. Only do it if something is untapped.
             oppCards = CardLists.filter(oppCards, CardPredicates.Presets.UNTAPPED);

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

@@ -162,21 +162,21 @@ public abstract class SpellAbilityAi {
      */
     protected boolean checkApiLogic(final Player ai, final SpellAbility sa) {
         if (ComputerUtil.preventRunAwayActivations(sa)) {
-            return false;   // prevent infinite loop
+            return false; // prevent infinite loop
         }
         return MyRandom.getRandom().nextFloat() < .8f; // random success
     }
     
     public final boolean doTriggerAI(final Player aiPlayer, final SpellAbility sa, final boolean mandatory) {
+        // this evaluation order is currently intentional as it does more stuff that helps avoiding some crashes
         if (!ComputerUtilCost.canPayCost(sa, aiPlayer) && !mandatory) {
             return false;
         }
 
         // a mandatory SpellAbility with targeting but without candidates,
         // does not need to go any deeper
-        if (sa.usesTargeting() && mandatory && !sa.isTargetNumberValid()
+        if (sa.usesTargeting() && mandatory && !sa.getTargetRestrictions().hasCandidates(sa)) {
-                && !sa.getTargetRestrictions().hasCandidates(sa)) {
+            return sa.isTargetNumberValid();
-            return false;
         }
 
         return doTriggerNoCostWithSubs(aiPlayer, sa, mandatory);

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

@@ -1699,19 +1699,21 @@ public class ChangeZoneAi extends SpellAbilityAi {
                 if (card.isToken()) {
                     return false;
                 }
-
                 if (card.isCreature() && ComputerUtilCard.isUselessCreature(decider, card)) {
                     return true;
-                } else if (card.isEquipped()) {
+                }
+                if (card.isEquipped()) {
                     return false;
-                } else if (card.isEnchanted()) {
+                }
+                if (card.isEnchanted()) {
                     for (Card enc : card.getEnchantedBy()) {
                         if (enc.getOwner().isOpponentOf(decider)) {
                             return true;
                         }
                     }
                     return false;
-                } else if (card.hasCounters()) {
+                }
+                if (card.hasCounters()) {
                     if (card.isPlaneswalker()) {
                         int maxLoyaltyToConsider = 2;
                         int loyaltyDiff = 2;

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

@@ -392,15 +392,19 @@ public class ChangeZoneAllAi extends SpellAbilityAi {
                         PlayerPredicates.isTargetableBy(sa)));
 
                 if (oppList.isEmpty()) {
+                    if (mandatory && !sa.isTargetNumberValid() && sa.canTarget(ai)) {
+                        sa.resetTargets();
+                        sa.getTargets().add(ai);
+                        return true;
+                    }
                     return false;
                 }
 
                 // get the one with the most handsize
-                Player oppTarget = Collections.max(oppList,
+                Player oppTarget = Collections.max(oppList, PlayerPredicates.compareByZoneSize(origin));
-                        PlayerPredicates.compareByZoneSize(origin));
 
                 // set the target
-                if (!oppTarget.getCardsIn(ZoneType.Hand).isEmpty()) {
+                if (!oppTarget.getCardsIn(ZoneType.Hand).isEmpty() || mandatory) {
                     sa.resetTargets();
                     sa.getTargets().add(oppTarget);
                 } else {
@@ -434,7 +438,12 @@ public class ChangeZoneAllAi extends SpellAbilityAi {
                         PlayerPredicates.isTargetableBy(sa)));
 
                 if (oppList.isEmpty()) {
-                    return false;
+                    if (mandatory && !sa.isTargetNumberValid() && sa.canTarget(ai)) {
+                        sa.resetTargets();
+                        sa.getTargets().add(ai);
+                        return true;
+                    }
+                    return sa.isTargetNumberValid();
                 }
 
                 // get the one with the most in graveyard
@@ -443,7 +452,7 @@ public class ChangeZoneAllAi extends SpellAbilityAi {
                         AiPlayerPredicates.compareByZoneValue(sa.getParam("ChangeType"), origin, sa));
 
                 // set the target
-                if (!oppTarget.getCardsIn(ZoneType.Graveyard).isEmpty()) {
+                if (!oppTarget.getCardsIn(ZoneType.Graveyard).isEmpty() || mandatory) {
                     sa.resetTargets();
                     sa.getTargets().add(oppTarget);
                 } else {

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

@@ -108,7 +108,7 @@ public class ChooseTypeAi extends SpellAbilityAi {
 
     @Override
     protected boolean doTriggerAINoCost(Player ai, SpellAbility sa, boolean mandatory) {
-        boolean isCurse = sa.hasParam("IsCurse");
+        boolean isCurse = sa.isCurse();
 
         if (sa.usesTargeting()) {
             final List<Player> oppList = Lists.newArrayList(Iterables.filter(

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

@@ -92,7 +92,7 @@ public class ClashAi extends SpellAbilityAi {
         if ("Creature".equals(valid)) {
             // Springjack Knight
             // TODO: Whirlpool Whelm also uses creature targeting but it's trickier to support
-            CardCollectionView aiCreats = CardLists.filter(ai.getCardsIn(ZoneType.Battlefield), CardPredicates.Presets.CREATURES);
+            CardCollectionView aiCreats = ai.getCreaturesInPlay();
             CardCollectionView oppCreats = CardLists.filter(ai.getOpponents().getCardsIn(ZoneType.Battlefield), CardPredicates.Presets.CREATURES);
 
             Card tgt = aiCreats.isEmpty() ? ComputerUtilCard.getWorstCreatureAI(oppCreats) : ComputerUtilCard.getBestCreatureAI(aiCreats);

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

@@ -76,8 +76,8 @@ public abstract class DamageAiBase extends SpellAbilityAi {
         Card hostcard = sa.getHostCard();
         for (Trigger trig : hostcard.getTriggers()) {
             if (trig.getMode() == TriggerType.DamageDone) {
-                if (("Opponent".equals(trig.getParam("ValidTarget")))
+                if ("Opponent".equals(trig.getParam("ValidTarget"))
-                        && (!"True".equals(trig.getParam("CombatDamage")))) {
+                        && !"True".equals(trig.getParam("CombatDamage"))) {
                     return true;
                 }
             }

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

@@ -448,7 +448,8 @@ public class DamageDealAi extends DamageAiBase {
 
         // We can hurt a planeswalker, so rank the one which is the best target
         if (!hPlay.isEmpty() && pl.isOpponentOf(ai) && activator.equals(ai)) {
-            return ComputerUtilCard.getBestPlaneswalkerToDamage(hPlay);
+            Card pw = ComputerUtilCard.getBestPlaneswalkerToDamage(hPlay);
+            return pw == null && mandatory ? hPlay.get(0) : pw;
         }
 
         return null;
@@ -713,7 +714,7 @@ public class DamageDealAi extends DamageAiBase {
                     }
                 }
 
-                if (freePing && sa.canTarget(enemy) && (!avoidTargetP(ai, sa))) {
+                if (freePing && sa.canTarget(enemy) && !avoidTargetP(ai, sa)) {
                     tcs.add(enemy);
                     if (divided) {
                         sa.addDividedAllocation(enemy, dmg);

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

@@ -371,7 +371,7 @@ public class DestroyAi extends SpellAbilityAi {
                             break;
                         }
                     } else {
-                        break;
+                        return true;
                     }
                 } else {
                     Card c = ComputerUtilCard.getBestAI(preferred);
@@ -380,7 +380,7 @@ public class DestroyAi extends SpellAbilityAi {
                 }
             }
 
-            while (sa.canAddMoreTarget()) {
+            while (!sa.isMinTargetChosen()) {
                 if (list.isEmpty()) {
                     break;
                 } else {

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

@@ -130,8 +130,7 @@ public class DigUntilAi extends SpellAbilityAi {
             if ("OathOfDruids".equals(logic)) {
                 final List<Card> creaturesInLibrary =
                         CardLists.filter(player.getCardsIn(ZoneType.Library), CardPredicates.Presets.CREATURES);
-                final List<Card> creaturesInBattlefield =
+                final List<Card> creaturesInBattlefield = player.getCreaturesInPlay();
-                        CardLists.filter(player.getCardsIn(ZoneType.Battlefield), CardPredicates.Presets.CREATURES);
                 // if there are at least 3 creatures in library,
                 // or none in play with one in library, oath
                 return creaturesInLibrary.size() > 2

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

@@ -66,7 +66,7 @@ public class FightAi extends SpellAbilityAi {
                 }
             }
             if (fighter1List.isEmpty()) {
-                return true; // FIXME: shouldn't this return "false" if nothing found?
+                return false;
             }
             Card fighter1 = fighter1List.get(0);
             for (Card humanCreature : humCreatures) {

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

@@ -122,9 +122,9 @@ public class LifeSetAi extends SpellAbilityAi {
         }
 
         // special cases when amount can't be calculated without targeting first
-        if (amount == 0 && opponent != null && "TargetedPlayer$StartingLife/HalfDown".equals(source.getSVar(amountStr))) {
+        if (amount == 0 && "TargetedPlayer$StartingLife/HalfDown".equals(source.getSVar(amountStr))) {
             // e.g. Torgaar, Famine Incarnate
-            return doHalfStartingLifeLogic(ai, opponent, sa);
+            return doHalfStartingLifeLogic(ai, opponent, sa) || mandatory;
         }
 
         if (sourceName.equals("Eternity Vessel")
@@ -160,9 +160,9 @@ public class LifeSetAi extends SpellAbilityAi {
 
     private boolean doHalfStartingLifeLogic(Player ai, Player opponent, SpellAbility sa) {
         int aiAmount = ai.getStartingLife() / 2;
-        int oppAmount = opponent.getStartingLife() / 2;
+        int oppAmount = opponent == null ? 0 : opponent.getStartingLife() / 2;
         int aiLife = ai.getLife();
-        int oppLife = opponent.getLife();
+        int oppLife = opponent == null ? 0 : opponent.getLife();
 
         sa.resetTargets();
 

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

@@ -15,6 +15,7 @@ import forge.ai.SpellAbilityAi;
 import forge.card.ColorSet;
 import forge.card.MagicColor;
 import forge.card.mana.ManaCost;
+import forge.game.GlobalRuleChange;
 import forge.game.ability.AbilityUtils;
 import forge.game.card.Card;
 import forge.game.card.CardCollection;
@@ -80,7 +81,7 @@ public class ManaEffectAi extends SpellAbilityAi {
         if (logic.startsWith("ManaRitual")) {
              return ph.is(PhaseType.MAIN2, ai) || ph.is(PhaseType.MAIN1, ai);
         } else if ("AtOppEOT".equals(logic)) {
-            return ph.is(PhaseType.END_OF_TURN) && ph.getNextTurn() == ai;
+            return !ai.getGame().getStaticEffects().getGlobalRuleChange(GlobalRuleChange.manaBurn) && ph.is(PhaseType.END_OF_TURN) && ph.getNextTurn() == ai;
         }
         return super.checkPhaseRestrictions(ai, sa, ph, logic);
     }

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

@@ -146,13 +146,13 @@ public class MillAi extends SpellAbilityAi {
 
             // can't target opponent?
             if (list.isEmpty()) {
-                if (mandatory && sa.canTarget(ai)) {
+                if (mandatory && !sa.isTargetNumberValid() && sa.canTarget(ai)) {
                     sa.getTargets().add(ai);
                     return true;
                 }
                 // TODO Obscure case when you know what your top card is so you might?
                 // want to mill yourself here
-                return false;
+                return sa.isTargetNumberValid();
             }
 
             // select Player which would cause the most damage

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

@@ -5,7 +5,6 @@ import java.util.Map;
 
 import com.google.common.base.Predicate;
 import com.google.common.collect.Iterables;
-import com.google.common.collect.Lists;
 
 import forge.ai.AiCardMemory;
 import forge.ai.ComputerUtilCard;
@@ -15,14 +14,12 @@ import forge.game.Game;
 import forge.game.ability.AbilityUtils;
 import forge.game.card.Card;
 import forge.game.card.CardLists;
-import forge.game.card.CardPredicates;
 import forge.game.combat.Combat;
 import forge.game.combat.CombatUtil;
 import forge.game.keyword.Keyword;
 import forge.game.phase.PhaseType;
 import forge.game.player.Player;
 import forge.game.spellability.SpellAbility;
-import forge.game.zone.ZoneType;
 
 public class MustBlockAi extends SpellAbilityAi {
 
@@ -76,7 +73,7 @@ public class MustBlockAi extends SpellAbilityAi {
             return false;
         }
 
-        Card attacker = null;
+        Card attacker = source;
         if (sa.hasParam("DefinedAttacker")) {
             final List<Card> cards = AbilityUtils.getDefinedCards(source, sa.getParam("DefinedAttacker"), sa);
             if (cards.isEmpty()) {
@@ -86,18 +83,12 @@ public class MustBlockAi extends SpellAbilityAi {
             attacker = cards.get(0);
         }
 
-        if (attacker == null) {
-            attacker = source;
-        }
-
-        final Card definedAttacker = attacker;
-
         boolean chance = false;
 
         if (sa.usesTargeting()) {
-            final List<Card> list = determineGoodBlockers(definedAttacker, ai, ai.getWeakestOpponent(), sa, true,true);
+            final List<Card> list = determineGoodBlockers(attacker, ai, ai.getWeakestOpponent(), sa, true, true);
             if (list.isEmpty()) {
-                return false;
+                return sa.isTargetNumberValid();
             }
             final Card blocker = ComputerUtilCard.getBestCreatureAI(list);
             if (blocker == null) {
@@ -160,8 +151,7 @@ public class MustBlockAi extends SpellAbilityAi {
 
     private List<Card> determineGoodBlockers(final Card attacker, final Player ai, Player defender, SpellAbility sa,
             final boolean onlyLethal, final boolean testTapped) {
-        List<Card> list = Lists.newArrayList();
+        List<Card> list = defender.getCreaturesInPlay();
-        list = CardLists.filter(defender.getCardsIn(ZoneType.Battlefield), CardPredicates.Presets.CREATURES);
 
         if (sa.usesTargeting()) {
             list = CardLists.getTargetableCards(list, sa);
@@ -187,7 +177,7 @@ public class MustBlockAi extends SpellAbilityAi {
         List<Card> better = determineBlockerFromList(attacker, ai, options, sa, false, false);
 
         if (!better.isEmpty()) {
-            return Iterables.getFirst(options, null);
+            return Iterables.getFirst(better, null);
         }
 
         return Iterables.getFirst(options, null);

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

@@ -63,7 +63,7 @@ public class PhasesAi extends SpellAbilityAi {
             return true;
         } else if (mandatory) {
             // not enough preferred targets, but mandatory so keep going:
-            return phasesUnpreferredTargeting(aiPlayer.getGame(), sa, mandatory);
+            return sa.isTargetNumberValid() || phasesUnpreferredTargeting(aiPlayer.getGame(), sa, mandatory);
         }
 
         return false;

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

@@ -1,6 +1,5 @@
 package forge.ai.ability;
 
-import java.util.Collections;
 import java.util.List;
 
 import forge.ai.ComputerUtilCard;
@@ -57,13 +56,13 @@ public class SacrificeAi extends SpellAbilityAi {
     private boolean sacrificeTgtAI(final Player ai, final SpellAbility sa, boolean mandatory) {
         final Card source = sa.getHostCard();
         final boolean destroy = sa.hasParam("Destroy");
-        final PlayerCollection targetableOpps = ai.getOpponents().filter(PlayerPredicates.isTargetableBy(sa));
-        final Player opp = Collections.max(targetableOpps, PlayerPredicates.compareByLife());
 
         if (sa.usesTargeting()) {
-            if (opp == null) {
+            final PlayerCollection targetableOpps = ai.getOpponents().filter(PlayerPredicates.isTargetableBy(sa));
+            if (targetableOpps.isEmpty()) {
                 return false;
             }
+            final Player opp = targetableOpps.max(PlayerPredicates.compareByLife());
             sa.resetTargets();
             sa.getTargets().add(opp);
             if (mandatory) {

forge-game/src/main/java/forge/game/ability/effects/ChangeZoneEffect.java

@@ -451,9 +451,10 @@ public class ChangeZoneEffect extends SpellAbilityEffect {
             sb.append(sa.getParam("AlternativeDestinationMessage"));
             Player alterDecider = player;
             if (sa.hasParam("AlternativeDecider")) {
-                alterDecider = AbilityUtils.getDefinedPlayers(hostCard, sa.getParam("AlternativeDecider"), sa).get(0);
+                PlayerCollection deciders = AbilityUtils.getDefinedPlayers(hostCard, sa.getParam("AlternativeDecider"), sa);
+                alterDecider = deciders.isEmpty() ? null : deciders.get(0);
             }
-            if (!alterDecider.getController().confirmAction(sa, PlayerActionConfirmMode.ChangeZoneToAltDestination, sb.toString())) {
+            if (alterDecider != null && !alterDecider.getController().confirmAction(sa, PlayerActionConfirmMode.ChangeZoneToAltDestination, sb.toString())) {
                 destination = ZoneType.smartValueOf(sa.getParam("DestinationAlternative"));
                 altDest = true;
             }

forge-game/src/main/java/forge/game/combat/AttackConstraints.java

@@ -47,7 +47,7 @@ public class AttackConstraints {
 
     public AttackConstraints(final Combat combat) {
         final Game game = combat.getAttackingPlayer().getGame();
-        possibleAttackers = CardLists.filter(combat.getAttackingPlayer().getCardsIn(ZoneType.Battlefield), CardPredicates.Presets.CREATURES);
+        possibleAttackers = combat.getAttackingPlayer().getCreaturesInPlay();
         possibleDefenders = combat.getDefenders();
         globalRestrictions = GlobalAttackRestrictions.getGlobalRestrictions(combat.getAttackingPlayer(), possibleDefenders);
 

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

@@ -3,6 +3,6 @@ ManaCost:2
 Types:Artifact
 A:AB$ Pump | Cost$ T Discard<1/Card> | TargetMin$ 0 | TargetMax$ 1 | ValidTgts$ Creature | TgtPrompt$ Select target creature | KW$ HIDDEN Unblockable | SpellDescription$ Up to one target creature can't be blocked this turn.
 T:Mode$ Untaps | ValidCard$ Card.Self | TriggerZones$ Battlefield | Execute$ TrigDraw | TriggerDescription$ Whenever CARDNAME becomes untapped, you may pay {2}. If you do, draw a card.
-SVar:TrigDraw:AB$Draw | Cost$ 2 | Defined$ You | NumCards$ 1
+SVar:TrigDraw:AB$ Draw | Cost$ 2 | Defined$ You | NumCards$ 1
 SVar:Picture:http://www.wizards.com/global/images/magic/general/key_to_the_city.jpg
 Oracle:{T}, Discard a card: Up to one target creature can't be blocked this turn.\nWhenever Key to the City becomes untapped, you may pay {2}. If you do, draw a card.

forge-gui/res/cardsfolder/s/subtlety.txt

@@ -5,6 +5,6 @@ PT:3/3
 K:Flash
 K:Flying
 T:Mode$ ChangesZone | Origin$ Any | Destination$ Battlefield | ValidCard$ Card.Self | Execute$ TrigTuck | TriggerDescription$ When CARDNAME enters the battlefield, choose up to one target creature spell or planeswalker spell. Its owner puts it on the top or bottom of their library.
-SVar:TrigTuck:DB$ ChangeZone | ValidTgts$ Card.inZoneStack+Creature,Card.inZoneStack+Planeswalker | TgtZone$ Stack | TgtPrompt$ Select up to one target creature spell or planeswalker spell | AlternativeDecider$ TargetedController | Origin$ Stack | Fizzle$ True | Destination$ Library | LibraryPosition$ 0 | DestinationAlternative$ Library | LibraryPositionAlternative$ -1 | AlternativeDestinationMessage$ Would you like to put the card on the top of your library (and not on the bottom)? | SpellDescription$ Choose up to one target creature spell or planeswalker spell. Its owner puts it on the top or bottom of their library.
+SVar:TrigTuck:DB$ ChangeZone | ValidTgts$ Card.inZoneStack+Creature,Card.inZoneStack+Planeswalker | TgtZone$ Stack | TargetMin$ 0 | TargetMax$ 1 | TgtPrompt$ Select up to one target creature spell or planeswalker spell | AlternativeDecider$ TargetedController | Origin$ Stack | Fizzle$ True | Destination$ Library | LibraryPosition$ 0 | DestinationAlternative$ Library | LibraryPositionAlternative$ -1 | AlternativeDestinationMessage$ Would you like to put the card on the top of your library (and not on the bottom)? | SpellDescription$ Choose up to one target creature spell or planeswalker spell. Its owner puts it on the top or bottom of their library.
 K:Evoke:ExileFromHand<1/Card.Blue+Other/blue card>
 Oracle:Flash\nFlying\nWhen Subtlety enters the battlefield, choose up to one target creature spell or planeswalker spell. Its owner puts it on the top or bottom of their library.\nEvoke—Exile a blue card from your hand.