From 4ede390e40eed49f09e289963fe1f574de3c1596 Mon Sep 17 00:00:00 2001 From: Evan Kranzler Date: Thu, 17 Feb 2022 18:08:22 -0500 Subject: [PATCH] some reworking of Equip, Fortify, and Reconfigure --- .../abilities/activated/ReconfigureTest.java | 23 +++++++++ .../mage/abilities/effects/EquipEffect.java | 39 --------------- .../effects/common/AttachEffect.java | 47 ++++++++++--------- .../effects/common/FortifyEffect.java | 38 --------------- .../mage/abilities/keyword/EquipAbility.java | 4 +- .../abilities/keyword/FortifyAbility.java | 17 +++---- .../abilities/keyword/ReconfigureAbility.java | 26 ++++------ Mage/src/main/java/mage/cards/CardImpl.java | 43 +++++++++++------ 8 files changed, 95 insertions(+), 142 deletions(-) delete mode 100644 Mage/src/main/java/mage/abilities/effects/EquipEffect.java delete mode 100644 Mage/src/main/java/mage/abilities/effects/common/FortifyEffect.java diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/abilities/activated/ReconfigureTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/abilities/activated/ReconfigureTest.java index 887a8604b35..1b078a7378c 100644 --- a/Mage.Tests/src/test/java/org/mage/test/cards/abilities/activated/ReconfigureTest.java +++ b/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); + } } diff --git a/Mage/src/main/java/mage/abilities/effects/EquipEffect.java b/Mage/src/main/java/mage/abilities/effects/EquipEffect.java deleted file mode 100644 index 392713bbf7b..00000000000 --- a/Mage/src/main/java/mage/abilities/effects/EquipEffect.java +++ /dev/null @@ -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); - } -} diff --git a/Mage/src/main/java/mage/abilities/effects/common/AttachEffect.java b/Mage/src/main/java/mage/abilities/effects/common/AttachEffect.java index 1918bc1c6c5..9290eccbd74 100644 --- a/Mage/src/main/java/mage/abilities/effects/common/AttachEffect.java +++ b/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 it activating on the stack then allow +1 zcc - 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 (sourcePermanent == null) { + return false; } - - return false; + // if it activating on the stack then allow +1 zcc + 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); } } diff --git a/Mage/src/main/java/mage/abilities/effects/common/FortifyEffect.java b/Mage/src/main/java/mage/abilities/effects/common/FortifyEffect.java deleted file mode 100644 index f62836ecc9f..00000000000 --- a/Mage/src/main/java/mage/abilities/effects/common/FortifyEffect.java +++ /dev/null @@ -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); - } -} diff --git a/Mage/src/main/java/mage/abilities/keyword/EquipAbility.java b/Mage/src/main/java/mage/abilities/keyword/EquipAbility.java index 1bc91d6cfcc..f3dcfe00fd4 100644 --- a/Mage/src/main/java/mage/abilities/keyword/EquipAbility.java +++ b/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; } diff --git a/Mage/src/main/java/mage/abilities/keyword/FortifyAbility.java b/Mage/src/main/java/mage/abilities/keyword/FortifyAbility.java index 14596029b34..be1b1ea368a 100644 --- a/Mage/src/main/java/mage/abilities/keyword/FortifyAbility.java +++ b/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.abilities.costs.Cost; -import mage.filter.common.FilterControlledLandPermanent; +import mage.filter.StaticFilters; 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() + ": Attach to target land you control. Fortify only as a sorcery.)"; } -} \ No newline at end of file +} diff --git a/Mage/src/main/java/mage/abilities/keyword/ReconfigureAbility.java b/Mage/src/main/java/mage/abilities/keyword/ReconfigureAbility.java index 370ce6ce0bb..61d672923a8 100644 --- a/Mage/src/main/java/mage/abilities/keyword/ReconfigureAbility.java +++ b/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 { diff --git a/Mage/src/main/java/mage/cards/CardImpl.java b/Mage/src/main/java/mage/cards/CardImpl.java index a06431ed075..daa45f8e5d0 100644 --- a/Mage/src/main/java/mage/cards/CardImpl.java +++ b/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)) { - Permanent attachment = game.getPermanent(permanentId); - if (attachment == null) { - attachment = game.getPermanentEntering(permanentId); - } - 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; - } - } + if (permanentId == null + || this.attachments.contains(permanentId) + || permanentId.equals(this.getId())) { + return false; } - 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