Ability refactor: face down;

Mage.Sets/src/mage/cards/m/MuragandaPetroglyphs.java

@@ -1,8 +1,5 @@
-
 package mage.cards.m;
 
-import java.util.Objects;
-import java.util.UUID;
 import mage.MageObject;
 import mage.abilities.Abilities;
 import mage.abilities.Ability;
@@ -20,8 +17,10 @@ import mage.filter.common.FilterCreaturePermanent;
 import mage.filter.predicate.Predicate;
 import mage.game.Game;
 
+import java.util.Objects;
+import java.util.UUID;
+
 /**
- *
  * @author anonymous
  */
 public final class MuragandaPetroglyphs extends CardImpl {
@@ -54,6 +53,12 @@ public final class MuragandaPetroglyphs extends CardImpl {
 
 class NoAbilityPredicate implements Predicate<MageObject> {
 
+    // Muraganda Petroglyphs gives a bonus only to creatures that have no rules text at all. This includes true vanilla
+    // creatures (such as Grizzly Bears), face-down creatures, many tokens, and creatures that have lost their abilities
+    // (due to Ovinize, for example). Any ability of any kind, whether or not the ability functions in the on the
+    // battlefield zone, including things like “Cycling {2}” means the creature doesn’t get the bonus.
+    // (2007-05-01)
+
     @Override
     public boolean apply(MageObject input, Game game) {
         boolean isFaceDown = false;
@@ -65,8 +70,20 @@ class NoAbilityPredicate implements Predicate<MageObject> {
             abilities = input.getAbilities();
         }
         if (isFaceDown) {
+            // Some Auras and Equipment grant abilities to creatures, meaning the affected creature would no longer
+            // get the +2/+2 bonus. For example, Flight grants flying to the enchanted creature. Other Auras and
+            // Equipment do not, meaning the affected creature would continue to get the +2/+2 bonus. For example,
+            // Dehydration states something now true about the enchanted creature, but doesn’t give it any abilities.
+            // Auras and Equipment that grant abilities will use the words “gains” or “has,” and they’ll list a keyword
+            // ability or an ability in quotation marks.
+            // (2007-05-01)
+
             for (Ability ability : abilities) {
-                if (!ability.getSourceId().equals(input.getId()) && !ability.getClass().equals(JohanVigilanceAbility.class)) {
+                if (ability.getWorksFaceDown()) {
+                    // inner face down abilities like turn up and becomes creature
+                    continue;
+                }
+                if (!Objects.equals(ability.getClass(), SpellAbility.class) && !ability.getClass().equals(JohanVigilanceAbility.class)) {
                     return false;
                 }
             }

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

@@ -274,9 +274,12 @@ public class ManifestTest extends CardTestPlayerBase {
 
     // Check if a Megamorph card is manifested and turned face up by their megamorph ability
     // it gets the +1/+1 counter.
+    // 701.33c
+    // If a card with morph is manifested, its controller may turn that card face up using
+    // either the procedure described in rule 702.36e to turn a face-down permanent with morph face up
+    // or the procedure described above to turn a manifested permanent face up.
     @Test
-    public void testManifestMegamorph() {
+    public void testManifestMegamorph_TurnUpByMegamorphCost() {
-
         addCard(Zone.BATTLEFIELD, playerB, "Swamp", 2);
         addCard(Zone.BATTLEFIELD, playerB, "Forest", 6);
         // {1}{B}, {T}, Sacrifice another creature: Manifest the top card of your library.
@@ -295,6 +298,7 @@ public class ManifestTest extends CardTestPlayerBase {
 
         activateAbility(2, PhaseStep.POSTCOMBAT_MAIN, playerB, "{5}{G}: Turn");
 
+        setStrictChooseMode(true);
         setStopAt(2, PhaseStep.END_TURN);
         execute();
         assertAllCommandsUsed();
@@ -310,7 +314,45 @@ public class ManifestTest extends CardTestPlayerBase {
         assertPowerToughness(playerB, "Aerie Bowmasters", 4, 5); // 3/4  and the +1/+1 counter from Megamorph
         Permanent aerie = getPermanent("Aerie Bowmasters", playerB);
         Assert.assertTrue("Aerie Bowmasters has to be green", aerie != null && aerie.getColor(currentGame).isGreen());
+    }
 
+    @Test
+    public void testManifestMegamorph_TurnUpBySimpleCost() {
+        addCard(Zone.BATTLEFIELD, playerB, "Swamp", 2);
+        addCard(Zone.BATTLEFIELD, playerB, "Forest", 4);
+        // {1}{B}, {T}, Sacrifice another creature: Manifest the top card of your library.
+        addCard(Zone.BATTLEFIELD, playerB, "Qarsi High Priest", 1);
+        addCard(Zone.BATTLEFIELD, playerB, "Silvercoat Lion", 1);
+
+        // {2}{G}{G}
+        // Reach (This creature can block creatures with flying.)
+        // Megamorph {5}{G}
+        addCard(Zone.LIBRARY, playerB, "Aerie Bowmasters", 1);
+        addCard(Zone.LIBRARY, playerB, "Mountain", 1);
+
+        skipInitShuffling();
+
+        activateAbility(2, PhaseStep.PRECOMBAT_MAIN, playerB, "{1}{B}, {T}, Sacrifice another creature");
+        setChoice(playerB, "Silvercoat Lion");
+
+        activateAbility(2, PhaseStep.POSTCOMBAT_MAIN, playerB, "{2}{G}{G}: Turn");
+
+        setStrictChooseMode(true);
+        setStopAt(2, PhaseStep.END_TURN);
+        execute();
+        assertAllCommandsUsed();
+
+        // no life gain
+        assertLife(playerA, 20);
+        assertLife(playerB, 20);
+
+        assertGraveyardCount(playerB, "Silvercoat Lion", 1);
+
+        assertPermanentCount(playerB, EmptyNames.FACE_DOWN_CREATURE.toString(), 0);
+        assertPermanentCount(playerB, "Aerie Bowmasters", 1);
+        assertPowerToughness(playerB, "Aerie Bowmasters", 3, 4); // 3/4 without counter (megamorph not used)
+        Permanent aerie = getPermanent("Aerie Bowmasters", playerB);
+        Assert.assertTrue("Aerie Bowmasters has to be green", aerie != null && aerie.getColor(currentGame).isGreen());
     }
 
     /**

Mage.Tests/src/test/java/org/mage/test/cards/single/fut/MuragandaPetroglyphsTest.java

@@ -1,5 +1,6 @@
 package org.mage.test.cards.single.fut;
 
+import mage.abilities.keyword.HasteAbility;
 import mage.constants.EmptyNames;
 import mage.constants.PhaseStep;
 import mage.constants.Zone;
@@ -31,8 +32,10 @@ public class MuragandaPetroglyphsTest extends CardTestPlayerBase {
 
         addCard(Zone.BATTLEFIELD, playerA, "Muraganda Petroglyphs", 1);
 
+        setStrictChooseMode(true);
         setStopAt(1, PhaseStep.PRECOMBAT_MAIN);
         execute();
+        assertAllCommandsUsed();
 
         assertPowerToughness(playerA, "Grizzly Bears", 4, 4, Filter.ComparisonScope.Any);
         assertPowerToughness(playerB, "Grizzly Bears", 4, 4, Filter.ComparisonScope.Any);
@@ -41,15 +44,20 @@ public class MuragandaPetroglyphsTest extends CardTestPlayerBase {
 
     @Test
     public void faceDownCreaturesTest() {
+        // Morph {4}{G}
         addCard(Zone.HAND, playerA, "Pine Walker");
         addCard(Zone.BATTLEFIELD, playerA, "Forest", 3);
+        //
+        // Creatures with no abilities get +2/+2.
         addCard(Zone.BATTLEFIELD, playerA, "Muraganda Petroglyphs", 1);
 
         castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Pine Walker");
         setChoice(playerA, "Yes"); // cast it face down as 2/2 creature
 
+        setStrictChooseMode(true);
         setStopAt(1, PhaseStep.BEGIN_COMBAT);
         execute();
+        assertAllCommandsUsed();
 
         assertPermanentCount(playerA, EmptyNames.FACE_DOWN_CREATURE.toString(), 1);
         assertPowerToughness(playerA, EmptyNames.FACE_DOWN_CREATURE.toString(), 4, 4);
@@ -57,21 +65,26 @@ public class MuragandaPetroglyphsTest extends CardTestPlayerBase {
 
     @Test
     public void faceDownGainedAbilityTest() {
+        // Morph {4}{G}
         addCard(Zone.HAND, playerA, "Pine Walker");
         addCard(Zone.BATTLEFIELD, playerA, "Forest", 5);
         addCard(Zone.BATTLEFIELD, playerA, "Mountain", 1);
         addCard(Zone.BATTLEFIELD, playerA, "Mass Hysteria"); // All creatures have haste.
 
+        // Creatures with no abilities get +2/+2.
         addCard(Zone.BATTLEFIELD, playerA, "Muraganda Petroglyphs", 1);
 
         castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Pine Walker");
         setChoice(playerA, "Yes"); // cast it face down as 2/2 creature
 
+        setStrictChooseMode(true);
         setStopAt(1, PhaseStep.BEGIN_COMBAT);
         execute();
+        assertAllCommandsUsed();
 
         assertPermanentCount(playerA, EmptyNames.FACE_DOWN_CREATURE.toString(), 1);
-        assertPowerToughness(playerA, EmptyNames.FACE_DOWN_CREATURE.toString(), 2, 2);
+        //assertPowerToughness(playerA, EmptyNames.FACE_DOWN_CREATURE.toString(), 2, 2); // no boost (permanent have haste)
+        assertAbility(playerA, EmptyNames.FACE_DOWN_CREATURE.toString(), HasteAbility.getInstance(), true);
     }
 
     @Test
@@ -83,8 +96,10 @@ public class MuragandaPetroglyphsTest extends CardTestPlayerBase {
 
         castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Raise the Alarm");
 
+        setStrictChooseMode(true);
         setStopAt(1, PhaseStep.BEGIN_COMBAT);
         execute();
+        assertAllCommandsUsed();
 
         assertPowerToughness(playerA, "Soldier", 3, 3);
     }
@@ -99,8 +114,10 @@ public class MuragandaPetroglyphsTest extends CardTestPlayerBase {
 
         castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Ovinize", "Goblin Guide");
 
+        setStrictChooseMode(true);
         setStopAt(1, PhaseStep.BEGIN_COMBAT);
         execute();
+        assertAllCommandsUsed();
 
         assertPowerToughness(playerB, "Goblin Guide", 2, 3);
     }
@@ -110,8 +127,10 @@ public class MuragandaPetroglyphsTest extends CardTestPlayerBase {
         addCard(Zone.BATTLEFIELD, playerA, "Hundroog", 1); // Cycling {3}, 4/7
         addCard(Zone.BATTLEFIELD, playerA, "Muraganda Petroglyphs", 1);
 
+        setStrictChooseMode(true);
         setStopAt(1, PhaseStep.BEGIN_COMBAT);
         execute();
+        assertAllCommandsUsed();
 
         assertPowerToughness(playerA, "Hundroog", 4, 7);
     }
@@ -126,11 +145,12 @@ public class MuragandaPetroglyphsTest extends CardTestPlayerBase {
         addCard(Zone.BATTLEFIELD, playerA, "Muraganda Petroglyphs", 1);
         addCard(Zone.HAND, playerA, "Vastwood Zendikon");
 
-
         castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Vastwood Zendikon", "Forest");
 
+        setStrictChooseMode(true);
         setStopAt(1, PhaseStep.BEGIN_COMBAT);
         execute();
+        assertAllCommandsUsed();
 
         assertPowerToughness(playerA, "Forest", 6, 4);
 
@@ -160,8 +180,10 @@ public class MuragandaPetroglyphsTest extends CardTestPlayerBase {
 
         castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Dehydration", "Runeclaw Bear");
 
+        setStrictChooseMode(true);
         setStopAt(1, PhaseStep.BEGIN_COMBAT);
         execute();
+        assertAllCommandsUsed();
 
         assertPowerToughness(playerA, "Grizzly Bears", 4, 2);
 
@@ -179,11 +201,14 @@ public class MuragandaPetroglyphsTest extends CardTestPlayerBase {
         addCard(Zone.BATTLEFIELD, playerA, "Grizzly Bears", 1);
 
         addCard(Zone.HAND, playerA, "Shadow Slice"); // {4}{B}
-        castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Shadow Slice");
+        castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Shadow Slice", playerB);
-        setChoice(playerA, "Grizzly Bears");
+        setChoice(playerA, "Yes"); // do cipher
+        addTarget(playerA, "Grizzly Bears");
 
+        setStrictChooseMode(true);
         setStopAt(1, PhaseStep.BEGIN_COMBAT);
         execute();
+        assertAllCommandsUsed();
 
         assertPowerToughness(playerA, "Grizzly Bears", 2, 2);
     }

Mage/src/main/java/mage/abilities/common/TurnFaceUpAbility.java

@@ -40,6 +40,7 @@ public class TurnFaceUpAbility extends SpecialAction {
         this.usesStack = false;
         this.abilityType = AbilityType.SPECIAL_ACTION;
         this.setRuleVisible(false); // will be made visible only to controller in CardView
+        this.setWorksFaceDown(true);
     }
 
     public TurnFaceUpAbility(final TurnFaceUpAbility ability) {

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

@@ -1,11 +1,5 @@
-
 package mage.abilities.effects.common.continuous;
 
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.UUID;
 import mage.MageObjectReference;
 import mage.ObjectColor;
 import mage.abilities.Ability;
@@ -13,23 +7,20 @@ import mage.abilities.common.TurnFaceUpAbility;
 import mage.abilities.effects.ContinuousEffectImpl;
 import mage.abilities.keyword.MorphAbility;
 import mage.cards.Card;
-import mage.constants.CardType;
+import mage.constants.*;
-import mage.constants.Duration;
-import mage.constants.Layer;
-import mage.constants.Outcome;
-import mage.constants.SubLayer;
 import mage.filter.FilterPermanent;
 import mage.game.Game;
 import mage.game.permanent.Permanent;
 
+import java.util.*;
+
 /**
- *
  * @author LevelX2
  */
 
 public class BecomesFaceDownCreatureAllEffect extends ContinuousEffectImpl implements SourceEffect {
 
-    protected Map<UUID,Ability> turnFaceUpAbilityMap = new HashMap<>();
+    protected Map<UUID, Ability> turnFaceUpAbilityMap = new HashMap<>();
     protected FilterPermanent filter;
 
     public BecomesFaceDownCreatureAllEffect(FilterPermanent filter) {
@@ -40,7 +31,7 @@ public class BecomesFaceDownCreatureAllEffect extends ContinuousEffectImpl imple
 
     public BecomesFaceDownCreatureAllEffect(final BecomesFaceDownCreatureAllEffect effect) {
         super(effect);
-        for (Map.Entry<UUID,Ability> entry: effect.turnFaceUpAbilityMap.entrySet()) {
+        for (Map.Entry<UUID, Ability> entry : effect.turnFaceUpAbilityMap.entrySet()) {
             this.turnFaceUpAbilityMap.put(entry.getKey(), entry.getValue());
         }
         this.filter = effect.filter.copy();
@@ -54,16 +45,16 @@ public class BecomesFaceDownCreatureAllEffect extends ContinuousEffectImpl imple
     @Override
     public void init(Ability source, Game game) {
         super.init(source, game);
-        for (Permanent perm: game.getBattlefield().getActivePermanents(filter, source.getControllerId(), source.getSourceId(), game)) {
+        for (Permanent perm : game.getBattlefield().getActivePermanents(filter, source.getControllerId(), source.getSourceId(), game)) {
             if (!perm.isFaceDown(game) && !perm.isTransformable()) {
                 affectedObjectList.add(new MageObjectReference(perm, game));
                 perm.setFaceDown(true, game);
                 // check for Morph
                 Card card = game.getCard(perm.getId());
                 if (card != null) {
-                    for (Ability ability: card.getAbilities()) {
+                    for (Ability ability : card.getAbilities()) {
                         if (ability instanceof MorphAbility) {
-                            this.turnFaceUpAbilityMap.put(card.getId(), new TurnFaceUpAbility(((MorphAbility)ability).getMorphCosts()));
+                            this.turnFaceUpAbilityMap.put(card.getId(), new TurnFaceUpAbility(((MorphAbility) ability).getMorphCosts()));
                         }
                     }
                 }
@@ -74,7 +65,7 @@ public class BecomesFaceDownCreatureAllEffect extends ContinuousEffectImpl imple
     @Override
     public boolean apply(Layer layer, SubLayer sublayer, Ability source, Game game) {
         boolean targetExists = false;
-        for (MageObjectReference mor: affectedObjectList) {
+        for (MageObjectReference mor : affectedObjectList) {
             Permanent permanent = mor.getPermanent(game);
             if (permanent != null && permanent.isFaceDown(game)) {
                 targetExists = true;
@@ -92,27 +83,35 @@ public class BecomesFaceDownCreatureAllEffect extends ContinuousEffectImpl imple
                         break;
                     case AbilityAddingRemovingEffects_6:
                         Card card = game.getCard(permanent.getId()); //
-                        List<Ability> abilities = new ArrayList<>();
+                        List<Ability> abilitiesToRemove = new ArrayList<>();
                         for (Ability ability : permanent.getAbilities()) {
+
+                            // keep gained abilities from other sources, removes only own (card text)
                             if (card != null && !card.getAbilities().contains(ability)) {
-                                // gained abilities from other sources won't be removed
                                 continue;
                             }
-                            // TODO: Add flag "works also face down" to ability and use it to control ability removement instead of instanceof check
+
+                            // 701.33c
+                            // If a card with morph is manifested, its controller may turn that card face up using
+                            // either the procedure described in rule 702.36e to turn a face-down permanent with morph face up
+                            // or the procedure described above to turn a manifested permanent face up.
+                            //
+                            // so keep all tune face up abilities and other face down compatible
                             if (ability.getWorksFaceDown()) {
                                 ability.setRuleVisible(false);
                                 continue;
                             }
+
                             if (!ability.getRuleVisible() && !ability.getEffects().isEmpty()) {
                                 if (ability.getEffects().get(0) instanceof BecomesFaceDownCreatureAllEffect) {
                                     continue;
                                 }
                             }
-                            abilities.add(ability);
+                            abilitiesToRemove.add(ability);
                         }
-                        permanent.getAbilities().removeAll(abilities);
+                        permanent.removeAbilities(abilitiesToRemove, source.getSourceId(), game);
                         if (turnFaceUpAbilityMap.containsKey(permanent.getId())) {
-                            permanent.addAbility(turnFaceUpAbilityMap.get(permanent.getId()), game);
+                            permanent.addAbility(turnFaceUpAbilityMap.get(permanent.getId()), source.getSourceId(), game);
                         }
                         break;
                     case PTChangingEffects_7:

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

@@ -1,8 +1,5 @@
-
 package mage.abilities.effects.common.continuous;
 
-import java.util.ArrayList;
-import java.util.List;
 import mage.MageObjectReference;
 import mage.ObjectColor;
 import mage.abilities.Ability;
@@ -12,14 +9,13 @@ import mage.abilities.costs.Costs;
 import mage.abilities.costs.CostsImpl;
 import mage.abilities.effects.ContinuousEffectImpl;
 import mage.cards.Card;
-import mage.constants.CardType;
+import mage.constants.*;
-import mage.constants.Duration;
-import mage.constants.Layer;
-import mage.constants.Outcome;
-import mage.constants.SubLayer;
 import mage.game.Game;
 import mage.game.permanent.Permanent;
 
+import java.util.ArrayList;
+import java.util.List;
+
 /**
  * This effect lets the card be a 2/2 face-down creature, with no text, no name,
  * no subtypes, and no mana cost, if it's face down on the battlefield. And it
@@ -149,21 +145,31 @@ public class BecomesFaceDownCreatureEffect extends ContinuousEffectImpl implemen
                     Card card = game.getCard(permanent.getId()); //
                     List<Ability> abilitiesToRemove = new ArrayList<>();
                     for (Ability ability : permanent.getAbilities()) {
+
+                        // keep gained abilities from other sources, removes only own (card text)
                         if (card != null && !card.getAbilities().contains(ability)) {
-                            // gained abilities from other sources won't be removed
                             continue;
                         }
+
+                        // 701.33c
+                        // If a card with morph is manifested, its controller may turn that card face up using
+                        // either the procedure described in rule 702.36e to turn a face-down permanent with morph face up
+                        // or the procedure described above to turn a manifested permanent face up.
+                        //
+                        // so keep all tune face up abilities and other face down compatible
                         if (ability.getWorksFaceDown()) {
                             ability.setRuleVisible(false);
                             continue;
-                        } else if (!ability.getRuleVisible() && !ability.getEffects().isEmpty()) {
+                        }
+
+                        if (!ability.getRuleVisible() && !ability.getEffects().isEmpty()) {
                             if (ability.getEffects().get(0) instanceof BecomesFaceDownCreatureEffect) {
                                 continue;
                             }
                         }
                         abilitiesToRemove.add(ability);
                     }
-                    permanent.getAbilities().removeAll(abilitiesToRemove);
+                    permanent.removeAbilities(abilitiesToRemove, source.getSourceId(), game);
                     if (turnFaceUpAbility != null) {
                         permanent.addAbility(turnFaceUpAbility, source.getSourceId(), game);
                     }

Mage/src/main/java/mage/game/permanent/Permanent.java

@@ -156,6 +156,7 @@ public interface Permanent extends Card, Controllable {
 
     void addAbility(Ability ability, UUID sourceId, Game game);
 
+    @Deprecated // use addAbility(Ability ability, UUID sourceId, Game game) instead
     void addAbility(Ability ability, UUID sourceId, Game game, boolean createNewId);
 
     void removeAllAbilities(UUID sourceId, Game game);

Mage/src/main/java/mage/game/permanent/PermanentImpl.java

@@ -365,7 +365,6 @@ public abstract class PermanentImpl extends CardImpl implements Permanent {
     }
 
     @Override
-    @Deprecated // use addAbility(Ability ability, UUID sourceId, Game game) instead
     public void addAbility(Ability ability, UUID sourceId, Game game, boolean createNewId) {
         // singleton abilities -- only one instance
         // other abilities -- any amount of instances