Small improvement to CardScriptParser

forge-ai/src/main/java/forge/ai/ability/PumpAi.java

@@ -596,7 +596,7 @@ public class PumpAi extends PumpAiBase {
         }
 
         return true;
-    } // pumpTgtAI()
+    }
 
     private boolean pumpMandatoryTarget(final Player ai, final SpellAbility sa) {
         final Game game = ai.getGame();
@@ -657,7 +657,7 @@ public class PumpAi extends PumpAiBase {
         }
 
         return true;
-    } // pumpMandatoryTarget()
+    }
 
     @Override
     protected boolean doTriggerAINoCost(Player ai, SpellAbility sa, boolean mandatory) {
@@ -706,7 +706,7 @@ public class PumpAi extends PumpAiBase {
         }
 
         return true;
-    } // pumpTriggerAI
+    }
 
     @Override
     public boolean chkAIDrawback(SpellAbility sa, Player ai) {
@@ -778,7 +778,7 @@ public class PumpAi extends PumpAiBase {
         }
 
         return true;
-    } // pumpDrawbackAI()
+    }
 
     @Override
     public boolean confirmAction(Player player, SpellAbility sa, PlayerActionConfirmMode mode, String message) {

forge-ai/src/main/java/forge/ai/ability/PumpAiBase.java

@@ -428,7 +428,7 @@ public abstract class PumpAiBase extends SpellAbilityAi {
             }
         });
         return list;
-    } // getPumpCreatures()
+    }
 
     /**
      * <p>
@@ -519,7 +519,7 @@ public abstract class PumpAiBase extends SpellAbilityAi {
         }
 
         return list;
-    } // getCurseCreatures()
+    }
 
     protected boolean containsNonCombatKeyword(final List<String> keywords) {
         for (final String keyword : keywords) {

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

@@ -50,7 +50,7 @@ import io.sentry.Sentry;
  */
 public final class AbilityFactory {
 
-    static final List<String> additionalAbilityKeys = Lists.newArrayList(
+    public static final List<String> additionalAbilityKeys = Lists.newArrayList(
             "WinSubAbility", "OtherwiseSubAbility", // Clash
             "BidSubAbility", // BidLifeEffect
             "ChooseNumberSubAbility", "Lowest", "Highest", "NotLowest", // ChooseNumber
@@ -62,7 +62,8 @@ public final class AbilityFactory {
             "FallbackAbility", // Complex Unless costs which can be unpayable
             "ChooseSubAbility", // Can choose a player via ChoosePlayer
             "CantChooseSubAbility", // Can't choose a player via ChoosePlayer
-            "AnimateSubAbility" // For ChangeZone Effects to Animate before ETB
+            "AnimateSubAbility", // For ChangeZone Effects to Animate before ETB
+            "ReturnAbility" // for Delayed Trigger on Magpie
         );
 
     public enum AbilityRecordType {
@@ -79,7 +80,7 @@ public final class AbilityFactory {
             return prefix;
         }
 
-        public SpellAbility buildSpellAbility(ApiType api, Card hostCard, Cost abCost, TargetRestrictions abTgt, Map<String, String> mapParams ) {
+        public SpellAbility buildSpellAbility(ApiType api, Card hostCard, Cost abCost, TargetRestrictions abTgt, Map<String, String> mapParams) {
             switch(this) {
                 case Ability: return new AbilityApiBased(api, hostCard, abCost, abTgt, mapParams);
                 case Spell: return new SpellApiBased(api, hostCard, abCost, abTgt, mapParams);

forge-gui/src/main/java/forge/gui/card/CardScriptParser.java

@@ -14,6 +14,8 @@ import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
 
 import forge.card.CardType;
+import forge.game.ability.AbilityFactory;
+import forge.game.ability.AbilityFactory.AbilityRecordType;
 import forge.game.ability.ApiType;
 import forge.game.replacement.ReplacementType;
 import forge.game.trigger.TriggerType;
@@ -74,6 +76,7 @@ public final class CardScriptParser {
                     bad = true;
                 }
             } else if (trimLine.startsWith("A:")) {
+                // TODO check if it's non-permanent, then Cost$ isn't mandatory
                 result.putAll(getActivatedAbilityErrors(trimLine.substring("A:".length()), index + "A:".length()));
             } else if (trimLine.startsWith("R:")) {
                 result.putAll(getReplacementErrors(trimLine.substring("R:".length()), index + "R:".length()));
@@ -221,7 +224,7 @@ public final class CardScriptParser {
                 if (trimValue.isEmpty()) {
                     isBadValue = true;
                 }
-            } else if (trimKey.equals("SubAbility")) {
+            } else if (trimKey.equals("SubAbility") || AbilityFactory.additionalAbilityKeys.contains(trimKey)) {
                 if (sVars.contains(trimValue)) {
                     sVarAbilities.add(trimValue);
                 } else {
@@ -315,7 +318,10 @@ public final class CardScriptParser {
 
     private static boolean isAbilityApiDeclarerLegal(final String declarer) {
         final String tDeclarer = declarer.trim();
-        return tDeclarer.equals("AB") || tDeclarer.equals("DB") || tDeclarer.equals("SP");
+        for (AbilityRecordType type : AbilityRecordType.values()) {
+            if (type.getPrefix().equals(tDeclarer)) return true;
+        }
+        return false;
     }
     private static boolean isAbilityApiLegal(final String api) {
         try {
@@ -506,7 +512,10 @@ public final class CardScriptParser {
                     "RememberMap", "wasCastFrom", "wasNotCastFrom", "set",
                     "inZone", "HasSVar");
 
-    private static boolean isValidExclusive(final String valid) {
+    private static boolean isValidExclusive(String valid) {
+        if (valid.charAt(0) == '!') {
+            valid = valid.substring(1);
+        }
         if (VALID_EXCLUSIVE.contains(valid)) {
             return true;
         }