Sacrificed cards are gathered to a list before actually sacrificing them, that returns a method of PlayerController

src/main/java/forge/Card.java

@@ -8897,6 +8897,10 @@ public class Card extends GameEntity implements Comparable<Card> {
         return Singletons.getModel().getGame().isCardInZone(this, zone);
     }
 
+    public final boolean canBeDestroyed() { 
+        return isInPlay() && (!hasKeyword("Indestructible") || (isCreature() && getNetDefense() <= 0));
+    }
+
     /**
      * Can target.
      * 
@@ -9210,4 +9214,17 @@ public class Card extends GameEntity implements Comparable<Card> {
         return getManaCost().getCMC() + xPaid;
     }
 
+    public final boolean canBeSacrificedBy(final SpellAbility source)
+    {
+        if (isImmutable()) {
+            System.out.println("Trying to sacrifice immutables: " + this);
+            return false;
+        }
+        if (source != null && !getController().isOpponentOf(source.getActivatingPlayer())
+                && getController().hasKeyword("Spells and abilities your opponents control can't cause you to sacrifice permanents.")) {
+            return false;
+        }
+        return true;
+    }
+
 } // end Card class

src/main/java/forge/CardPredicates.java

@@ -147,6 +147,16 @@ public final class CardPredicates {
         };
     }
 
+    public static final Predicate<Card> canBeSacrificedBy(final SpellAbility sa) {
+        return new Predicate<Card>() {
+            @Override
+            public boolean apply(final Card c) {
+                return c.canBeSacrificedBy(sa);
+            }
+        };
+    };
+    
+
     public static class Presets {
 
         /**
@@ -347,6 +357,12 @@ public final class CardPredicates {
                 return c.isPlaneswalker();
             }
         };
+        public static final Predicate<Card> CAN_BE_DESTROYED = new Predicate<Card>() {
+            @Override
+            public boolean apply(final Card c) {
+                return c.canBeDestroyed();
+            }
+        };
     }
 
     public static class Accessors {

src/main/java/forge/card/ability/ai/AttachAi.java

@@ -400,7 +400,7 @@ public class AttachAi extends SpellAiLogic {
                     @Override
                     public boolean apply(final Card c) {
                         // Don't enchant creatures that can survive
-                        if (c.hasKeyword("Indestructible") || c.getNetCombatDamage() < c.getNetDefense()) {
+                        if (!c.canBeDestroyed() || c.getNetCombatDamage() < c.getNetDefense()) {
                             return false;
                         }
                         return true;

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

@@ -1,6 +1,5 @@
 package forge.card.ability.effects;
 
-import java.util.ArrayList;
 import java.util.List;
 
 import forge.Card;
@@ -8,10 +7,9 @@ import forge.Singletons;
 import forge.card.ability.AbilityUtils;
 import forge.card.ability.SpellEffect;
 import forge.card.spellability.SpellAbility;
-import forge.game.ai.ComputerUtil;
+import forge.game.GameState;
 import forge.game.player.Player;
 import forge.game.zone.ZoneType;
-import forge.gui.GuiChoose;
 import forge.util.Aggregates;
 
 public class SacrificeEffect extends SpellEffect {
@@ -19,6 +17,7 @@ public class SacrificeEffect extends SpellEffect {
     @Override
     public void resolve(SpellAbility sa) {
         final Card card = sa.getSourceCard();
+        final GameState game = Singletons.getModel().getGame();
 
         // Expand Sacrifice keyword here depending on what we need out of it.
         final String num = sa.hasParam("Amount") ? sa.getParam("Amount") : "1";
@@ -41,29 +40,31 @@ public class SacrificeEffect extends SpellEffect {
         final boolean remSacrificed = sa.hasParam("RememberSacrificed");
 
         if (valid.equals("Self")) {
-            if (Singletons.getModel().getGame().getZoneOf(card).is(ZoneType.Battlefield)) {
+            if (game.getZoneOf(card).is(ZoneType.Battlefield)) {
-                if (Singletons.getModel().getGame().getAction().sacrifice(card, sa) && remSacrificed) {
+                if (game.getAction().sacrifice(card, sa) && remSacrificed) {
                     card.addRemembered(card);
                 }
             }
         }
         else {
-            List<Card> sacList = null;
+            List<Card> choosenToSacrifice = null;
             for (final Player p : tgts) {
+                List<Card> battlefield = p.getCardsIn(ZoneType.Battlefield);
+                List<Card> validTargets = AbilityUtils.filterListByType(battlefield, valid, sa);
+                
                 if (sa.hasParam("Random")) {
-                    sacList = sacrificeRandom(p, amount, valid, sa, destroy);
+                    choosenToSacrifice = Aggregates.random(validTargets, Math.min(amount, validTargets.size()));
-                } else if (p.isComputer()) {
-                    if (sa.hasParam("Optional") && sa.getActivatingPlayer().isOpponentOf(p)) {
-                        continue;
-                    }
-                    sacList = sacrificeAI(p, amount, valid, sa, destroy);
                 } else {
-                    sacList = sacrificeHuman(p, amount, valid, sa, destroy,
+                    boolean isOptional = sa.hasParam("Optional");
-                            sa.hasParam("Optional"));
+                    choosenToSacrifice = p.getController().choosePermanentsToSacrifice(validTargets, amount, sa, destroy, isOptional);
                 }
-                if (remSacrificed) {
+                
-                    for (int i = 0; i < sacList.size(); i++) {
+                for(Card sac : choosenToSacrifice) {
-                        card.addRemembered(sacList.get(i));
+                    boolean wasSacrificed = !destroy && game.getAction().sacrifice(sac, sa);
+                    boolean wasDestroyed = destroy && game.getAction().destroy(sac);
+                    
+                    if ( remSacrificed && (wasDestroyed || wasSacrificed) ) {
+                        card.addRemembered(sac);
                     }
                 }
             }
@@ -111,115 +112,4 @@ public class SacrificeEffect extends SpellEffect {
 
         return sb.toString();
     }
-
-    /**
-     * <p>
-     * sacrificeAI.
-     * </p>
-     * 
-     * @param p
-     *            a {@link forge.game.player.Player} object.
-     * @param amount
-     *            a int.
-     * @param valid
-     *            a {@link java.lang.String} object.
-     * @param sa
-     *            a {@link forge.card.spellability.SpellAbility} object.
-     */
-    private List<Card> sacrificeAI(final Player p, final int amount, final String valid, final SpellAbility sa,
-            final boolean destroy) {
-        List<Card> battlefield = p.getCardsIn(ZoneType.Battlefield);
-        List<Card> sacList = AbilityUtils.filterListByType(battlefield, valid, sa);
-        sacList = ComputerUtil.sacrificePermanents(p, amount, sacList, destroy, sa);
-
-        return sacList;
-    }
-
-    /**
-     * <p>
-     * sacrificeHuman.
-     * </p>
-     * 
-     * @param p
-     *            a {@link forge.game.player.Player} object.
-     * @param amount
-     *            a int.
-     * @param valid
-     *            a {@link java.lang.String} object.
-     * @param sa
-     *            a {@link forge.card.spellability.SpellAbility} object.
-     * @param message
-     *            a {@link java.lang.String} object.
-     */
-    public static List<Card> sacrificeHuman(final Player p, final int amount, final String valid, final SpellAbility sa,
-            final boolean destroy, final boolean optional) {
-        List<Card> list = AbilityUtils.filterListByType(p.getCardsIn(ZoneType.Battlefield), valid, sa);
-        List<Card> sacList = new ArrayList<Card>();
-
-        for (int i = 0; i < amount; i++) {
-            if (list.isEmpty()) {
-                break;
-            }
-            Card c;
-            if (optional) {
-                c = GuiChoose.oneOrNone("Select a card to sacrifice", list);
-            } else {
-                c = GuiChoose.one("Select a card to sacrifice", list);
-            }
-            if (c != null) {
-                if (destroy) {
-                    if (Singletons.getModel().getGame().getAction().destroy(c)) {
-                        sacList.add(c);
-                    }
-                } else {
-                    if (Singletons.getModel().getGame().getAction().sacrifice(c, sa)) {
-                        sacList.add(c);
-                    }
-                }
-
-                list.remove(c);
-
-            } else {
-                return sacList;
-            }
-        }
-        return sacList;
-    }
-
-    /**
-     * <p>
-     * sacrificeRandom.
-     * </p>
-     * 
-     * @param p
-     *            a {@link forge.game.player.Player} object.
-     * @param amount
-     *            a int.
-     * @param valid
-     *            a {@link java.lang.String} object.
-     * @param sa
-     *            a {@link forge.card.spellability.SpellAbility} object.
-     */
-    private List<Card> sacrificeRandom(final Player p, final int amount, final String valid, final SpellAbility sa,
-            final boolean destroy) {
-        List<Card> sacList = new ArrayList<Card>();
-        for (int i = 0; i < amount; i++) {
-            List<Card> battlefield = p.getCardsIn(ZoneType.Battlefield);
-            List<Card> list = AbilityUtils.filterListByType(battlefield, valid, sa);
-            if (list.size() != 0) {
-                final Card sac = Aggregates.random(list);
-                if (destroy) {
-                    if (Singletons.getModel().getGame().getAction().destroy(sac)) {
-                        sacList.add(sac);
-                    }
-                } else {
-                    if (Singletons.getModel().getGame().getAction().sacrifice(sac, sa)) {
-                        sacList.add(sac);
-                    }
-                }
-            }
-        }
-        return sacList;
-    }
-
 }

src/main/java/forge/game/GameAction.java

@@ -1166,27 +1166,11 @@ public class GameAction {
         }
     } // destroyLegendaryCreatures()
 
-    /**
+
-     * <p>
-     * sacrifice.
-     * </p>
-     * 
-     * @param c
-     *            a {@link forge.Card} object.
-     * @param source
-     *            a SpellAbility object.
-     * @return a boolean.
-     */
     public final boolean sacrifice(final Card c, final SpellAbility source) {
-        if (c.isImmutable()) {
+        if(!c.canBeSacrificedBy(source))
-            System.out.println("Trying to sacrifice immutables: " + c);
             return false;
-        }
+        
-        if (source != null && !c.getController().equals(source.getActivatingPlayer())
-                && c.getController().hasKeyword("Spells and abilities your opponents control can't cause"
-                        + " you to sacrifice permanents.")) {
-            return false;
-        }
         this.sacrificeDestroy(c);
 
         // Play the Sacrifice sound
@@ -1196,7 +1180,6 @@ public class GameAction {
         final HashMap<String, Object> runParams = new HashMap<String, Object>();
         runParams.put("Card", c);
         game.getTriggerHandler().runTrigger(TriggerType.Sacrificed, runParams, false);
-
         return true;
     }
 
@@ -1210,8 +1193,7 @@ public class GameAction {
      * @return a boolean.
      */
     public final boolean destroy(final Card c) {
-        if (!c.isInPlay()
+        if (!c.canBeDestroyed()) {
-                || (c.hasKeyword("Indestructible") && (!c.isCreature() || c.getNetDefense() > 0))) {
             return false;
         }
 
@@ -1242,10 +1224,8 @@ public class GameAction {
      * @return a boolean.
      */
     public final boolean destroyNoRegeneration(final Card c) {
-        if (!c.isInPlay()
+        if ( !c.canBeDestroyed() )
-                || (c.hasKeyword("Indestructible") && (!c.isCreature() || c.getNetDefense() > 0))) {
             return false;
-        }
 
         if (c.isEnchanted()) {
             List<Card> list = new ArrayList<Card>(c.getEnchantedBy());

src/main/java/forge/game/ai/ComputerUtil.java

@@ -625,56 +625,64 @@ public class ComputerUtil {
      * <p>
      * sacrificePermanents.
      * </p>
-     * 
      * @param amount
      *            a int.
-     * @param list
-     *            a {@link forge.CardList} object.
-     * @param destroy
-     *            the destroy
      * @param source
      *            the source SpellAbility
+     * @param destroy
+     *            the destroy
+     * @param list
+     *            a {@link forge.CardList} object.
+     * 
      * @return the card list
      */
-    public static List<Card> sacrificePermanents(final Player ai, final int amount, final List<Card> cardlist, final boolean destroy,
+    public static List<Card> choosePermanentsToSacrifice(final Player ai, final List<Card> cardlist, final int amount, SpellAbility source, 
-            SpellAbility source) {
+            final boolean destroy, final boolean isOptional) {
-        final List<Card> list = new ArrayList<Card>(cardlist);
+        final List<Card> remaining = new ArrayList<Card>(cardlist);
-        final List<Card> sacList = new ArrayList<Card>();
+        final List<Card> sacrificed = new ArrayList<Card>();
-        // used in Annihilator and AF_Sacrifice
+
-        int max = list.size();
+        if (isOptional && source.getActivatingPlayer().isOpponentOf(ai)) { 
-        if (max > amount) {
+            return sacrificed; // sacrifice none 
-            max = amount;
         }
         
-        CardLists.sortCMC(list);
+        CardLists.sortCMC(remaining);
-        Collections.reverse(list);
+        Collections.reverse(remaining);
+
+        final int max = Math.min(remaining.size(), amount);
 
         for (int i = 0; i < max; i++) {
-            Card c = null;
+            Card c = chooseCardToSacrifice(remaining, ai, destroy);
+            remaining.remove(c);
+            sacrificed.add(c);
+        }
+        return sacrificed;
+    }
 
+    // Precondition it wants: remaining are reverse-sorted by CMC
+    private static Card chooseCardToSacrifice(final List<Card> remaining, final Player ai, final boolean destroy) {
         if (destroy) {
-                final List<Card> indestructibles = CardLists.getKeyword(list, "Indestructible");
+            final List<Card> indestructibles = CardLists.getKeyword(remaining, "Indestructible");
             if (!indestructibles.isEmpty()) {
-                    c = indestructibles.get(0);
+                return indestructibles.get(0);
             }
         }
         for (int ip = 0; ip < 6; ip++) { // priority 0 is the lowest, priority 5 the highest
             final int priority = 6 - ip;
-                for (Card card : list) {
+            for (Card card : remaining) {
                 if (!card.getSVar("SacMe").equals("") && Integer.parseInt(card.getSVar("SacMe")) == priority) {
-                        c = card;
+                    return card;
-                        break;
                 }
             }
         }
 
-            if (c == null) {
+        Card c;
-                if (CardLists.getNotType(list, "Creature").size() == 0) {
+
-                    c = CardFactoryUtil.getWorstCreatureAI(list);
+        if (CardLists.getNotType(remaining, "Creature").size() == 0) {
-                } else if (CardLists.getNotType(list, "Land").size() == 0) {
+            c = CardFactoryUtil.getWorstCreatureAI(remaining);
-                    c = CardFactoryUtil.getWorstLand(ai);
+        } else if (CardLists.getNotType(remaining, "Land").size() == 0) {
+            c = CardFactoryUtil.getWorstLand(CardLists.filter(remaining, CardPredicates.Presets.LANDS));
         } else {
-                    c = CardFactoryUtil.getWorstPermanentAI(list, false, false, false, false);
+            c = CardFactoryUtil.getWorstPermanentAI(remaining, false, false, false, false);
         }
 
         final ArrayList<Card> auras = c.getEnchantedBy();
@@ -683,26 +691,12 @@ public class ComputerUtil {
             // TODO: choose "worst" controlled enchanting Aura
             for (int j = 0; j < auras.size(); j++) {
                 final Card aura = auras.get(j);
-                        if (aura.getController().equals(c.getController()) && list.contains(aura)) {
+                if (aura.getController().equals(c.getController()) && remaining.contains(aura)) {
-                            c = aura;
+                    return aura;
-                            break;
                 }
             }
         }
-            }
+        return c;
-            if (destroy) {
-                if (!Singletons.getModel().getGame().getAction().destroy(c)) {
-                    continue;
-                }
-            } else {
-                if (!Singletons.getModel().getGame().getAction().sacrifice(c, source)) {
-                    continue;
-                }
-            }
-            list.remove(c);
-            sacList.add(c);
-        }
-        return sacList;
     }
 
     /**

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

@@ -37,7 +37,6 @@ import forge.Constant;
 import forge.GameEntity;
 import forge.Singletons;
 import forge.card.SpellManaCost;
-import forge.card.ability.effects.SacrificeEffect;
 import forge.card.cardfactory.CardFactoryUtil;
 import forge.card.cost.Cost;
 import forge.card.cost.CostUtil;
@@ -1247,37 +1246,34 @@ public class CombatUtil {
         // Annihilator:
         if (!c.getDamageHistory().getCreatureAttackedThisCombat()) {
             final ArrayList<String> kws = c.getKeyword();
-            final Pattern p = Pattern.compile("Annihilator [0-9]+");
-            Matcher m;
             for (final String key : kws) {
-                m = p.matcher(key);
+                if( !key.startsWith("Annihilator ") ) continue;
-                if (m.find()) {
+                final String[] k = key.split(" ", 2);
-                    final String[] k = key.split(" ");
                 final int a = Integer.valueOf(k[1]);
-                    final Card crd = c;
 
                 final Ability ability = new Ability(c, SpellManaCost.ZERO) {
                     @Override
                     public void resolve() {
-                            final Player cp = crd.getController();
                         final Player opponent = Singletons.getModel().getGame().getCombat().getDefendingPlayerRelatedTo(c);
-                            if (cp.isHuman()) {
+                        //List<Card> list = AbilityUtils.filterListByType(opponent.getCardsIn(ZoneType.Battlefield), "Permanent", this);
                         final List<Card> list = opponent.getCardsIn(ZoneType.Battlefield);
-                                ComputerUtil.sacrificePermanents(opponent,  a, list, false, this);
+                        List<Card> toSac = opponent.getController().choosePermanentsToSacrifice(list, a, this, false, false);
-                            } else {
+
-                                SacrificeEffect.sacrificeHuman(opponent, a, "Permanent", this, false, false);
+                        for(Card sacd : toSac) {
+                            final GameState game = Singletons.getModel().getGame(); 
+                            game.getAction().sacrifice(sacd, this);
                         }
+                        
                     }
                 };
-                    final StringBuilder sb = new StringBuilder();
+                String sb = String.format("Annihilator - Defending player sacrifices %d permanents.", a);
-                    sb.append("Annihilator - Defending player sacrifices ").append(a).append(" permanents.");
+                ability.setStackDescription(sb);
-                    ability.setStackDescription(sb.toString());
+                ability.setDescription(sb);
-                    ability.setDescription(sb.toString());
                 ability.setActivatingPlayer(c.getController());
                 ability.setTrigger(true);
 
                 Singletons.getModel().getGame().getStack().add(ability);
-                } // find
+
             } // for
         } // creatureAttacked
           // Annihilator

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

@@ -93,4 +93,6 @@ public abstract class PlayerController {
     public abstract Map<Card, Integer> assignCombatDamage(Card attacker, List<Card> blockers, int damageDealt, GameEntity defender);
 
     public abstract String announceRequirements(SpellAbility ability, String announce);
+
+    public abstract List<Card> choosePermanentsToSacrifice(List<Card> validTargets, int amount, SpellAbility sa, boolean destroy, boolean isOptional);
 }

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

@@ -180,5 +180,13 @@ public class PlayerControllerAi extends PlayerController {
         return null;
     }
 
+    /* (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, int amount, SpellAbility sa, boolean destroy, boolean isOptional) {
+        return ComputerUtil.choosePermanentsToSacrifice(player, validTargets, amount, sa, destroy, isOptional);
+    }
+
 
 }

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

@@ -1,5 +1,6 @@
 package forge.game.player;
 
+import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -210,5 +211,32 @@ public class PlayerControllerHuman extends PlayerController {
         return JOptionPane.showInputDialog(sb.toString());
     }
 
+    /* (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, int amount, SpellAbility sa, boolean destroy, boolean isOptional) {
+        List<Card> result = new ArrayList<Card>();
+
+        for (int i = 0; i < amount; i++) {
+            if (validTargets.isEmpty()) {
+                break;
+            }
+            Card c;
+            if (isOptional) {
+                c = GuiChoose.oneOrNone("Select a card to sacrifice", validTargets);
+            } else {
+                c = GuiChoose.one("Select a card to sacrifice", validTargets);
+            }
+            if (c != null) {
+                result.add(c);
+                validTargets.remove(c);
+            } else {
+                return result;
+            }
+        }
+        return result;
+    }
+
 
 }