got sick and tired of 7-parameter function choosePermanentsToSacrifice - made 2 overloads with 4 params.

When min = 0 cancel button is enabled

src/main/java/forge/card/ability/effects/SacrificeEffect.java

@@ -61,7 +61,9 @@ public class SacrificeEffect extends SpellAbilityEffect {
                     choosenToSacrifice = Aggregates.random(validTargets, Math.min(amount, validTargets.size()));
                 } else {
                     boolean isOptional = sa.hasParam("Optional");
-                    choosenToSacrifice = p.getController().choosePermanentsToSacrifice(validTargets, valid, amount, sa, destroy, isOptional, false);
+                    choosenToSacrifice = destroy ? 
+                        p.getController().choosePermanentsToDestroy(sa, isOptional ? 0 : amount, amount, validTargets, valid) :
+                        p.getController().choosePermanentsToSacrifice(sa, isOptional ? 0 : amount, amount, validTargets, valid);
                 }
                 
                 for(Card sac : choosenToSacrifice) {

src/main/java/forge/card/mana/ManaCostBeingPaid.java

@@ -665,8 +665,9 @@ public class ManaCostBeingPaid {
         List<Card> canOffer = CardLists.filter(spell.getActivatingPlayer().getCardsIn(ZoneType.Battlefield),
                 CardPredicates.isType(offeringType));
 
-        final List<Card> toSacList = sa.getSourceCard().getController().getController().choosePermanentsToSacrifice(canOffer,
+        final List<Card> toSacList = sa.getSourceCard().getController().getController().choosePermanentsToSacrifice(spell, 0, 1, canOffer,
-                offeringType, 1, spell, false, false, true);
+                offeringType);
+
         if (!toSacList.isEmpty()) {
             toSac = toSacList.get(0);
         } else {

src/main/java/forge/game/Game.java

@@ -19,7 +19,6 @@ package forge.game;
 
 import java.util.ArrayList;
 
-import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;

src/main/java/forge/game/phase/CombatUtil.java

@@ -1048,7 +1048,7 @@ public class CombatUtil {
                         final Player opponent = game.getCombat().getDefendingPlayerRelatedTo(c).get(0);
                         //List<Card> list = AbilityUtils.filterListByType(opponent.getCardsIn(ZoneType.Battlefield), "Permanent", this);
                         final List<Card> list = opponent.getCardsIn(ZoneType.Battlefield);
-                        List<Card> toSac = opponent.getController().choosePermanentsToSacrifice(list, "Card", a, this, false, false, false);
+                        List<Card> toSac = opponent.getController().choosePermanentsToSacrifice(this, a, a, list, "Card");
 
                         for(Card sacd : toSac) {
                             game.getAction().sacrifice(sacd, this);

src/main/java/forge/game/phase/Upkeep.java

@@ -455,7 +455,7 @@ public class Upkeep extends Phase {
                             lowest.add(c);
                         }
 
-                        List<Card> toSac = player.getController().choosePermanentsToSacrifice(lowest, "Select creature with power: " + power + " to destroy.", 1, this, true, false, false);
+                        List<Card> toSac = player.getController().choosePermanentsToDestroy(this, 1, 1, lowest, "Select creature with power: " + power + " to destroy.");
                         game.getAction().destroyNoRegeneration(toSac.get(0), this);
                     }
                 } // resolve

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

@@ -90,7 +90,8 @@ public abstract class PlayerController {
     public abstract Map<Card, Integer> assignCombatDamage(Card attacker, List<Card> blockers, int damageDealt, GameEntity defender);
 
     public abstract Integer announceRequirements(SpellAbility ability, String announce, boolean allowZero);
-    public abstract List<Card> choosePermanentsToSacrifice(List<Card> validTargets, String validMessage, int amount, SpellAbility sa, boolean destroy, boolean isOptional, boolean canCancel);
+    public abstract List<Card> choosePermanentsToSacrifice(SpellAbility sa, int min, int max, List<Card> validTargets, String message);
+    public abstract List<Card> choosePermanentsToDestroy(SpellAbility sa, int min, int max, List<Card> validTargets, String message);
     public abstract Target chooseTargets(SpellAbility ability);
 
     public Card chooseSingleCardForEffect(List<Card> sourceList, SpellAbility sa, String title) { return chooseSingleCardForEffect(sourceList, sa, title, false); }

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

@@ -118,10 +118,16 @@ public class PlayerControllerAi extends PlayerController {
     }
 
     @Override
-    public List<Card> choosePermanentsToSacrifice(List<Card> validTargets, String validMessage, int amount, SpellAbility sa, boolean destroy, boolean isOptional, boolean canCancel) {
+    public List<Card> choosePermanentsToSacrifice(SpellAbility sa, int min, int max, List<Card> validTargets, String message) {
-        return ComputerUtil.choosePermanentsToSacrifice(player, validTargets, amount, sa, destroy, isOptional);
+        return ComputerUtil.choosePermanentsToSacrifice(player, validTargets, max, sa, false, min == 0);
     }
 
+    @Override
+    public List<Card> choosePermanentsToDestroy(SpellAbility sa, int min, int max, List<Card> validTargets, String message) {
+        return ComputerUtil.choosePermanentsToSacrifice(player, validTargets, max, sa, true, min == 0);
+    }
+
+    
     @Override
     public Card chooseSingleCardForEffect(List<Card> options, SpellAbility sa, String title, boolean isOptional) {
         return getAi().chooseSingleCardForEffect(options, sa, title, isOptional);

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

@@ -11,6 +11,8 @@ import javax.swing.JOptionPane;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.tuple.ImmutablePair;
 
+import com.google.common.collect.Lists;
+
 import forge.Card;
 import forge.GameEntity;
 import forge.Singletons;
@@ -202,34 +204,33 @@ public class PlayerControllerHuman extends PlayerController {
                 if (val == 0 && canChooseZero || val > 0) 
                     return val;
             }
-            JOptionPane.showMessageDialog(null, "You have to enter a valid number", "Announce value", JOptionPane.WARNING_MESSAGE);
+            GuiDialog.message("You have to enter a valid number", "Announce value");
         }
     }
 
-    /* (non-Javadoc)
+
-     * @see forge.game.player.PlayerController#choosePermanentsToSacrifice(java.util.List, int, forge.card.spellability.SpellAbility, boolean, boolean)
-     */
     @Override
-    public List<Card> choosePermanentsToSacrifice(List<Card> validTargets, String validMessage, int amount, SpellAbility sa, boolean destroy, boolean isOptional, boolean canCancel) {
+    public List<Card> choosePermanentsToSacrifice(SpellAbility sa, int min, int max, List<Card> valid, String message) {
-        int max = Math.min(amount, validTargets.size());
+        String outerMessage = "Select %d " + message + "(s) to sacrifice";
+        return choosePermanentsTo(min, max, valid, outerMessage); 
+    }
+
+    @Override
+    public List<Card> choosePermanentsToDestroy(SpellAbility sa, int min, int max, List<Card> valid, String message) {
+        String outerMessage = "Select %d " + message + "(s) to be destroyed";
+        return choosePermanentsTo(min, max, valid, outerMessage); 
+    }
+
+    private List<Card> choosePermanentsTo(int min, int max, List<Card> valid, String outerMessage) {
+        max = Math.min(max, valid.size());
         if (max <= 0)
             return new ArrayList<Card>();
 
-        int min = isOptional ? 0 : amount;
+        InputSelectCards inp = new InputSelectCardsFromList(min == 0 ? 1 : min, max, valid);
-        if (min > max) {
+        inp.setMessage(outerMessage);
-            min = max;
+        inp.setCancelAllowed(min == 0);
-        }
-
-        InputSelectCards inp = new InputSelectCardsFromList(min, max, validTargets);
-        // TODO: Either compose a message here, or pass it as parameter from caller. 
-        inp.setMessage("Select %d " + validMessage + "(s) to sacrifice");
-        inp.setCancelAllowed(canCancel);
-        
         Singletons.getControl().getInputQueue().setInputAndWait(inp);
-        if (inp.hasCancelled()) {
+        return inp.hasCancelled() ? Lists.<Card>newArrayList() : inp.getSelected();
-            return new ArrayList<Card>();
-        }
-        else return inp.getSelected(); 
     }
 
     @Override