Attacker/blocker indexing fix - updated TestPlayer and unit tests

Mage.Tests/src/test/java/org/mage/test/cards/abilities/keywords/PersistTest.java

@@ -142,8 +142,8 @@ public class PersistTest extends CardTestPlayerBase {
         addCard(Zone.BATTLEFIELD, playerB, "Torpor Orb", 1);
 
         attack(2, playerB, "Wurmcoil Engine");
-        block(2, playerA, "Kitchen Finks", "Wurmcoil Engine");
+        block(2, playerA, "Kitchen Finks:0", "Wurmcoil Engine");
-        block(2, playerA, "Kitchen Finks", "Wurmcoil Engine");
+        block(2, playerA, "Kitchen Finks:1", "Wurmcoil Engine");
 
         setChoice(playerB, "Creatures entering the battlefield don't cause abilities to trigger");
         setChoice(playerB, "Creatures entering the battlefield don't cause abilities to trigger");

Mage.Tests/src/test/java/org/mage/test/cards/copy/CryptoplasmTest.java

@@ -5,6 +5,13 @@ import mage.constants.Zone;
 import org.junit.Test;
 import org.mage.test.serverside.base.CardTestPlayerBase;
 
+/**
+ * Mana cost: 1UU
+ * Type: Creature — Shapeshifter
+ * Effect of card: At the beginning of your upkeep, you may have Cryptoplasm become a copy of another target creature. If you do, Cryptoplasm gains this ability.
+ * Power/Toughness: 2/2
+ */
+
 public class CryptoplasmTest extends CardTestPlayerBase {
 
     @Test
@@ -100,8 +107,8 @@ public class CryptoplasmTest extends CardTestPlayerBase {
         addTarget(playerB, "Divinity of Pride");
 
         attack(3, playerA, "Divinity of Pride");
-        block(3, playerB, "Divinity of Pride", "Divinity of Pride");
+        block(3, playerB, "Divinity of Pride:0", "Divinity of Pride");
-        block(3, playerB, "Divinity of Pride", "Divinity of Pride");
+        block(3, playerB, "Divinity of Pride:1", "Divinity of Pride");
 
         setStopAt(3, PhaseStep.POSTCOMBAT_MAIN);
         execute();
@@ -115,6 +122,7 @@ public class CryptoplasmTest extends CardTestPlayerBase {
         assertLife(playerA, 25);
     }
 
+
     @Test
     public void testTransformMultipleTime() {
         // At the beginning of your upkeep, you may have Cryptoplasm become a copy of another target creature. If you do, Cryptoplasm gains this ability.

Mage.Tests/src/test/java/org/mage/test/cards/triggers/dies/XathridNecromancerTest.java

@@ -67,8 +67,8 @@ public class XathridNecromancerTest extends CardTestPlayerBase {
         attack(2, playerB, "Pillarfield Ox");
         attack(2, playerB, "Siege Mastodon");
 
-        block(2, playerA, "Human", "Silvercoat Lion");
+        block(2, playerA, "Human:0", "Silvercoat Lion");
-        block(2, playerA, "Human", "Pillarfield Ox");
+        block(2, playerA, "Human:1", "Pillarfield Ox");
         block(2, playerA, "Xathrid Necromancer", "Siege Mastodon");
         
         setStopAt(2, PhaseStep.POSTCOMBAT_MAIN);

Mage.Tests/src/test/java/org/mage/test/combat/AttackBlockRestrictionsTest.java

@@ -231,16 +231,16 @@ public class AttackBlockRestrictionsTest extends CardTestPlayerBase {
      */
     @Test
     public void testCantBeBlocked() {
-        addCard(Zone.BATTLEFIELD, playerB, "Blighted Agent");
 
         addCard(Zone.BATTLEFIELD, playerA, "Blighted Agent");
+        addCard(Zone.BATTLEFIELD, playerB, "Blighted Agent");
         addCard(Zone.BATTLEFIELD, playerA, "Llanowar Elves");
         addCard(Zone.BATTLEFIELD, playerA, "Birds of Paradise");
 
-        attack(2, playerB, "Blighted Agent");
+        attack(2, playerB, "Blighted Agent:0");
-        block(2, playerA, "Blighted Agent", "Blighted Agent");
+        block(2, playerA, "Blighted Agent:0", "Blighted Agent:0");
-        block(2, playerA, "Llanowar Elves", "Blighted Agent");
+        block(2, playerA, "Llanowar Elves:0", "Blighted Agent:0");
-        block(2, playerA, "Birds of Paradise", "Blighted Agent");
+        block(2, playerA, "Birds of Paradise:0", "Blighted Agent:0");
 
         setStopAt(2, PhaseStep.END_TURN);
         execute();
@@ -468,8 +468,8 @@ public class AttackBlockRestrictionsTest extends CardTestPlayerBase {
         addCard(Zone.BATTLEFIELD, playerB, "Memnite", 2); // 1/1
 
         attack(3, playerA, "Underworld Cerberus");
-        block(3, playerB, "Memnite", "Underworld Cerberus");
+        block(3, playerB, "Memnite:0", "Underworld Cerberus");
-        block(3, playerB, "Memnite", "Underworld Cerberus");
+        block(3, playerB, "Memnite:1", "Underworld Cerberus");
 
         setStopAt(3, PhaseStep.POSTCOMBAT_MAIN);
 
@@ -495,9 +495,9 @@ public class AttackBlockRestrictionsTest extends CardTestPlayerBase {
 
         // Blocked by 3 creatures - this is acceptable
         attack(3, playerA, "Underworld Cerberus");
-        block(3, playerB, "Memnite", "Underworld Cerberus");
+        block(3, playerB, "Memnite:0", "Underworld Cerberus");
-        block(3, playerB, "Memnite", "Underworld Cerberus");
+        block(3, playerB, "Memnite:1", "Underworld Cerberus");
-        block(3, playerB, "Memnite", "Underworld Cerberus");
+        block(3, playerB, "Memnite:2", "Underworld Cerberus");
 
         setStopAt(3, PhaseStep.POSTCOMBAT_MAIN);
 
@@ -524,7 +524,7 @@ public class AttackBlockRestrictionsTest extends CardTestPlayerBase {
         // Blocked by 10 creatures - this is acceptable as it's >3
         attack(3, playerA, "Underworld Cerberus");
         for(int i = 0; i < 10; i++) {
-            block(3, playerB, "Memnite", "Underworld Cerberus");
+            block(3, playerB, "Memnite:" + i, "Underworld Cerberus");
         }
 
         setStopAt(3, PhaseStep.POSTCOMBAT_MAIN);

Mage.Tests/src/test/java/org/mage/test/combat/CanBlockMultipleCreaturesTest.java

@@ -139,7 +139,7 @@ public class CanBlockMultipleCreaturesTest extends CardTestPlayerBase {
             execute();
             fail("Expected exception not thrown");
         } catch(UnsupportedOperationException e) {
-            assertEquals("Hundred-Handed One cannot block Fabled Hero", e.getMessage());
+            assertEquals("Hundred-Handed One cannot block Fabled Hero it is already blocking the maximum amount of creatures.", e.getMessage());
         }
     }
 

Mage.Tests/src/test/java/org/mage/test/commander/duel/AnafenzaTest.java

@@ -73,8 +73,8 @@ public class AnafenzaTest extends CardTestCommanderDuelBase {
         castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Anafenza, the Foremost");
 
         attack(3, playerA, "Anafenza, the Foremost");
-        block(3, playerB, "Runed Servitor", "Anafenza, the Foremost");
+        block(3, playerB, "Runed Servitor:0", "Anafenza, the Foremost");
-        block(3, playerB, "Runed Servitor", "Anafenza, the Foremost");
+        block(3, playerB, "Runed Servitor:1", "Anafenza, the Foremost");
 
         setStopAt(3, PhaseStep.POSTCOMBAT_MAIN);
         execute();
@@ -115,9 +115,9 @@ public class AnafenzaTest extends CardTestCommanderDuelBase {
         playLand(4, PhaseStep.PRECOMBAT_MAIN, playerB, "Forest");
 
         attack(5, playerA, "Acidic Slime");
-        block(5, playerB, "Elemental", "Acidic Slime");
+        block(5, playerB, "Elemental:0", "Acidic Slime");
         attack(5, playerA, "Anafenza, the Foremost");
-        block(5, playerB, "Elemental", "Anafenza, the Foremost");
+        block(5, playerB, "Elemental:1", "Anafenza, the Foremost");
         addTarget(playerB, playerA);
 
         setStopAt(5, PhaseStep.POSTCOMBAT_MAIN);

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

@@ -29,6 +29,9 @@ package org.mage.test.player;
 
 import java.io.Serializable;
 import java.util.*;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
 import mage.MageObject;
 import mage.MageObjectReference;
 import mage.abilities.Abilities;
@@ -98,8 +101,8 @@ import mage.target.common.TargetPermanentOrPlayer;
 import org.junit.Ignore;
 
 /**
- *
  * @author BetaSteward_at_googlemail.com
+ * @author Simown
  */
 @Ignore
 public class TestPlayer implements Player {
@@ -124,7 +127,6 @@ public class TestPlayer implements Player {
     public TestPlayer(final TestPlayer testPlayer) {
         this.AIPlayer = testPlayer.AIPlayer;
         this.foundNoAction = testPlayer.foundNoAction;
-
         this.actions.addAll(testPlayer.actions);
         this.choices.addAll(testPlayer.choices);
         this.targets.addAll(testPlayer.targets);
@@ -156,25 +158,61 @@ public class TestPlayer implements Player {
     }
 
     /**
-     *
      * @param maxCallsWithoutAction max number of priority passes a player may
-     * have for this test (default = 100)
+     *                              have for this test (default = 100)
      */
     public void setMaxCallsWithoutAction(int maxCallsWithoutAction) {
         this.maxCallsWithoutAction = maxCallsWithoutAction;
     }
 
-    protected Permanent findPermanent(FilterPermanent filter, UUID controllerId, Game game) {
+
-        List<Permanent> permanents = game.getBattlefield().getAllActivePermanents(filter, controllerId, game);
+    private Permanent findPermanent(FilterPermanent filter, String name, UUID controllerID, Game game) {
-        if (!permanents.isEmpty()) {
+        return findPermanent(filter, name, controllerID, game, true);
-            return permanents.get(0);
-        }
-        return null;
     }
 
-    // Gets all permanents that match the filter
+    /**
-    protected List<Permanent> findPermanents(FilterPermanent filter, UUID controllerId, Game game) {
+     * Finds a permanent based on a general filter an their name and possible index.
-        return game.getBattlefield().getAllActivePermanents(filter, controllerId, game);
+     *
+     * An index is permitted after the permanent's name to denote their index on the battlefield
+     * Either use name="<permanent>" which will get the first permanent with that name on the battlefield
+     * that meets the filter criteria or name="<permanent>:<index>" to get the named permanent with that index on
+     * the battlefield.
+     *
+     * Permanents are zero indexed in the order they entered the battlefield for each controller:
+     *
+     * findPermanent(new AttackingCreatureFilter(), "Human", <controllerID>, <game>)
+     * Will find the first "Human" creature that entered the battlefield under this controller and is attacking.
+     *
+     * findPermanent(new FilterControllerPermanent(), "Fabled Hero:3", <controllerID>, <game>)
+     * Will find the 4th permanent named "Fabled Hero" that entered the battlefield under this controller
+     *
+     * An exception will be thrown if no permanents match the criteria or the index is larger than the number
+     * of permanents found with that name.
+     *
+     * failOnNotFound boolean controls if this function returns null for a permanent not found on the battlefield. Currently
+     * used only as a workaround for attackers  in selectAttackers() being able to attack multiple times each combat. See issue #3038
+     */
+    private Permanent findPermanent(FilterPermanent filter, String name, UUID controllerID, Game game, boolean failOnNotFound) {
+        String filteredName = name;
+        Pattern indexedName = Pattern.compile("^([\\w| ]+):(\\d+)$"); // Ends with <:number>
+        Matcher indexedMatcher = indexedName.matcher(filteredName);
+        int index = 0;
+        if (indexedMatcher.matches()) {
+            filteredName = indexedMatcher.group(1);
+            index = Integer.valueOf(indexedMatcher.group(2));
+        }
+        filter.add(new NamePredicate(filteredName));
+        List<Permanent> allPermanents = game.getBattlefield().getAllActivePermanents(filter, controllerID, game);
+        if (allPermanents.isEmpty()) {
+            if (failOnNotFound)
+                throw new UnsupportedOperationException("No permanents found called " + filteredName + " that match the filter criteria \"" + filter.getMessage() + "\"");
+            return null;
+        } else if (allPermanents.size() - 1 < index) {
+            if (failOnNotFound)
+                throw new UnsupportedOperationException("Cannot find " + filteredName + ":" + index + " that match the filter criteria \"" + filter.getMessage() + "\"" + ".\n Only " + allPermanents.size() + " called " + filteredName + " found for this controller(zero indexed).");
+            return null;
+        }
+        return allPermanents.get(index);
     }
 
     private boolean checkExecuteCondition(String[] groups, Game game) {
@@ -543,10 +581,10 @@ public class TestPlayer implements Player {
                     }
                 }
                 FilterCreatureForCombat filter = new FilterCreatureForCombat();
-                filter.add(new NamePredicate(groups[0]));
                 filter.add(Predicates.not(new AttackingPredicate()));
                 filter.add(Predicates.not(new SummoningSicknessPredicate()));
-                Permanent attacker = findPermanent(filter, computerPlayer.getId(), game);
+                // TODO: Cannot enforce legal attackers multiple times per combat. See issue #3038
+                Permanent attacker = findPermanent(filter, groups[0], computerPlayer.getId(), game, false);
                 if (attacker != null && attacker.canAttack(defenderId, game)) {
                     computerPlayer.declareAttacker(attacker.getId(), defenderId, game, false);
                 }
@@ -554,6 +592,7 @@ public class TestPlayer implements Player {
         }
     }
 
+
     @Override
     public void selectBlockers(Game game, UUID defendingPlayerId) {
         UUID opponentId = game.getOpponents(computerPlayer.getId()).iterator().next();
@@ -564,27 +603,14 @@ public class TestPlayer implements Player {
                 String command = action.getAction();
                 command = command.substring(command.indexOf("block:") + 6);
                 String[] groups = command.split("\\$");
-                FilterAttackingCreature filterAttacker = new FilterAttackingCreature();
+                String blockerName = groups[0];
-                filterAttacker.add(new NamePredicate(groups[1]));
+                String attackerName = groups[1];
-                Permanent attacker = findPermanent(filterAttacker, opponentId, game);
+                Permanent attacker = findPermanent(new FilterAttackingCreature(), attackerName, opponentId, game);
-                FilterControlledPermanent filterPermanent = new FilterControlledPermanent();
+                Permanent blocker = findPermanent(new FilterControlledPermanent(), blockerName, computerPlayer.getId(), game);
-                filterPermanent.add(new NamePredicate(groups[0]));
+                if (canBlockAnother(game, blocker, attacker, blockedCreaturesByCreature)) {
-                // Get all possible blockers - those with the same name on the battlefield
+                    computerPlayer.declareBlocker(defendingPlayerId, blocker.getId(), attacker.getId(), game);
-                List<Permanent> possibleBlockers = findPermanents(filterPermanent, computerPlayer.getId(), game);
+                } else {
-                if (!possibleBlockers.isEmpty() && attacker != null) {
+                    throw new UnsupportedOperationException(blockerName + " cannot block " + attackerName + " it is already blocking the maximum amount of creatures.");
-                    boolean blockerFound = false;
-                    for (Permanent blocker : possibleBlockers) {
-                        // See if it can block this creature
-                        if (canBlockAnother(game, blocker, attacker, blockedCreaturesByCreature)) {
-                            computerPlayer.declareBlocker(defendingPlayerId, blocker.getId(), attacker.getId(), game);
-                            blockerFound = true;
-                            break;
-                        }
-                    }
-                    // If we haven't found a blocker then an illegal block has been made in the test
-                    if (!blockerFound) {
-                        throw new UnsupportedOperationException(groups[0] + " cannot block " + groups[1]);
-                    }
                 }
             }
         }
@@ -998,6 +1024,7 @@ public class TestPlayer implements Player {
         return computerPlayer.chooseTarget(outcome, target, source, game);
     }
 
+
     @Override
     public boolean chooseTarget(Outcome outcome, Cards cards, TargetCard target, Ability source, Game game) {
         if (!targets.isEmpty()) {
@@ -2098,32 +2125,19 @@ public class TestPlayer implements Player {
     public boolean choose(Outcome outcome, Cards cards, TargetCard target, Game game) {
         if (!choices.isEmpty()) {
             for (String choose2 : choices) {
+                // TODO: More targetting to fix
                 String[] targetList = choose2.split("\\^");
                 boolean targetFound = false;
                 for (String targetName : targetList) {
-                    boolean originOnly = false;
-                    boolean copyOnly = false;
-                    if (targetName.endsWith("]")) {
-                        if (targetName.endsWith("[no copy]")) {
-                            originOnly = true;
-                            targetName = targetName.substring(0, targetName.length() - 9);
-                        }
-                        if (targetName.endsWith("[only copy]")) {
-                            copyOnly = true;
-                            targetName = targetName.substring(0, targetName.length() - 11);
-                        }
-                    }
                     for (Card card : cards.getCards(game)) {
                         if (target.getTargets().contains(card.getId())) {
                             continue;
                         }
                         if (card.getName().equals(targetName)) {
                             if (target.isNotTarget() || target.canTarget(card.getId(), game)) {
-                                if ((card.isCopy() && !originOnly) || (!card.isCopy() && !copyOnly)) {
+                                target.add(card.getId(), game);
-                                    target.add(card.getId(), game);
+                                targetFound = true;
-                                    targetFound = true;
+                                break;
-                                    break;
-                                }
                             }
                         }
                     }

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

@@ -3,6 +3,9 @@ package org.mage.test.serverside.base.impl;
 import java.io.FileNotFoundException;
 import java.util.List;
 import java.util.UUID;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
 import mage.abilities.Ability;
 import mage.cards.Card;
 import mage.cards.decks.Deck;
@@ -158,12 +161,6 @@ public abstract class CardTestPlayerAPIImpl extends MageTestPlayerBase implement
         return player;
     }
 
-    /**
-     * Starts testing card by starting current game.
-     *
-     * @throws IllegalStateException In case game wasn't created previously. Use
-     * {@link #load} method to initialize the game.
-     */
     public void execute() throws IllegalStateException {
         if (currentGame == null || activePlayer == null) {
             throw new IllegalStateException("Game is not initialized. Use load method to load a test case and initialize a game.");
@@ -952,7 +949,7 @@ public abstract class CardTestPlayerAPIImpl extends MageTestPlayerBase implement
     }
 
     /**
-     * Asserts added actions count. Usefull to make sure that all actions were
+     * Asserts added actions count. Useful to make sure that all actions were
      * executed.
      *
      * @param player
@@ -966,17 +963,29 @@ public abstract class CardTestPlayerAPIImpl extends MageTestPlayerBase implement
         Assert.assertEquals("message", currentGame.getState().getActivePlayerId(), player.getId());
     }
 
-    public Permanent getPermanent(String cardName) {
+    
+    public Permanent getPermanent(String cardName, UUID controller) {
         Permanent found = null;
+        Pattern indexedName = Pattern.compile("^([\\w| ]+):(\\d+)$"); // Ends with <:number>
+        Matcher indexedMatcher = indexedName.matcher(cardName);
+        int index = 0;
+        int count = 0;
+        if(indexedMatcher.matches()) {
+        	cardName = indexedMatcher.group(1);
+        	index = Integer.valueOf(indexedMatcher.group(2));
+        }
         for (Permanent permanent : currentGame.getBattlefield().getAllActivePermanents()) {
             if (permanent.getName().equals(cardName)) {
-                found = permanent;
+            	if (controller == null || permanent.getControllerId().equals(controller)) {
-                break;
+	            	found = permanent;
+	            	if(count != index) {
+	            		count++;
+	            	}
+            	}
             }
         }
-
         Assert.assertNotNull("Couldn't find a card with specified name: " + cardName, found);
-
+        Assert.assertEquals("Only " + count + " permanents were found and " + cardName + ":" + index + " was requested", index, count);
         return found;
     }
     
@@ -984,20 +993,8 @@ public abstract class CardTestPlayerAPIImpl extends MageTestPlayerBase implement
         return getPermanent(cardName, player.getId());
     }
     
-    public Permanent getPermanent(String cardName, UUID controller) {
+    public Permanent getPermanent(String cardName) {
-        Permanent permanent0 = null;
+    	return getPermanent(cardName, (UUID)null);
-        int count = 0;
-        for (Permanent permanent : currentGame.getBattlefield().getAllPermanents()) {
-            if (permanent.getControllerId().equals(controller)) {
-                if (permanent.getName().equals(cardName)) {
-                    permanent0 = permanent;
-                    count++;
-                }
-            }
-        }
-        Assert.assertNotNull("Couldn't find a card with specified name: " + cardName, permanent0);
-        Assert.assertEquals("More than one permanent was found: " + cardName + '(' + count + ')', 1, count);
-        return permanent0;
     }
 
     public void playLand(int turnNum, PhaseStep step, TestPlayer player, String cardName) {
@@ -1065,6 +1062,7 @@ public abstract class CardTestPlayerAPIImpl extends MageTestPlayerBase implement
         castSpell(turnNum, step, player, cardName, targetName, spellOnStack, StackClause.WHILE_ON_STACK);
     }
 
+
     /**
      * Spell will only be cast, if a spell / ability with the given name IS or
      * IS NOT on the stack
@@ -1112,8 +1110,8 @@ public abstract class CardTestPlayerAPIImpl extends MageTestPlayerBase implement
         player.addAction(turnNum, step, "activate:" + ability + "$targetPlayer=" + target.getName());
     }
 
-    public void activateAbility(int turnNum, PhaseStep step, TestPlayer player, String ability, String targetName) {
+    public void activateAbility(int turnNum, PhaseStep step, TestPlayer player, String ability, String ... targetNames) {
-        player.addAction(turnNum, step, "activate:" + ability + "$target=" + targetName);
+        player.addAction(turnNum, step, "activate:" + ability + "$target=" + String.join("^", targetNames));
     }
 
     public void activateAbility(int turnNum, PhaseStep step, TestPlayer player, String ability, String targetName, String spellOnStack) {
@@ -1245,33 +1243,6 @@ public abstract class CardTestPlayerAPIImpl extends MageTestPlayerBase implement
         gameOptions.skipInitShuffling = true;
     }
 
-    protected ExpectedType getExpectedType(String line) {
-        if (line.startsWith("turn:")) {
-            return ExpectedType.TURN_NUMBER;
-        }
-        if (line.startsWith("result:")) {
-            return ExpectedType.RESULT;
-        }
-        if (line.startsWith("life:")) {
-            return ExpectedType.LIFE;
-        }
-        if (line.startsWith("battlefield:")) {
-            return ExpectedType.BATTLEFIELD;
-        }
-        if (line.startsWith("graveyard:")) {
-            return ExpectedType.GRAVEYARD;
-        }
-        return ExpectedType.UNKNOWN;
-    }
-
-    protected String getStringParam(String line, int index) {
-        String[] params = line.split(":");
-        if (index > params.length - 1) {
-            throw new IllegalArgumentException("Not correct line: " + line);
-        }
-        return params[index];
-    }
-
     protected void checkPermanentPT(Player player, String cardName, int power, int toughness, Filter.ComparisonScope scope) {
         if (currentGame == null) {
             throw new IllegalStateException("Current game is null");
@@ -1288,13 +1259,4 @@ public abstract class CardTestPlayerAPIImpl extends MageTestPlayerBase implement
             }
         }
     }
-
-    protected int getIntParam(String line, int index) {
-        String[] params = line.split(":");
-        if (index > params.length - 1) {
-            throw new IllegalArgumentException("Not correct line: " + line);
-        }
-        return Integer.parseInt(params[index]);
-    }
-
 }