Merge branch 'trig' into 'master'

makeChoices for Trigger: move to stack entry

See merge request core-developers/forge!5028

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

@@ -510,7 +510,6 @@ public class AiController {
             landList = unreflectedLands;
         }
 
-
         //try to skip lands that enter the battlefield tapped
         if (!nonLandsInHand.isEmpty()) {
             CardCollection nonTappedLands = new CardCollection();
@@ -534,6 +533,7 @@ public class AiController {
                     }
                 }
 
+                // TODO if this is the only source for a color we need badly prioritize it instead
                 if (foundTapped) {
                     continue;
                 }
@@ -813,7 +813,7 @@ public class AiController {
         }
         else {
             Cost payCosts = sa.getPayCosts();
-            if(payCosts != null) {
+            if (payCosts != null) {
                 ManaCost mana = payCosts.getTotalMana();
                 if (mana != null) {
                     if (mana.countX() > 0) {
@@ -1916,7 +1916,7 @@ public class AiController {
         if (sa.hasParam("AIMaxAmount")) {
             max = AbilityUtils.calculateAmount(sa.getHostCard(), sa.getParam("AIMaxAmount"), sa);
         }
-        switch(sa.getApi()) {
+        switch (sa.getApi()) {
             case TwoPiles:
                 // TODO: improve AI
                 Card biggest = null;
@@ -2013,7 +2013,7 @@ public class AiController {
 
         final CardCollection library = new CardCollection(in);
         CardLists.shuffle(library);
-    
+
         // remove all land, keep non-basicland in there, shuffled
         CardCollection land = CardLists.filter(library, CardPredicates.Presets.LANDS);
         for (Card c : land) {
@@ -2021,7 +2021,7 @@ public class AiController {
                 library.remove(c);
             }
         }
-    
+
         try {
             // mana weave, total of 7 land
             // The Following have all been reduced by 1, to account for the
@@ -2038,19 +2038,14 @@ public class AiController {
             System.err.println("Error: cannot smooth mana curve, not enough land");
             return in;
         }
-    
+
         // add the rest of land to the end of the deck
         for (int i = 0; i < land.size(); i++) {
             if (!library.contains(land.get(i))) {
                 library.add(land.get(i));
             }
         }
-    
+
-        // check
-        for (int i = 0; i < library.size(); i++) {
-            System.out.println(library.get(i));
-        }
-    
         return library;
     } // smoothComputerManaCurve()
 
@@ -2224,7 +2219,7 @@ public class AiController {
         }
         return ComputerUtil.chooseSacrificeType(player, type, ability, ability.getTargetCard(), amount);
     }
-    
+
     private boolean checkAiSpecificRestrictions(final SpellAbility sa) {
         // AI-specific restrictions specified as activation parameters in spell abilities
 
@@ -2276,5 +2271,5 @@ public class AiController {
         // AI logic for choosing which replacement effect to apply happens here.
         return Iterables.getFirst(list, null);
     }
-    
+
 }

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

@@ -1899,8 +1899,7 @@ public class ComputerUtilMana {
             if (!res.contains(a)) {
                 if (cost.isReusuableResource()) {
                     res.add(0, a);
-                }
+                } else {
-                else {
                     res.add(res.size(), a);
                 }
             }

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

@@ -31,8 +31,6 @@ import forge.game.IHasSVars;
 import forge.game.ability.AbilityFactory;
 import forge.game.ability.AbilityKey;
 import forge.game.ability.AbilityUtils;
-import forge.game.ability.ApiType;
-import forge.game.ability.effects.CharmEffect;
 import forge.game.card.*;
 import forge.game.player.Player;
 import forge.game.spellability.AbilitySub;
@@ -346,8 +344,7 @@ public class TriggerHandler {
         }
     }
 
-    private boolean runNonStaticTriggersForPlayer(final Player player, final TriggerWaiting wt, final List<Trigger> delayedTriggersWorkingCopy ) {
+    private boolean runNonStaticTriggersForPlayer(final Player player, final TriggerWaiting wt, final List<Trigger> delayedTriggersWorkingCopy) {
-
         final TriggerType mode = wt.getMode();
         final Map<AbilityKey, Object> runParams = wt.getParams();
         final List<Trigger> triggers = wt.getTriggers() != null ? wt.getTriggers() : activeTriggers;
@@ -503,7 +500,6 @@ public class TriggerHandler {
     // runs it if so.
     // Return true if the trigger went off, false otherwise.
     private void runSingleTriggerInternal(final Trigger regtrig, final Map<AbilityKey, Object> runParams) {
-
         // All tests passed, execute ability.
         if (regtrig instanceof TriggerTapsForMana) {
             final SpellAbility abMana = (SpellAbility) runParams.get(AbilityKey.AbilityMana);
@@ -519,8 +515,7 @@ public class TriggerHandler {
         if (sa == null) {
             if (!regtrig.hasParam("Execute")) {
                 sa = new SpellAbility.EmptySa(host);
-            }
+            } else {
-            else {
                 String name = regtrig.getParam("Execute");
                 if (!host.getCurrentState().hasSVar(name)) {
                     System.err.println("Warning: tried to run a trigger for card " + host + " referencing a SVar " + name + " not present on the current state " + host.getCurrentState() + ". Aborting trigger execution to prevent a crash.");
@@ -577,12 +572,6 @@ public class TriggerHandler {
         }
 
         sa.setStackDescription(sa.toString());
-        if (sa.getApi() == ApiType.Charm && !sa.isWrapper()) {
-            if (!CharmEffect.makeChoices(sa)) {
-                // 603.3c If no mode is chosen, the ability is removed from the stack.
-                return;
-            }
-        }
 
         Player decider = null;
         boolean isMandatory = false;
@@ -592,8 +581,7 @@ public class TriggerHandler {
         }
         else if (sa instanceof AbilitySub || !sa.hasParam("Cost") || sa.getParam("Cost").equals("0")) {
             isMandatory = true;
-        }
+        } else { // triggers with a cost can't be mandatory
-        else { // triggers with a cost can't be mandatory
             sa.setOptionalTrigger(true);
             decider = sa.getActivatingPlayer();
         }
@@ -605,8 +593,7 @@ public class TriggerHandler {
         wrapperAbility.setLastStateBattlefield(game.getLastStateBattlefield());
         if (regtrig.isStatic()) {
             wrapperAbility.getActivatingPlayer().getController().playTrigger(host, wrapperAbility, isMandatory);
-        }
+        } else {
-        else {
             game.getStack().addSimultaneousStackEntry(wrapperAbility);
         }
 

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

@@ -549,7 +549,6 @@ public class WrappedAbility extends Ability {
         sa.setXManaCostPaid(n);
     }
 
-
     public CardState getCardState() {
         return sa.getCardState();
     }

forge-game/src/main/java/forge/game/zone/MagicStack.java

@@ -40,6 +40,7 @@ import forge.game.GameObject;
 import forge.game.ability.AbilityKey;
 import forge.game.ability.AbilityUtils;
 import forge.game.ability.ApiType;
+import forge.game.ability.effects.CharmEffect;
 import forge.game.card.Card;
 import forge.game.card.CardCollection;
 import forge.game.card.CardUtil;
@@ -224,8 +225,7 @@ public class MagicStack /* extends MyObservable */ implements Iterable<SpellAbil
                 undoStackOwner = activator;
             }
             undoStack.push(sp);
-        }
+        } else {
-        else {
             clearUndoStack();
         }
 
@@ -628,8 +628,7 @@ public class MagicStack /* extends MyObservable */ implements Iterable<SpellAbil
         if (sa.usesTargeting()) {
             if (sa.isZeroTargets()) {
                 // Nothing targeted, and nothing needs to be targeted.
-            }
+            } else {
-            else {
                 // Some targets were chosen, fizzling for this subability is now possible
                 //fizzle = true;
                 // With multi-targets, as long as one target is still legal,
@@ -675,8 +674,7 @@ public class MagicStack /* extends MyObservable */ implements Iterable<SpellAbil
         }
         else if (sa.getTargetCard() != null) {
             fizzle = !sa.canTarget(sa.getTargetCard());
-        }
+        } else {
-        else {
             // Set fizzle to the same as the parent if there's no target info
             fizzle = parentFizzled;
         }
@@ -799,7 +797,7 @@ public class MagicStack /* extends MyObservable */ implements Iterable<SpellAbil
             result |= chooseOrderOfSimultaneousStackEntry(whoAddsToStack);
             // 2014-08-10 Fix infinite loop when a player dies during a multiplayer game during their turn
             whoAddsToStack = game.getNextPlayerAfter(whoAddsToStack);
-        } while( whoAddsToStack != null && whoAddsToStack != playerTurn);
+        } while (whoAddsToStack != null && whoAddsToStack != playerTurn);
         return result;
     }
 
@@ -809,9 +807,19 @@ public class MagicStack /* extends MyObservable */ implements Iterable<SpellAbil
         }
 
         final List<SpellAbility> activePlayerSAs = Lists.newArrayList();
+        final List<SpellAbility> failedSAs = Lists.newArrayList();
         for (int i = 0; i < simultaneousStackEntryList.size(); i++) {
             SpellAbility sa = simultaneousStackEntryList.get(i);
             Player activator = sa.getActivatingPlayer();
+
+            if (sa.getApi() == ApiType.Charm) {
+                if (!CharmEffect.makeChoices(sa)) {
+                    // 603.3c If no mode is chosen, the ability is removed from the stack.
+                    failedSAs.add(sa);
+                    continue;
+                }
+            }
+
             if (activator == null) {
                 if (sa.getHostCard().getController().equals(activePlayer)) {
                     activePlayerSAs.add(sa);
@@ -823,6 +831,7 @@ public class MagicStack /* extends MyObservable */ implements Iterable<SpellAbil
             }
         }
         simultaneousStackEntryList.removeAll(activePlayerSAs);
+        simultaneousStackEntryList.removeAll(failedSAs);
 
         if (activePlayerSAs.isEmpty()) {
             return false;
@@ -875,8 +884,7 @@ public class MagicStack /* extends MyObservable */ implements Iterable<SpellAbil
     public final void addCastCommand(final String valid, final GameCommand c) {
         if (commandList.containsKey(valid)) {
             commandList.get(valid).add(0, c);
-        }
+        } else {
-        else {
             commandList.put(valid, Lists.newArrayList(c));
         }
     }