AI: improved combat support:

* added support to attack battle permanents (#10246);
* fixed that AI was able to attack multiple targets by single creature (#7434);
* added docs;
* added todos with another bugs or possible problems with AI combat;

Mage.Server.Plugins/Mage.Player.AI.MA/src/mage/player/ai/ComputerPlayer6.java

@@ -43,7 +43,7 @@ import java.util.stream.Collectors;
 /**
  * AI: server side bot with game simulations (mad bot, part of implementation)
  *
- * @author nantuko
+ * @author nantuko, JayDi85
  */
 public class ComputerPlayer6 extends ComputerPlayer {
 
@@ -887,7 +887,7 @@ public class ComputerPlayer6 extends ComputerPlayer {
         if (!game.replaceEvent(GameEvent.getEvent(GameEvent.EventType.DECLARING_ATTACKERS, activePlayerId, activePlayerId))) {
             Player attackingPlayer = game.getPlayer(activePlayerId);
 
-            // check alpha strike first (all in attack to kill)
+            // check alpha strike first (all in attack to kill a player)
             for (UUID defenderId : game.getOpponents(playerId)) {
                 Player defender = game.getPlayer(defenderId);
                 if (!defender.isInGame()) {
@@ -908,7 +908,9 @@ public class ComputerPlayer6 extends ComputerPlayer {
                 }
             }
 
-            // check all other actions
+            // TODO: add game simulations here to find best attackers/blockers combination
+
+            // find safe attackers (can't be killed by blockers)
             for (UUID defenderId : game.getOpponents(playerId)) {
                 Player defender = game.getPlayer(defenderId);
                 if (!defender.isInGame()) {
@@ -983,37 +985,64 @@ public class ComputerPlayer6 extends ComputerPlayer {
                     }
                 }
 
-                // now we have a list of all attackers that can safely attack:
-                // first check to see if any Planeswalkers can be killed
+                // find possible target for attack (priority: planeswalker -> battle -> player)
                 int totalPowerOfAttackers = 0;
-                int loyaltyCounters = 0;
-                for (Permanent planeswalker : game.getBattlefield().getAllActivePermanents(StaticFilters.FILTER_PERMANENT_PLANESWALKER, defender.getId(), game)) {
-                    if (planeswalker != null) {
-                        loyaltyCounters = planeswalker.getCounters(game).getCount(CounterType.LOYALTY);
-                        // verify the attackers can kill the planeswalker, otherwise attack the player
+                int usedPowerOfAttackers = 0;
                 for (Permanent attacker : attackersToCheck) {
                     totalPowerOfAttackers += attacker.getPower().getValue();
                 }
-                        if (totalPowerOfAttackers < loyaltyCounters) {
+
+                // TRY ATTACK PLANESWALKER + BATTLE
+                List<Permanent> possiblePermanentDefenders = new ArrayList<>();
+                // planeswalker first priority
+                game.getBattlefield().getActivePermanents(StaticFilters.FILTER_PERMANENT_PLANESWALKER, activePlayerId, game)
+                        .stream()
+                        .filter(p -> p.canBeAttacked(null, defenderId, game))
+                        .forEach(possiblePermanentDefenders::add);
+                // battle second priority
+                game.getBattlefield().getActivePermanents(StaticFilters.FILTER_PERMANENT_BATTLE, activePlayerId, game)
+                        .stream()
+                        .filter(p -> p.canBeAttacked(null, defenderId, game))
+                        .forEach(possiblePermanentDefenders::add);
+
+                for (Permanent permanentDefender : possiblePermanentDefenders) {
+                    if (usedPowerOfAttackers >= totalPowerOfAttackers) {
                         break;
                     }
-                        // kill the Planeswalker
-                        for (Permanent attacker : attackersToCheck) {
-                            loyaltyCounters -= attacker.getPower().getValue();
-                            attackingPlayer.declareAttacker(attacker.getId(), planeswalker.getId(), game, true);
-                            if (loyaltyCounters <= 0) {
-                                break;
+                    int currentCounters;
+                    if (permanentDefender.isPlaneswalker(game)) {
+                        currentCounters = permanentDefender.getCounters(game).getCount(CounterType.LOYALTY);
+                    } else if (permanentDefender.isBattle(game)) {
+                        currentCounters = permanentDefender.getCounters(game).getCount(CounterType.DEFENSE);
+                    } else {
+                        // impossible error (SBA must remove all planeswalkers/battles with 0 counters before declare attackers)
+                        throw new IllegalStateException("AI: can't find counters for defending permanent " + permanentDefender.getName(), new Throwable());
                     }
+
+                    // attack anyway (for kill or damage)
+                    // TODO: add attackers optimization here (1 powerfull + min number of additional permanents,
+                    //  current code uses random/etb order)
+                    for (Permanent attackingPermanent : attackersToCheck) {
+                        if (attackingPermanent.isAttacking()) {
+                            // already used for another target
+                            continue;
+                        }
+                        attackingPlayer.declareAttacker(attackingPermanent.getId(), permanentDefender.getId(), game, true);
+                        currentCounters -= attackingPermanent.getPower().getValue();
+                        usedPowerOfAttackers += attackingPermanent.getPower().getValue();
+                        if (currentCounters <= 0) {
+                            break;
                         }
                     }
                 }
 
+                // TRY ATTACK PLAYER
                 // any remaining attackers go for the player
                 for (Permanent attackingPermanent : attackersToCheck) {
-                    // if not already attacking a Planeswalker...
-                    if (!attackingPermanent.isAttacking()) {
-                        attackingPlayer.declareAttacker(attackingPermanent.getId(), defenderId, game, true);
+                    if (attackingPermanent.isAttacking()) {
+                        continue;
                     }
+                    attackingPlayer.declareAttacker(attackingPermanent.getId(), defenderId, game, true);
                 }
             }
         }

Mage.Server.Plugins/Mage.Player.AI.MA/src/mage/player/ai/ComputerPlayer7.java

@@ -44,7 +44,7 @@ public class ComputerPlayer7 extends ComputerPlayer6 {
     private boolean priorityPlay(Game game) {
         if (lastLoggedTurn != game.getTurnNum()) {
             lastLoggedTurn = game.getTurnNum();
-            logger.info("======================= Turn: " + game.getTurnNum() + " [" + game.getPlayer(game.getActivePlayerId()).getName() + "] =========================================");
+            logger.info("======================= Turn: " + game.getState().toString() + " [" + game.getPlayer(game.getActivePlayerId()).getName() + "] =========================================");
         }
         logState(game);
         logger.debug("Priority -- Step: " + (game.getTurnStepType() + "                       ").substring(0, 25) + " ActivePlayer-" + game.getPlayer(game.getActivePlayerId()).getName() + " PriorityPlayer-" + name);

Mage.Server.Plugins/Mage.Player.AI.MA/src/mage/player/ai/util/CombatUtil.java

@@ -198,9 +198,13 @@ public final class CombatUtil {
         return blockers;
     }
 
+    /**
+     * @deprecated TODO: unused, can be deleted?
+     */
     public static SurviveInfo willItSurvive(Game game, UUID attackingPlayerId, UUID defendingPlayerId, Permanent attacker, Permanent blocker) {
         Game sim = game.copy();
 
+        // TODO: bugged, miss combat.clear code (possible bugs - wrong blocker declare by AI on multiple options?)
         Combat combat = sim.getCombat();
         combat.setAttacker(attackingPlayerId);
         combat.setDefenders(sim);
@@ -232,40 +236,6 @@ public final class CombatUtil {
         return new SurviveInfo(!sim.getBattlefield().containsPermanent(attacker.getId()), !sim.getBattlefield().containsPermanent(blocker.getId()));
     }
 
-    public static SurviveInfo getCombatInfo(Game game, UUID attackingPlayerId, UUID defendingPlayerId, Permanent attacker) {
-        Game sim = game.copy();
-
-        Combat combat = sim.getCombat();
-        combat.setAttacker(attackingPlayerId);
-        combat.setDefenders(sim);
-
-        UUID defenderId = sim.getCombat().getDefenders().iterator().next();
-        boolean triggered = false;
-
-        sim.fireEvent(GameEvent.getEvent(GameEvent.EventType.DECLARED_BLOCKERS, defendingPlayerId, defendingPlayerId));
-
-        sim.checkStateAndTriggered();
-        while (!sim.getStack().isEmpty()) {
-            triggered = true;
-            sim.getStack().resolve(sim);
-            sim.applyEffects();
-        }
-        sim.fireEvent(GameEvent.getEvent(GameEvent.EventType.DECLARE_BLOCKERS_STEP_POST, sim.getActivePlayerId(), sim.getActivePlayerId()));
-
-        simulateStep(sim, new CombatDamageStep(true));
-        simulateStep(sim, new CombatDamageStep(false));
-        simulateStep(sim, new EndOfCombatStep());
-        // The following commented out call produces random freezes.
-        //sim.checkStateAndTriggered();
-        while (!sim.getStack().isEmpty()) {
-            triggered = true;
-            sim.getStack().resolve(sim);
-            sim.applyEffects();
-        }
-
-        return new SurviveInfo(!sim.getBattlefield().containsPermanent(attacker.getId()), false, sim.getPlayer(defenderId), triggered);
-    }
-
     protected static void simulateStep(Game game, Step step) {
         game.getPhase().setStep(step);
         if (!step.skipStep(game, game.getActivePlayerId())) {
@@ -339,6 +309,7 @@ public final class CombatUtil {
 
         Game sim = game.copy();
 
+        // TODO: bugged, miss combat.clear code (possible bugs - wrong blocker declare by AI on multiple options?)
         Combat combat = sim.getCombat();
         combat.setAttacker(attackingPlayerId);
         combat.setDefenders(sim);

Mage.Tests/src/test/java/org/mage/test/cards/battle/BattleBaseTest.java

@@ -3,20 +3,22 @@ package org.mage.test.cards.battle;
 import mage.game.permanent.Permanent;
 import mage.players.Player;
 import org.junit.jupiter.api.Assertions;
-import org.mage.test.serverside.base.CardTestPlayerBase;
+import org.mage.test.serverside.base.CardTestPlayerBaseWithAIHelps;
 
 /**
  * @author TheElk801
  */
-public class BattleBaseTest extends CardTestPlayerBase {
+public class BattleBaseTest extends CardTestPlayerBaseWithAIHelps {
 
     protected static final String belenon = "Invasion of Belenon";
     protected static final String warAnthem = "Belenon War Anthem";
     protected static final String kaladesh = "Invasion of Kaladesh";
     protected static final String bear = "Grizzly Bears";
+    protected static final String bearWithFlyingAndVigilance = "Abbey Griffin";
     protected static final String confiscate = "Confiscate";
     protected static final String impact = "Explosive Impact";
     protected static final String stifle = "Stifle";
+    protected static final String fayden = "Dack Fayden";
 
     protected void assertBattle(Player controller, Player protector, String name) {
         assertPermanentCount(controller, name, 1);

Mage.Tests/src/test/java/org/mage/test/cards/battle/BattleDuelTest.java

@@ -6,7 +6,7 @@ import mage.counters.CounterType;
 import org.junit.Test;
 
 /**
- * @author TheElk801
+ * @author TheElk801, JayDi85
  */
 public class BattleDuelTest extends BattleBaseTest {
 
@@ -155,6 +155,7 @@ public class BattleDuelTest extends BattleBaseTest {
         assertGraveyardCount(playerA, kaladesh, 1);
         assertPermanentCount(playerA, "Thopter Token", 1);
     }
+
     @Test
     public void testSpellCardTypeTrigger() {
         addCard(Zone.BATTLEFIELD, playerA, "Plateau", 3 + 6);
@@ -176,4 +177,91 @@ public class BattleDuelTest extends BattleBaseTest {
         assertPermanentCount(playerA, "Warrior Token", 1);
         assertPowerToughness(playerA, "Deeproot Champion", 3, 3);
     }
+
+    @Test
+    public void test_AI_CastBattle() {
+        addCard(Zone.BATTLEFIELD, playerA, "Plains", 3);
+        addCard(Zone.HAND, playerA, belenon);
+
+        aiPlayPriority(1, PhaseStep.PRECOMBAT_MAIN, playerA);
+
+        setStrictChooseMode(true);
+        setStopAt(1, PhaseStep.END_TURN);
+        execute();
+
+        assertBattle(playerA, playerB, belenon);
+        assertPermanentCount(playerA, "Knight Token", 1);
+    }
+
+    @Test
+    public void test_AI_AttackPriority_TargetBattleInsteadPlayer() {
+        addCard(Zone.BATTLEFIELD, playerA, "Plains", 3);
+        addCard(Zone.BATTLEFIELD, playerA, bear);
+        addCard(Zone.HAND, playerA, belenon);
+
+        castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, belenon);
+
+        // ai must attack planeswalker instead player
+        aiPlayStep(1, PhaseStep.DECLARE_ATTACKERS, playerA);
+
+        setStrictChooseMode(true);
+        setStopAt(1, PhaseStep.END_TURN);
+        execute();
+
+        assertBattle(playerA, playerB, belenon);
+        assertPermanentCount(playerA, "Knight Token", 1);
+        assertTapped(bear, true);
+        assertLife(playerB, 20);
+        assertCounterCount(belenon, CounterType.DEFENSE, 5 - 2);
+    }
+
+    @Test
+    public void test_AI_AttackPriority_TargetPlaneswalkerInsteadBattle() {
+        addCard(Zone.BATTLEFIELD, playerA, "Plains", 3);
+        addCard(Zone.BATTLEFIELD, playerA, bear);
+        addCard(Zone.BATTLEFIELD, playerB, fayden); // 3
+        addCard(Zone.HAND, playerA, belenon);
+
+        castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, belenon);
+
+        // ai must attack planeswalker instead battle/player
+        aiPlayStep(1, PhaseStep.DECLARE_ATTACKERS, playerA);
+
+        setStrictChooseMode(true);
+        setStopAt(1, PhaseStep.END_TURN);
+        execute();
+
+        assertBattle(playerA, playerB, belenon);
+        assertPermanentCount(playerA, "Knight Token", 1);
+        assertTapped(bear, true);
+        assertLife(playerB, 20);
+        assertCounterCount(belenon, CounterType.DEFENSE, 5);
+        assertCounterCount(fayden, CounterType.LOYALTY, 3 - 2);
+    }
+
+    @Test
+    public void test_AI_MustNotAttackMultipleTargets() {
+        // bug with multiple targets for single attacker: https://github.com/magefree/mage/issues/7434
+        addCard(Zone.BATTLEFIELD, playerA, "Plains", 3);
+        addCard(Zone.BATTLEFIELD, playerA, bearWithFlyingAndVigilance);
+        addCard(Zone.BATTLEFIELD, playerB, fayden); // 3
+        addCard(Zone.HAND, playerA, belenon);
+
+        castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, belenon);
+
+        // ai must attack planeswalker instead battle/player
+        // ai must attack only single target
+        aiPlayStep(1, PhaseStep.DECLARE_ATTACKERS, playerA);
+
+        setStrictChooseMode(true);
+        setStopAt(1, PhaseStep.END_TURN);
+        execute();
+
+        assertBattle(playerA, playerB, belenon);
+        assertPermanentCount(playerA, "Knight Token", 1);
+        assertTapped(bearWithFlyingAndVigilance, false); // vigilance
+        assertLife(playerB, 20);
+        assertCounterCount(belenon, CounterType.DEFENSE, 5);
+        assertCounterCount(fayden, CounterType.LOYALTY, 3 - 2);
+    }
 }

Mage.Tests/src/test/java/org/mage/test/serverside/base/CardTestCommander4PlayersWithAIHelps.java

@@ -14,7 +14,7 @@ public abstract class CardTestCommander4PlayersWithAIHelps extends CardTestComma
     @Override
     protected TestPlayer createPlayer(String name, RangeOfInfluence rangeOfInfluence) {
         // use same RangeOfInfluence.ALL as CardTestCommander4Players do
-        TestPlayer testPlayer = new TestPlayer(new TestComputerPlayer7(name, RangeOfInfluence.ALL, 6));
+        TestPlayer testPlayer = new TestPlayer(new TestComputerPlayer7(name, rangeOfInfluence, 6));
         testPlayer.setAIPlayer(false); // AI can't play it by itself, use AI commands
         return testPlayer;
     }

Mage.Tests/src/test/java/org/mage/test/serverside/base/CardTestPlayerBaseWithAIHelps.java

@@ -15,7 +15,7 @@ public abstract class CardTestPlayerBaseWithAIHelps extends CardTestPlayerBase {
 
     @Override
     protected TestPlayer createPlayer(String name, RangeOfInfluence rangeOfInfluence) {
-        TestPlayer testPlayer = new TestPlayer(new TestComputerPlayer7(name, RangeOfInfluence.ONE, 6));
+        TestPlayer testPlayer = new TestPlayer(new TestComputerPlayer7(name, rangeOfInfluence, 6));
         testPlayer.setAIPlayer(false); // AI can't play it by itself, use AI commands
         return testPlayer;
     }

Mage.Tests/src/test/java/org/mage/test/serverside/base/impl/CardTestPlayerAPIImpl.java

@@ -1681,6 +1681,8 @@ public abstract class CardTestPlayerAPIImpl extends MageTestPlayerBase implement
      * AI play STEP to the end with multi game simulations (calcs and play best
      * actions until step ends, can be called in the middle of the step) All
      * choices must be made by AI (e.g. strict mode possible)
+     * <p>
+     * Can be used for AI's declare of attackers/blockers
      */
     public void aiPlayStep(int turnNum, PhaseStep step, TestPlayer player) {
         assertAiPlayAndGameCompatible(player);

Mage/src/main/java/mage/abilities/effects/ContinuousEffects.java

@@ -247,10 +247,10 @@ public class ContinuousEffects implements Serializable {
                 .collect(Collectors.toList());
     }
 
-    public Map<RequirementEffect, Set<Ability>> getApplicableRequirementEffects(Permanent permanent, boolean playerRealted, Game game) {
+    public Map<RequirementEffect, Set<Ability>> getApplicableRequirementEffects(Permanent permanent, boolean playerRelated, Game game) {
         Map<RequirementEffect, Set<Ability>> effects = new HashMap<>();
         for (RequirementEffect effect : requirementEffects) {
-            if (playerRealted == effect.isPlayerRelated()) {
+            if (playerRelated == effect.isPlayerRelated()) {
                 Set<Ability> abilities = requirementEffects.getAbility(effect.getId());
                 Set<Ability> applicableAbilities = new HashSet<>();
                 for (Ability ability : abilities) {

Mage/src/main/java/mage/abilities/effects/RequirementEffect.java

@@ -58,6 +58,7 @@ public abstract class RequirementEffect extends ContinuousEffectImpl {
 
     /**
      * Defines the defender a attacker has to attack
+     * TODO: WTF, it bugged, usage code need only players to attack, but effects return controller, source id, other broken data!!!
      *
      * @param source
      * @param game

Mage/src/main/java/mage/game/combat/Combat.java

@@ -60,7 +60,7 @@ public class Combat implements Serializable, Copyable<Combat> {
 
     protected List<CombatGroup> groups = new ArrayList<>();
     protected Map<UUID, CombatGroup> blockingGroups = new HashMap<>();
-    // player and planeswalker ids
+    // all possible defenders (players, planeswalkers or battle)
     protected Set<UUID> defenders = new HashSet<>();
     // how many creatures attack defending player
     protected Map<UUID, Set<UUID>> numberCreaturesDefenderAttackedBy = new HashMap<>();
@@ -115,7 +115,7 @@ public class Combat implements Serializable, Copyable<Combat> {
     }
 
     /**
-     * Get all possible defender (players and planeswalkers) That does not mean
+     * Get all possible defender (players, planeswalkers and battles) That does not mean
      * necessarily mean that they are really attacked
      *
      * @return
@@ -447,13 +447,18 @@ public class Combat implements Serializable, Copyable<Combat> {
         game.informPlayers(sb.toString());
     }
 
+    /**
+     * Force "must attack" creatures to attack
+     */
     protected void checkAttackRequirements(Player player, Game game) {
         //20101001 - 508.1d
         for (Permanent creature : player.getAvailableAttackers(game)) {
+
+            // find must attack targets
             boolean mustAttack = false;
-            Set<UUID> defendersForcedToAttack = new HashSet<>();
+            Set<UUID> defendersForcedToAttack = new HashSet<>(); // contains only forced defenders
             if (creature.getGoadingPlayers().isEmpty()) {
-                // check if a creature has to attack
+                // must attack effects (not goad)
                 for (Map.Entry<RequirementEffect, Set<Ability>> entry : game.getContinuousEffects().getApplicableRequirementEffects(creature, false, game).entrySet()) {
                     RequirementEffect effect = entry.getKey();
                     if (!effect.mustAttack(game)) {
@@ -466,6 +471,8 @@ public class Combat implements Serializable, Copyable<Combat> {
                         if (defenderId != null) {
                             // creature is not forced to attack players that are no longer in the game
                             if (game.getPermanentOrLKIBattlefield(defenderId) == null && game.getPlayer(defenderId).hasLost()) {
+                                // TODO: must attack bugged (not goad)? Must use not LKI here or must check controller's lost too?
+                                // TODO: multiple must attack bugged (not goad)? It skip ALL other effects on outdated one of the effects (must continue instead?)
                                 return;
                             } else if (defenders.contains(defenderId)) {
                                 defendersForcedToAttack.add(defenderId);
@@ -475,9 +482,10 @@ public class Combat implements Serializable, Copyable<Combat> {
                     }
                 }
             } else {
+                // goad effects
                 // if creature is goaded then we start with assumption that it needs to attack any player
                 mustAttack = true;
-                // Filter out the planeswalkers
+                // filter only players
                 defendersForcedToAttack.addAll(defenders.stream()
                         .map(game::getPlayer)
                         .filter(Objects::nonNull)
@@ -487,9 +495,17 @@ public class Combat implements Serializable, Copyable<Combat> {
             if (!mustAttack) {
                 continue;
             }
-            // check which defenders the forced to attack creature can attack without paying a cost
-            Set<UUID> defendersCostlessAttackable = new HashSet<>(defenders);
+
+            // If a goaded creature can't attack for any reason (such as being tapped or having come under
+            // that player's control that turn), then it doesn't attack. If there's a cost associated with having
+            // it attack, its controller isn't forced to pay that cost, so it doesn't have to attack in that case
+            // either.
+            // (2020-04-17)
+
+            // remove costable targets from require list
+            Set<UUID> defendersCostlessAttackable = new HashSet<>(defenders); // contains all defenders (forced + own)
             for (UUID defenderId : defenders) {
+                // filter must pay to attack
                 if (game.getContinuousEffects().checkIfThereArePayCostToAttackBlockEffects(
                         new DeclareAttackerEvent(defenderId, creature.getId(), creature.getControllerId()), game
                 )) {
@@ -497,6 +513,8 @@ public class Combat implements Serializable, Copyable<Combat> {
                     defendersForcedToAttack.remove(defenderId);
                     continue;
                 }
+
+                // filter can't attack
                 for (Map.Entry<RestrictionEffect, Set<Ability>> entry : game.getContinuousEffects().getApplicableRestrictionEffects(creature, game).entrySet()) {
                     if (entry
                             .getValue()
@@ -511,17 +529,34 @@ public class Combat implements Serializable, Copyable<Combat> {
                     break;
                 }
             }
-            // Remove all the players which have goaded the attacker
+
+            // If a goaded creature doesn't meet any of the above exceptions and can attack,
+            // it must attack a player other than a player who goaded it if able.
+            // It the creature can't attack any of those players but could otherwise attack,
+            // it must attack an opposing planeswalker (controlled by any opponent) or a player who goaded it.
+            // (2020-04-17)
+
+            // filter goaded players
             defendersForcedToAttack.removeAll(creature.getGoadingPlayers());
 
-            // force attack only if a defender can be attacked without paying a cost
+            // if no free and valid targets then skip forced attack at all
             if (defendersCostlessAttackable.isEmpty()) {
                 continue;
             }
+
+            // OK, creature can be forced to attack something here
+
+            // TODO: bugged, can't attack own planeswalker on restricted opponent? Must store defendersToChooseFrom?
             creaturesForcedToAttack.put(creature.getId(), defendersForcedToAttack);
-            // No need to attack a special defender
+
+            // If a creature you control has been goaded by multiple opponents, it must attack one of your opponents
+            // who hasn't goaded it. If a creature you control has been goaded by each of your opponents,
+            // you choose which opponent it attacks.
+            // (2020-04-17)
             Set<UUID> defendersToChooseFrom = defendersForcedToAttack.isEmpty() ? defendersCostlessAttackable : defendersForcedToAttack;
+
             if (defendersToChooseFrom.size() == 1) {
+                // TODO: add game log here about forced to attack?
                 player.declareAttacker(creature.getId(), defendersToChooseFrom.iterator().next(), game, false);
                 continue;
             }
@@ -1271,9 +1306,12 @@ public class Combat implements Serializable, Copyable<Combat> {
     }
 
     public void setDefenders(Game game) {
+        // player + planeswalkers
         for (UUID playerId : getAttackablePlayers(game)) {
-            addDefender(playerId, game);
+            addDefendersFromPlayer(playerId, game);
         }
+
+        // battles
         for (Permanent permanent : game.getBattlefield().getActivePermanents(filterBattles, attackingPlayerId, game)) {
             defenders.add(permanent.getId());
         }
@@ -1316,22 +1354,31 @@ public class Combat implements Serializable, Copyable<Combat> {
         return attackablePlayers;
     }
 
-    private void addDefender(UUID defenderId, Game game) {
-        if (!defenders.contains(defenderId)) {
+    private void addDefendersFromPlayer(UUID playerId, Game game) {
+        if (!defenders.contains(playerId)) {
+
+            // workaround to find max number of attackers, example: Mirri, Weatherlight Duelist
+            // TODO: must research - is Integer.MIN_VALUE logic works fine on maxAttackers usage
             if (maxAttackers < Integer.MAX_VALUE) {
-                Player defendingPlayer = game.getPlayer(defenderId);
+                Player defendingPlayer = game.getPlayer(playerId);
                 if (defendingPlayer != null) {
                     if (defendingPlayer.getMaxAttackedBy() == Integer.MAX_VALUE) {
                         maxAttackers = Integer.MAX_VALUE;
                     } else if (maxAttackers == Integer.MIN_VALUE) {
+                        // first player
                         maxAttackers = defendingPlayer.getMaxAttackedBy();
                     } else {
+                        // second+ player
                         maxAttackers += defendingPlayer.getMaxAttackedBy();
                     }
                 }
             }
-            defenders.add(defenderId);
-            for (Permanent permanent : game.getBattlefield().getAllActivePermanents(StaticFilters.FILTER_PERMANENT_PLANESWALKER, defenderId, game)) {
+
+            // player
+            defenders.add(playerId);
+
+            // planeswalkers
+            for (Permanent permanent : game.getBattlefield().getAllActivePermanents(StaticFilters.FILTER_PERMANENT_PLANESWALKER, playerId, game)) {
                 defenders.add(permanent.getId());
             }
         }

Mage/src/main/java/mage/game/combat/CombatGroup.java

@@ -31,12 +31,12 @@ public class CombatGroup implements Serializable, Copyable<CombatGroup> {
     protected List<UUID> attackerOrder = new ArrayList<>();
     protected Map<UUID, UUID> players = new HashMap<>();
     protected boolean blocked;
-    protected UUID defenderId; // planeswalker or player, can be null after remove from combat (e.g. due damage)
+    protected UUID defenderId; // planeswalker, player, or battle id, can be null after remove from combat (e.g. due damage)
     protected UUID defendingPlayerId;
     protected boolean defenderIsPermanent;
 
     /**
-     * @param defenderId          the player that controls the defending permanents
+     * @param defenderId          player, planeswalker or battle that defending
      * @param defenderIsPermanent is the defender a permanent
      * @param defendingPlayerId   regular controller of the defending permanents
      */

Mage/src/main/java/mage/game/permanent/Permanent.java

@@ -310,7 +310,15 @@ public interface Permanent extends Card, Controllable {
 
     boolean canBlockAny(Game game);
 
-    boolean canBeAttacked(UUID attackerId, UUID playerToAttack, Game game);
+    /**
+     * Fast check for attacking possibilities (is it possible to attack permanent/planeswalker/battle)
+     *
+     * @param attackerId creature to attack, can be null
+     * @param defendingPlayerId defending player
+     * @param game
+     * @return
+     */
+    boolean canBeAttacked(UUID attackerId, UUID defendingPlayerId, Game game);
 
     /**
      * Checks by restriction effects if the permanent can use activated

Mage/src/main/java/mage/game/permanent/PermanentImpl.java

@@ -1435,12 +1435,12 @@ public abstract class PermanentImpl extends CardImpl implements Permanent {
     }
 
     @Override
-    public boolean canBeAttacked(UUID attackerId, UUID playerToAttack, Game game) {
+    public boolean canBeAttacked(UUID attackerId, UUID defendingPlayerId, Game game) {
         if (isPlaneswalker(game)) {
-            return isControlledBy(playerToAttack);
+            return isControlledBy(defendingPlayerId);
         }
         if (isBattle(game)) {
-            return isProtectedBy(playerToAttack);
+            return isProtectedBy(defendingPlayerId);
         }
         return false;
     }

Mage/src/main/java/mage/players/Player.java

@@ -786,6 +786,7 @@ public interface Player extends MageItem, Copyable<Player> {
 
     void pickCard(List<Card> cards, Deck deck, Draft draft);
 
+    // TODO: add result, process it in AI code (if something put creature to attack then it can broke current AI logic)
     void declareAttacker(UUID attackerId, UUID defenderId, Game game, boolean allowUndo);
 
     void declareBlocker(UUID defenderId, UUID blockerId, UUID attackerId, Game game);

Mage/src/main/java/mage/players/PlayerImpl.java

@@ -2710,6 +2710,7 @@ public abstract class PlayerImpl implements Player, Serializable {
     @Override
     public void declareAttacker(UUID attackerId, UUID defenderId, Game game, boolean allowUndo) {
         if (allowUndo) {
+            // TODO: allowUndo is smells bad, must be researched (what will be restored on allowUndo = false and why there are so diff usage), 2024-01-16
             setStoredBookmark(game.bookmarkState()); // makes it possible to UNDO a declared attacker with costs from e.g. Propaganda
         }
         Permanent attacker = game.getPermanent(attackerId);