* AI: reworked computer choices for "target permanent" targets:

* Fixed that computer was able to target own permanents for bad effects and opponent permanents for good effect;
 * Improved target choices on card score (as example: boost most valuable, discard most weaker);
 * Tests: fixed false positive tests (AI depended);
 * Tests: fixed that test player can't find permanents for some filters/predicates;

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

@@ -132,6 +132,12 @@ public class ComputerPlayer extends PlayerImpl implements Player {
             log.debug("chooseTarget: " + outcome.toString() + ':' + target.toString());
         }
 
+        // controller hints:
+        // - target.getTargetController(), this.getId() -- player that must makes choices (must be same with this.getId)
+        // - target.getAbilityController(), abilityControllerId -- affected player/controller for all actions/filters
+        // - affected controler can be different from target controller (another player makes choices for controller)
+
+
         // sometimes a target selection can be made from a player that does not control the ability
         UUID abilityControllerId = playerId;
         if (target.getTargetController() != null
@@ -440,6 +446,18 @@ public class ComputerPlayer extends PlayerImpl implements Player {
         if (log.isDebugEnabled()) {
             log.debug("chooseTarget: " + outcome.toString() + ':' + target.toString());
         }
+
+        // source can be null (as example: legendary rule permanent selection)
+        UUID sourceId = source != null ? source.getSourceId() : null;
+
+        // temp lists
+        List<Permanent> goodList = new ArrayList<>();
+        List<Permanent> badList = new ArrayList<>();
+        List<Permanent> allList = new ArrayList<>();
+        List<Permanent> goodList2 = new ArrayList<>();
+        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) {
@@ -457,17 +475,17 @@ public class ComputerPlayer extends PlayerImpl implements Player {
         }
 
         if (target.getOriginalTarget() instanceof TargetPlayer) {
-            return setTargetPlayer(outcome, target, source, source.getSourceId(), abilityControllerId, randomOpponentId, game);
+            return setTargetPlayer(outcome, target, source, sourceId, abilityControllerId, randomOpponentId, game);
         }
 
         if (target.getOriginalTarget() instanceof TargetDiscard
                 || target.getOriginalTarget() instanceof TargetCardInHand) {
             if (outcome.isGood()) {
                 // good
-                Cards cards = new CardsImpl(target.possibleTargets(source.getSourceId(), getId(), game));
+                Cards cards = new CardsImpl(target.possibleTargets(sourceId, getId(), game));
                 ArrayList<Card> cardsInHand = new ArrayList<>(cards.getCards(game));
                 while (!target.isChosen()
-                        && !target.possibleTargets(source.getSourceId(), getId(), game).isEmpty()
+                        && !target.possibleTargets(sourceId, getId(), game).isEmpty()
                         && target.getMaxNumberOfTargets() > target.getTargets().size()) {
                     Card card = pickBestCard(cardsInHand, null, target, source, game);
                     if (card != null) {
@@ -510,7 +528,7 @@ public class ComputerPlayer extends PlayerImpl implements Player {
         if (target.getOriginalTarget() instanceof TargetControlledPermanent) {
             TargetControlledPermanent origTarget = (TargetControlledPermanent) target.getOriginalTarget();
             List<Permanent> targets;
-            targets = threats(abilityControllerId, source.getSourceId(), origTarget.getFilter(), game, target.getTargets());
+            targets = threats(abilityControllerId, sourceId, origTarget.getFilter(), game, target.getTargets());
             if (!outcome.isGood()) {
                 Collections.reverse(targets);
             }
@@ -526,30 +544,38 @@ public class ComputerPlayer extends PlayerImpl implements Player {
 
         }
 
+        // TODO: implemented findBestPlayerTargets
+        // TODO: add findBest*Targets for all target types
         if (target.getOriginalTarget() instanceof TargetPermanent) {
-            List<Permanent> targets;
             TargetPermanent origTarget = (TargetPermanent) target.getOriginalTarget();
-            boolean outcomeTargets = true;
+            findBestPermanentTargets(outcome, abilityControllerId, sourceId, ((TargetPermanent) target).getFilter(),
-            if (outcome.isGood()) {
+                    game, target, goodList, badList, allList);
-                targets = threats(abilityControllerId, source == null ? null : source.getSourceId(), ((TargetPermanent) target).getFilter(), game, target.getTargets());
+            findBestPermanentTargets(outcome, abilityControllerId, sourceId, origTarget.getFilter(),
-            } else {
+                    game, target, goodList2, badList2, allList2);
-                targets = threats(randomOpponentId, source == null ? null : source.getSourceId(), ((TargetPermanent) target).getFilter(), game, target.getTargets());
+            if (goodList.size() != goodList2.size() || badList.size() != badList2.size() || allList.size() != allList2.size()
+                    || !origTarget.getFilter().equals(target.getFilter())) {
+                // TODO: remove double check after servers testing
+                log.error("Different filters in target and origTarget: " + target.getClass().getName() + " - " + origTarget.getClass().getName());
             }
-            if (targets.isEmpty() && target.isRequired(source)) {
+
-                targets = threats(null, source == null ? null : source.getSourceId(), ((TargetPermanent) target).getFilter(), game, target.getTargets());
+            // use good list all the time and add maximum targets
-                Collections.reverse(targets);
+            for (Permanent permanent : goodList) {
-                outcomeTargets = false;
-                //targets = game.getBattlefield().getActivePermanents(((TargetPermanent)target).getFilter(), playerId, game);
-            }
-            if (targets.isEmpty() && target.isRequired()) {
-                targets = game.getBattlefield().getActivePermanents(origTarget.getFilter(), playerId, game);
-            }
-            for (Permanent permanent : targets) {
                 if (target.canTarget(abilityControllerId, permanent.getId(), source, game)) {
-                    target.addTarget(permanent.getId(), source, game);
+                    if (target.getTargets().size() >= target.getMaxNumberOfTargets()) {
-                    if (!outcomeTargets || target.getMaxNumberOfTargets() <= target.getTargets().size()) {
+                        break;
-                        return true;
                     }
+                    target.addTarget(permanent.getId(), 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()) {
+                        break;
+                    }
+                    target.addTarget(permanent.getId(), source, game);
                 }
             }
             return target.isChosen();
@@ -559,9 +585,9 @@ public class ComputerPlayer extends PlayerImpl implements Player {
             List<Permanent> targets;
             TargetCreatureOrPlayer origTarget = ((TargetCreatureOrPlayer) target);
             if (outcome.isGood()) {
-                targets = threats(abilityControllerId, source.getSourceId(), ((FilterCreatureOrPlayer) origTarget.getFilter()).getCreatureFilter(), game, target.getTargets());
+                targets = threats(abilityControllerId, sourceId, ((FilterCreatureOrPlayer) origTarget.getFilter()).getCreatureFilter(), game, target.getTargets());
             } else {
-                targets = threats(randomOpponentId, source.getSourceId(), ((FilterCreatureOrPlayer) origTarget.getFilter()).getCreatureFilter(), game, target.getTargets());
+                targets = threats(randomOpponentId, sourceId, ((FilterCreatureOrPlayer) origTarget.getFilter()).getCreatureFilter(), game, target.getTargets());
             }
 
             if (targets.isEmpty()) {
@@ -602,9 +628,9 @@ public class ComputerPlayer extends PlayerImpl implements Player {
             List<Permanent> targets;
             TargetAnyTarget origTarget = ((TargetAnyTarget) target);
             if (outcome.isGood()) {
-                targets = threats(abilityControllerId, source.getSourceId(), ((FilterCreaturePlayerOrPlaneswalker) origTarget.getFilter()).getCreatureFilter(), game, target.getTargets());
+                targets = threats(abilityControllerId, sourceId, ((FilterCreaturePlayerOrPlaneswalker) origTarget.getFilter()).getCreatureFilter(), game, target.getTargets());
             } else {
-                targets = threats(randomOpponentId, source.getSourceId(), ((FilterCreaturePlayerOrPlaneswalker) origTarget.getFilter()).getCreatureFilter(), game, target.getTargets());
+                targets = threats(randomOpponentId, sourceId, ((FilterCreaturePlayerOrPlaneswalker) origTarget.getFilter()).getCreatureFilter(), game, target.getTargets());
             }
 
             if (targets.isEmpty()) {
@@ -2441,6 +2467,58 @@ public class ComputerPlayer extends PlayerImpl implements Player {
         return worst;
     }
 
+    protected void findBestPermanentTargets(Outcome outcome, UUID abilityControllerId, UUID sourceId, FilterPermanent filter, Game game, Target target,
+                                            List<Permanent> goodList, List<Permanent> badList, List<Permanent> allList) {
+        // searching for most valuable/powerfull permanents
+        goodList.clear();
+        badList.clear();
+        allList.clear();
+        List<UUID> usedTargets = target.getTargets();
+
+        // search all
+        for (Permanent permanent : game.getBattlefield().getActivePermanents(filter, abilityControllerId, sourceId, game)) {
+            if (usedTargets.contains(permanent.getId())) {
+                continue;
+            }
+
+            if (outcome.isGood()) {
+                // good effect
+                if (permanent.isControlledBy(abilityControllerId)) {
+                    goodList.add(permanent);
+                } else {
+                    badList.add(permanent);
+                }
+            } else {
+                // bad effect
+                if (permanent.isControlledBy(abilityControllerId)) {
+                    badList.add(permanent);
+                } else {
+                    goodList.add(permanent);
+                }
+            }
+        }
+
+        // sort from tiny to big (more valuable)
+        PermanentComparator comparator = new PermanentComparator(game);
+        goodList.sort(comparator);
+        badList.sort(comparator);
+
+        // real sort
+        if (outcome.isGood()) {
+            // good effect -- most valueable goes first
+            Collections.reverse(goodList);
+            // Collections.reverse(badList);
+        } else {
+            // bad effect - most weakest goes first, no need in reverse
+            // Collections.reverse(goodList);
+            Collections.reverse(badList);
+        }
+
+        allList.addAll(goodList);
+        allList.addAll(badList);
+    }
+
+
     protected List<Permanent> threats(UUID playerId, UUID sourceId, FilterPermanent filter, Game game, List<UUID> targets) {
         return threats(playerId, sourceId, filter, game, targets, true);
     }

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

@@ -214,9 +214,11 @@ public class LoadTest {
         // playing until game over
         boolean startToWatching = false;
         while (true) {
+            GameView gameView = monitor.client.getLastGameView();
+
             checkGame = monitor.getTable(tableId);
             TableState state = checkGame.get().getTableState();
-            logger.warn(state);
+            logger.warn((gameView != null ? "Turn " + gameView.getTurn() + ", " + gameView.getStep().toString() + " - " : "") + state);
 
             if (state == TableState.FINISHED) {
                 break;
@@ -227,7 +229,6 @@ public class LoadTest {
                 startToWatching = true;
             }
 
-            GameView gameView = monitor.client.getLastGameView();
             if (gameView != null) {
                 for (PlayerView p : gameView.getPlayers()) {
                     logger.info(p.getName() + " - Life=" + p.getLife() + "; Lib=" + p.getLibraryCount());
@@ -249,21 +250,21 @@ public class LoadTest {
     }
 
     @Test
-    @Ignore
+    //@Ignore
     public void test_TwoAIPlayGame_Multiple() {
 
         // save random seeds for repeated results
-        Integer gamesAmount = 1000;
+        int gamesAmount = 1000;
         List<Integer> seedsList = new ArrayList<>();
         for (int i = 1; i <= gamesAmount; i++) {
             seedsList.add(RandomUtil.nextInt());
         }
 
-        for (int i = 1; i <= 1000; i++) {
+        for (int i = 1; i <= gamesAmount; i++) {
             long randomSeed = seedsList.get(i);
-            logger.info("RANDOM seed: " + randomSeed);
+            logger.info("Game " + i + " of " + gamesAmount + ", RANDOM seed: " + randomSeed);
             RandomUtil.setSeed(randomSeed);
-            playTwoAIGame("WGUBR", "SWS");
+            playTwoAIGame("WGUBR", "ELD");
         }
     }
 

Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java

@@ -1453,7 +1453,8 @@ public class TestPlayer implements Player {
 
     private void chooseStrictModeFailed(Game game, String reason) {
         if (strictChooseMode) {
-            Assert.fail("Missing target/choice def for turn " + game.getTurnNum() + ", " + game.getStep().getType().name() + ": " + reason);
+            Assert.fail("Missing target/choice def for turn " + game.getTurnNum() + ", " + this.getName() + ", "
+                    + game.getStep().getType().name() + ": " + reason);
         }
     }
 
@@ -1523,6 +1524,11 @@ public class TestPlayer implements Player {
 
     @Override
     public boolean choose(Outcome outcome, Target target, UUID sourceId, Game game, Map<String, Serializable> options) {
+        UUID abilityControllerId = computerPlayer.getId();
+        if (target.getTargetController() != null && target.getAbilityController() != null) {
+            abilityControllerId = target.getAbilityController();
+        }
+
         if (!choices.isEmpty()) {
 
             List<String> usedChoices = new ArrayList<>();
@@ -1557,13 +1563,12 @@ public class TestPlayer implements Player {
                                 targetName = targetName.substring(0, targetName.length() - 11);
                             }
                         }
-                        for (Permanent permanent : game.getBattlefield().getActivePermanents(filterPermanent, getId(), sourceId, game)) {
+                        for (Permanent permanent : game.getBattlefield().getActivePermanents(filterPermanent, abilityControllerId, sourceId, game)) {
                             if (target.getTargets().contains(permanent.getId())) {
                                 continue;
                             }
                             if (permanent.getName().equals(targetName)) {
-
+                                if (target.isNotTarget() || target.canTarget(abilityControllerId, permanent.getId(), source, game)) {
-                                if (target.isNotTarget() || target.canTarget(computerPlayer.getId(), permanent.getId(), source, game)) {
                                     if ((permanent.isCopy() && !originOnly) || (!permanent.isCopy() && !copyOnly)) {
                                         target.add(permanent.getId(), game);
                                         targetFound = true;
@@ -1571,7 +1576,7 @@ public class TestPlayer implements Player {
                                     }
                                 }
                             } else if ((permanent.getName() + '-' + permanent.getExpansionSetCode()).equals(targetName)) {
-                                if (target.isNotTarget() || target.canTarget(computerPlayer.getId(), permanent.getId(), source, game)) {
+                                if (target.isNotTarget() || target.canTarget(abilityControllerId, permanent.getId(), source, game)) {
                                     if ((permanent.isCopy() && !originOnly) || (!permanent.isCopy() && !copyOnly)) {
                                         target.add(permanent.getId(), game);
                                         targetFound = true;
@@ -1592,7 +1597,7 @@ public class TestPlayer implements Player {
                 for (Player player : game.getPlayers().values()) {
                     for (String choose2 : choices) {
                         if (player.getName().equals(choose2)) {
-                            if (target.canTarget(computerPlayer.getId(), player.getId(), null, game) && !target.getTargets().contains(player.getId())) {
+                            if (target.canTarget(abilityControllerId, player.getId(), null, game) && !target.getTargets().contains(player.getId())) {
                                 target.add(player.getId(), game);
                                 choices.remove(choose2);
                                 return true;
@@ -1623,7 +1628,7 @@ public class TestPlayer implements Player {
 
                     CheckOneChoice:
                     for (String possibleChoice : possibleChoices) {
-                        Set<UUID> possibleCards = target.possibleTargets(sourceId, target.getTargetController() == null ? getId() : target.getTargetController(), game);
+                        Set<UUID> possibleCards = target.possibleTargets(sourceId, abilityControllerId, game);
                         CheckTargetsList:
                         for (UUID targetId : possibleCards) {
                             MageObject targetObject = game.getObject(targetId);
@@ -1672,7 +1677,7 @@ public class TestPlayer implements Player {
             if (target instanceof TargetSource) {
                 Set<UUID> possibleTargets;
                 TargetSource t = ((TargetSource) target);
-                possibleTargets = t.possibleTargets(sourceId, computerPlayer.getId(), game);
+                possibleTargets = t.possibleTargets(sourceId, abilityControllerId, game);
                 for (String choose2 : choices) {
                     String[] targetList = choose2.split("\\^");
                     boolean targetFound = false;
@@ -1732,18 +1737,20 @@ public class TestPlayer implements Player {
 
         // how to fix: change target definition for addTarget in test's code or update choose from targets implementation in TestPlayer
         if ((foundMulti && !canMulti) || (foundSpecialStart && !canSpecialStart) || (foundSpecialClose && !canSpecialClose) || (foundEquals && !canEquals)) {
-            Assert.fail("Targets list was setup by addTarget with " + targets + ", but target definition [" + targetDefinition + "]"
+            Assert.fail(this.getName() + " - Targets list was setup by addTarget with " + targets + ", but target definition [" + targetDefinition + "]"
                     + " is not supported by [" + canSupportChars + "] for target class " + needTarget.getClass().getSimpleName());
         }
     }
 
     @Override
     public boolean chooseTarget(Outcome outcome, Target target, Ability source, Game game) {
-        if (!targets.isEmpty()) {
         UUID abilityControllerId = computerPlayer.getId();
         if (target.getTargetController() != null && target.getAbilityController() != null) {
             abilityControllerId = target.getAbilityController();
         }
+        UUID sourceId = source != null ? source.getSourceId() : null;
+
+        if (!targets.isEmpty()) {
 
             // do not select
             if (targets.get(0).equals(TARGET_SKIP)) {
@@ -1811,7 +1818,7 @@ public class TestPlayer implements Player {
                         if (filter instanceof FilterPlaneswalkerOrPlayer) {
                             filter = ((FilterPlaneswalkerOrPlayer) filter).getFilterPermanent();
                         }
-                        for (Permanent permanent : game.getBattlefield().getAllActivePermanents((FilterPermanent) filter, game)) {
+                        for (Permanent permanent : game.getBattlefield().getActivePermanents((FilterPermanent) filter, abilityControllerId, sourceId, game)) {
                             if (permanent.getName().equals(targetName) || (permanent.getName() + '-' + permanent.getExpansionSetCode()).equals(targetName)) {
                                 if (target.canTarget(abilityControllerId, permanent.getId(), source, game) && !target.getTargets().contains(permanent.getId())) {
                                     if ((permanent.isCopy() && !originOnly) || (!permanent.isCopy() && !copyOnly)) {
@@ -1989,13 +1996,13 @@ public class TestPlayer implements Player {
             String message;
 
             if (source != null) {
-                message = "Targets list was setup by addTarget with " + targets + ", but not used in ["
+                message = this.getName() + " - Targets list was setup by addTarget with " + targets + ", but not used in ["
                         + "card " + source.getSourceObject(game)
                         + " -> ability " + source.getClass().getSimpleName() + " (" + source.getRule().substring(0, Math.min(20, source.getRule().length()) - 1) + "..." + ")"
                         + " -> target " + target.getClass().getSimpleName() + " (" + target.getMessage() + ")"
                         + "]";
             } else {
-                message = "Targets list was setup by addTarget with " + targets + ", but not used in ["
+                message = this.getName() + " - Targets list was setup by addTarget with " + targets + ", but not used in ["
                         + "card XXX"
                         + " -> target " + target.getClass().getSimpleName() + " (" + target.getMessage() + ")"
                         + "]";

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

@@ -235,7 +235,7 @@ public class Battlefield implements Serializable {
 
     /**
      * Returns all {@link Permanent} on the battlefield that match the supplied
-     * filter. This method ignores the range of influence.
+     * filter. This method ignores the range of influence. It's ignore controllers preficate
      *
      * @param filter
      * @param game