some reworking of Equip, Fortify, and Reconfigure

Mage.Tests/src/test/java/org/mage/test/cards/abilities/activated/ReconfigureTest.java

@@ -16,6 +16,8 @@ public class ReconfigureTest extends CardTestPlayerBase {
     private static final String lion = "Silvercoat Lion";
     private static final String boar = "Bronzeplate Boar";
     private static final String aid = "Sigarda's Aid";
+    private static final String paladin = "Puresteel Paladin";
+    private static final String relic = "Darksteel Relic";
 
     @Test
     public void testAttach() {
@@ -79,4 +81,25 @@ public class ReconfigureTest extends CardTestPlayerBase {
         assertPowerToughness(playerA, lion, 2 + 3, 2 + 2);
         assertAbility(playerA, lion, TrampleAbility.getInstance(), true);
     }
+
+    @Test
+    public void testPuresteelPaladin() {
+        addCard(Zone.BATTLEFIELD, playerA, lion);
+        addCard(Zone.BATTLEFIELD, playerA, boar);
+        addCard(Zone.BATTLEFIELD, playerA, paladin);
+        addCard(Zone.BATTLEFIELD, playerA, relic, 2);
+
+        activateAbility(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Equip", lion);
+
+        setStrictChooseMode(true);
+        setStopAt(1, PhaseStep.END_TURN);
+        execute();
+        assertAllCommandsUsed();
+
+        assertType(boar, CardType.CREATURE, false);
+        assertSubtype(boar, SubType.EQUIPMENT);
+        assertIsAttachedTo(playerA, boar, lion);
+        assertPowerToughness(playerA, lion, 2 + 3, 2 + 2);
+        assertAbility(playerA, lion, TrampleAbility.getInstance(), true);
+    }
 }

Mage/src/main/java/mage/abilities/effects/EquipEffect.java

@@ -1,39 +0,0 @@
-package mage.abilities.effects;
-
-import mage.abilities.Ability;
-import mage.abilities.effects.common.AttachEffect;
-import mage.constants.Outcome;
-import mage.constants.SubType;
-import mage.game.Game;
-import mage.game.permanent.Permanent;
-
-public class EquipEffect extends AttachEffect {
-
-    public EquipEffect(Outcome outcome) {
-        super(outcome, "Equip");
-    }
-
-    public EquipEffect(EquipEffect effect) {
-        super(effect);
-    }
-
-    @Override
-    public boolean apply(Game game, Ability source) {
-        Permanent sourcePermanent = game.getPermanent(source.getSourceId());
-        //301.5c An Equipment that’s also a creature can’t equip a creature. An Equipment that loses the subtype
-        // “Equipment” can’t equip a creature. An Equipment can’t equip itself. An Equipment that equips an illegal or
-        // nonexistent permanent becomes unattached from that permanent but remains on the battlefield. (This is a
-        // state-based action. See rule 704.) An Equipment can’t equip more than one creature. If a spell or ability
-        // would cause an Equipment to equip more than one creature, the Equipment’s controller chooses which creature
-        // it equips.
-        if (sourcePermanent != null && sourcePermanent.hasSubtype(SubType.EQUIPMENT, game) && !sourcePermanent.isCreature(game)) {
-            return super.apply(game, source);
-        }
-        return false;
-    }
-
-    @Override
-    public EquipEffect copy(){
-        return new EquipEffect(this);
-    }
-}

Mage/src/main/java/mage/abilities/effects/common/AttachEffect.java

@@ -10,6 +10,8 @@ import mage.players.Player;
 import mage.target.TargetCard;
 import mage.util.CardUtil;
 
+import java.util.UUID;
+
 /**
  * @author BetaSteward_at_googlemail.com
  */
@@ -36,30 +38,29 @@ public class AttachEffect extends OneShotEffect {
     @Override
     public boolean apply(Game game, Ability source) {
         Permanent sourcePermanent = game.getPermanent(source.getSourceId());
-        if (sourcePermanent != null) {
+        if (sourcePermanent == null) {
-            // if it activating on the stack then allow +1 zcc
+            return false;
-            int zcc = game.getState().getZoneChangeCounter(sourcePermanent.getId());
-            if (zcc == CardUtil.getActualSourceObjectZoneChangeCounter(game, source)
-                    || zcc == CardUtil.getActualSourceObjectZoneChangeCounter(game, source) + 1) {
-                Permanent permanent = game.getPermanent(getTargetPointer().getFirst(game, source));
-                if (permanent != null) {
-                    return permanent.addAttachment(source.getSourceId(), source, game);
-                } else {
-                    Player player = game.getPlayer(getTargetPointer().getFirst(game, source));
-                    if (player != null) {
-                        return player.addAttachment(source.getSourceId(), source, game);
-                    }
-                    if (!source.getTargets().isEmpty() && source.getTargets().get(0) instanceof TargetCard) { // e.g. Spellweaver Volute
-                        Card card = game.getCard(getTargetPointer().getFirst(game, source));
-                        if (card != null) {
-                            return card.addAttachment(source.getSourceId(), source, game);
-                        }
-                    }
-                }
-            }
         }
-
+        // if it activating on the stack then allow +1 zcc
-        return false;
+        int zcc = game.getState().getZoneChangeCounter(sourcePermanent.getId());
+        if (zcc != CardUtil.getActualSourceObjectZoneChangeCounter(game, source)
+                && zcc != CardUtil.getActualSourceObjectZoneChangeCounter(game, source) + 1) {
+            return false;
+        }
+        UUID targetId = getTargetPointer().getFirst(game, source);
+        Permanent permanent = game.getPermanent(targetId);
+        if (permanent != null) {
+            return permanent.addAttachment(source.getSourceId(), source, game);
+        }
+        Player player = game.getPlayer(targetId);
+        if (player != null) {
+            return player.addAttachment(source.getSourceId(), source, game);
+        }
+        if (source.getTargets().isEmpty() || !(source.getTargets().get(0) instanceof TargetCard)) {
+            return false;
+        }
+        Card card = game.getCard(targetId);
+        return card != null && card.addAttachment(source.getSourceId(), source, game);
     }
 
 }

Mage/src/main/java/mage/abilities/effects/common/FortifyEffect.java

@@ -1,38 +0,0 @@
-package mage.abilities.effects.common;
-
-import mage.abilities.Ability;
-import mage.constants.Outcome;
-import mage.constants.SubType;
-import mage.game.Game;
-import mage.game.permanent.Permanent;
-
-public class FortifyEffect extends AttachEffect{
-
-    public FortifyEffect(Outcome outcome) {
-        super(outcome, "Fortify");
-    }
-
-    public FortifyEffect(FortifyEffect effect) {
-        super(effect);
-    }
-
-    @Override
-    public boolean apply(Game game, Ability source) {
-        Permanent sourcePermanent = game.getPermanent(source.getSourceId());
-        //Some artifacts have the subtype “Fortification.” A Fortification can be attached to a land. It can’t legally
-        // be attached to an object that isn’t a land. Fortification’s analog to the equip keyword ability is the
-        // fortify keyword ability. Rules 301.5a–e apply to Fortifications in relation to lands just as they apply to
-        // Equipment in relation to creatures, with one clarification relating to rule 301.5c: a Fortification that’s
-        // also a creature (not a land) can’t fortify a land. (See rule 702.66, “Fortify.”)
-        if (sourcePermanent != null && sourcePermanent.hasSubtype(SubType.FORTIFICATION, game) && !sourcePermanent.isCreature(game)
-                && !sourcePermanent.isLand(game)) {
-            return super.apply(game, source);
-        }
-        return false;
-    }
-
-    @Override
-    public FortifyEffect copy(){
-        return new FortifyEffect(this);
-    }
-}

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

@@ -3,7 +3,7 @@ package mage.abilities.keyword;
 import mage.abilities.ActivatedAbilityImpl;
 import mage.abilities.costs.Cost;
 import mage.abilities.costs.mana.GenericManaCost;
-import mage.abilities.effects.EquipEffect;
+import mage.abilities.effects.common.AttachEffect;
 import mage.constants.Outcome;
 import mage.constants.TimingRule;
 import mage.constants.Zone;
@@ -26,7 +26,7 @@ public class EquipAbility extends ActivatedAbilityImpl {
     }
 
     public EquipAbility(Outcome outcome, Cost cost, Target target) {
-        super(Zone.BATTLEFIELD, new EquipEffect(outcome), cost);
+        super(Zone.BATTLEFIELD, new AttachEffect(outcome, "Equip"), cost);
         this.addTarget(target);
         this.timing = TimingRule.SORCERY;
     }

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

@@ -1,20 +1,17 @@
-
-
 package mage.abilities.keyword;
 
+import mage.abilities.ActivatedAbilityImpl;
+import mage.abilities.costs.Cost;
 import mage.abilities.costs.mana.GenericManaCost;
-import mage.abilities.effects.common.FortifyEffect;
+import mage.abilities.effects.common.AttachEffect;
 import mage.constants.Outcome;
 import mage.constants.TimingRule;
 import mage.constants.Zone;
-import mage.abilities.ActivatedAbilityImpl;
+import mage.filter.StaticFilters;
-import mage.abilities.costs.Cost;
-import mage.filter.common.FilterControlledLandPermanent;
 import mage.target.Target;
 import mage.target.TargetPermanent;
 
 /**
- *
  * @author BetaSteward_at_googlemail.com
  */
 
@@ -26,11 +23,11 @@ public class FortifyAbility extends ActivatedAbilityImpl {
     }
 
     public FortifyAbility(Outcome outcome, Cost cost) {
-        this(outcome, cost, new TargetPermanent(new FilterControlledLandPermanent()));
+        this(outcome, cost, new TargetPermanent(StaticFilters.FILTER_CONTROLLED_PERMANENT_LAND));
     }
 
     public FortifyAbility(Outcome outcome, Cost cost, Target target) {
-        super(Zone.BATTLEFIELD, new FortifyEffect(outcome), cost);
+        super(Zone.BATTLEFIELD, new AttachEffect(outcome, "Fortify"), cost);
         this.addTarget(target);
         this.timing = TimingRule.SORCERY;
     }
@@ -49,4 +46,4 @@ public class FortifyAbility extends ActivatedAbilityImpl {
     public String getRule() {
         return "Fortify " + costs.getText() + manaCosts.getText() + " (" + manaCosts.getText() + ": <i>Attach to target land you control. Fortify only as a sorcery.)</i>";
     }
-}
+}

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

@@ -3,7 +3,6 @@ package mage.abilities.keyword;
 import mage.abilities.Ability;
 import mage.abilities.ActivatedAbilityImpl;
 import mage.abilities.common.SimpleStaticAbility;
-import mage.abilities.condition.Condition;
 import mage.abilities.costs.mana.ManaCostsImpl;
 import mage.abilities.effects.ContinuousEffectImpl;
 import mage.abilities.effects.OneShotEffect;
@@ -52,23 +51,9 @@ public class ReconfigureAbility extends ActivatedAbilityImpl {
 
 class ReconfigureUnattachAbility extends ActivatedAbilityImpl {
 
-    private static enum ReconfigureUnattachAbilityCondition implements Condition {
-        instance;
-
-        @Override
-        public boolean apply(Game game, Ability source) {
-            Permanent equipment = source.getSourcePermanentIfItStillExists(game);
-            if (equipment == null) {
-                return false;
-            }
-            Permanent permanent = game.getPermanent(equipment.getAttachedTo());
-            return permanent != null && permanent.isCreature(game);
-        }
-    }
-
     protected ReconfigureUnattachAbility(String manaString) {
         super(Zone.BATTLEFIELD, new ReconfigureUnattachEffect(), new ManaCostsImpl<>(manaString));
-        this.condition = ReconfigureUnattachAbilityCondition.instance;
+        this.condition = ReconfigureUnattachAbility::checkForCreature;
         this.timing = TimingRule.SORCERY;
         this.setRuleVisible(false);
     }
@@ -86,6 +71,15 @@ class ReconfigureUnattachAbility extends ActivatedAbilityImpl {
     public String getRule() {
         return super.getRule() + " Activate only if this permanent is attached to a creature and only as a sorcery.";
     }
+
+    private static boolean checkForCreature(Game game, Ability source) {
+        Permanent equipment = source.getSourcePermanentIfItStillExists(game);
+        if (equipment == null) {
+            return false;
+        }
+        Permanent permanent = game.getPermanent(equipment.getAttachedTo());
+        return permanent != null && permanent.isCreature(game);
+    }
 }
 
 class ReconfigureUnattachEffect extends OneShotEffect {

Mage/src/main/java/mage/cards/CardImpl.java

@@ -8,6 +8,7 @@ import mage.abilities.common.SimpleStaticAbility;
 import mage.abilities.effects.common.continuous.HasSubtypesSourceEffect;
 import mage.abilities.keyword.ChangelingAbility;
 import mage.abilities.keyword.FlashbackAbility;
+import mage.abilities.keyword.ReconfigureAbility;
 import mage.abilities.mana.ActivatedManaAbilityImpl;
 import mage.cards.repository.PluginClassloaderRegistery;
 import mage.constants.*;
@@ -813,21 +814,35 @@ public abstract class CardImpl extends MageObjectImpl implements Card {
 
     @Override
     public boolean addAttachment(UUID permanentId, Ability source, Game game) {
-        if (!this.attachments.contains(permanentId)) {
+        if (permanentId == null
-            Permanent attachment = game.getPermanent(permanentId);
+                || this.attachments.contains(permanentId)
-            if (attachment == null) {
+                || permanentId.equals(this.getId())) {
-                attachment = game.getPermanentEntering(permanentId);
+            return false;
-            }
-            if (attachment != null) {
-                if (!game.replaceEvent(new AttachEvent(objectId, attachment, source))) {
-                    this.attachments.add(permanentId);
-                    attachment.attachTo(objectId, source, game);
-                    game.fireEvent(new AttachedEvent(objectId, attachment, source));
-                    return true;
-                }
-            }
         }
-        return false;
+        Permanent attachment = game.getPermanent(permanentId);
+        if (attachment == null) {
+            attachment = game.getPermanentEntering(permanentId);
+        }
+        if (attachment == null) {
+            return false;
+        }
+        if (attachment.hasSubtype(SubType.EQUIPMENT, game)
+                && (attachment.isCreature(game)
+                && !attachment.getAbilities(game).containsClass(ReconfigureAbility.class)
+                || !this.isCreature(game))) {
+            return false;
+        }
+        if (attachment.hasSubtype(SubType.FORTIFICATION, game)
+                && (attachment.isCreature(game) || !this.isLand(game))) {
+            return false;
+        }
+        if (game.replaceEvent(new AttachEvent(objectId, attachment, source))) {
+            return false;
+        }
+        this.attachments.add(permanentId);
+        attachment.attachTo(objectId, source, game);
+        game.fireEvent(new AttachedEvent(objectId, attachment, source));
+        return true;
     }
 
     @Override