Test framework:

* added choices support for discard spells;
 * fixes TargetCard choices (one choice record per target);

Mage.Tests/src/test/java/org/mage/test/cards/single/MisdirectionTest.java

@@ -1,4 +1,3 @@
-
 package org.mage.test.cards.single;
 
 import mage.constants.PhaseStep;
@@ -7,8 +6,7 @@ import org.junit.Test;
 import org.mage.test.serverside.base.CardTestPlayerBase;
 
 /**
- *
- * @author LevelX2
+ * @author LevelX2, JayDi85
  */
 
 public class MisdirectionTest extends CardTestPlayerBase {
@@ -17,10 +15,58 @@ public class MisdirectionTest extends CardTestPlayerBase {
      * Tests if Misdirection for target opponent works correctly
      * https://github.com/magefree/mage/issues/574
      */
+
     @Test
-    public void testChangeTargetOpponent() {
+    public void test_RakshaDiscardWorks() {
         // Target opponent discards two cards. Put the top two cards of your library into your graveyard.
-        addCard(Zone.HAND, playerA, "Rakshasa's Secret");
+        addCard(Zone.HAND, playerA, "Rakshasa's Secret"); // {2}{B}
+        addCard(Zone.BATTLEFIELD, playerA, "Swamp", 3);
+        addCard(Zone.HAND, playerB, "Silvercoat Lion", 2);
+        addCard(Zone.HAND, playerB, "Ashcoat Bear", 5);
+
+        // A cast discard
+        castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Rakshasa's Secret", playerB);
+        setChoice(playerB, "Silvercoat Lion"); // select target 1
+        setChoice(playerB, "Silvercoat Lion"); // select target 2
+        checkHandCardCount("B haven't lions", 1, PhaseStep.POSTCOMBAT_MAIN, playerB, "Silvercoat Lion", 0);
+        checkHandCardCount("B have 5 bears", 1, PhaseStep.POSTCOMBAT_MAIN, playerB, "Ashcoat Bear", 5);
+
+        setStopAt(1, PhaseStep.END_TURN);
+        execute();
+        assertAllCommandsUsed();
+    }
+
+    @Test
+    public void test_MisdirectionRetargetWorks() {
+        // Return target permanent to its owner’s hand.
+        addCard(Zone.HAND, playerA, "Boomerang", 1); // {U}{U}
+        addCard(Zone.BATTLEFIELD, playerA, "Island", 2);
+        addCard(Zone.BATTLEFIELD, playerA, "Ashcoat Bear", 1);
+        // Change the target of target spell with a single target.
+        addCard(Zone.HAND, playerB, "Misdirection"); // {3}{U}{U}
+        addCard(Zone.BATTLEFIELD, playerB, "Island", 5);
+        addCard(Zone.BATTLEFIELD, playerB, "Silvercoat Lion", 1);
+
+        // A cast Boomerang to remove lion
+        castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Boomerang", "Silvercoat Lion");
+        // B counter it by Misdirection and remove bear
+        castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerB, "Misdirection", "Boomerang", "Boomerang");
+        addTarget(playerB, "Ashcoat Bear");
+
+        setStopAt(1, PhaseStep.END_TURN);
+        execute();
+        assertAllCommandsUsed();
+
+        assertGraveyardCount(playerA, "Boomerang", 1);
+        assertPermanentCount(playerA, "Ashcoat Bear", 0);
+        assertGraveyardCount(playerB, "Misdirection", 1);
+        assertPermanentCount(playerB, "Silvercoat Lion", 1);
+    }
+
+    @Test
+    public void test_MisdirectionCantTargetToIllegal() {
+        // Target opponent discards two cards. Put the top two cards of your library into your graveyard.
+        addCard(Zone.HAND, playerA, "Rakshasa's Secret"); // {2}{B}
         addCard(Zone.BATTLEFIELD, playerA, "Swamp", 3);
         /*    
         Misdirection {3}{U}{U}
@@ -30,23 +76,31 @@ public class MisdirectionTest extends CardTestPlayerBase {
         */
         addCard(Zone.HAND, playerB, "Misdirection");
         addCard(Zone.HAND, playerB, "Silvercoat Lion", 2);
+        addCard(Zone.HAND, playerB, "Ashcoat Bear", 5);
         addCard(Zone.BATTLEFIELD, playerB, "Island", 5);
-        
+
+        // cast Raksha and select B
         castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Rakshasa's Secret", playerB);
+        // cast misdir, but it's not apply and taget will be same
         castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerB, "Misdirection", "Rakshasa's Secret", "Rakshasa's Secret");
-        addTarget(playerB, playerA); // only legal target is player B as opponent - so player A should not be allowed
-        
-        setStopAt(1, PhaseStep.BEGIN_COMBAT);
+        // B must select cards to discard (2 lions, not bears)
+        setChoice(playerB, "Silvercoat Lion"); // select target 1
+        setChoice(playerB, "Silvercoat Lion"); // select target 2
+        checkHandCardCount("B haven't lions", 1, PhaseStep.POSTCOMBAT_MAIN, playerB, "Silvercoat Lion", 0);
+        checkHandCardCount("B have 5 bears", 1, PhaseStep.POSTCOMBAT_MAIN, playerB, "Ashcoat Bear", 5);
+
+        setStopAt(1, PhaseStep.END_TURN);
         execute();
+        assertAllCommandsUsed();
 
         assertGraveyardCount(playerA, "Rakshasa's Secret", 1);
         assertGraveyardCount(playerB, "Misdirection", 1);
         assertHandCount(playerB, "Silvercoat Lion", 0);
     }
-    
+
     // check to change target permanent creature legal to to a creature the opponent of the spell controller controls
     @Test
-    public void testChangePublicExecution() {
+    public void test_ChangePublicExecution() {
         // Destroy target creature an opponent controls. Each other creature that player controls gets -2/-0 until end of turn.
         addCard(Zone.HAND, playerA, "Public Execution");
         addCard(Zone.BATTLEFIELD, playerA, "Swamp", 6);
@@ -60,26 +114,27 @@ public class MisdirectionTest extends CardTestPlayerBase {
         addCard(Zone.BATTLEFIELD, playerB, "Pillarfield Ox", 1);
         addCard(Zone.BATTLEFIELD, playerB, "Custodian of the Trove", 1); // 4/3
         addCard(Zone.BATTLEFIELD, playerB, "Island", 5);
-        
+
         castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Public Execution", "Pillarfield Ox");
         castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerB, "Misdirection", "Public Execution", "Public Execution");
-        addTarget(playerB, "Custodian of the Trove"); 
-        
+        addTarget(playerB, "Custodian of the Trove");
+
         setStopAt(1, PhaseStep.BEGIN_COMBAT);
         execute();
+        assertAllCommandsUsed();
 
         assertGraveyardCount(playerA, "Public Execution", 1);
         assertGraveyardCount(playerB, "Misdirection", 1);
-        
-        assertGraveyardCount(playerB, "Custodian of the Trove",1);
+
+        assertGraveyardCount(playerB, "Custodian of the Trove", 1);
         assertPermanentCount(playerB, "Pillarfield Ox", 1);
         assertPowerToughness(playerB, "Pillarfield Ox", 0, 4);
 
-    } 
-    
+    }
+
     // check to change target permanent creature not legal to to a creature the your opponent controls
     @Test
-    public void testChangePublicExecution2() {
+    public void test_ChangePublicExecution2() {
         // Destroy target creature an opponent controls. Each other creature that player controls gets -2/-0 until end of turn.
         addCard(Zone.HAND, playerA, "Public Execution");
         addCard(Zone.BATTLEFIELD, playerA, "Swamp", 6);
@@ -94,13 +149,13 @@ public class MisdirectionTest extends CardTestPlayerBase {
         addCard(Zone.BATTLEFIELD, playerB, "Pillarfield Ox", 1);
         addCard(Zone.BATTLEFIELD, playerB, "Custodian of the Trove", 1); // 4/3
         addCard(Zone.BATTLEFIELD, playerB, "Island", 5);
-        
+
         castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Public Execution", "Custodian of the Trove");
         castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerB, "Misdirection", "Public Execution", "Public Execution");
-        addTarget(playerB, "Keeper of the Lens"); 
-        
+
         setStopAt(1, PhaseStep.BEGIN_COMBAT);
         execute();
+        assertAllCommandsUsed();
 
         assertGraveyardCount(playerA, "Public Execution", 1);
         assertGraveyardCount(playerB, "Misdirection", 1);
@@ -108,8 +163,7 @@ public class MisdirectionTest extends CardTestPlayerBase {
 
         assertPermanentCount(playerB, "Pillarfield Ox", 1);
         assertPowerToughness(playerB, "Pillarfield Ox", 0, 4);
-        
-        assertGraveyardCount(playerB, "Custodian of the Trove",1);
 
-    }        
+        assertGraveyardCount(playerB, "Custodian of the Trove", 1);
+    }
 }

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

@@ -3,9 +3,11 @@ package org.mage.test.player;
 import mage.MageObject;
 import mage.abilities.ActivatedAbility;
 import mage.abilities.SpellAbility;
+import mage.constants.Outcome;
 import mage.constants.RangeOfInfluence;
 import mage.game.Game;
 import mage.player.ai.ComputerPlayer;
+import mage.target.Target;
 
 import java.util.LinkedHashMap;
 import java.util.UUID;
@@ -59,4 +61,13 @@ public class TestComputerPlayer extends ComputerPlayer {
         // default implementation by AI
         return super.chooseSpellAbilityForCast(ability, game, noMana);
     }
+
+    @Override
+    public boolean choose(Outcome outcome, Target target, UUID sourceId, Game game) {
+        // copy-paste for TestComputerXXX
+
+        // workaround for discard spells
+        // reason: TestPlayer uses outer computerPlayer to discard but inner code uses choose
+        return testPlayerLink.choose(outcome, target, sourceId, game);
+    }
 }

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

@@ -3,9 +3,11 @@ package org.mage.test.player;
 import mage.MageObject;
 import mage.abilities.ActivatedAbility;
 import mage.abilities.SpellAbility;
+import mage.constants.Outcome;
 import mage.constants.RangeOfInfluence;
 import mage.game.Game;
 import mage.player.ai.ComputerPlayer7;
+import mage.target.Target;
 
 import java.util.LinkedHashMap;
 import java.util.UUID;
@@ -59,4 +61,13 @@ public class TestComputerPlayer7 extends ComputerPlayer7 {
         // default implementation by AI
         return super.chooseSpellAbilityForCast(ability, game, noMana);
     }
+
+    @Override
+    public boolean choose(Outcome outcome, Target target, UUID sourceId, Game game) {
+        // copy-paste for TestComputerXXX
+
+        // workaround for discard spells
+        // reason: TestPlayer uses outer computerPlayer to discard but inner code uses choose
+        return testPlayerLink.choose(outcome, target, sourceId, game);
+    }
 }

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

@@ -1160,11 +1160,16 @@ public class TestPlayer implements Player {
     @Override
     public boolean choose(Outcome outcome, Target target, UUID sourceId, Game game, Map<String, Serializable> options) {
         if (!choices.isEmpty()) {
+
+            List<String> usedChoices = new ArrayList<>();
+            List<UUID> usedTargets = new ArrayList<>();
+
             Ability source = null;
             StackObject stackObject = game.getStack().getStackObject(sourceId);
             if (stackObject != null) {
                 source = stackObject.getStackAbility();
             }
+
             if ((target instanceof TargetPermanent) || (target instanceof TargetPermanentOrPlayer)) { // player target not implemted yet
                 FilterPermanent filterPermanent;
                 if (target instanceof TargetPermanentOrPlayer) {
@@ -1218,6 +1223,7 @@ public class TestPlayer implements Player {
                     }
                 }
             }
+
             if (target instanceof TargetPlayer) {
                 for (Player player : game.getPlayers().values()) {
                     for (String choose2 : choices) {
@@ -1231,42 +1237,74 @@ public class TestPlayer implements Player {
                     }
                 }
             }
+
+            // TODO: add same choices fixes for other target types (one choice must uses only one time for one target)
             if (target instanceof TargetCard) {
-                TargetCard targetCard = ((TargetCard) target);
-                Set<UUID> possibleTargets = targetCard.possibleTargets(sourceId, target.getTargetController() == null ? getId() : target.getTargetController(), game);
-                for (String choose2 : choices) {
-                    String[] targetList = choose2.split("\\^");
+                // one choice per target
+                // only unique targets
+                //TargetCard targetFull = ((TargetCard) target);
+
+                usedChoices.clear();
+                usedTargets.clear();
+                boolean targetCompleted = false;
+
+                CheckAllChoices:
+                for (String choiceRecord : choices) {
+                    if (targetCompleted) {
+                        break CheckAllChoices;
+                    }
+
                     boolean targetFound = false;
-                    Choice:
-                    for (String targetName : targetList) {
-                        for (UUID targetId : possibleTargets) {
+                    String[] possibleChoices = choiceRecord.split("\\^");
+
+                    CheckOneChoice:
+                    for (String possibleChoice : possibleChoices) {
+                        Set<UUID> possibleCards = target.possibleTargets(sourceId, target.getTargetController() == null ? getId() : target.getTargetController(), game);
+                        CheckTargetsList:
+                        for (UUID targetId : possibleCards) {
                             MageObject targetObject = game.getObject(targetId);
-                            if (targetObject != null) {
-                                if (targetObject.getName().equals(targetName)) {
-                                    if (targetCard.canTarget(targetObject.getId(), game)) {
-                                        if (targetCard.getTargets() != null && !targetCard.getTargets().contains(targetObject.getId())) {
-                                            targetCard.add(targetObject.getId(), game);
-                                            targetFound = true;
-                                            if (target.getTargets().size() >= target.getMaxNumberOfTargets()) {
-                                                break Choice;
-                                            }
-                                        }
+                            if (targetObject != null && targetObject.getName().equals(possibleChoice)) {
+                                if (target.canTarget(targetObject.getId(), game)) {
+                                    // only unique targets
+                                    if (usedTargets.contains(targetObject.getId())) {
+                                        continue;
                                     }
+
+                                    // OK, can use it
+                                    target.add(targetObject.getId(), game);
+                                    targetFound = true;
+                                    usedTargets.add(targetObject.getId());
+
+                                    // break on full targets list
+                                    if (target.getTargets().size() >= target.getMaxNumberOfTargets()) {
+                                        targetCompleted = true;
+                                        break CheckOneChoice;
+                                    }
+
+                                    // restart search
+                                    break CheckTargetsList;
                                 }
                             }
-
                         }
                     }
+
                     if (targetFound) {
-                        if (targetCard.isChosen()) {
-                            choices.remove(choose2);
-                            return true;
-                        } else {
-                            target.clearChosen();
-                        }
+                        usedChoices.add(choiceRecord);
+                    }
+                }
+
+                // apply only on ALL targets or revert
+                if (usedChoices.size() > 0) {
+                    if (target.isChosen()) {
+                        choices.removeAll(usedChoices);
+                        return true;
+                    } else {
+                        Assert.fail("Not full targets list.");
+                        target.clearChosen();
                     }
                 }
             }
+
             if (target instanceof TargetSource) {
                 Set<UUID> possibleTargets;
                 TargetSource t = ((TargetSource) target);
@@ -2774,7 +2812,7 @@ public class TestPlayer implements Player {
 
     @Override
     public SpellAbility chooseSpellAbilityForCast(SpellAbility ability, Game game, boolean noMana) {
-        Assert.fail("That's method calls only from computerPlayer->cast(), see TestComputerPlayerXXX");
+        Assert.fail("That's method must calls only from computerPlayer->cast(), see TestComputerPlayerXXX");
         return computerPlayer.chooseSpellAbilityForCast(ability, game, noMana);
     }
 

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

@@ -1132,17 +1132,17 @@ public abstract class CardTestPlayerAPIImpl extends MageTestPlayerBase implement
     }
 
     public void assertActionsCount(TestPlayer player, int count) throws AssertionError {
-        Assert.assertEquals("(Actions " + player.getName() + ") Count are not equel (founded ["
+        Assert.assertEquals("(Actions of " + player.getName() + ") Count are not equel (founded ["
                 + player.getActions().stream().map(PlayerAction::getAction).collect(Collectors.joining(", "))
                 + "])", count, player.getActions().size());
     }
 
     public void assertChoicesCount(TestPlayer player, int count) throws AssertionError {
-        Assert.assertEquals("(Choices " + player.getName() + ") Count are not equel (founded " + player.getChoices() + ")", count, player.getChoices().size());
+        Assert.assertEquals("(Choices of " + player.getName() + ") Count are not equel (founded " + player.getChoices() + ")", count, player.getChoices().size());
     }
 
     public void assertTargetsCount(TestPlayer player, int count) throws AssertionError {
-        Assert.assertEquals("(Targets " + player.getName() + ") Count are not equel (founded " + player.getTargets() + ")", count, player.getTargets().size());
+        Assert.assertEquals("(Targets of " + player.getName() + ") Count are not equel (founded " + player.getTargets() + ")", count, player.getTargets().size());
     }
 
     public void assertAllCommandsUsed() throws AssertionError {