fixed an issue with modular keyword and LKI

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

@@ -8,7 +8,6 @@ import org.junit.Test;
 import org.mage.test.serverside.base.CardTestPlayerBase;
 
 /**
- *
  * @author BetaSteward
  */
 public class ModularTest extends CardTestPlayerBase {
@@ -33,7 +32,6 @@ public class ModularTest extends CardTestPlayerBase {
      * Arcbound Hybrid Artifact Creature — Beast 0/0, 4 (4) Haste Modular 2
      * (This enters the battlefield with two +1/+1 counters on it. When it dies,
      * you may put its +1/+1 counters on target artifact creature.)
-     *
      */
     @Test
     public void testModularEnters() {
@@ -105,4 +103,71 @@ public class ModularTest extends CardTestPlayerBase {
 
     }
 
+    /**
+     * If a creature with modular dies due to -1/-1 counters, it still had those counters when it left
+     * and therefore will still transfer them to a creature on the battlefield
+     */
+    @Test
+    public void testMinusCounters() {
+        addCard(Zone.BATTLEFIELD, playerA, "Mountain", 3);
+        addCard(Zone.BATTLEFIELD, playerA, "Arcbound Bruiser");
+        addCard(Zone.BATTLEFIELD, playerA, "Memnite");
+        addCard(Zone.HAND, playerA, "Puncture Blast");
+
+        castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Puncture Blast", "Arcbound Bruiser");
+        setChoice(playerA, "Yes");
+
+        setStopAt(1, PhaseStep.END_TURN);
+        execute();
+
+        assertGraveyardCount(playerA, "Arcbound Bruiser", 1);
+        assertGraveyardCount(playerA, "Puncture Blast", 1);
+        assertCounterCount("Memnite", CounterType.P1P1, 3);
+    }
+
+    /**
+     * If a creature with modular dies and returns and dies again before any modular triggers resolve,
+     * the modular triggers should use that creature's counters from each time it died
+     * rather than the most recent time it died
+     */
+    @Test
+    public void testReturnedAndKilledAgain() {
+        addCard(Zone.BATTLEFIELD, playerA, "Badlands", 11);
+        addCard(Zone.BATTLEFIELD, playerA, "Arcbound Lancer");
+        addCard(Zone.BATTLEFIELD, playerA, "Memnite");
+        addCard(Zone.HAND, playerA, "Makeshift Mannequin");
+        addCard(Zone.HAND, playerA, "Puncture Blast");
+        addCard(Zone.HAND, playerA, "Flame Slash");
+        addCard(Zone.HAND, playerA, "Murder");
+
+        // put three -1/-1 counters on lancer, which leaves it with one +1/+1
+        castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Puncture Blast", "Arcbound Lancer");
+        setChoice(playerA, "Yes", 2);
+        checkStackSize("stack1", 1, PhaseStep.PRECOMBAT_MAIN, playerA, 1);
+
+        // kill lancer with one +1/+1 counter on it
+        castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Flame Slash", "Arcbound Lancer");
+        waitStackResolved(1, PhaseStep.PRECOMBAT_MAIN, true);
+        checkStackSize("stack2", 1, PhaseStep.PRECOMBAT_MAIN, playerA, 1);
+
+        // in response to modular trigger, return lancer to the battlefield
+        castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Makeshift Mannequin", "Arcbound Lancer");
+        waitStackResolved(1, PhaseStep.PRECOMBAT_MAIN, true);
+        checkStackSize("stack3", 1, PhaseStep.PRECOMBAT_MAIN, playerA, 2);
+
+        // kill lancer again with original modular trigger on the stack
+        castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Murder", "Arcbound Lancer");
+        checkStackSize("stack4", 1, PhaseStep.PRECOMBAT_MAIN, playerA, 2);
+
+        setStopAt(1, PhaseStep.END_TURN);
+        execute();
+        assertAllCommandsUsed();
+
+        assertGraveyardCount(playerA, "Arcbound Lancer", 1);
+        assertGraveyardCount(playerA, "Puncture Blast", 1);
+        assertGraveyardCount(playerA, "Flame Slash", 1);
+        assertGraveyardCount(playerA, "Murder", 1);
+        // Memnite should have 1 counter for the first trigger and 4 from the second
+        assertCounterCount("Memnite", CounterType.P1P1, 1 + 4);
+    }
 }

Mage/src/main/java/mage/abilities/keyword/ModularAbility.java

@@ -1,8 +1,5 @@
 package mage.abilities.keyword;
 
-import java.util.ArrayList;
-import java.util.List;
-import java.util.UUID;
 import mage.abilities.Ability;
 import mage.abilities.StaticAbility;
 import mage.abilities.common.DiesSourceTriggeredAbility;
@@ -20,14 +17,16 @@ import mage.game.events.GameEvent;
 import mage.game.events.ZoneChangeEvent;
 import mage.game.permanent.Permanent;
 import mage.players.Player;
-import mage.target.Target;
 import mage.target.common.TargetArtifactPermanent;
 import mage.util.CardUtil;
 
+import java.util.ArrayList;
+import java.util.List;
+import java.util.UUID;
+
 /**
- *
  * 702.41. Modular
- *
+ * <p>
  * 702.41a Modular represents both a static ability and a triggered ability.
  * "Modular N" means "This permanent enters the battlefield with N +1/+1
  * counters on it" and "When this permanent is put into a graveyard from the
@@ -35,7 +34,6 @@ import mage.util.CardUtil;
  * +1/+1 counter on this permanent." 702.41b If a creature has multiple
  * instances of modular, each one works separately.
  *
- *
  * @author Loki, LevelX2
  */
 public class ModularAbility extends DiesSourceTriggeredAbility {
@@ -45,8 +43,9 @@ public class ModularAbility extends DiesSourceTriggeredAbility {
     static {
         filter.add(CardType.CREATURE.getPredicate());
     }
-    private int amount;
+
-    private boolean sunburst;
+    private final int amount;
+    private final boolean sunburst;
 
     public ModularAbility(Card card, int amount) {
         this(card, amount, false);
@@ -54,8 +53,7 @@ public class ModularAbility extends DiesSourceTriggeredAbility {
 
     public ModularAbility(Card card, int amount, boolean sunburst) {
         super(new ModularDistributeCounterEffect(), true);
-        Target target = new TargetArtifactPermanent(filter);
+        this.addTarget(new TargetArtifactPermanent(filter));
-        this.addTarget(target);
         this.amount = amount;
         this.sunburst = sunburst;
         if (sunburst) {
@@ -67,7 +65,7 @@ public class ModularAbility extends DiesSourceTriggeredAbility {
         }
     }
 
-    public ModularAbility(ModularAbility ability) {
+    private ModularAbility(ModularAbility ability) {
         super(ability);
         this.amount = ability.amount;
         this.sunburst = ability.sunburst;
@@ -102,20 +100,19 @@ public class ModularAbility extends DiesSourceTriggeredAbility {
         }
         return sb.toString();
     }
-
 }
 
 class ModularStaticAbility extends StaticAbility {
 
-    private String ruleText;
+    private final String ruleText;
 
-    public ModularStaticAbility(int amount) {
+    ModularStaticAbility(int amount) {
         super(Zone.ALL, new EntersBattlefieldEffect(new AddCountersSourceEffect(CounterType.P1P1.createInstance(amount))));
         ruleText = "This enters the battlefield with " + CardUtil.numberToText(amount, "a") + " +1/+1 counter" + (amount != 1 ? "s" : "") + " on it.";
         this.setRuleVisible(false);
     }
 
-    public ModularStaticAbility(final ModularStaticAbility ability) {
+    private ModularStaticAbility(final ModularStaticAbility ability) {
         super(ability);
         this.ruleText = ability.ruleText;
     }
@@ -133,18 +130,12 @@ class ModularStaticAbility extends StaticAbility {
 
 class ModularDistributeCounterEffect extends OneShotEffect {
 
-    private static final FilterArtifactPermanent filter = new FilterArtifactPermanent("artifact creature");
+    ModularDistributeCounterEffect() {
-
-    static {
-        filter.add(CardType.CREATURE.getPredicate());
-    }
-
-    public ModularDistributeCounterEffect() {
         super(Outcome.BoostCreature);
         this.staticText = "you may put a +1/+1 counter on target artifact creature for each +1/+1 counter on this permanent";
     }
 
-    public ModularDistributeCounterEffect(final ModularDistributeCounterEffect effect) {
+    private ModularDistributeCounterEffect(final ModularDistributeCounterEffect effect) {
         super(effect);
     }
 
@@ -155,7 +146,7 @@ class ModularDistributeCounterEffect extends OneShotEffect {
 
     @Override
     public boolean apply(Game game, Ability source) {
-        Permanent sourcePermanent = (Permanent) game.getLastKnownInformation(source.getSourceId(), Zone.BATTLEFIELD);
+        Permanent sourcePermanent = (Permanent) getValue("permanentLeftBattlefield");
         Permanent targetArtifact = game.getPermanent(targetPointer.getFirst(game, source));
         Player player = game.getPlayer(source.getControllerId());
         if (sourcePermanent != null && targetArtifact != null && player != null) {