Merge pull request #6251 from Dilnu/Bonecrusher

Only fire one TARGETTED event per target selected while changing targets.

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

@@ -77,6 +77,101 @@ public class CopySpellTest extends CardTestPlayerBase {
         assertAbility(playerB, "Silvercoat Lion", FlyingAbility.getInstance(), false);
     }
 
+    @Test
+    public void BonecrusherGiantChangeTargets_BoneTargetBoth() {
+        // Whenever Bonecrusher Giant becomes the target of a spell, Bonecrusher Giant deals 2 damage to that spell’s controller.
+        addCard(Zone.BATTLEFIELD, playerA, "Bonecrusher Giant");
+        //
+        // Target creature gets +2/+2 until end of turn.
+        // Conspire (As you cast this spell, you may tap two untapped creatures you control that share a color with it. When you do, copy it and you may choose a new target for the copy.)
+        addCard(Zone.HAND, playerA, "Barkshell Blessing");
+        addCard(Zone.BATTLEFIELD, playerA, "Plains");
+        addCard(Zone.BATTLEFIELD, playerA, "Grizzly Bears");
+        addCard(Zone.BATTLEFIELD, playerA, "Savannah Lions");
+
+        castSpell(1, PhaseStep.UPKEEP, playerA, "Barkshell Blessing");
+        setChoice(playerA, "Yes"); // use Conspire
+        addTarget(playerA, "Bonecrusher Giant"); // target bone
+        setChoice(playerA, "Grizzly Bears"); // pay for conspire
+        setChoice(playerA, "Savannah Lions"); // pay for conspire
+        setChoice(playerA, "When you pay"); // Put Conspire on the stack first.
+        setChoice(playerA, "No"); // both spells target bone
+
+        setStrictChooseMode(true);
+        setStopAt(1, PhaseStep.PRECOMBAT_MAIN);
+        execute();
+        assertAllCommandsUsed();
+
+        assertPowerToughness(playerA, "Bonecrusher Giant", 4 + 2 * 2, 3 + 2 * 2);
+        assertPowerToughness(playerA, "Grizzly Bears", 2, 2);
+        assertPowerToughness(playerA, "Savannah Lions", 2, 1);
+        assertLife(playerA, 20 - 2 * 2); // bone trigger from both spells
+    }
+
+    @Test
+    public void BonecrusherGiantChangeTargets_BoneTargetFirst() {
+        // Whenever Bonecrusher Giant becomes the target of a spell, Bonecrusher Giant deals 2 damage to that spell’s controller.
+        addCard(Zone.BATTLEFIELD, playerA, "Bonecrusher Giant");
+        //
+        // Target creature gets +2/+2 until end of turn.
+        // Conspire (As you cast this spell, you may tap two untapped creatures you control that share a color with it. When you do, copy it and you may choose a new target for the copy.)
+        addCard(Zone.HAND, playerA, "Barkshell Blessing");
+        addCard(Zone.BATTLEFIELD, playerA, "Plains");
+        addCard(Zone.BATTLEFIELD, playerA, "Grizzly Bears");
+        addCard(Zone.BATTLEFIELD, playerA, "Savannah Lions");
+
+        castSpell(1, PhaseStep.UPKEEP, playerA, "Barkshell Blessing");
+        setChoice(playerA, "Yes"); // use Conspire
+        addTarget(playerA, "Bonecrusher Giant"); // target bone
+        setChoice(playerA, "Grizzly Bears"); // pay for conspire
+        setChoice(playerA, "Savannah Lions"); // pay for conspire
+        setChoice(playerA, "When you pay"); // Put Conspire on the stack first.
+        setChoice(playerA, "Yes"); // new target for copy: bear
+        addTarget(playerA, "Grizzly Bears");
+
+        setStrictChooseMode(true);
+        setStopAt(1, PhaseStep.PRECOMBAT_MAIN);
+        execute();
+        assertAllCommandsUsed();
+
+        assertPowerToughness(playerA, "Bonecrusher Giant", 4 + 2, 3 + 2);
+        assertPowerToughness(playerA, "Grizzly Bears", 2 + 2, 2 + 2);
+        assertPowerToughness(playerA, "Savannah Lions", 2, 1);
+        assertLife(playerA, 20 - 2); // one trigger
+    }
+
+    @Test
+    public void BonecrusherGiantChangeTargets_BoneTargetSecond() {
+        // Whenever Bonecrusher Giant becomes the target of a spell, Bonecrusher Giant deals 2 damage to that spell’s controller.
+        addCard(Zone.BATTLEFIELD, playerA, "Bonecrusher Giant");
+        //
+        // Target creature gets +2/+2 until end of turn.
+        // Conspire (As you cast this spell, you may tap two untapped creatures you control that share a color with it. When you do, copy it and you may choose a new target for the copy.)
+        addCard(Zone.HAND, playerA, "Barkshell Blessing");
+        addCard(Zone.BATTLEFIELD, playerA, "Plains");
+        addCard(Zone.BATTLEFIELD, playerA, "Grizzly Bears");
+        addCard(Zone.BATTLEFIELD, playerA, "Savannah Lions");
+
+        castSpell(1, PhaseStep.UPKEEP, playerA, "Barkshell Blessing");
+        setChoice(playerA, "Yes"); // use Conspire
+        addTarget(playerA, "Grizzly Bears"); // target bear
+        setChoice(playerA, "Grizzly Bears"); // pay for conspire
+        setChoice(playerA, "Savannah Lions"); // pay for conspire
+        setChoice(playerA, "Yes"); // new target for copy: bone
+        addTarget(playerA, "Bonecrusher Giant");
+        // setChoice(playerA, "When {this} becomes the target of a spell"); // must be one trigger from bone, not two
+
+        setStrictChooseMode(true);
+        setStopAt(1, PhaseStep.PRECOMBAT_MAIN);
+        execute();
+        assertAllCommandsUsed();
+
+        assertPowerToughness(playerA, "Bonecrusher Giant", 4 + 2, 3 + 2);
+        assertPowerToughness(playerA, "Grizzly Bears", 2 + 2, 2 + 2);
+        assertPowerToughness(playerA, "Savannah Lions", 2, 1);
+        assertLife(playerA, 20 - 2); // one trigger
+    }
+
     /*
      * Reported bug: "Silverfur Partisan and fellow wolves did not trigger off
      * of copies of Strength of Arms made by Zada, Hedron Grinder. Not sure

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

@@ -1883,7 +1883,7 @@ public class TestPlayer implements Player {
                         for (Player player : game.getPlayers().values()) {
                             if (player.getName().equals(playerName)
                                     && target.canTarget(computerPlayer.getId(), player.getId(), source, game)) {
-                                target.add(player.getId(), game);
+                                target.addTarget(player.getId(), source, game);
                                 targets.remove(targetDefinition);
                                 return true;
                             }
@@ -1932,7 +1932,7 @@ public class TestPlayer implements Player {
                             if (isObjectHaveTargetNameOrAlias(permanent, targetName) || (permanent.getName() + '-' + permanent.getExpansionSetCode()).equals(targetName)) { // TODO: remove exp code search?
                                 if (target.canTarget(abilityControllerId, permanent.getId(), source, game) && !target.getTargets().contains(permanent.getId())) {
                                     if ((permanent.isCopy() && !originOnly) || (!permanent.isCopy() && !copyOnly)) {
-                                        target.add(permanent.getId(), game);
+                                        target.addTarget(permanent.getId(), source, game);
                                         targetFound = true;
                                         break; // return to for (String targetName
                                     }
@@ -1957,7 +1957,7 @@ public class TestPlayer implements Player {
                         for (Card card : computerPlayer.getHand().getCards(((TargetCardInHand) target.getOriginalTarget()).getFilter(), game)) {
                             if (isObjectHaveTargetNameOrAlias(card, targetName) || (card.getName() + '-' + card.getExpansionSetCode()).equals(targetName)) { // TODO: remove set code search?
                                 if (target.canTarget(abilityControllerId, card.getId(), source, game) && !target.getTargets().contains(card.getId())) {
-                                    target.add(card.getId(), game);
+                                    target.addTarget(card.getId(), source, game);
                                     targetFound = true;
                                     break; // return to for (String targetName
                                 }
@@ -1982,7 +1982,7 @@ public class TestPlayer implements Player {
                         for (Card card : game.getExile().getCards(targetFull.getFilter(), game)) {
                             if (isObjectHaveTargetNameOrAlias(card, targetName) || (card.getName() + '-' + card.getExpansionSetCode()).equals(targetName)) { // TODO: remove set code search?
                                 if (target.canTarget(abilityControllerId, card.getId(), source, game) && !target.getTargets().contains(card.getId())) {
-                                    target.add(card.getId(), game);
+                                    target.addTarget(card.getId(), source, game);
                                     targetFound = true;
                                     break; // return to for (String targetName
                                 }
@@ -2057,7 +2057,7 @@ public class TestPlayer implements Player {
                             for (Card card : player.getGraveyard().getCards(targetFull.getFilter(), game)) {
                                 if (isObjectHaveTargetNameOrAlias(card, targetName) || (card.getName() + '-' + card.getExpansionSetCode()).equals(targetName)) { // TODO: remove set code search?
                                     if (target.canTarget(abilityControllerId, card.getId(), source, game) && !target.getTargets().contains(card.getId())) {
-                                        target.add(card.getId(), game);
+                                        target.addTarget(card.getId(), source, game);
                                         targetFound = true;
                                         break IterateGraveyards;  // return to for (String targetName
                                     }
@@ -2084,7 +2084,7 @@ public class TestPlayer implements Player {
                         for (StackObject stackObject : game.getStack()) {
                             if (isObjectHaveTargetNameOrAlias(stackObject, targetName)) {
                                 if (target.canTarget(abilityControllerId, stackObject.getId(), source, game) && !target.getTargets().contains(stackObject.getId())) {
-                                    target.add(stackObject.getId(), game);
+                                    target.addTarget(stackObject.getId(), source, game);
                                     targetFound = true;
                                     break; // return to for (String targetName
                                 }
@@ -2133,7 +2133,7 @@ public class TestPlayer implements Player {
                 for (String targetName : targetList) {
                     for (Card card : cards.getCards(game)) {
                         if (isObjectHaveTargetNameOrAlias(card, targetName) && !target.getTargets().contains(card.getId())) {
-                            target.add(card.getId(), game);
+                            target.addTarget(card.getId(), source, game);
                             targetFound = true;
                             break;
                         }

Mage/src/main/java/mage/game/stack/StackObjImpl.java

@@ -151,6 +151,7 @@ public abstract class StackObjImpl implements StackObject {
      */
     private Target chooseNewTarget(Player targetController, Ability ability, Mode mode, Target target, boolean forceChange, FilterPermanent filterNewTarget, Game game) {
         Target newTarget = target.copy();
+        newTarget.setEventReporting(false);
         if (!targetController.getId().equals(getControllerId())) {
             newTarget.setTargetController(targetController.getId()); // target controller for the change is different from spell controller
             newTarget.setAbilityController(getControllerId());
@@ -199,6 +200,7 @@ public abstract class StackObjImpl implements StackObject {
                 } else {
                     // build a target definition with exactly one possible target to select that replaces old target
                     Target tempTarget = target.copy();
+                    tempTarget.setEventReporting(false);
                     if (target instanceof TargetAmount) {
                         ((TargetAmount) tempTarget).setAmountDefinition(StaticValue.get(target.getTargetAmount(targetId)));
                     }
@@ -215,7 +217,7 @@ public abstract class StackObjImpl implements StackObject {
                         if (!tempTarget.chooseTarget(outcome, getControllerId(), ability, game)) {
                             if (targetController.chooseUse(Outcome.Benefit, "No target object selected. Reset to original target?", ability, game)) {
                                 // use previous target no target was selected
-                                newTarget.addTarget(targetId, target.getTargetAmount(targetId), ability, game, false);
+                                newTarget.addTarget(targetId, target.getTargetAmount(targetId), ability, game, true);
                             } else {
                                 again = true;
                             }
@@ -225,12 +227,12 @@ public abstract class StackObjImpl implements StackObject {
                                 if (targetController.isHuman()) {
                                     if (targetController.chooseUse(Outcome.Benefit, "This target was already selected from origin spell. Reset to original target?", ability, game)) {
                                         // use previous target no target was selected
-                                        newTarget.addTarget(targetId, target.getTargetAmount(targetId), ability, game, false);
+                                        newTarget.addTarget(targetId, target.getTargetAmount(targetId), ability, game, true);
                                     } else {
                                         again = true;
                                     }
                                 } else {
-                                    newTarget.addTarget(targetId, target.getTargetAmount(targetId), ability, game, false);
+                                    newTarget.addTarget(targetId, target.getTargetAmount(targetId), ability, game, true);
                                 }
                             } else if (!target.canTarget(getControllerId(), tempTarget.getFirstTarget(), ability, game)) {
                                 if (targetController.isHuman()) {
@@ -238,7 +240,7 @@ public abstract class StackObjImpl implements StackObject {
                                     again = true;
                                 } else {
                                     // keep the old
-                                    newTarget.addTarget(targetId, target.getTargetAmount(targetId), ability, game, false);
+                                    newTarget.addTarget(targetId, target.getTargetAmount(targetId), ability, game, true);
                                 }
                             } else if (newTarget.getFirstTarget() != null && filterNewTarget != null) {
                                 Permanent newTargetPermanent = game.getPermanent(newTarget.getFirstTarget());
@@ -248,14 +250,14 @@ public abstract class StackObjImpl implements StackObject {
                                 }
                             } else {
                                 // valid target was selected, add it to the new target definition
-                                newTarget.addTarget(tempTarget.getFirstTarget(), target.getTargetAmount(targetId), ability, game, false);
+                                newTarget.addTarget(tempTarget.getFirstTarget(), target.getTargetAmount(targetId), ability, game, true);
                             }
                         }
                     } while (again && targetController.canRespond());
                 }
             } // keep the target
             else {
-                newTarget.addTarget(targetId, target.getTargetAmount(targetId), ability, game, false);
+                newTarget.addTarget(targetId, target.getTargetAmount(targetId), ability, game, true);
             }
         }
         return newTarget;

Mage/src/main/java/mage/target/Target.java

@@ -136,4 +136,6 @@ public interface Target extends Serializable {
     void setTargetAmount(UUID targetId, int amount, Game game);
 
     Target withChooseHint(String chooseHint);
+
+    void setEventReporting(boolean shouldReport);
 }

Mage/src/main/java/mage/target/TargetImpl.java

@@ -37,6 +37,7 @@ public abstract class TargetImpl implements Target {
 
     protected int targetTag; // can be set if other target check is needed (AnotherTargetPredicate)
     protected String chooseHint = null; // UI choose hints after target name
+    protected boolean shouldReportEvents = true;
 
     @Override
     public abstract TargetImpl copy();
@@ -65,6 +66,7 @@ public abstract class TargetImpl implements Target {
         this.abilityController = target.abilityController;
         this.targetTag = target.targetTag;
         this.chooseHint = target.chooseHint;
+        this.shouldReportEvents = target.shouldReportEvents;
     }
 
     @Override
@@ -213,12 +215,12 @@ public abstract class TargetImpl implements Target {
         //20100423 - 113.3
         if (getMaxNumberOfTargets() == 0 || targets.size() < getMaxNumberOfTargets()) {
             if (!targets.containsKey(id)) {
-                if (source != null && !skipEvent) {
+                if (source != null && !skipEvent && shouldReportEvents) {
                     if (!game.replaceEvent(GameEvent.getEvent(EventType.TARGET, id, source.getSourceId(), source.getControllerId()))) {
                         targets.put(id, 0);
                         rememberZoneChangeCounter(id, game);
                         chosen = targets.size() >= getNumberOfTargets();
-                        if (!skipEvent) {
+                        if (!skipEvent && shouldReportEvents) {
                             game.addSimultaneousEvent(GameEvent.getEvent(EventType.TARGETED, id, source.getSourceId(), source.getControllerId()));
                         }
                     }
@@ -251,12 +253,12 @@ public abstract class TargetImpl implements Target {
         if (targets.containsKey(id)) {
             amount += targets.get(id);
         }
-        if (source != null && !skipEvent) {
+        if (source != null && !skipEvent && shouldReportEvents) {
             if (!game.replaceEvent(GameEvent.getEvent(EventType.TARGET, id, source.getSourceId(), source.getControllerId()))) {
                 targets.put(id, amount);
                 rememberZoneChangeCounter(id, game);
                 chosen = targets.size() >= getNumberOfTargets();
-                if (!skipEvent) {
+                if (!skipEvent && shouldReportEvents) {
                     game.fireEvent(GameEvent.getEvent(EventType.TARGETED, id, source.getSourceId(), source.getControllerId()));
                 }
             }
@@ -551,4 +553,9 @@ public abstract class TargetImpl implements Target {
         this.chooseHint = chooseHint;
         return this;
     }
+
+    @Override
+    public void setEventReporting(boolean shouldReport) {
+        this.shouldReportEvents = shouldReport;
+    }
 }