From 7f1267563b6569b1e149f1c7c53908a03d1d19c1 Mon Sep 17 00:00:00 2001 From: Jeff Date: Thu, 10 Jan 2019 17:10:25 -0600 Subject: [PATCH] - Fixed #5500. If you note any interactions that I missed testing, do tell. --- Mage.Sets/src/mage/cards/b/BloodMoon.java | 32 +++++-- Mage.Sets/src/mage/cards/c/Conversion.java | 69 ++++++++++---- Mage.Sets/src/mage/cards/g/Glaciers.java | 57 +++++++++--- .../src/mage/cards/i/IllusionaryTerrain.java | 89 +++++++------------ .../src/mage/cards/p/PhantasmalTerrain.java | 3 +- .../LandTypeChangingEffectsTest.java | 19 ++-- 6 files changed, 156 insertions(+), 113 deletions(-) diff --git a/Mage.Sets/src/mage/cards/b/BloodMoon.java b/Mage.Sets/src/mage/cards/b/BloodMoon.java index ca52c9989c..1eef0357ac 100644 --- a/Mage.Sets/src/mage/cards/b/BloodMoon.java +++ b/Mage.Sets/src/mage/cards/b/BloodMoon.java @@ -1,11 +1,14 @@ - package mage.cards.b; import java.util.UUID; import mage.abilities.Ability; import mage.abilities.common.SimpleStaticAbility; import mage.abilities.effects.ContinuousEffectImpl; +import mage.abilities.mana.BlackManaAbility; +import mage.abilities.mana.BlueManaAbility; +import mage.abilities.mana.GreenManaAbility; import mage.abilities.mana.RedManaAbility; +import mage.abilities.mana.WhiteManaAbility; import mage.cards.CardImpl; import mage.cards.CardSetInfo; import mage.constants.*; @@ -22,7 +25,7 @@ import mage.game.permanent.Permanent; public final class BloodMoon extends CardImpl { public BloodMoon(UUID ownerId, CardSetInfo setInfo) { - super(ownerId,setInfo,new CardType[]{CardType.ENCHANTMENT},"{2}{R}"); + super(ownerId, setInfo, new CardType[]{CardType.ENCHANTMENT}, "{2}{R}"); // Nonbasic lands are Mountains. this.addAbility(new SimpleStaticAbility(Zone.BATTLEFIELD, new BloodMoonEffect())); @@ -72,12 +75,28 @@ class BloodMoonEffect extends ContinuousEffectImpl { case TypeChangingEffects_4: // 305.7 Note that this doesn't remove any abilities that were granted to the land by other effects // So the ability removing has to be done before Layer 6 - land.removeAllAbilities(source.getSourceId(), game); - land.getSubtype(game).removeAll(SubType.getLandTypes(false)); + //land.getSubtype(game).removeAll(SubType.getLandTypes(false)); + land.getSubtype(game).clear(); land.getSubtype(game).add(SubType.MOUNTAIN); + land.removeAllAbilities(source.getSourceId(), game); break; case AbilityAddingRemovingEffects_6: - land.addAbility(new RedManaAbility(), source.getSourceId(), game); + land.removeAllAbilities(source.getSourceId(), game); + if (land.getSubtype(game).contains(SubType.FOREST)) { + land.addAbility(new GreenManaAbility(), source.getSourceId(), game); + } + if (land.getSubtype(game).contains(SubType.PLAINS)) { + land.addAbility(new WhiteManaAbility(), source.getSourceId(), game); + } + if (land.getSubtype(game).contains(SubType.MOUNTAIN)) { + land.addAbility(new RedManaAbility(), source.getSourceId(), game); + } + if (land.getSubtype(game).contains(SubType.ISLAND)) { + land.addAbility(new BlueManaAbility(), source.getSourceId(), game); + } + if (land.getSubtype(game).contains(SubType.SWAMP)) { + land.addAbility(new BlackManaAbility(), source.getSourceId(), game); + } break; } } @@ -86,6 +105,7 @@ class BloodMoonEffect extends ContinuousEffectImpl { @Override public boolean hasLayer(Layer layer) { - return layer == Layer.AbilityAddingRemovingEffects_6 || layer == Layer.TypeChangingEffects_4; + return layer == Layer.AbilityAddingRemovingEffects_6 + || layer == Layer.TypeChangingEffects_4; } } diff --git a/Mage.Sets/src/mage/cards/c/Conversion.java b/Mage.Sets/src/mage/cards/c/Conversion.java index 24f0ef8d1f..8731aaba76 100644 --- a/Mage.Sets/src/mage/cards/c/Conversion.java +++ b/Mage.Sets/src/mage/cards/c/Conversion.java @@ -1,4 +1,3 @@ - package mage.cards.c; import java.util.List; @@ -13,11 +12,14 @@ import mage.abilities.effects.ContinuousEffect; import mage.abilities.effects.ContinuousEffectImpl; import mage.abilities.effects.Effect; import mage.abilities.effects.common.SacrificeSourceUnlessPaysEffect; +import mage.abilities.mana.BlackManaAbility; +import mage.abilities.mana.BlueManaAbility; +import mage.abilities.mana.GreenManaAbility; +import mage.abilities.mana.RedManaAbility; import mage.abilities.mana.WhiteManaAbility; import mage.cards.CardImpl; import mage.cards.CardSetInfo; import mage.constants.*; -import mage.filter.common.FilterLandPermanent; import mage.game.Game; import mage.game.permanent.Permanent; @@ -27,13 +29,15 @@ import mage.game.permanent.Permanent; */ public final class Conversion extends CardImpl { - private static final FilterLandPermanent filter = new FilterLandPermanent(SubType.MOUNTAIN, "Mountains"); - public Conversion(UUID ownerId, CardSetInfo setInfo) { - super(ownerId,setInfo,new CardType[]{CardType.ENCHANTMENT},"{2}{W}{W}"); + super(ownerId, setInfo, new CardType[]{CardType.ENCHANTMENT}, "{2}{W}{W}"); // At the beginning of your upkeep, sacrifice Conversion unless you pay {W}{W}. - this.addAbility(new BeginningOfUpkeepTriggeredAbility(new SacrificeSourceUnlessPaysEffect(new ManaCostsImpl("{W}{W}")), TargetController.YOU, false)); + this.addAbility(new BeginningOfUpkeepTriggeredAbility( + new SacrificeSourceUnlessPaysEffect( + new ManaCostsImpl("{W}{W}")), + TargetController.YOU, + false)); // All Mountains are Plains. this.addAbility(new SimpleStaticAbility(Zone.BATTLEFIELD, new ConversionEffect())); @@ -72,15 +76,45 @@ public final class Conversion extends CardImpl { @Override public boolean apply(Layer layer, SubLayer sublayer, Ability source, Game game) { - for (Permanent land : game.getBattlefield().getActivePermanents(filter, source.getControllerId(), game)) { + for (Permanent land : game.getBattlefield().getAllActivePermanents(CardType.LAND)) { switch (layer) { - case AbilityAddingRemovingEffects_6: - land.removeAllAbilities(source.getSourceId(), game); - land.addAbility(new WhiteManaAbility(), source.getSourceId(), game); - break; case TypeChangingEffects_4: - land.getSubtype(game).clear(); - land.getSubtype(game).add(SubType.PLAINS); + if (land.getSubtype(game).contains(SubType.MOUNTAIN)) { + land.getSubtype(game).clear(); + land.getSubtype(game).add(SubType.PLAINS); + game.getState().setValue("conversion" + + source.getId() + + land.getId() + + land.getZoneChangeCounter(game), + "true"); + } + break; + case AbilityAddingRemovingEffects_6: + if (game.getState().getValue("conversion" + + source.getId() + + land.getId() + + land.getZoneChangeCounter(game)) != null + && game.getState().getValue("conversion" + + source.getId() + + land.getId() + + land.getZoneChangeCounter(game)).equals("true")) { + land.removeAllAbilities(source.getSourceId(), game); + if (land.getSubtype(game).contains(SubType.FOREST)) { + land.addAbility(new GreenManaAbility(), source.getSourceId(), game); + } + if (land.getSubtype(game).contains(SubType.PLAINS)) { + land.addAbility(new WhiteManaAbility(), source.getSourceId(), game); + } + if (land.getSubtype(game).contains(SubType.MOUNTAIN)) { + land.addAbility(new RedManaAbility(), source.getSourceId(), game); + } + if (land.getSubtype(game).contains(SubType.ISLAND)) { + land.addAbility(new BlueManaAbility(), source.getSourceId(), game); + } + if (land.getSubtype(game).contains(SubType.SWAMP)) { + land.addAbility(new BlackManaAbility(), source.getSourceId(), game); + } + } break; } } @@ -89,20 +123,17 @@ public final class Conversion extends CardImpl { @Override public boolean hasLayer(Layer layer) { - return layer == Layer.AbilityAddingRemovingEffects_6 || layer == Layer.TypeChangingEffects_4; + return layer == Layer.AbilityAddingRemovingEffects_6 + || layer == Layer.TypeChangingEffects_4; } @Override public Set isDependentTo(List allEffectsInLayer) { - // the dependent classes needs to be an enclosed class for dependent check of continuous effects return allEffectsInLayer .stream() - .filter(effect->effect.getDependencyTypes().contains(DependencyType.BecomeMountain)) + .filter(effect -> effect.getDependencyTypes().contains(DependencyType.BecomePlains)) .map(Effect::getId) .collect(Collectors.toSet()); - } - } - } diff --git a/Mage.Sets/src/mage/cards/g/Glaciers.java b/Mage.Sets/src/mage/cards/g/Glaciers.java index d0c6fa2be8..6d2ae05de9 100644 --- a/Mage.Sets/src/mage/cards/g/Glaciers.java +++ b/Mage.Sets/src/mage/cards/g/Glaciers.java @@ -12,24 +12,23 @@ import mage.abilities.mana.WhiteManaAbility; import mage.cards.CardImpl; import mage.cards.CardSetInfo; import mage.constants.*; -import mage.filter.common.FilterLandPermanent; import mage.game.Game; import mage.game.permanent.Permanent; - import java.util.List; import java.util.Set; import java.util.UUID; import java.util.stream.Collectors; +import mage.abilities.mana.BlackManaAbility; +import mage.abilities.mana.BlueManaAbility; +import mage.abilities.mana.GreenManaAbility; +import mage.abilities.mana.RedManaAbility; /** * * @author jmharmon */ - public final class Glaciers extends CardImpl { - private static final FilterLandPermanent filter = new FilterLandPermanent(SubType.MOUNTAIN, "Mountains"); - public Glaciers(UUID ownerId, CardSetInfo setInfo) { super(ownerId, setInfo, new CardType[]{CardType.ENCHANTMENT}, "{2}{W}{U}"); @@ -72,15 +71,44 @@ public final class Glaciers extends CardImpl { @Override public boolean apply(Layer layer, SubLayer sublayer, Ability source, Game game) { - for (Permanent land : game.getBattlefield().getActivePermanents(filter, source.getControllerId(), game)) { + for (Permanent land : game.getBattlefield().getAllActivePermanents(CardType.LAND)) { switch (layer) { - case AbilityAddingRemovingEffects_6: - land.removeAllAbilities(source.getSourceId(), game); - land.addAbility(new WhiteManaAbility(), source.getSourceId(), game); - break; case TypeChangingEffects_4: - land.getSubtype(game).clear(); - land.getSubtype(game).add(SubType.PLAINS); + if (land.getSubtype(game).contains(SubType.MOUNTAIN)) { + land.getSubtype(game).clear(); + land.getSubtype(game).add(SubType.PLAINS); + game.getState().setValue("glaciers" + + source.getId() + + land.getId() + + land.getZoneChangeCounter(game), "true"); + } + break; + case AbilityAddingRemovingEffects_6: + if (game.getState().getValue("glaciers" + + source.getId() + + land.getId() + + land.getZoneChangeCounter(game)) != null + && game.getState().getValue("glaciers" + + source.getId() + + land.getId() + + land.getZoneChangeCounter(game)).equals("true")) { + land.removeAllAbilities(source.getSourceId(), game); + if (land.getSubtype(game).contains(SubType.FOREST)) { + land.addAbility(new GreenManaAbility(), source.getSourceId(), game); + } + if (land.getSubtype(game).contains(SubType.PLAINS)) { + land.addAbility(new WhiteManaAbility(), source.getSourceId(), game); + } + if (land.getSubtype(game).contains(SubType.MOUNTAIN)) { + land.addAbility(new RedManaAbility(), source.getSourceId(), game); + } + if (land.getSubtype(game).contains(SubType.ISLAND)) { + land.addAbility(new BlueManaAbility(), source.getSourceId(), game); + } + if (land.getSubtype(game).contains(SubType.SWAMP)) { + land.addAbility(new BlackManaAbility(), source.getSourceId(), game); + } + } break; } } @@ -89,14 +117,15 @@ public final class Glaciers extends CardImpl { @Override public boolean hasLayer(Layer layer) { - return layer == Layer.AbilityAddingRemovingEffects_6 || layer == Layer.TypeChangingEffects_4; + return layer == Layer.AbilityAddingRemovingEffects_6 + || layer == Layer.TypeChangingEffects_4; } @Override public Set isDependentTo(List allEffectsInLayer) { return allEffectsInLayer .stream() - .filter(effect->effect.getDependencyTypes().contains(DependencyType.BecomeMountain)) + .filter(effect -> effect.getDependencyTypes().contains(DependencyType.BecomePlains)) .map(Effect::getId) .collect(Collectors.toSet()); } diff --git a/Mage.Sets/src/mage/cards/i/IllusionaryTerrain.java b/Mage.Sets/src/mage/cards/i/IllusionaryTerrain.java index 102e49dc61..d61f2916e1 100644 --- a/Mage.Sets/src/mage/cards/i/IllusionaryTerrain.java +++ b/Mage.Sets/src/mage/cards/i/IllusionaryTerrain.java @@ -89,76 +89,46 @@ class IllusionaryTerrainEffect extends ContinuousEffectImpl { && firstChoice != null && secondChoice != null) { for (Permanent land : lands) { - if (land != null - && land.isBasic()) { + if (land.isBasic()) { switch (layer) { case TypeChangingEffects_4: - if (sublayer == SubLayer.NA) { + if (land.getSubtype(game).contains(firstChoice)) { land.getSubtype(game).clear(); land.getSubtype(game).add(secondChoice); + game.getState().setValue("illusionaryTerrain" + + source.getId() + + land.getId() + + land.getZoneChangeCounter(game), + "true"); } break; case AbilityAddingRemovingEffects_6: - if (sublayer == SubLayer.NA) { - boolean addAbility = true; - land.getAbilities().clear(); - if (secondChoice.equals(SubType.FOREST)) { - for (Ability existingAbility : land.getAbilities()) { - if (existingAbility instanceof GreenManaAbility) { - addAbility = false; - break; - } - } - if (addAbility) { - land.addAbility(new GreenManaAbility(), source.getSourceId(), game); - } + if (game.getState().getValue("illusionaryTerrain" + + source.getId() + + land.getId() + + land.getZoneChangeCounter(game)) != null + && game.getState().getValue("illusionaryTerrain" + + source.getId() + + land.getId() + + land.getZoneChangeCounter(game)).equals("true")) { + land.removeAllAbilities(source.getSourceId(), game); + if (land.getSubtype(game).contains(SubType.FOREST)) { + land.addAbility(new GreenManaAbility(), source.getSourceId(), game); } - if (secondChoice.equals(SubType.PLAINS)) { - for (Ability existingAbility : land.getAbilities()) { - if (existingAbility instanceof WhiteManaAbility) { - addAbility = false; - break; - } - } - if (addAbility) { - land.addAbility(new WhiteManaAbility(), source.getSourceId(), game); - } + if (land.getSubtype(game).contains(SubType.PLAINS)) { + land.addAbility(new WhiteManaAbility(), source.getSourceId(), game); } - if (secondChoice.equals(SubType.MOUNTAIN)) { - for (Ability existingAbility : land.getAbilities()) { - if (existingAbility instanceof RedManaAbility) { - addAbility = false; - break; - } - } - if (addAbility) { - land.addAbility(new RedManaAbility(), source.getSourceId(), game); - } + if (land.getSubtype(game).contains(SubType.MOUNTAIN)) { + land.addAbility(new RedManaAbility(), source.getSourceId(), game); } - if (secondChoice.equals(SubType.ISLAND)) { - for (Ability existingAbility : land.getAbilities()) { - if (existingAbility instanceof BlueManaAbility) { - addAbility = false; - break; - } - } - if (addAbility) { - land.addAbility(new BlueManaAbility(), source.getSourceId(), game); - } + if (land.getSubtype(game).contains(SubType.ISLAND)) { + land.addAbility(new BlueManaAbility(), source.getSourceId(), game); } - if (secondChoice.equals(SubType.SWAMP)) { - for (Ability existingAbility : land.getAbilities()) { - if (existingAbility instanceof BlackManaAbility) { - addAbility = false; - break; - } - } - if (addAbility) { - land.addAbility(new BlackManaAbility(), source.getSourceId(), game); - } + if (land.getSubtype(game).contains(SubType.SWAMP)) { + land.addAbility(new BlackManaAbility(), source.getSourceId(), game); } + break; } - break; } } } @@ -174,8 +144,9 @@ class IllusionaryTerrainEffect extends ContinuousEffectImpl { @Override public boolean hasLayer(Layer layer) { - return layer == Layer.AbilityAddingRemovingEffects_6 - || layer == Layer.TypeChangingEffects_4; + return layer == Layer.TypeChangingEffects_4 + || layer == Layer.AbilityAddingRemovingEffects_6; + } } diff --git a/Mage.Sets/src/mage/cards/p/PhantasmalTerrain.java b/Mage.Sets/src/mage/cards/p/PhantasmalTerrain.java index f5bb97875c..cb425f1d88 100644 --- a/Mage.Sets/src/mage/cards/p/PhantasmalTerrain.java +++ b/Mage.Sets/src/mage/cards/p/PhantasmalTerrain.java @@ -125,7 +125,8 @@ class PhantasmalTerrainContinuousEffect extends ContinuousEffectImpl { @Override public boolean hasLayer(Layer layer) { - return layer == Layer.AbilityAddingRemovingEffects_6 || layer == Layer.TypeChangingEffects_4; + return layer == Layer.AbilityAddingRemovingEffects_6 + || layer == Layer.TypeChangingEffects_4; } } diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/continuous/LandTypeChangingEffectsTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/continuous/LandTypeChangingEffectsTest.java index 12b594c155..ce9d973e1b 100644 --- a/Mage.Tests/src/test/java/org/mage/test/cards/continuous/LandTypeChangingEffectsTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/cards/continuous/LandTypeChangingEffectsTest.java @@ -1,4 +1,3 @@ - package org.mage.test.cards.continuous; import mage.abilities.keyword.IndestructibleAbility; @@ -11,7 +10,6 @@ import mage.counters.CounterType; import mage.filter.StaticFilters; import mage.game.permanent.Permanent; import org.junit.Assert; -import org.junit.Ignore; import org.junit.Test; import org.mage.test.serverside.base.CardTestPlayerBase; @@ -114,10 +112,6 @@ public class LandTypeChangingEffectsTest extends CardTestPlayerBase { String bloodmoon = "Blood Moon"; String canopyvista = "Canopy Vista"; - /* - TODO: NOTE: this test is currently failing due to bug in code. See issue #3072 - */ - //@Ignore @Test public void testBloodMoonBeforeUrborg() { // Blood Moon 2R @@ -147,10 +141,6 @@ public class LandTypeChangingEffectsTest extends CardTestPlayerBase { Assert.assertTrue("The mana the land can produce should be [{R}] but it's " + playerB.getManaAvailable(currentGame).toString(), playerB.getManaAvailable(currentGame).toString().equals("[{R}]")); } - /* - TODO: NOTE: this test is currently failing due to bug in code. See issue #3072 - */ - //@Ignore @Test public void testBloodMoonAfterUrborg() { // Blood Moon 2R @@ -186,6 +176,7 @@ public class LandTypeChangingEffectsTest extends CardTestPlayerBase { In terms of time-stamp order, Urborg was down first, then Kormus Bell, then Quicksilver. When I put a flood counter on a basic swamp, it would become a 0/0 instead of a 1/1 and die. */ + @Test public void testCormusBellAfterUrborg() { // Land - Legendary @@ -250,19 +241,19 @@ public class LandTypeChangingEffectsTest extends CardTestPlayerBase { addCard(Zone.BATTLEFIELD, playerA, "Blood Sun"); // all lands lose all abilities except for mana-producing addCard(Zone.BATTLEFIELD, playerA, "Stormtide Leviathan"); // all lands are islands in addition to their other types addCard(Zone.BATTLEFIELD, playerA, "Darksteel Citadel"); // land has indestructible ability - + setStopAt(3, PhaseStep.POSTCOMBAT_MAIN); execute(); - + Permanent darksteel = getPermanent("Darksteel Citadel", playerA.getId()); Assert.assertNotNull(darksteel); Assert.assertFalse(darksteel.getAbilities().contains(IndestructibleAbility.getInstance())); // The ability is removed - + /* If a continuous effect has started applying in an earlier layer, it will continue to apply in later layers even if the ability that created that effect has been removed. Urborg ability is applied in the 4th layer. The Blood Sun works in the 6th. So the effect still applies to the lands. - */ + */ assertType(urborgtoy, CardType.LAND, SubType.SWAMP); assertType("Mountain", CardType.LAND, SubType.SWAMP); assertType(urborgtoy, CardType.LAND, SubType.ISLAND);