Fix deadlock for impossible blocking requirements

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

@@ -368,7 +368,7 @@ public class AiBlockController {
                         // if the total damage of the blockgang was not enough
                         // without but is enough with this blocker finish the blockgang
                         if (ComputerUtilCombat.totalFirstStrikeDamageOfBlockers(attacker, blockGang) < damageNeeded
-                                || CombatUtil.needsBlockers(attacker) > blockGang.size()) {
+                                || CombatUtil.getMinNumBlockersForAttacker(attacker, ai) > blockGang.size()) {
                             blockGang.add(blocker);
                             if (ComputerUtilCombat.totalFirstStrikeDamageOfBlockers(attacker, blockGang) >= damageNeeded) {
                                 currentAttackers.remove(attacker);
@@ -406,7 +406,7 @@ public class AiBlockController {
             boolean foundDoubleBlock = false; // if true, a good double block is found
 
             // AI can't handle good blocks with more than three creatures yet
-            if (CombatUtil.needsBlockers(attacker) > (considerTripleBlock ? 3 : 2)) {
+            if (CombatUtil.getMinNumBlockersForAttacker(attacker, ai) > (considerTripleBlock ? 3 : 2)) {
                 continue;
             }
 
@@ -443,7 +443,7 @@ public class AiBlockController {
                 final int addedValue = ComputerUtilCard.evaluateCreature(blocker);
                 final int damageNeeded = ComputerUtilCombat.getDamageToKill(attacker)
                         + ComputerUtilCombat.predictToughnessBonusOfAttacker(attacker, blocker, combat, false);
-                if ((damageNeeded > currentDamage || CombatUtil.needsBlockers(attacker) > blockGang.size())
+                if ((damageNeeded > currentDamage || CombatUtil.getMinNumBlockersForAttacker(attacker, ai) > blockGang.size())
                         && !(damageNeeded > currentDamage + additionalDamage)
                         // The attacker will be killed
                         && (absorbedDamage2 + absorbedDamage > attacker.getNetCombatDamage()
@@ -494,7 +494,7 @@ public class AiBlockController {
                     final int addedValue3 = ComputerUtilCard.evaluateCreature(secondBlocker);
                     final int netCombatDamage = attacker.getNetCombatDamage();
 
-                    if ((damageNeeded > currentDamage || CombatUtil.needsBlockers(attacker) > blockGang.size())
+                    if ((damageNeeded > currentDamage || CombatUtil.getMinNumBlockersForAttacker(attacker, ai) > blockGang.size())
                             && !(damageNeeded > currentDamage + additionalDamage2 + additionalDamage3)
                             // The attacker will be killed
                             && ((absorbedDamage2 + absorbedDamage > netCombatDamage && absorbedDamage3 + absorbedDamage > netCombatDamage
@@ -1093,7 +1093,7 @@ public class AiBlockController {
         chumpBlockers.addAll(CardLists.getKeyword(blockersLeft, "CARDNAME blocks each combat if able."));
         // if an attacker with lure attacks - all that can block
         for (final Card blocker : blockersLeft) {
-            if (CombatUtil.mustBlockAnAttacker(blocker, combat)) {
+            if (CombatUtil.mustBlockAnAttacker(blocker, combat, null)) {
                 chumpBlockers.add(blocker);
             }
         }
@@ -1103,7 +1103,7 @@ public class AiBlockController {
                 blockers = getPossibleBlockers(combat, attacker, chumpBlockers, false);
                 for (final Card blocker : blockers) {
                     if (CombatUtil.canBlock(attacker, blocker, combat) && blockersLeft.contains(blocker)
-                            && (CombatUtil.mustBlockAnAttacker(blocker, combat)
+                            && (CombatUtil.mustBlockAnAttacker(blocker, combat, null)
                                     || blocker.hasKeyword("CARDNAME blocks each turn if able.")
                                     || blocker.hasKeyword("CARDNAME blocks each combat if able."))) {
                         combat.addBlocker(attacker, blocker);
@@ -1122,7 +1122,6 @@ public class AiBlockController {
             }
         }
 
-
         // check to see if it's possible to defend a Planeswalker under attack with a chump block,
         // unless life is low enough to be more worried about saving preserving the life total
         if (ai.getController().isAI() && !ComputerUtilCombat.lifeInDanger(ai, combat)) {

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

@@ -412,7 +412,6 @@ public class ComputerUtilCombat {
             return false;
         }
 
-
         // check for creatures that must be blocked
         final List<Card> attackers = combat.getAttackersOf(ai);
 

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

@@ -31,9 +31,7 @@ public class CamouflageEffect extends SpellAbilityEffect {
                 }
             }
 
-            if (attacker.hasKeyword("CARDNAME can't be blocked unless all creatures defending player controls block it.") &&
+            if (blockers.size() < CombatUtil.getMinNumBlockersForAttacker(attacker, defender)) {
-                    blockers.size() < defender.getCreaturesInPlay().size() ||
-                    blockers.size() < CombatUtil.needsBlockers(attacker)) {
                 // If not enough remaining creatures to block, don't add them as blocker
                 continue;
             }

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

@@ -412,10 +412,10 @@ public class CombatUtil {
             return false;
         }
         if (combat == null) {
-            return CombatUtil.canBlock(blocker);
+            return canBlock(blocker);
         }
 
-        if (!CombatUtil.canBlockMoreCreatures(blocker, combat.getAttackersBlockedBy(blocker))) {
+        if (!canBlockMoreCreatures(blocker, combat.getAttackersBlockedBy(blocker))) {
             return false;
         }
         final Game game = blocker.getGame();
@@ -434,7 +434,7 @@ public class CombatUtil {
             return false;
         }
 
-        return CombatUtil.canBlock(blocker);
+        return canBlock(blocker);
     }
 
     // can the creature block at all?
@@ -517,7 +517,7 @@ public class CombatUtil {
                 return false;
             }
         }
-        return CombatUtil.canBeBlocked(attacker, defendingPlayer);
+        return canBeBlocked(attacker, defendingPlayer);
     }
 
     // can the attacker be blocked at all?
@@ -642,7 +642,7 @@ public class CombatUtil {
      */
     public static boolean canBlockAtLeastOne(final Card blocker, final Iterable<Card> attackers) {
         for (Card attacker : attackers) {
-            if (CombatUtil.canBlock(attacker, blocker)) {
+            if (canBlock(attacker, blocker)) {
                 return true;
             }
         }
@@ -661,7 +661,7 @@ public class CombatUtil {
     public static boolean canBeBlocked(final Card attacker, final List<Card> blockers, final Combat combat) {
         int blocks = 0;
         for (final Card blocker : blockers) {
-            if (CombatUtil.canBeBlocked(attacker, blocker.getController()) && CombatUtil.canBlock(attacker, blocker)) {
+            if (canBeBlocked(attacker, blocker.getController()) && canBlock(attacker, blocker)) {
                 blocks++;
             }
         }
@@ -676,7 +676,7 @@ public class CombatUtil {
         }
 
         for (final Card blocker : blockers) {
-            if (CombatUtil.canBeBlocked(attacker, blocker.getController()) && CombatUtil.canBlock(attacker, blocker)) {
+            if (canBeBlocked(attacker, blocker.getController()) && canBlock(attacker, blocker)) {
                 potentialBlockers.add(blocker);
             }
         }
@@ -693,27 +693,29 @@ public class CombatUtil {
         return minBlockerList;
     }
 
-    /**
+    // return all creatures that could help satisfy a blocking requirement without breaking another
-     * <p>
+    // TODO according to 509.1c, this should really check if the maximum possible is already fulfilled
-     * needsMoreBlockers.
+    public static List<Card> findFreeBlockers(List<Card> defendersArmy, List<Card> attackers, Combat combat) {
-     * </p>
+        final CardCollection freeBlockers = new CardCollection();
-     * 
+        for (Card blocker : defendersArmy) {
-     * @param attacker
+            if (canBlock(blocker) && !mustBlockAnAttacker(blocker, combat, null)) {
-     *            a {@link forge.game.card.Card} object.
+                CardCollection blockedAttackers = combat.getAttackersBlockedBy(blocker);
-     * @return a boolean.
+                boolean blockChange = blockedAttackers.isEmpty();
-     */
+                for (Card attacker : blockedAttackers) {
-    public static int needsBlockers(final Card attacker) {
+                    // check if we could unblock something
-
+                    List<Card> blockersReduced = Lists.newArrayList(combat.getBlockers(attacker));
-        if (attacker == null) {
+                    blockersReduced.remove(blocker);
-            return 0;
+                    if (canBlockMoreCreatures(blocker, blockedAttackers) || canBeBlocked(attacker, blockersReduced, combat)) {
+                        blockChange = true;
+                        break;
+                    }
+                }
+                if (blockChange) {
+                    freeBlockers.add(blocker);
+                }
+            }
         }
-        // TODO: remove CantBeBlockedByAmount LT2
+        return freeBlockers;
-        if (attacker.hasKeyword("CantBeBlockedByAmount LT2") || attacker.hasKeyword(Keyword.MENACE)) {
-            return 2;
-        } else if (attacker.hasKeyword("CantBeBlockedByAmount LT3")) {
-            return 3;
-        } else
-            return 1;
     }
 
     // Has the human player chosen all mandatory blocks?
@@ -730,32 +732,44 @@ public class CombatUtil {
         final List<Card> defendersArmy = defending.getCreaturesInPlay();
         final List<Card> attackers = combat.getAttackers();
         final List<Card> blockers = CardLists.filterControlledBy(combat.getAllBlockers(), defending);
+        final List<Card> freeBlockers = findFreeBlockers(defendersArmy, attackers, combat);
 
         // if a creature does not block but should, return false
         for (final Card blocker : defendersArmy) {
             if (blocker.getMustBlockCards() != null) {
-               final CardCollectionView blockedSoFar = combat.getAttackersBlockedBy(blocker);
+                final CardCollectionView blockedSoFar = combat.getAttackersBlockedBy(blocker);
-               for (Card cardToBeBlocked : blocker.getMustBlockCards()) {
+                for (Card cardToBeBlocked : blocker.getMustBlockCards()) {
-                 if (!blockedSoFar.contains(cardToBeBlocked) && CombatUtil.canBlockMoreCreatures(blocker, blockedSoFar) 
+                    int additionalBlockers = getMinNumBlockersForAttacker(cardToBeBlocked, defending) -1;
-                         && combat.isAttacking(cardToBeBlocked) && CombatUtil.canBlock(cardToBeBlocked, blocker)) {
+                    int potentialBlockers = 0;
-                     return TextUtil.concatWithSpace(blocker.toString(),"must still block", TextUtil.addSuffix(cardToBeBlocked.toString(),"."));
+                    // if the attacker can only be blocked with multiple creatures check if that's possible
-                 }
+                    for (int i = 0; i < additionalBlockers; i++) {
-               } 
+                        for (Card freeBlocker: new CardCollection(freeBlockers)) {
+                            if (freeBlocker != blocker && canBlock(cardToBeBlocked, freeBlocker)) {
+                                freeBlockers.remove(freeBlocker);
+                                potentialBlockers++;
+                            }
+                        }
+                    }
+                    if (potentialBlockers >= additionalBlockers && !blockedSoFar.contains(cardToBeBlocked) && canBlockMoreCreatures(blocker, blockedSoFar) 
+                            && combat.isAttacking(cardToBeBlocked) && canBlock(cardToBeBlocked, blocker)) {
+                        return TextUtil.concatWithSpace(blocker.toString(),"must still block", TextUtil.addSuffix(cardToBeBlocked.toString(),"."));
+                    }
+                } 
             }
             // lure effects
-            if (!blockers.contains(blocker) && CombatUtil.mustBlockAnAttacker(blocker, combat)) {
+            if (!blockers.contains(blocker) && mustBlockAnAttacker(blocker, combat, freeBlockers)) {
                 return TextUtil.concatWithSpace(blocker.toString(),"must block an attacker, but has not been assigned to block any.");
             }
 
             // "CARDNAME blocks each turn/combat if able."
             if (!blockers.contains(blocker) && (blocker.hasKeyword("CARDNAME blocks each turn if able.") || blocker.hasKeyword("CARDNAME blocks each combat if able."))) {
                 for (final Card attacker : attackers) {
-                    if (CombatUtil.canBlock(attacker, blocker, combat)) {
+                    if (canBlock(attacker, blocker, combat)) {
                         boolean must = true;
                         if (attacker.hasStartOfKeyword("CantBeBlockedByAmount LT") || attacker.hasKeyword(Keyword.MENACE)) {
                             final List<Card> possibleBlockers = Lists.newArrayList(defendersArmy);
                             possibleBlockers.remove(blocker);
-                            if (!CombatUtil.canBeBlocked(attacker, possibleBlockers, combat)) {
+                            if (!canBeBlocked(attacker, possibleBlockers, combat)) {
                                 must = false;
                             }
                         }
@@ -813,12 +827,12 @@ public class CombatUtil {
      *            a {@link forge.game.combat.Combat} object.
      * @return a boolean.
      */
-    public static boolean mustBlockAnAttacker(final Card blocker, final Combat combat) {
+    public static boolean mustBlockAnAttacker(final Card blocker, final Combat combat, List<Card> freeBlockers) {
         if (blocker == null || combat == null) {
             return false;
         }
 
-        if (!CombatUtil.canBlock(blocker, combat)) {
+        if (!canBlock(blocker, combat)) {
             return false;
         }
 
@@ -859,12 +873,12 @@ public class CombatUtil {
 
         final Player defender = blocker.getController();
         for (final Card attacker : attackersWithLure) {
-            if (CombatUtil.canBeBlocked(attacker, combat, defender) && CombatUtil.canBlock(attacker, blocker)) {
+            if (canBeBlocked(attacker, combat, defender) && canBlock(attacker, blocker)) {
                 boolean canBe = true;
                 if (attacker.hasStartOfKeyword("CantBeBlockedByAmount LT") || attacker.hasKeyword(Keyword.MENACE)) {
                     final List<Card> blockers = combat.getDefenderPlayerByAttacker(attacker).getCreaturesInPlay();
                     blockers.remove(blocker);
-                    if (!CombatUtil.canBeBlocked(attacker, blockers, combat)) {
+                    if (!canBeBlocked(attacker, blockers, combat)) {
                         canBe = false;
                     }
                 }
@@ -876,13 +890,13 @@ public class CombatUtil {
 
         if (blocker.getMustBlockCards() != null) {
             for (final Card attacker : blocker.getMustBlockCards()) {
-                if (CombatUtil.canBeBlocked(attacker, combat, defender) && CombatUtil.canBlock(attacker, blocker)
+                if (canBeBlocked(attacker, combat, defender) && canBlock(attacker, blocker)
                         && combat.isAttacking(attacker)) {
                     boolean canBe = true;
                     if (attacker.hasStartOfKeyword("CantBeBlockedByAmount LT") || attacker.hasKeyword(Keyword.MENACE)) {
-                        final List<Card> blockers = combat.getDefenderPlayerByAttacker(attacker).getCreaturesInPlay();
+                        final List<Card> blockers = freeBlockers != null ? new CardCollection(freeBlockers) : combat.getDefenderPlayerByAttacker(attacker).getCreaturesInPlay();
                         blockers.remove(blocker);
-                        if (!CombatUtil.canBeBlocked(attacker, blockers, combat)) {
+                        if (!canBeBlocked(attacker, blockers, combat)) {
                             canBe = false;
                         }
                     }
@@ -917,7 +931,7 @@ public class CombatUtil {
 
         for (Card c : creatures) {
             for (Card a : attackers) {
-                if (CombatUtil.canBlock(a, c, combat)) {
+                if (canBlock(a, c, combat)) {
                     return true;
                 }
             }
@@ -944,10 +958,10 @@ public class CombatUtil {
             return false;
         }
 
-        if (!CombatUtil.canBlock(blocker, combat)) {
+        if (!canBlock(blocker, combat)) {
             return false;
         }
-        if (!CombatUtil.canBeBlocked(attacker, combat, blocker.getController())) {
+        if (!canBeBlocked(attacker, combat, blocker.getController())) {
             return false;
         }
         if (combat != null && combat.isBlocking(blocker, attacker)) { // Can't block if already blocking the attacker
@@ -984,11 +998,11 @@ public class CombatUtil {
                 && !(attacker.hasKeyword("CARDNAME must be blocked by two or more creatures if able.") && combat.getBlockers(attacker).size() < 2)
                 && !(blocker.getMustBlockCards() != null && blocker.getMustBlockCards().contains(attacker))
                 && !mustBeBlockedBy
-                && CombatUtil.mustBlockAnAttacker(blocker, combat)) {
+                && mustBlockAnAttacker(blocker, combat, null)) {
             return false;
         }
 
-        return CombatUtil.canBlock(attacker, blocker);
+        return canBlock(attacker, blocker);
     }
 
     // can the blocker block the attacker?
@@ -1025,10 +1039,10 @@ public class CombatUtil {
         }
 
         final Game game = attacker.getGame();
-        if (!CombatUtil.canBlock(blocker, nextTurn)) {
+        if (!canBlock(blocker, nextTurn)) {
             return false;
         }
-        if (!CombatUtil.canBeBlocked(attacker, blocker.getController())) {
+        if (!canBeBlocked(attacker, blocker.getController())) {
             return false;
         }
         
@@ -1091,11 +1105,10 @@ public class CombatUtil {
                 // TODO: a better fix is needed here (to prevent a hard NPE, e.g. when the AI attacks with Tromokratis).
                 System.out.println("Warning: defender was 'null' in CombatUtil::canAttackerBeBlockedWithAmount for the card " + attacker + ", attempting to deduce defender.");
                 defender = combat.getDefendingPlayers().getFirst();
-                if (defender != null) {
+                if (defender == null) {
-                    return amount >= defender.getCreaturesInPlay().size();
+                    System.out.println("Warning: it was impossible to deduce the defending player in CombatUtil#canAttackerBeBlockedWithAmount, returning 'true' (safest default).");
+                    return true;
                 }
-                System.out.println("Warning: it was impossible to deduce the defending player in CombatUtil#canAttackerBeBlockedWithAmount, returning 'true' (safest default).");
-                return true;
             }
             return amount >= defender.getCreaturesInPlay().size();
         }