Fixed AI freeze with non available targets

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

@@ -1,14 +1,14 @@
-
 package mage.player.ai;
 
-import java.util.LinkedList;
 import mage.abilities.Ability;
 import mage.constants.RangeOfInfluence;
 import mage.game.Game;
 import org.apache.log4j.Logger;
 
+import java.util.Date;
+import java.util.LinkedList;
+
 /**
- *
  * @author ayratn
  */
 public class ComputerPlayer7 extends ComputerPlayer6 {
@@ -107,6 +107,7 @@ private boolean priorityPlay(Game game) {
 
     protected void calculateActions(Game game) {
         if (!getNextAction(game)) {
+            Date startTime = new Date();
             currentScore = GameStateEvaluator2.evaluate(playerId, game);
             Game sim = createSimulation(game);
             SimulationNode2.resetCount();
@@ -137,6 +138,15 @@ protected void calculateActions(Game game) {
             } else {
                 logger.info('[' + game.getPlayer(playerId).getName() + "][pre] Action: skip");
             }
+            Date endTime = new Date();
+            this.setLastThinkTime((endTime.getTime() - startTime.getTime()));
+
+            /*
+            logger.warn("Last think time: " + this.getLastThinkTime()
+                    + "; actions: " + actions.size()
+                    + "; hand: " + this.getHand().size()
+                    + "; permanents: " + game.getBattlefield().getAllPermanents().size());
+             */
         } else {
             logger.debug("Next Action exists!");
         }

Mage.Server.Plugins/Mage.Player.AI/src/main/java/mage/player/ai/ComputerPlayer.java

@@ -70,6 +70,7 @@
 public class ComputerPlayer extends PlayerImpl implements Player {
 
     private static final Logger log = Logger.getLogger(ComputerPlayer.class);
+    private long lastThinkTime = 0; // msecs for last AI actions calc
 
     protected int PASSIVITY_PENALTY = 5; // Penalty value for doing nothing if some actions are available
     protected boolean ALLOW_INTERRUPT = true;     // change this for test to false / debugging purposes to false to switch off interrupts while debugging
@@ -450,6 +451,18 @@ public boolean chooseTarget(Outcome outcome, Target target, Ability source, Game
         // source can be null (as example: legendary rule permanent selection)
         UUID sourceId = source != null ? source.getSourceId() : null;
 
+        // sometimes a target selection can be made from a player that does not control the ability
+        UUID abilityControllerId = playerId;
+        if (target.getAbilityController() != null) {
+            abilityControllerId = target.getAbilityController();
+        }
+
+        boolean required = target.isRequired(sourceId, game);
+        Set<UUID> possibleTargets = target.possibleTargets(sourceId, abilityControllerId, game);
+        if (possibleTargets.isEmpty() || target.getTargets().size() >= target.getNumberOfTargets()) {
+            required = false;
+        }
+
         // temp lists
         List<Permanent> goodList = new ArrayList<>();
         List<Permanent> badList = new ArrayList<>();
@@ -458,12 +471,6 @@ public boolean chooseTarget(Outcome outcome, Target target, Ability source, Game
         List<Permanent> badList2 = new ArrayList<>();
         List<Permanent> allList2 = new ArrayList<>();
 
-        // sometimes a target selection can be made from a player that does not control the ability
-        UUID abilityControllerId = playerId;
-        if (target.getAbilityController() != null) {
-            abilityControllerId = target.getAbilityController();
-        }
-
         // TODO: improve to process multiple opponents instead random
         UUID randomOpponentId;
         if (target.getTargetController() != null) {
@@ -569,7 +576,6 @@ public boolean chooseTarget(Outcome outcome, Target target, Ability source, Game
             }
 
             // use bad list only on required target and add minimum targets
-            boolean required = target.isRequiredExplicitlySet() ? required = target.isRequired() : target.isRequired(source); // got that code from HumanPlayer.chooseTarget
             if (required) {
                 for (Permanent permanent : badList) {
                     if (target.getTargets().size() >= target.getMinNumberOfTargets()) {
@@ -2787,4 +2793,12 @@ public void restore(Player player) {
         // all human players converted to computer and analyse
         this.human = false;
     }
+
+    public long getLastThinkTime() {
+        return lastThinkTime;
+    }
+
+    public void setLastThinkTime(long lastThinkTime) {
+        this.lastThinkTime = lastThinkTime;
+    }
 }

Mage.Sets/src/mage/cards/r/RedcapMelee.java

@@ -43,7 +43,7 @@ class RedcapMeleeEffect extends OneShotEffect {
     private static final Effect effect = new SacrificeControllerEffect(StaticFilters.FILTER_LAND, 1, "");
 
     RedcapMeleeEffect() {
-        super(Outcome.Benefit);
+        super(Outcome.Damage);
         staticText = "{this} deals 4 damage to target creature or planeswalker. " +
                 "If a nonred permanent is dealt damage this way, you sacrifice a land.";
     }

Mage.Tests/src/test/java/org/mage/test/AI/basic/TargetRequiredTest.java

@@ -0,0 +1,101 @@
+package org.mage.test.AI.basic;
+
+import mage.constants.PhaseStep;
+import mage.constants.Zone;
+import org.junit.Test;
+import org.mage.test.serverside.base.CardTestPlayerBase;
+
+/**
+ * @author JayDi85
+ */
+public class TargetRequiredTest extends CardTestPlayerBase {
+
+    /*
+        Redcap must sacrifice target land -- it's required target, but AI don't known about that
+        (target can be copied as new target in effect's code)
+     */
+
+    @Test
+    public void test_chooseBadTargetOnSacrifice_WithTargets_User() {
+        // Redcap Melee deals 4 damage to target creature or planeswalker. If a nonred permanent is dealt damage this way, you sacrifice a land.
+        addCard(Zone.HAND, playerA, "Redcap Melee", 1);
+        addCard(Zone.BATTLEFIELD, playerA, "Mountain", 3);
+        //
+        addCard(Zone.BATTLEFIELD, playerB, "Silvercoat Lion", 1);
+
+        castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Redcap Melee", "Silvercoat Lion");
+        addTarget(playerA, "Mountain");
+
+        setStrictChooseMode(true);
+        setStopAt(1, PhaseStep.END_TURN);
+        execute();
+        assertAllCommandsUsed();
+
+        assertGraveyardCount(playerA, "Redcap Melee", 1);
+        assertGraveyardCount(playerA, "Mountain", 1);
+        assertPermanentCount(playerA, "Mountain", 3 - 1);
+        assertGraveyardCount(playerB, "Silvercoat Lion", 1);
+    }
+
+    @Test
+    public void test_chooseBadTargetOnSacrifice_WithTargets_AI() {
+        // Redcap Melee deals 4 damage to target creature or planeswalker. If a nonred permanent is dealt damage this way, you sacrifice a land.
+        addCard(Zone.HAND, playerA, "Redcap Melee", 1);
+        addCard(Zone.BATTLEFIELD, playerA, "Mountain", 3);
+        //
+        addCard(Zone.BATTLEFIELD, playerB, "Silvercoat Lion", 1);
+
+        castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Redcap Melee", "Silvercoat Lion");
+        //addTarget(playerA, "Mountain"); AI must select targets
+
+        //setStrictChooseMode(true); AI must select targets
+        setStopAt(1, PhaseStep.END_TURN);
+        execute();
+        assertAllCommandsUsed();
+
+        assertGraveyardCount(playerA, "Redcap Melee", 1);
+        assertGraveyardCount(playerA, "Mountain", 1);
+        assertPermanentCount(playerA, "Mountain", 3 - 1);
+        assertGraveyardCount(playerB, "Silvercoat Lion", 1);
+    }
+
+    @Test
+    public void test_chooseBadTargetOnSacrifice_WithoutTargets_User() {
+        // Redcap Melee deals 4 damage to target creature or planeswalker. If a nonred permanent is dealt damage this way, you sacrifice a land.
+        addCard(Zone.HAND, playerA, "Redcap Melee", 1);
+        addCard(Zone.BATTLEFIELD, playerA, "Atarka Monument", 1); // {T}: Add {R} or {G}.
+        //
+        addCard(Zone.BATTLEFIELD, playerB, "Silvercoat Lion", 1);
+
+        castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Redcap Melee", "Silvercoat Lion");
+        //addTarget(playerA, "Mountain"); no lands to sacrifice
+
+        setStrictChooseMode(true);
+        setStopAt(1, PhaseStep.END_TURN);
+        execute();
+        assertAllCommandsUsed();
+
+        assertGraveyardCount(playerA, "Redcap Melee", 1);
+        assertGraveyardCount(playerB, "Silvercoat Lion", 1);
+    }
+
+    @Test
+    public void test_chooseBadTargetOnSacrifice_WithoutTargets_AI() {
+        // Redcap Melee deals 4 damage to target creature or planeswalker. If a nonred permanent is dealt damage this way, you sacrifice a land.
+        addCard(Zone.HAND, playerA, "Redcap Melee", 1);
+        addCard(Zone.BATTLEFIELD, playerA, "Atarka Monument", 1); // {T}: Add {R} or {G}.
+        //
+        addCard(Zone.BATTLEFIELD, playerB, "Silvercoat Lion", 1);
+
+        castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Redcap Melee", "Silvercoat Lion");
+        //addTarget(playerA, "Mountain"); no lands to sacrifice
+
+        //setStrictChooseMode(true);
+        setStopAt(1, PhaseStep.END_TURN);
+        execute();
+        assertAllCommandsUsed();
+
+        assertGraveyardCount(playerA, "Redcap Melee", 1);
+        assertGraveyardCount(playerB, "Silvercoat Lion", 1);
+    }
+}

Mage.Tests/src/test/java/org/mage/test/load/LoadTest.java

@@ -22,7 +22,10 @@
 
 import java.time.Instant;
 import java.time.temporal.ChronoUnit;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Optional;
+import java.util.UUID;
 
 /**
  * Intended to test Mage server under different load patterns.
@@ -188,7 +191,7 @@ public void test_TwoUsersPlayGameUntilEnd() {
         }
     }
 
-    public void playTwoAIGame(String deckColors, String deckAllowedSets) {
+    public void playTwoAIGame(String gameName, String deckColors, String deckAllowedSets) {
         Assert.assertFalse("need deck colors", deckColors.isEmpty());
         Assert.assertFalse("need allowed sets", deckAllowedSets.isEmpty());
 
@@ -197,7 +200,7 @@ public void playTwoAIGame(String deckColors, String deckAllowedSets) {
 
         // game by monitor
         GameTypeView gameType = monitor.session.getGameTypes().get(0);
-        MatchOptions gameOptions = createSimpleGameOptionsForAI(gameType, monitor.session);
+        MatchOptions gameOptions = createSimpleGameOptionsForAI(gameType, monitor.session, gameName);
         TableView game = monitor.session.createTable(monitor.roomID, gameOptions);
         UUID tableId = game.getTableId();
 
@@ -218,7 +221,11 @@ public void playTwoAIGame(String deckColors, String deckAllowedSets) {
 
             checkGame = monitor.getTable(tableId);
             TableState state = checkGame.get().getTableState();
-            logger.warn((gameView != null ? "Turn " + gameView.getTurn() + ", " + gameView.getStep().toString() + " - " : "") + state);
+
+            logger.warn(checkGame.get().getTableName()
+                    + (gameView != null ? ", turn " + gameView.getTurn() + ", " + gameView.getStep().toString() : "")
+                    + (gameView != null ? ", active " + gameView.getActivePlayerName() : "")
+                    + ", " + state);
 
             if (state == TableState.FINISHED) {
                 break;
@@ -246,25 +253,34 @@ public void playTwoAIGame(String deckColors, String deckAllowedSets) {
     @Test
     @Ignore
     public void test_TwoAIPlayGame_One() {
-        playTwoAIGame("GR", "GRN");
+        playTwoAIGame("Single AI game", "GR", "GRN");
     }
 
     @Test
     @Ignore
     public void test_TwoAIPlayGame_Multiple() {
 
-        // save random seeds for repeated results
-        int gamesAmount = 1000;
+        int singleGameSID = -1554824422; // for one game test with same deck
+        int singleGamePlaysAmount = 1000; // multiple run of one game test
+
+        // save random seeds for repeated results (in decks generating)
         List<Integer> seedsList = new ArrayList<>();
-        for (int i = 1; i <= gamesAmount; i++) {
-            seedsList.add(RandomUtil.nextInt());
+        if (singleGameSID != 0) {
+            for (int i = 1; i <= singleGamePlaysAmount; i++) {
+                seedsList.add(singleGameSID);
+            }
+        } else {
+            int gamesAmount = 1000;
+            for (int i = 1; i <= gamesAmount; i++) {
+                seedsList.add(RandomUtil.nextInt());
+            }
         }
 
-        for (int i = 1; i <= gamesAmount; i++) {
+        for (int i = 0; i <= seedsList.size() - 1; i++) {
             long randomSeed = seedsList.get(i);
-            logger.info("Game " + i + " of " + gamesAmount + ", RANDOM seed: " + randomSeed);
+            logger.info("Game " + (i + 1) + " of " + seedsList.size() + ", RANDOM seed: " + randomSeed);
             RandomUtil.setSeed(randomSeed);
-            playTwoAIGame("WGUBR", "ELD");
+            playTwoAIGame("AI game #" + (i + 1), "WGUBR", "ELD");
         }
     }
 
@@ -454,8 +470,8 @@ private MatchOptions createSimpleGameOptionsForBots(GameTypeView gameTypeView, S
         return createSimpleGameOptions("Bots test game", gameTypeView, session, PlayerType.HUMAN);
     }
 
-    private MatchOptions createSimpleGameOptionsForAI(GameTypeView gameTypeView, Session session) {
-        return createSimpleGameOptions("AI test game", gameTypeView, session, PlayerType.COMPUTER_MAD);
+    private MatchOptions createSimpleGameOptionsForAI(GameTypeView gameTypeView, Session session, String gameName) {
+        return createSimpleGameOptions(gameName, gameTypeView, session, PlayerType.COMPUTER_MAD);
     }
 
     private class LoadPlayer {

Mage/src/main/java/mage/abilities/effects/common/CopySpellForEachItCouldTargetEffect.java

@@ -10,14 +10,14 @@
 import mage.filter.FilterInPlay;
 import mage.filter.predicate.mageobject.FromSetPredicate;
 import mage.game.Game;
+import mage.game.events.GameEvent;
 import mage.game.stack.Spell;
 import mage.players.Player;
 import mage.target.Target;
 import mage.target.TargetImpl;
 import mage.util.TargetAddress;
 
 import java.util.*;
-import mage.game.events.GameEvent;
 
 /**
  * @param <T>
@@ -259,15 +259,20 @@ public int getNumberOfTargets() {
     }
 
     @Override
-    public int getMaxNumberOfTargets() {
-        return originalTarget.getMaxNumberOfTargets();
+    public int getMinNumberOfTargets() {
+        return originalTarget.getMinNumberOfTargets();
     }
 
     @Override
     public void setMinNumberOfTargets(int minNumberOfTargets) {
         originalTarget.setMinNumberOfTargets(minNumberOfTargets);
     }
 
+    @Override
+    public int getMaxNumberOfTargets() {
+        return originalTarget.getMaxNumberOfTargets();
+    }
+
     @Override
     public void setMaxNumberOfTargets(int maxNumberOfTargets) {
         originalTarget.setMaxNumberOfTargets(maxNumberOfTargets);