[LRW] added test for Aquitect's Will (partially fails) (#7597)

Mage.Sets/src/mage/cards/a/AquitectsWill.java

@@ -1,7 +1,7 @@
-
 package mage.cards.a;
 
 import mage.abilities.Ability;
+import mage.abilities.condition.Condition;
 import mage.abilities.condition.common.PermanentsOnTheBattlefieldCondition;
 import mage.abilities.decorator.ConditionalOneShotEffect;
 import mage.abilities.effects.common.DrawCardSourceControllerEffect;
@@ -19,19 +19,15 @@ import mage.target.common.TargetLandPermanent;
 import java.util.UUID;
 
 /**
- *
  * @author ilcartographer
  */
 public final class AquitectsWill extends CardImpl {
 
-    private static final FilterControlledPermanent filter = new FilterControlledPermanent("Merfolk");
+    private static final FilterControlledPermanent filter = new FilterControlledPermanent(SubType.MERFOLK);
-
+    private static final Condition condition = new PermanentsOnTheBattlefieldCondition(filter);
-    static {
-        filter.add(SubType.MERFOLK.getPredicate());
-    }
 
     public AquitectsWill(UUID ownerId, CardSetInfo setInfo) {
-        super(ownerId,setInfo,new CardType[]{CardType.TRIBAL,CardType.SORCERY},"{U}");
+        super(ownerId, setInfo, new CardType[]{CardType.TRIBAL, CardType.SORCERY}, "{U}");
         this.subtype.add(SubType.MERFOLK);
 
         // Put a flood counter on target land.
@@ -39,13 +35,13 @@ public final class AquitectsWill extends CardImpl {
         this.getSpellAbility().addTarget(new TargetLandPermanent());
 
         // That land is an Island in addition to its other types for as long as it has a flood counter on it.
-        this.getSpellAbility().addEffect(new AquitectsWillEffect(Duration.Custom, false, false, SubType.ISLAND));
+        this.getSpellAbility().addEffect(new AquitectsWillEffect());
 
         // If you control a Merfolk, draw a card.
         this.getSpellAbility().addEffect(new ConditionalOneShotEffect(
-                new DrawCardSourceControllerEffect(1),
+                new DrawCardSourceControllerEffect(1), condition,
-                new PermanentsOnTheBattlefieldCondition(filter),
+                "If you control a Merfolk, draw a card"
-                "If you control a Merfolk, draw a card"));
+        ));
     }
 
     private AquitectsWill(final AquitectsWill card) {
@@ -60,26 +56,23 @@ public final class AquitectsWill extends CardImpl {
 
 class AquitectsWillEffect extends BecomesBasicLandTargetEffect {
 
-    public AquitectsWillEffect(Duration duration, boolean chooseLandType, boolean loseType, SubType... landNames) {
+    AquitectsWillEffect() {
-        super(duration, chooseLandType, loseType, landNames);
+        super(Duration.Custom, false, false, SubType.ISLAND);
         staticText = "That land is an Island in addition to its other types for as long as it has a flood counter on it";
     }
 
-    public AquitectsWillEffect(final AquitectsWillEffect effect) {
+    private AquitectsWillEffect(final AquitectsWillEffect effect) {
         super(effect);
     }
 
     @Override
     public boolean apply(Layer layer, SubLayer sublayer, Ability source, Game game) {
         Permanent land = game.getPermanent(this.targetPointer.getFirst(game, source));
-        if (land == null) {
+        if (land == null || land.getCounters(game).getCount(CounterType.FLOOD) < 1) {
-            // if permanent left battlefield the effect can be removed because it was only valid for that object
             this.discard();
-        } else if (land.getCounters(game).getCount(CounterType.FLOOD) > 0) {
+            return false;
-            // only if Flood counter is on the object it becomes an Island.(it would be possible to remove and return the counters with e.g. Fate Transfer if the land becomes a creature too)
-            super.apply(layer, sublayer, source, game);
         }
-        return true;
+        return super.apply(layer, sublayer, source, game);
     }
 
     @Override

Mage.Sets/src/mage/cards/c/CyclopeanTomb.java

@@ -49,7 +49,7 @@ public final class CyclopeanTomb extends CardImpl {
         Ability ability = new ConditionalActivatedAbility(Zone.BATTLEFIELD, new AddCountersTargetEffect(CounterType.MIRE.createInstance()), new GenericManaCost(2), new IsStepCondition(PhaseStep.UPKEEP), "{2}, {T}: Put a mire counter on target non-Swamp land. That land is a Swamp for as long as it has a mire counter on it. Activate this ability only during your upkeep.");
         ability.addCost(new TapSourceCost());
         ability.addTarget(new TargetLandPermanent(filter));
-        ability.addEffect(new BecomeSwampEffect(Duration.Custom, false, true, SubType.SWAMP));
+        ability.addEffect(new BecomeSwampEffect());
         this.addAbility(ability, new CyclopeanTombCounterWatcher());
 
         // When Cyclopean Tomb is put into a graveyard from the battlefield, at the beginning of each of your upkeeps for the rest of the game, remove all mire counters from a land that a mire counter was put onto with Cyclopean Tomb but that a mire counter has not been removed from with Cyclopean Tomb.
@@ -68,8 +68,8 @@ public final class CyclopeanTomb extends CardImpl {
 
 class BecomeSwampEffect extends BecomesBasicLandTargetEffect {
 
-    BecomeSwampEffect(Duration duration, boolean chooseLandType, boolean loseOther, SubType... landNames) {
+    BecomeSwampEffect() {
-        super(duration, chooseLandType, loseOther, landNames);
+        super(Duration.Custom, false, true, SubType.SWAMP);
         staticText = "That land is a Swamp for as long as it has a mire counter on it";
     }
 
@@ -80,14 +80,11 @@ class BecomeSwampEffect extends BecomesBasicLandTargetEffect {
     @Override
     public boolean apply(Layer layer, SubLayer sublayer, Ability source, Game game) {
         Permanent land = game.getPermanent(this.targetPointer.getFirst(game, source));
-        if (land == null) {
+        if (land == null || land.getCounters(game).getCount(CounterType.MIRE) < 1) {
-            // if permanent left battlefield the effect can be removed because it was only valid for that object
             this.discard();
-        } else if (land.getCounters(game).getCount(CounterType.MIRE) > 0) {
+            return false;
-            // only if Mire counter is on the object it becomes a Swamp.
-            super.apply(layer, sublayer, source, game);
         }
-        return true;
+        return super.apply(layer, sublayer, source, game);
     }
 
     @Override

Mage.Tests/src/test/java/org/mage/test/cards/single/lrw/AquitectsWillTest.java

@@ -0,0 +1,93 @@
+package org.mage.test.cards.single.lrw;
+
+import mage.constants.PhaseStep;
+import mage.constants.SubType;
+import mage.constants.Zone;
+import mage.counters.CounterType;
+import org.junit.Ignore;
+import org.junit.Test;
+import org.mage.test.serverside.base.CardTestPlayerBase;
+
+/**
+ * @author TheElk801
+ */
+public class AquitectsWillTest extends CardTestPlayerBase {
+
+    private static final String mountain = "Mountain";
+    private static final String will = "Aquitect's Will";
+    private static final String recall = "Ancestral Recall";
+    private static final String hexmage = "Vampire Hexmage";
+
+    @Ignore // TODO: this test currently fails, the mana option is there but can't be selected for some reason
+    @Test
+    public void testProduceBlueDuringCast() {
+        addCard(Zone.BATTLEFIELD, playerA, "Island");
+        addCard(Zone.BATTLEFIELD, playerA, mountain);
+        addCard(Zone.HAND, playerA, will);
+        addCard(Zone.HAND, playerA, recall);
+
+        setStrictChooseMode(true);
+        castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, will, mountain);
+
+        castSpell(1, PhaseStep.POSTCOMBAT_MAIN, playerA, recall, playerA);
+
+        setStopAt(1, PhaseStep.END_TURN);
+        execute();
+        assertAllCommandsUsed();
+
+        assertHandCount(playerA, 3);
+        assertGraveyardCount(playerA, will, 1);
+        assertGraveyardCount(playerA, recall, 1);
+        assertCounterCount(playerA, mountain, CounterType.FLOOD, 1);
+        assertSubtype(mountain, SubType.MOUNTAIN);
+        assertSubtype(mountain, SubType.ISLAND);
+    }
+
+    @Test
+    public void testProduceBlueOutsideCast() {
+        addCard(Zone.BATTLEFIELD, playerA, "Island");
+        addCard(Zone.BATTLEFIELD, playerA, mountain);
+        addCard(Zone.HAND, playerA, will);
+        addCard(Zone.HAND, playerA, recall);
+
+        setStrictChooseMode(true);
+        castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, will, mountain);
+
+        activateManaAbility(1, PhaseStep.POSTCOMBAT_MAIN, playerA, "{T}: Add {U}");
+        castSpell(1, PhaseStep.POSTCOMBAT_MAIN, playerA, recall, playerA);
+
+        setStopAt(1, PhaseStep.END_TURN);
+        execute();
+        assertAllCommandsUsed();
+
+        assertHandCount(playerA, 3);
+        assertGraveyardCount(playerA, will, 1);
+        assertGraveyardCount(playerA, recall, 1);
+        assertCounterCount(playerA, mountain, CounterType.FLOOD, 1);
+        assertSubtype(mountain, SubType.MOUNTAIN);
+        assertSubtype(mountain, SubType.ISLAND);
+    }
+
+    @Test
+    public void testEffectTiedToCounter() {
+        addCard(Zone.BATTLEFIELD, playerA, "Island");
+        addCard(Zone.BATTLEFIELD, playerA, mountain);
+        addCard(Zone.BATTLEFIELD, playerA, hexmage);
+        addCard(Zone.HAND, playerA, will);
+
+        setStrictChooseMode(true);
+        castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, will, mountain);
+
+        activateAbility(1, PhaseStep.POSTCOMBAT_MAIN, playerA, "Sacrifice", mountain);
+
+        setStopAt(1, PhaseStep.END_TURN);
+        execute();
+        assertAllCommandsUsed();
+
+        assertGraveyardCount(playerA, will, 1);
+        assertGraveyardCount(playerA, hexmage, 1);
+        assertCounterCount(playerA, mountain, CounterType.FLOOD, 0);
+        assertSubtype(mountain, SubType.MOUNTAIN);
+        assertNotSubtype(mountain, SubType.ISLAND);
+    }
+}

Mage/src/main/java/mage/abilities/effects/common/continuous/BecomesBasicLandTargetEffect.java

@@ -24,8 +24,8 @@ public class BecomesBasicLandTargetEffect extends ContinuousEffectImpl {
 
     protected boolean chooseLandType;
     protected List<SubType> landTypes = new ArrayList<>();
-    protected List<SubType> landTypesToAdd = new ArrayList<>();
+    private final List<SubType> landTypesToAdd = new ArrayList<>();
-    protected boolean loseOther;  // loses all other abilities, card types, and creature types
+    private final boolean loseOther;  // loses all other abilities, card types, and creature types
 
     public BecomesBasicLandTargetEffect(Duration duration) {
         this(duration, true);
@@ -40,7 +40,7 @@ public class BecomesBasicLandTargetEffect extends ContinuousEffectImpl {
     }
 
     public BecomesBasicLandTargetEffect(Duration duration, boolean chooseLandType, boolean loseOther, SubType... landNames) {
-        super(duration, Outcome.Detriment);
+        super(duration, Layer.TypeChangingEffects_4, SubLayer.NA, Outcome.Detriment);
         this.landTypes.addAll(Arrays.asList(landNames));
         if (landTypes.contains(SubType.MOUNTAIN)) {
             dependencyTypes.add(DependencyType.BecomeMountain);
@@ -71,11 +71,6 @@ public class BecomesBasicLandTargetEffect extends ContinuousEffectImpl {
         this.loseOther = effect.loseOther;
     }
 
-    @Override
-    public boolean apply(Game game, Ability source) {
-        return false;
-    }
-
     @Override
     public BecomesBasicLandTargetEffect copy() {
         return new BecomesBasicLandTargetEffect(this);
@@ -101,53 +96,49 @@ public class BecomesBasicLandTargetEffect extends ContinuousEffectImpl {
     }
 
     @Override
-    public boolean apply(Layer layer, SubLayer sublayer, Ability source, Game game) {
+    public boolean apply(Game game, Ability source) {
         for (UUID targetPermanent : targetPointer.getTargets(game, source)) {
             Permanent land = game.getPermanent(targetPermanent);
-            if (land != null) {
+            if (land == null) {
-                switch (layer) {
+                continue;
-                    case TypeChangingEffects_4:
+            }
-                        // Attention: Cards like Unstable Frontier that use this class do not give the "Basic" supertype to the target
+            if (!land.isLand()) {
-                        if (!land.isLand()) {
+                land.addCardType(CardType.LAND);
-                            land.addCardType(CardType.LAND);
+            }
-                        }
+            if (loseOther) {
-                        if (loseOther) {
+                // 305.7 Note that this doesn't remove any abilities
-                            // 305.7 Note that this doesn't remove any abilities 
+                // that were granted to the land by other effects
-                            // that were granted to the land by other effects
+                // So the ability removing has to be done before Layer 6
-                            // So the ability removing has to be done before Layer 6
+                land.removeAllAbilities(source.getSourceId(), game);
-                            land.removeAllAbilities(source.getSourceId(), game);
+                // 305.7
-                            // 305.7
+                land.removeAllSubTypes(game, SubTypeSet.NonBasicLandType);
-                            land.removeAllSubTypes(game, SubTypeSet.NonBasicLandType);
+                land.addSubType(game, landTypes);
-                            land.addSubType(game, landTypes);
+            } else {
-                        } else {
+                landTypesToAdd.clear();
-                            landTypesToAdd.clear();
+                for (SubType subtype : landTypes) {
-                            for (SubType subtype : landTypes) {
+                    if (!land.hasSubtype(subtype, game)) {
-                                if (!land.hasSubtype(subtype, game)) {
+                        land.addSubType(game, subtype);
-                                    land.addSubType(game, subtype);
+                        landTypesToAdd.add(subtype);
-                                    landTypesToAdd.add(subtype);
+                    }
-                                }
+                }
-                            }
+            }
-                        }
+            // add intrinsic land abilities here not in layer 6
-                        // add intrinsic land abilities here not in layer 6
+            for (SubType landType : landTypesToAdd) {
-                        for (SubType landType : landTypesToAdd) {
+                switch (landType) {
-                            switch (landType) {
+                    case PLAINS:
-                                case SWAMP:
+                        land.addAbility(new WhiteManaAbility(), source.getSourceId(), game);
-                                    land.addAbility(new BlackManaAbility(), source.getSourceId(), game);
+                        break;
-                                    break;
+                    case ISLAND:
-                                case MOUNTAIN:
+                        land.addAbility(new BlueManaAbility(), source.getSourceId(), game);
-                                    land.addAbility(new RedManaAbility(), source.getSourceId(), game);
+                        break;
-                                    break;
+                    case SWAMP:
-                                case FOREST:
+                        land.addAbility(new BlackManaAbility(), source.getSourceId(), game);
-                                    land.addAbility(new GreenManaAbility(), source.getSourceId(), game);
+                        break;
-                                    break;
+                    case MOUNTAIN:
-                                case ISLAND:
+                        land.addAbility(new RedManaAbility(), source.getSourceId(), game);
-                                    land.addAbility(new BlueManaAbility(), source.getSourceId(), game);
+                        break;
-                                    break;
+                    case FOREST:
-                                case PLAINS:
+                        land.addAbility(new GreenManaAbility(), source.getSourceId(), game);
-                                    land.addAbility(new WhiteManaAbility(), source.getSourceId(), game);
-                                    break;
-                            }
-                        }
                         break;
                 }
             }
@@ -155,11 +146,6 @@ public class BecomesBasicLandTargetEffect extends ContinuousEffectImpl {
         return true;
     }
 
-    @Override
-    public boolean hasLayer(Layer layer) {
-        return layer == Layer.TypeChangingEffects_4;
-    }
-
     private String setText() {
         StringBuilder sb = new StringBuilder();
         if (chooseLandType) {