refactor LandAbility to be created by CardFactory (#5047)

---------

Co-authored-by: tool4EvEr <tool4EvEr@192.168.0.60>

forge-game/src/main/java/forge/game/CardTraitBase.java

@@ -170,7 +170,7 @@ public abstract class CardTraitBase extends GameObject implements IHasCardView,
      *
      * @return a boolean.
      */
-    public final boolean isSecondary() {
+    public boolean isSecondary() {
         return getParamOrDefault("Secondary", "False").equals("True");
     }
 

forge-game/src/main/java/forge/game/GameActionUtil.java

@@ -91,10 +91,10 @@ public final class GameActionUtil {
             return alternatives;
         }
 
-        if (sa.isSpell()) {
+        if (sa.isSpell() || sa.isLandAbility()) {
             boolean lkicheck = false;
 
-            Card newHost = ((Spell)sa).getAlternateHost(source);
+            Card newHost = sa.getAlternateHost(source);
             if (newHost != null) {
                 source = newHost;
                 lkicheck = true;

forge-game/src/main/java/forge/game/ability/AbilityUtils.java

@@ -2953,7 +2953,7 @@ public class AbilityUtils {
         }
 
         for (SpellAbility s : list) {
-            if (s instanceof LandAbility) {
+            if (s.isLandAbility()) {
                 // CR 305.3
                 if (controller.getGame().getPhaseHandler().isPlayerTurn(controller) && controller.canPlayLand(tgtCard, true, s)) {
                     sas.add(s);
@@ -2981,9 +2981,7 @@ public class AbilityUtils {
 
     private static void collectSpellsForPlayEffect(final List<SpellAbility> result, final CardState state, final Player controller, final boolean withAltCost) {
         if (state.getType().isLand()) {
-            LandAbility la = new LandAbility(state.getCard(), controller, null);
-            la.setCardState(state);
-            result.add(la);
+            result.add(state.getFirstSpellAbility());
         }
         final Iterable<SpellAbility> spells = state.getSpellAbilities();
         for (SpellAbility sa : spells) {

forge-game/src/main/java/forge/game/ability/effects/DiscoverEffect.java

@@ -1,7 +1,5 @@
 package forge.game.ability.effects;
 
-import com.google.common.base.Predicates;
-import com.google.common.collect.Iterables;
 import forge.game.Game;
 import forge.game.ability.AbilityKey;
 import forge.game.ability.AbilityUtils;
@@ -15,7 +13,7 @@ import forge.game.cost.CostPart;
 import forge.game.cost.CostReveal;
 import forge.game.player.Player;
 import forge.game.player.PlayerCollection;
-import forge.game.spellability.LandAbility;
+
 import forge.game.spellability.SpellAbility;
 import forge.game.trigger.TriggerType;
 import forge.game.zone.PlayerZone;
@@ -94,7 +92,7 @@ public class DiscoverEffect extends SpellAbilityEffect {
                     List<SpellAbility> sas = AbilityUtils.getBasicSpellsFromPlayEffect(found, p);
 
                     // filter out land abilities due to MDFC or similar
-                    Iterables.removeIf(sas, Predicates.instanceOf(LandAbility.class));
+                    sas.removeIf(sp -> sp.isLandAbility());
                     // the spell must also have a mana value equal to or less than the discover number
                     sas.removeIf(sp -> sp.getPayCosts().getTotalMana().getCMC() > num);
 

forge-game/src/main/java/forge/game/ability/effects/PlayEffect.java

@@ -39,7 +39,7 @@ import forge.game.replacement.ReplacementEffect;
 import forge.game.replacement.ReplacementHandler;
 import forge.game.replacement.ReplacementLayer;
 import forge.game.spellability.AlternativeCost;
-import forge.game.spellability.LandAbility;
+
 import forge.game.spellability.SpellAbility;
 import forge.game.spellability.SpellAbilityPredicates;
 import forge.game.zone.Zone;
@@ -343,7 +343,7 @@ public class PlayEffect extends SpellAbilityEffect {
             final Zone originZone = tgtCard.getZone();
 
             // lands will be played
-            if (tgtSA instanceof LandAbility) {
+            if (tgtSA.isLandAbility()) {
                 tgtSA.resolve();
                 amount--;
                 if (remember) {

forge-game/src/main/java/forge/game/ability/effects/PlayLandVariantEffect.java

@@ -57,7 +57,7 @@ public class PlayLandVariantEffect extends SpellAbilityEffect {
             PaperCard ran = Aggregates.random(cards);
             random = CardFactory.getCard(ran, activator, game);
             cards.remove(ran);
-        } while (!activator.canPlayLand(random, false));
+        } while (!activator.canPlayLand(random, false, random.getFirstSpellAbility()));
 
         source.addCloneState(CardFactory.getCloneStates(random, source, sa), game.getNextTimestamp());
         source.updateStateForView();

forge-game/src/main/java/forge/game/card/Card.java

@@ -7430,7 +7430,7 @@ public class Card extends GameEntity implements Comparable<Card>, IHasSVars {
             for (SpellAbility sa : getState(CardStateName.Modal).getSpellAbilities()) {
                 //add alternative costs as additional spell abilities
                 // only add Spells there
-                if (sa.isSpell()) {
+                if (sa.isSpell() || sa.isLandAbility()) {
                     abilities.add(sa);
                     abilities.addAll(GameActionUtil.getAlternativeCosts(sa, player, false));
                 }
@@ -7466,106 +7466,6 @@ public class Card extends GameEntity implements Comparable<Card>, IHasSVars {
         }
         abilities.removeAll(toRemove);
 
-        // Land Abilities below, move them to CardFactory after MayPlayRefactor
-        if (getLastKnownZone().is(ZoneType.Battlefield)) {
-            return abilities;
-        }
-        if (getState(CardStateName.Original).getType().isLand()) {
-            LandAbility la = new LandAbility(this, player, null);
-            la.setCardState(oState);
-            if (la.canPlay()) {
-                abilities.add(la);
-            }
-
-            Card source = this;
-            boolean lkicheck = false;
-
-            // if Card is Facedown, need to check if MayPlay still applies
-            if (isFaceDown()) {
-                lkicheck = true;
-                source = CardCopyService.getLKICopy(source);
-                source.forceTurnFaceUp();
-            }
-
-            if (lkicheck) {
-                // double freeze tracker, so it doesn't update view
-                game.getTracker().freeze();
-                CardCollection preList = new CardCollection(source);
-                game.getAction().checkStaticAbilities(false, Sets.newHashSet(source), preList);
-            }
-
-            // extra for MayPlay
-            for (CardPlayOption o : source.mayPlay(player)) {
-                la = new LandAbility(this, player, o);
-                la.setCardState(oState);
-                if (la.canPlay()) {
-                    abilities.add(la);
-                }
-            }
-
-            // reset static abilities
-            if (lkicheck) {
-                game.getAction().checkStaticAbilities(false);
-                // clear delayed changes, this check should not have updated the view
-                game.getTracker().clearDelayed();
-                // need to unfreeze tracker
-                game.getTracker().unfreeze();
-            }
-        }
-
-        if (isModal() && hasState(CardStateName.Modal)) {
-            CardState modal = getState(CardStateName.Modal);
-            if (modal.getType().isLand()) {
-                LandAbility la = new LandAbility(this, player, null);
-                la.setCardState(modal);
-
-                Card source = CardCopyService.getLKICopy(this);
-                boolean lkicheck = true;
-
-                // if Card is Facedown, need to check if MayPlay still applies
-                if (isFaceDown()) {
-                    source.forceTurnFaceUp();
-                }
-
-                // the modal state is not copied with lki, need to copy it extra
-                if (!source.hasState(CardStateName.Modal)) {
-                    source.addAlternateState(CardStateName.Modal, false);
-                    source.getState(CardStateName.Modal).copyFrom(this.getState(CardStateName.Modal), true);
-                }
-
-                source.setSplitStateToPlayAbility(la);
-
-                if (la.canPlay(source)) {
-                    abilities.add(la);
-                }
-
-                if (lkicheck) {
-                    // double freeze tracker, so it doesn't update view
-                    game.getTracker().freeze();
-                    CardCollection preList = new CardCollection(source);
-                    game.getAction().checkStaticAbilities(false, Sets.newHashSet(source), preList);
-                }
-
-                // extra for MayPlay
-                for (CardPlayOption o : source.mayPlay(player)) {
-                    la = new LandAbility(this, player, o);
-                    la.setCardState(modal);
-                    if (la.canPlay(source)) {
-                        abilities.add(la);
-                    }
-                }
-
-                // reset static abilities
-                if (lkicheck) {
-                    game.getAction().checkStaticAbilities(false);
-                    // clear delayed changes, this check should not have updated the view
-                    game.getTracker().clearDelayed();
-                    // need to unfreeze tracker
-                    game.getTracker().unfreeze();
-                }
-            }
-        }
-
         return abilities;
     }
 

forge-game/src/main/java/forge/game/card/CardFactory.java

@@ -413,17 +413,16 @@ public class CardFactory {
 
         // SpellPermanent only for Original State
         if (c.getCurrentStateName() == CardStateName.Original || c.getCurrentStateName() == CardStateName.Modal || c.getCurrentStateName().toString().startsWith("Specialize")) {
-            // this is the "default" spell for permanents like creatures and artifacts
-            if (c.isPermanent() && !c.isAura() && !c.isLand()) {
+            if (c.isLand()) {
+                SpellAbility sa = new LandAbility(c);
+                sa.setCardState(c.getCurrentState());
+                c.addSpellAbility(sa);
+            } else if (c.isPermanent() && !c.isAura()) {
+                // this is the "default" spell for permanents like creatures and artifacts
                 SpellAbility sa = new SpellPermanent(c);
-
-                // Currently only for Modal, might react different when state is always set
-                //if (c.getCurrentStateName() == CardStateName.Modal) {
-                    sa.setCardState(c.getCurrentState());
-                //}
+                sa.setCardState(c.getCurrentState());
                 c.addSpellAbility(sa);
             }
-            // TODO add LandAbility there when refactor MayPlay
         }
 
         CardFactoryUtil.addAbilityFactoryAbilities(c, face.getAbilities());

forge-game/src/main/java/forge/game/phase/PhaseHandler.java

@@ -32,7 +32,7 @@ import forge.game.event.*;
 import forge.game.player.Player;
 import forge.game.replacement.ReplacementResult;
 import forge.game.replacement.ReplacementType;
-import forge.game.spellability.LandAbility;
+
 import forge.game.spellability.SpellAbility;
 import forge.game.trigger.Trigger;
 import forge.game.trigger.TriggerType;
@@ -1064,7 +1064,7 @@ public class PhaseHandler implements java.io.Serializable {
                         final Zone currentZone = saHost.getZone();
 
                         // Need to check if Zone did change
-                        if (currentZone != null && originZone != null && !currentZone.equals(originZone) && (sa.isSpell() || sa instanceof LandAbility)) {
+                        if (currentZone != null && originZone != null && !currentZone.equals(originZone) && (sa.isSpell() || sa.isLandAbility())) {
                             // currently there can be only one Spell put on the Stack at once, or Land Abilities be played
                             final CardZoneTable triggerList = new CardZoneTable(game.getLastStateBattlefield(), game.getLastStateGraveyard());
                             triggerList.put(originZone.getZoneType(), currentZone.getZoneType(), saHost);

forge-game/src/main/java/forge/game/player/Player.java

@@ -43,7 +43,7 @@ import forge.game.replacement.ReplacementHandler;
 import forge.game.replacement.ReplacementResult;
 import forge.game.replacement.ReplacementType;
 import forge.game.spellability.AbilitySub;
-import forge.game.spellability.LandAbility;
+
 import forge.game.spellability.SpellAbility;
 import forge.game.staticability.*;
 import forge.game.trigger.Trigger;
@@ -1691,9 +1691,9 @@ public class Player extends GameEntity implements Comparable<Player> {
         game.fireEvent(new GameEventShuffle(this));
     }
 
-    public final boolean playLand(final Card land, final boolean ignoreZoneAndTiming) {
+    public final boolean playLand(final Card land, final boolean ignoreZoneAndTiming, SpellAbility cause) {
         // Dakkon Blackblade Avatar will use a similar effect
-        if (canPlayLand(land, ignoreZoneAndTiming)) {
+        if (canPlayLand(land, ignoreZoneAndTiming, cause)) {
             playLandNoCheck(land, null);
             return true;
         }
@@ -1706,7 +1706,7 @@ public class Player extends GameEntity implements Comparable<Player> {
         land.setController(this, 0);
         if (land.isFaceDown()) {
             land.turnFaceUp(null);
-            if (cause instanceof LandAbility) {
+            if (cause.isLandAbility()) {
                 land.changeToState(cause.getCardStateName());
             }
         }
@@ -1730,12 +1730,6 @@ public class Player extends GameEntity implements Comparable<Player> {
         return c;
     }
 
-    public final boolean canPlayLand(final Card land) {
-        return canPlayLand(land, false);
-    }
-    public final boolean canPlayLand(final Card land, final boolean ignoreZoneAndTiming) {
-        return canPlayLand(land, ignoreZoneAndTiming, null);
-    }
     public final boolean canPlayLand(final Card land, final boolean ignoreZoneAndTiming, SpellAbility landSa) {
         if (!ignoreZoneAndTiming) {
             // CR 305.3

forge-game/src/main/java/forge/game/spellability/LandAbility.java

@@ -21,24 +21,20 @@ import forge.card.CardStateName;
 import forge.card.mana.ManaCost;
 import forge.game.card.Card;
 import forge.game.card.CardCopyService;
-import forge.game.card.CardPlayOption;
-import forge.game.cost.Cost;
 import forge.game.player.Player;
 import forge.game.staticability.StaticAbility;
+import forge.game.zone.ZoneType;
 import forge.util.CardTranslation;
 import forge.util.Localizer;
 import org.apache.commons.lang3.ObjectUtils;
 import org.apache.commons.lang3.StringUtils;
 
-public class LandAbility extends Ability {
+public class LandAbility extends AbilityStatic {
 
-    public LandAbility(Card sourceCard, Player p, CardPlayOption mayPlay) {
-        super(sourceCard, new Cost(ManaCost.NO_COST, false));
-        setActivatingPlayer(p);
-        setMayPlay(mayPlay);
-    }
     public LandAbility(Card sourceCard) {
-        this(sourceCard, sourceCard.getController(), null);
+        super(sourceCard, ManaCost.NO_COST);
+
+        getRestrictions().setZone(ZoneType.Hand);
     }
 
     public boolean canPlay(Card newHost) {
@@ -46,11 +42,21 @@ public class LandAbility extends Ability {
         return p.canPlayLand(newHost, false, this);
     }
 
+    @Override
+    public boolean isLandAbility() { return true; }
+
+    @Override
+    public boolean isSecondary() {
+        return true;
+    }
+
     @Override
     public boolean canPlay() {
         Card land = this.getHostCard();
         final Player p = this.getActivatingPlayer();
-
+        if (p == null) {
+            return false;
+        }
         if (this.getCardState() != null && land.getCurrentStateName() != this.getCardStateName()) {
             if (!land.isLKI()) {
                 land = CardCopyService.getLKICopy(land);
@@ -113,4 +119,41 @@ public class LandAbility extends Ability {
         return sb.toString();
     }
 
+    @Override
+    public Card getAlternateHost(Card source) {
+        boolean lkicheck = false;
+
+        // need to be done before so it works with Vivien and Zoetic Cavern
+        if (source.isFaceDown() && source.isInZone(ZoneType.Exile)) {
+            if (!source.isLKI()) {
+                source = CardCopyService.getLKICopy(source);
+            }
+
+            source.forceTurnFaceUp();
+            lkicheck = true;
+        }
+
+        if (getCardState() != null && source.getCurrentStateName() != getCardStateName()) {
+            if (!source.isLKI()) {
+                source = CardCopyService.getLKICopy(source);
+            }
+            CardStateName stateName = getCardState().getStateName();
+            if (!source.hasState(stateName)) {
+                source.addAlternateState(stateName, false);
+                source.getState(stateName).copyFrom(getHostCard().getState(stateName), true);
+            }
+
+            source.setState(stateName, false);
+            if (getHostCard().isDoubleFaced()) {
+                source.setBackSide(getHostCard().getRules().getSplitType().getChangedStateName().equals(stateName));
+            }
+
+            // need to reset CMC
+            source.setLKICMC(-1);
+            source.setLKICMC(source.getCMC());
+            lkicheck = true;
+        }
+
+        return lkicheck ? source : null;
+    }
 }

forge-game/src/main/java/forge/game/spellability/Spell.java

@@ -153,6 +153,7 @@ public abstract class Spell extends SpellAbility implements java.io.Serializable
         this.castFaceDown = faceDown;
     }
 
+    @Override
     public Card getAlternateHost(Card source) {
         boolean lkicheck = false;
 

forge-game/src/main/java/forge/game/spellability/SpellAbility.java

@@ -527,6 +527,7 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
     public boolean isSpell() { return false; }
     public boolean isAbility() { return true; }
     public boolean isActivatedAbility() { return false; }
+    public boolean isLandAbility() { return false; }
 
     public boolean isTurnFaceUp() {
         return isMorphUp() || isDisguiseUp() || isManifestUp() || isCloakUp();
@@ -2185,7 +2186,7 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
             }
         }
         else if (incR[0].contains("LandAbility")) {
-            if (!(root instanceof LandAbility)) {
+            if (!(root.isLandAbility())) {
                 return testFailed;
             }
         }
@@ -2544,7 +2545,7 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
         if (getRestrictions().isInstantSpeed()) {
             return true;
         }
-        if ((isSpell() || this instanceof LandAbility) && (isCastFromPlayEffect() || host.isInstant() || host.hasKeyword(Keyword.FLASH))) {
+        if ((isSpell() || this.isLandAbility()) && (isCastFromPlayEffect() || host.isInstant() || host.hasKeyword(Keyword.FLASH))) {
             return true;
         }
 
@@ -2589,6 +2590,10 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
         return true;
     }
 
+    public Card getAlternateHost(Card source) {
+        return null;
+    }
+
     public boolean hasOptionalKeywordAmount(KeywordInterface kw) {
         return this.optionalKeywordAmount.contains(kw.getKeyword(), Pair.of(kw.getIdx(), kw.getStaticId()));
     }