Refactor confirmTrigger into controller class

forge-gui/src/main/java/forge/game/player/PlayerController.java

@@ -10,6 +10,7 @@ import org.apache.commons.lang3.tuple.ImmutablePair;
 import org.apache.commons.lang3.tuple.Pair;
 
 import com.google.common.base.Predicate;
+
 import forge.card.ColorSet;
 import forge.deck.Deck;
 import forge.game.Game;
@@ -27,6 +28,7 @@ import forge.game.spellability.AbilitySub;
 import forge.game.spellability.SpellAbility;
 import forge.game.spellability.SpellAbilityStackInstance;
 import forge.game.spellability.TargetChoices;
+import forge.game.trigger.Trigger;
 import forge.game.zone.ZoneType;
 import forge.item.PaperCard;
 
@@ -135,8 +137,9 @@ public abstract class PlayerController {
     public abstract SpellAbility chooseSingleSpellForEffect(List<SpellAbility> spells, SpellAbility sa, String title);
 
     public abstract boolean confirmAction(SpellAbility sa, PlayerActionConfirmMode mode, String message);
-    public abstract boolean getWillPlayOnFirstTurn(boolean isFirstGame);
     public abstract boolean confirmStaticApplication(Card hostCard, GameEntity affected, String logic, String message);
+    public abstract boolean confirmTrigger(SpellAbility sa, Trigger regtrig, Map<String, String> triggerParams, boolean isMandatory);
+    public abstract boolean getWillPlayOnFirstTurn(boolean isFirstGame);
 
     public abstract List<Card> orderBlockers(Card attacker, List<Card> blockers);
     public abstract List<Card> orderAttackers(Card blocker, List<Card> attackers);

forge-gui/src/main/java/forge/game/player/PlayerControllerAi.java

@@ -47,6 +47,7 @@ import forge.game.spellability.Spell;
 import forge.game.spellability.SpellAbility;
 import forge.game.spellability.SpellAbilityStackInstance;
 import forge.game.spellability.TargetChoices;
+import forge.game.trigger.Trigger;
 import forge.game.zone.ZoneType;
 import forge.item.PaperCard;
 import forge.util.Aggregates;
@@ -73,9 +74,11 @@ public class PlayerControllerAi extends PlayerController {
     public SpellAbility getAbilityToPlay(List<SpellAbility> abilities, MouseEvent triggerEvent) {
         if (abilities.size() == 0) {
             return null;
-        } else 
+        }
+        else {
             return abilities.get(0);
-//        } else {
+        }
+//      else {
 //          return GuiChoose.oneOrNone("Choose ability for AI to play", abilities); // some day network interaction will be here
 //      }
     }
@@ -168,21 +171,61 @@ public class PlayerControllerAi extends PlayerController {
         return api.getAi().chooseSingleSpellAbility(player, sa, spells);
     }
 
-    
     @Override
     public boolean confirmAction(SpellAbility sa, PlayerActionConfirmMode mode, String message) {
         return getAi().confirmAction(sa, mode, message);
     }
 
-    @Override
-    public boolean getWillPlayOnFirstTurn(boolean isFirstGame) {
-        return true; // AI is brave :)
-    }
     @Override
     public boolean confirmStaticApplication(Card hostCard, GameEntity affected, String logic, String message) {
         return getAi().confirmStaticApplication(hostCard, affected, logic, message);
     }
 
+    @Override
+    public boolean confirmTrigger(SpellAbility sa, Trigger regtrig, Map<String, String> triggerParams, boolean isMandatory) {
+        if (triggerParams.containsKey("DelayedTrigger")) {
+            //TODO: The only card with an optional delayed trigger is Shirei, Shizo's Caretaker,
+            //      needs to be expanded when a more difficult cards comes up
+            return true;
+        }
+        // Store/replace target choices more properly to get this SA cleared.
+        TargetChoices tc = null;
+        TargetChoices subtc = null;
+        boolean storeChoices = sa.getTargetRestrictions() != null;
+        final SpellAbility sub = sa.getSubAbility();
+        boolean storeSubChoices = sub != null && sub.getTargetRestrictions() != null;
+        boolean ret = true;
+
+        if (storeChoices) {
+            tc = sa.getTargets();
+            sa.resetTargets();
+        }
+        if (storeSubChoices) {
+            subtc = sub.getTargets();
+            sub.resetTargets();
+        }
+        // There is no way this doTrigger here will have the same target as stored above
+        // So it's possible it's making a different decision here than will actually happen
+        if (!sa.doTrigger(isMandatory, player)) {
+            ret = false;
+        }
+        if (storeChoices) {
+            sa.resetTargets();
+            sa.setTargets(tc);
+        }
+        if (storeSubChoices) {
+            sub.resetTargets();
+            sub.setTargets(subtc);
+        }
+
+        return ret;
+    }
+
+    @Override
+    public boolean getWillPlayOnFirstTurn(boolean isFirstGame) {
+        return true; // AI is brave :)
+    }
+
     @Override
     public List<Card> orderBlockers(Card attacker, List<Card> blockers) {
         return AiBlockController.orderBlockers(attacker, blockers);
@@ -207,18 +250,19 @@ public class PlayerControllerAi extends PlayerController {
         List<Card> toTop = new ArrayList<Card>();
 
         for (Card c: topN) {
-            if (ComputerUtil.scryWillMoveCardToBottomOfLibrary(player, c))
+            if (ComputerUtil.scryWillMoveCardToBottomOfLibrary(player, c)) {
                 toBottom.add(c);
-            else 
+            }
+            else {
                 toTop.add(c);
             }
+        }
 
         // put the rest on top in random order
         Collections.shuffle(toTop);
         return ImmutablePair.of(toTop, toBottom);
     }
 
-
     @Override
     public boolean willPutCardOnTop(Card c) {
         return true; // AI does not know what will happen next (another clash or that would become his topdeck)
@@ -232,8 +276,9 @@ public class PlayerControllerAi extends PlayerController {
 
     @Override
     public List<Card> chooseCardsToDiscardFrom(Player p, SpellAbility sa, List<Card> validCards, int min, int max) {
-        if ( p == player )
+        if (p == player) {
             return brains.getCardsToDiscard(min, max, validCards, sa);
+        }
 
         boolean isTargetFriendly = !p.isOpponentOf(player);
 
@@ -259,7 +304,8 @@ public class PlayerControllerAi extends PlayerController {
                 if (!spell.canPlayFromEffectAI(player, true, true)) {
                     return; // is this legal at all?
                 }
-            } else {
+            }
+            else {
                 copySA.canPlayAI(player);
             }
         }
@@ -339,21 +385,23 @@ public class PlayerControllerAi extends PlayerController {
 
     @Override
     public List<Card> getCardsToMulligan(boolean isCommander, Player firstPlayer)  {
-        if( !ComputerUtil.wantMulligan(player) )
+        if (!ComputerUtil.wantMulligan(player)) {
             return null;
+        }
 
-        if (!isCommander) 
+        if (!isCommander) {
             return player.getCardsIn(ZoneType.Hand);
-        else
+        }
+        else {
             return ComputerUtil.getPartialParisCandidates(player);
         }
+    }
 
     @Override
     public void declareAttackers(Player attacker, Combat combat) {
         brains.declareAttackers(attacker, combat);
     }
 
-   
     @Override
     public void declareBlockers(Player defender, Combat combat) {
         brains.declareBlockersFor(defender, combat);
@@ -361,8 +409,9 @@ public class PlayerControllerAi extends PlayerController {
 
     @Override
     public void takePriority() {
-        if ( !game.isGameOver() )
+        if (!game.isGameOver()) {
             brains.onPriorityRecieved();
+        }
         // use separate thread for AI?
     }
 
@@ -436,7 +485,7 @@ public class PlayerControllerAi extends PlayerController {
 
     @Override
     public boolean chooseBinary(SpellAbility sa, String question, BinaryChoiceType kindOfChoice) {
-        if(kindOfChoice == BinaryChoiceType.TapOrUntap) return true;
+        if (kindOfChoice == BinaryChoiceType.TapOrUntap) { return true; }
         return MyRandom.getRandom().nextBoolean();
     }
 

forge-gui/src/main/java/forge/game/player/PlayerControllerHuman.java

@@ -49,6 +49,7 @@ import forge.game.spellability.SpellAbility;
 import forge.game.spellability.SpellAbilityStackInstance;
 import forge.game.spellability.TargetChoices;
 import forge.game.spellability.TargetSelection;
+import forge.game.trigger.Trigger;
 import forge.game.zone.Zone;
 import forge.game.zone.ZoneType;
 import forge.gui.GuiChoose;
@@ -403,6 +404,36 @@ public class PlayerControllerHuman extends PlayerController {
         return GuiDialog.confirm(hostCard, message);
     }
 
+    @Override
+    public boolean confirmTrigger(SpellAbility sa, Trigger regtrig, Map<String, String> triggerParams, boolean isMandatory) {
+        if (this.shouldAlwaysAcceptTrigger(regtrig.getId())) {
+            return true;
+        }
+        if (this.shouldAlwaysDeclineTrigger(regtrig.getId())) {
+            return false;
+        }
+        
+        String triggerDesc = triggerParams.get("TriggerDescription").replace("CARDNAME", regtrig.getHostCard().getName());
+        final StringBuilder buildQuestion = new StringBuilder("Use triggered ability of ");
+        buildQuestion.append(regtrig.getHostCard().toString()).append("?");
+        buildQuestion.append("\r\n(").append(triggerDesc).append(")\r\n");
+        HashMap<String, Object> tos = sa.getTriggeringObjects();
+        if (tos.containsKey("Attacker")) {
+            buildQuestion.append("[Attacker: " + tos.get("Attacker") + "]");
+        }
+        if (tos.containsKey("Card")) {
+            Card card = (Card) tos.get("Card");
+            if (card != null && (card.getController() == player || game.getZoneOf(card) == null 
+                    || game.getZoneOf(card).getZoneType().isKnown())) {
+                buildQuestion.append("[Triggering card: " + tos.get("Card") + "]");
+            }
+        }
+        if (!GuiDialog.confirm(regtrig.getHostCard(), buildQuestion.toString())) {
+            return false;
+        }
+        return true;
+    }
+
     @Override
     public boolean getWillPlayOnFirstTurn(boolean isFirstGame) {
         InputPlayOrDraw inp = new InputPlayOrDraw(player, isFirstGame);
@@ -890,8 +921,9 @@ public class PlayerControllerHuman extends PlayerController {
 
     @Override
     public CounterType chooseCounterType(Collection<CounterType> options, SpellAbility sa, String prompt) {
-        if( options.size() <= 1)
+        if (options.size() <= 1) {
             return Iterables.getFirst(options, null);
+        }
         return GuiChoose.one(prompt, options);
     }
 }

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

@@ -18,7 +18,6 @@ import forge.game.spellability.SpellAbility;
 import forge.game.spellability.SpellAbilityRestriction;
 import forge.game.spellability.TargetChoices;
 import forge.game.spellability.TargetRestrictions;
-import forge.gui.GuiDialog;
 
 // Wrapper ability that checks the requirements again just before
 // resolving, for intervening if clauses.
@@ -356,8 +355,9 @@ public class WrappedAbility extends Ability implements ISpellAbility {
         TriggerHandler th = game.getTriggerHandler();
         Map<String, String> triggerParams = regtrig.getMapParams();
 
-        if (decider != null && !confirmTrigger(decider, triggerParams)) 
+        if (decider != null && !decider.getController().confirmTrigger(sa, regtrig, triggerParams, this.isMandatory())) {
             return;
+        }
 
         getActivatingPlayer().getController().playSpellAbilityNoStack(sa, false);
 
@@ -370,71 +370,4 @@ public class WrappedAbility extends Ability implements ISpellAbility {
             th.registerDelayedTrigger(deltrig);
         }
     }
-    
-    private boolean confirmTrigger(Player decider, Map<String, String> triggerParams) {
-        if (decider.isHuman()) {
-            if(decider.getController().shouldAlwaysAcceptTrigger(regtrig.getId()))
-                return true;
-            else if(decider.getController().shouldAlwaysDeclineTrigger(regtrig.getId()))
-                return false;
-            
-            String triggerDesc = triggerParams.get("TriggerDescription").replace("CARDNAME", regtrig.getHostCard().getName());
-            final StringBuilder buildQuestion = new StringBuilder("Use triggered ability of ");
-            buildQuestion.append(regtrig.getHostCard().toString()).append("?");
-            buildQuestion.append("\r\n(").append(triggerDesc).append(")\r\n");
-            HashMap<String, Object> tos = sa.getTriggeringObjects();
-            if (tos.containsKey("Attacker")) {
-                buildQuestion.append("[Attacker: " + tos.get("Attacker") + "]");
-            }
-            if (tos.containsKey("Card")) {
-                Card card = (Card) tos.get("Card");
-                if (card != null && (card.getController() == decider || decider.getGame().getZoneOf(card) == null 
-                        || decider.getGame().getZoneOf(card).getZoneType().isKnown())) {
-                    buildQuestion.append("[Triggering card: " + tos.get("Card") + "]");
-                }
-            }
-            if (!GuiDialog.confirm(regtrig.getHostCard(), buildQuestion.toString())) {
-                return false;
-            }
-            return true;
-        } // human end
-
-        if (triggerParams.containsKey("DelayedTrigger")) {
-            //TODO: The only card with an optional delayed trigger is Shirei, Shizo's Caretaker, 
-            //      needs to be expanded when a more difficult cards comes up
-            return true;
-        }
-        // Store/replace target choices more properly to get this SA cleared.
-        TargetChoices tc = null;
-        TargetChoices subtc = null;
-        boolean storeChoices = sa.getTargetRestrictions() != null;
-        final SpellAbility sub = sa.getSubAbility();
-        boolean storeSubChoices = sub != null && sub.getTargetRestrictions() != null;
-        boolean ret = true;
-
-        if (storeChoices) {
-            tc = sa.getTargets();
-            sa.resetTargets();
-        }
-        if (storeSubChoices) {
-            subtc = sub.getTargets();
-            sub.resetTargets();
-        }
-        // There is no way this doTrigger here will have the same target as stored above
-        // So it's possible it's making a different decision here than will actually happen
-        if (!sa.doTrigger(this.isMandatory(), decider)) {
-            ret = false;
-        }
-        if (storeChoices) {
-            sa.resetTargets();
-            sa.setTargets(tc);
-        }
-        if (storeSubChoices) {
-            sub.resetTargets();
-            sub.setTargets(subtc);
-        }
-
-        return ret;
-    }
-
 }

forge-gui/src/test/java/forge/gamesimulationtests/util/PlayerControllerForTests.java

@@ -41,6 +41,7 @@ import forge.game.spellability.AbilitySub;
 import forge.game.spellability.SpellAbility;
 import forge.game.spellability.SpellAbilityStackInstance;
 import forge.game.spellability.TargetChoices;
+import forge.game.trigger.Trigger;
 import forge.game.zone.ZoneType;
 import forge.gamesimulationtests.util.card.CardSpecification;
 import forge.gamesimulationtests.util.card.CardSpecificationHandler;
@@ -182,12 +183,17 @@ public class PlayerControllerForTests extends PlayerController {
 	}
 
 	@Override
-	public boolean getWillPlayOnFirstTurn( boolean isFirstGame ) {
+	public boolean confirmStaticApplication(Card hostCard, GameEntity affected, String logic, String message) {
 		return true;
 	}
 
 	@Override
-	public boolean confirmStaticApplication( Card hostCard, GameEntity affected, String logic, String message ) {
+    public boolean confirmTrigger(SpellAbility sa, Trigger regtrig, Map<String, String> triggerParams, boolean isMandatory) {
+        return true;
+    }
+
+    @Override
+    public boolean getWillPlayOnFirstTurn(boolean isFirstGame) {
         return true;
     }