ReplacementEffect: rewrite for Riot and Spark Double

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

@@ -466,7 +466,7 @@ public class Game {
     }
 
     public Zone getZoneOf(final Card card) {
-        return card.getZone();
+        return card.getLastKnownZone();
     }
 
     public synchronized CardCollectionView getCardsIn(final ZoneType zone) {

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

@@ -29,7 +29,6 @@ import forge.game.ability.effects.AttachEffect;
 import forge.game.card.*;
 import forge.game.event.*;
 import forge.game.keyword.KeywordInterface;
-import forge.game.keyword.KeywordsChange;
 import forge.game.player.GameLossReason;
 import forge.game.player.Player;
 import forge.game.replacement.ReplacementEffect;
@@ -289,33 +288,6 @@ public class GameAction {
                 copied.getOwner().addInboundToken(copied);
             }
 
-            if (toBattlefield) {
-                // HACK for making the RIOT enchantment look into the Future
-                // need to check the Keywords what it would have on the Battlefield
-                Card riotLKI = CardUtil.getLKICopy(copied);
-                riotLKI.setLastKnownZone(zoneTo);
-                CardCollection preList = new CardCollection(riotLKI);
-                checkStaticAbilities(false, Sets.newHashSet(riotLKI), preList);
-
-                List<Long> changedTimeStamps = Lists.newArrayList();
-                for(Map.Entry<Long, KeywordsChange> e : riotLKI.getChangedCardKeywords().entrySet()) {
-                    if (!copied.hasChangedCardKeywords(e.getKey())) {
-                        KeywordsChange o = e.getValue();
-                        o.setHostCard(copied);
-                        for (KeywordInterface k : o.getKeywords()) {
-                            for (ReplacementEffect re : k.getReplacements()) {
-                                // this param need to be set, otherwise in ReplaceMoved it fails
-                                re.getMapParams().put("BypassEtbCheck", "True");
-                            }
-                        }
-                        copied.addChangedCardKeywordsInternal(o, e.getKey());
-                        changedTimeStamps.add(e.getKey());
-                    }
-                }
-
-                checkStaticAbilities(false);
-            }
-
             Map<String, Object> repParams = Maps.newHashMap();
             repParams.put("Event", "Moved");
             repParams.put("Affected", copied);

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

@@ -10,8 +10,11 @@ import forge.game.player.Player;
 import forge.game.spellability.SpellAbility;
 import forge.game.zone.ZoneType;
 
+import java.util.Arrays;
 import java.util.List;
 
+import com.google.common.collect.Lists;
+
 public class CloneEffect extends SpellAbilityEffect {
     // TODO update this method
 
@@ -48,6 +51,11 @@ public class CloneEffect extends SpellAbilityEffect {
         final Player activator = sa.getActivatingPlayer();
         Card tgtCard = host;
         final Game game = activator.getGame();
+        final List<String> pumpKeywords = Lists.newArrayList();
+
+        if (sa.hasParam("PumpKeywords")) {
+            pumpKeywords.addAll(Arrays.asList(sa.getParam("PumpKeywords").split(" & ")));
+        }
 
         // find cloning source i.e. thing to be copied
         Card cardToCopy = null;
@@ -115,6 +123,10 @@ public class CloneEffect extends SpellAbilityEffect {
             tgtCard.setTapped(true);
         }
 
+        if (!pumpKeywords.isEmpty()) {
+            tgtCard.addChangedCardKeywords(pumpKeywords, Lists.<String>newArrayList(), false, false, ts);
+        }
+
         tgtCard.updateStateForView();
 
         //Clear Remembered and Imprint lists

forge-game/src/main/java/forge/game/replacement/ReplaceMoved.java

@@ -1,16 +1,13 @@
 package forge.game.replacement;
 
 import forge.game.card.Card;
-import forge.game.card.CardCollection;
+
-import forge.game.card.CardUtil;
 import forge.game.player.Player;
 import forge.game.spellability.SpellAbility;
 import forge.game.zone.ZoneType;
 
 import java.util.Map;
 
-import com.google.common.collect.Sets;
-
 /** 
  * TODO: Write javadoc for this type.
  *
@@ -50,56 +47,23 @@ public class ReplaceMoved extends ReplacementEffect {
             }
         }
 
-        boolean matchedZone = false;
         if (hasParam("Origin")) {
-            for(ZoneType z : ZoneType.listValueOf(getParam("Origin"))) {
+            ZoneType zt = (ZoneType) runParams.get("Origin");
-                if(z == (ZoneType) runParams.get("Origin"))
+            if (!ZoneType.listValueOf(getParam("Origin")).contains(zt)) {
-                    matchedZone =  true;
-            }
-
-            if(!matchedZone)
-            {
                 return false;
             }
         }        
 
         if (hasParam("Destination")) {
-            matchedZone = false;
             ZoneType zt = (ZoneType) runParams.get("Destination");
-            for(ZoneType z : ZoneType.listValueOf(getParam("Destination"))) {
+            if (!ZoneType.listValueOf(getParam("Destination")).contains(zt)) {
-                if(z == zt)
-                    matchedZone =  true;
-            }
-
-            if(!matchedZone)
-            {
                 return false;
             }
-
-            if (zt.equals(ZoneType.Battlefield) && getHostCard().equals(affected) && !hasParam("BypassEtbCheck")) {
-                // would be an etb replacement effect that enters the battlefield
-                Card lki = CardUtil.getLKICopy(affected);
-                lki.setLastKnownZone(lki.getController().getZone(zt));
-
-                CardCollection preList = new CardCollection(lki);
-                getHostCard().getGame().getAction().checkStaticAbilities(false, Sets.newHashSet(lki), preList);
-
-                // check if when entering the battlefield would still has this RE or is suppressed
-                if (!lki.hasReplacementEffect(this) || lki.getReplacementEffect(getId()).isSuppressed()) {
-                    return false;
-                }
-            }
         }
         
         if (hasParam("ExcludeDestination")) {
-            matchedZone = false;
+            ZoneType zt = (ZoneType) runParams.get("Destination");
-            for(ZoneType z : ZoneType.listValueOf(getParam("ExcludeDestination"))) {
+            if (ZoneType.listValueOf(getParam("ExcludeDestination")).contains(zt)) {
-                if(z == (ZoneType) runParams.get("Destination"))
-                    matchedZone =  true;
-            }
-            
-            if(matchedZone)
-            {
                 return false;
             }
         }

forge-game/src/main/java/forge/game/replacement/ReplacementEffect.java

@@ -44,6 +44,8 @@ public abstract class ReplacementEffect extends TriggerReplacementBase {
     /** The has run. */
     private boolean hasRun = false;
 
+    private List<ReplacementEffect> otherChoices = null;
+
     /**
      * Gets the id.
      *
@@ -102,6 +104,13 @@ public abstract class ReplacementEffect extends TriggerReplacementBase {
         this.hasRun = hasRun;
     }
 
+    public List<ReplacementEffect> getOtherChoices() {
+        return otherChoices;
+    }
+    public void setOtherChoices(List<ReplacementEffect> choices) {
+        this.otherChoices = choices;
+    }
+
     /**
      * Can replace.
      *
@@ -174,6 +183,7 @@ public abstract class ReplacementEffect extends TriggerReplacementBase {
         if (!lki) {
             res.setId(nextId());
             res.setHasRun(false);
+            res.setOtherChoices(null);
         }
 
         res.setHostCard(host);

forge-game/src/main/java/forge/game/replacement/ReplacementHandler.java

@@ -23,6 +23,8 @@ import forge.game.GameLogEntryType;
 import forge.game.ability.AbilityFactory;
 import forge.game.ability.AbilityUtils;
 import forge.game.card.Card;
+import forge.game.card.CardCollection;
+import forge.game.card.CardUtil;
 import forge.game.player.Player;
 import forge.game.spellability.SpellAbility;
 import forge.game.zone.Zone;
@@ -34,6 +36,7 @@ import forge.util.Visitor;
 import org.apache.commons.lang3.StringUtils;
 
 import com.google.common.collect.Lists;
+import com.google.common.collect.Sets;
 
 import java.util.*;
 
@@ -74,6 +77,34 @@ public class ReplacementHandler {
     }
 
     public List<ReplacementEffect> getReplacementList(final Map<String, Object> runParams, final ReplacementLayer layer) {
+
+        final CardCollection preList = new CardCollection();
+        boolean checkAgain = false;
+        Card affectedLKI = null;
+        Card affectedCard = null;
+
+        if ("Moved".equals(runParams.get("Event")) && ZoneType.Battlefield.equals(runParams.get("Destination"))) {
+            // if it was caused by an replacement effect, use the already calculated RE list
+            // otherwise the RIOT card would cause a StackError
+            SpellAbility cause = (SpellAbility) runParams.get("Cause");
+            if (cause != null && cause.isReplacementAbility()) {
+                final ReplacementEffect re = cause.getReplacementEffect();
+                // only return for same layer
+                if (layer.equals(re.getLayer())) {
+                    return re.getOtherChoices();
+                }
+            }
+
+            // Rule 614.12 Enter the Battlefield Replacement Effects look at what the card would be on the battlefield
+            affectedCard = (Card) runParams.get("Affected");
+            affectedLKI = CardUtil.getLKICopy(affectedCard);
+            affectedLKI.setLastKnownZone(affectedCard.getController().getZone(ZoneType.Battlefield));
+            preList.add(affectedLKI);
+            game.getAction().checkStaticAbilities(false, Sets.newHashSet(affectedLKI), preList);
+            checkAgain = true;
+            runParams.put("Affected", affectedLKI);
+        }
+
         final List<ReplacementEffect> possibleReplacers = Lists.newArrayList();
         // Round up Non-static replacement effects ("Until EOT," or
         // "The next time you would..." etc)
@@ -87,17 +118,19 @@ public class ReplacementHandler {
         game.forEachCardInGame(new Visitor<Card>() {
             @Override
             public boolean visit(Card crd) {
-                for (final ReplacementEffect replacementEffect : crd.getReplacementEffects()) {
+                final Card c = preList.get(crd);
+
+                for (final ReplacementEffect replacementEffect : c.getReplacementEffects()) {
 
                     // Use "CheckLKIZone" parameter to test for effects that care abut where the card was last (e.g. Kalitas, Traitor of Ghet
                     // getting hit by mass removal should still produce tokens).
-                    Zone cardZone = "True".equals(replacementEffect.getMapParams().get("CheckSelfLKIZone")) ? game.getChangeZoneLKIInfo(crd).getLastKnownZone() : game.getZoneOf(crd);
+                    Zone cardZone = "True".equals(replacementEffect.getParam("CheckSelfLKIZone")) ? game.getChangeZoneLKIInfo(c).getLastKnownZone() : game.getZoneOf(c);
 
                     // Replacement effects that are tied to keywords (e.g. damage prevention effects - if the keyword is removed, the replacement
                     // effect should be inactive)
                     if (replacementEffect.hasParam("TiedToKeyword")) {
                         String kw = replacementEffect.getParam("TiedToKeyword");
-                        if (!crd.hasKeyword(kw)) {
+                        if (!c.hasKeyword(kw)) {
                             continue;
                         }
                     }
@@ -115,6 +148,19 @@ public class ReplacementHandler {
             }
 
         });
+
+        if (checkAgain) {
+            if (affectedLKI != null && affectedCard != null) {
+                // need to set the Host Card there so it is not connected to LKI anymore?
+                // need to be done after canReplace check
+                for (final ReplacementEffect re : affectedLKI.getReplacementEffects()) {
+                    re.setHostCard(affectedCard);
+                }
+                runParams.put("Affected", affectedCard);
+            }
+            game.getAction().checkStaticAbilities(false);
+        }
+
         return possibleReplacers;
     }
 
@@ -138,15 +184,18 @@ public class ReplacementHandler {
         possibleReplacers.remove(chosenRE);
 
         chosenRE.setHasRun(true);
-        ReplacementResult res = this.executeReplacement(runParams, chosenRE, decider, game);
+        chosenRE.setOtherChoices(possibleReplacers);
+        ReplacementResult res = executeReplacement(runParams, chosenRE, decider, game);
         if (res == ReplacementResult.NotReplaced) {
             if (!possibleReplacers.isEmpty()) {
                 res = run(runParams);
             }
             chosenRE.setHasRun(false);
+            chosenRE.setOtherChoices(null);
             return res;
         }
         chosenRE.setHasRun(false);
+        chosenRE.setOtherChoices(null);
         String message = chosenRE.toString();
         if ( !StringUtils.isEmpty(message))
         	if (chosenRE.getHostCard() != null) {
@@ -210,7 +259,6 @@ public class ReplacementHandler {
                 effectSA.setIntrinsic(true);
                 effectSA.changeText();
             }
-            effectSA.setReplacementAbility(true);
             effectSA.setReplacementEffect(replacementEffect);
         }
 
@@ -278,9 +326,11 @@ public class ReplacementHandler {
      * @return A finished instance
      */
     public static ReplacementEffect parseReplacement(final String repParse, final Card host, final boolean intrinsic) {
+        return ReplacementHandler.parseReplacement(parseParams(repParse), host, intrinsic);
+    }
 
-        final Map<String, String> mapParams = FileSection.parseToMap(repParse, "$", "|");
+    public static Map<String, String> parseParams(final String repParse) {
-        return ReplacementHandler.parseReplacement(mapParams, host, intrinsic);
+        return FileSection.parseToMap(repParse, "$", "|");
     }
 
     /**

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

@@ -91,7 +91,6 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
     private boolean trigger = false;
     private Trigger triggerObj = null;
     private boolean optionalTrigger = false;
-    private boolean replacementAbility = false;
     private ReplacementEffect replacementEffect = null;
     private int sourceTrigger = -1;
     private List<Object> triggerRemembered = Lists.newArrayList();
@@ -952,13 +951,13 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
     }
 
     public boolean isReplacementAbility() {
-        return replacementAbility;
+        return getParent() != null ? getParent().isReplacementAbility() : replacementEffect != null;
-    }
-    public void setReplacementAbility(boolean replacement) {
-        replacementAbility = replacement;
     }
 
     public ReplacementEffect getReplacementEffect() {
+        if (getParent() != null) {
+            return getParent().getReplacementEffect();
+        }
         return replacementEffect;
     }
 

forge-game/src/main/java/forge/game/staticability/StaticAbility.java

@@ -540,7 +540,7 @@ public class StaticAbility extends CardTraitBase implements Comparable<StaticAbi
 
         if (hasParam("EffectZone")) {
             if (!getParam("EffectZone").equals("All")) {
-                Zone zone = getHostCard().getZone();
+                Zone zone = game.getZoneOf(getHostCard());
                 if (zone == null || !ZoneType.listValueOf(getParam("EffectZone")).contains(zone.getZoneType())) {
                     return false;
                 }

forge-gui-desktop/src/test/java/forge/ai/simulation/GameSimulatorTest.java

@@ -1830,6 +1830,37 @@ public class GameSimulatorTest extends SimulationTestCase {
     }
 
 
+    public void testSparkDoubleAndGideon() {
+        Game game = initAndCreateGame();
+        Player p = game.getPlayers().get(0);
+        game.getPhaseHandler().devModeSet(PhaseType.MAIN1, p);
+
+        for (int i=0; i<7; i++) { addCardToZone("Plains", p, ZoneType.Battlefield); }
+        for (int i=0; i<7; i++) { addCardToZone("Island", p, ZoneType.Battlefield); }
+
+        Card gideon = addCardToZone("Gideon Blackblade", p, ZoneType.Hand);
+        Card sparkDouble = addCardToZone("Spark Double", p, ZoneType.Hand);
+
+        SpellAbility gideonSA = gideon.getFirstSpellAbility();
+        SpellAbility sparkDoubleSA = sparkDouble.getFirstSpellAbility();
+
+        game.getPhaseHandler().devModeSet(PhaseType.MAIN1, p);
+        game.getAction().checkStateEffects(true);
+
+        GameSimulator sim = createSimulator(game, p);
+        sim.simulateSpellAbility(gideonSA);
+        sim.simulateSpellAbility(sparkDoubleSA);
+
+        Game simGame = sim.getSimulatedGameState();
+        Card simSpark = (Card)sim.getGameCopier().find(sparkDouble);
+
+        assert(simSpark != null);
+        assert(simSpark.getZone().is(ZoneType.Battlefield));
+        assert(simSpark.getCounters(CounterType.P1P1) == 1);
+        assert(simSpark.getCounters(CounterType.LOYALTY) == 5);
+    }
+
+
     @SuppressWarnings("unused")
     public void broken_testCloneDimir() {
         Game game = initAndCreateGame();

forge-gui/res/cardsfolder/upcoming/spark_double.txt

@@ -0,0 +1,7 @@
+Name:Spark Double
+ManaCost:3 U
+Types:Creature Shapeshifter
+PT:0/0
+K:ETBReplacement:Copy:DBCopy:Optional
+SVar:DBCopy:DB$ Clone | Choices$ Creature.Other+YouCtrl,Planeswalker.Other+YouCtrl | NonLegendary$ True | PumpKeywords$ etbCounter:P1P1:1:ValidCard$ Creature.Self:CARDNAME enters with an additional +1/+1 counter on it if it’s a creature & etbCounter:LOYALTY:1:ValidCard$ Planeswalker.Self:CARDNAME enters with an additional loyalty counter on it if it’s a planeswalker | SpellDescription$ You may have CARDNAME enter the battlefield as a copy of a creature or planeswalker you control, except it enters with an additional +1/+1 counter on it if it’s a creature, it enters with an additional loyalty counter on it if it’s a planeswalker, and it isn’t legendary if that permanent is legendary.
+Oracle:You may have Spark Double enter the battlefield as a copy of a creature or planeswalker you control, except it enters with an additional +1/+1 counter on it if it’s a creature, it enters with an additional loyalty counter on it if it’s a planeswalker, and it isn’t legendary if that permanent is legendary.