From 7c19d7f25a9f6415140895d1fa9757127f2b71f1 Mon Sep 17 00:00:00 2001 From: MostCromulent <201167372+MostCromulent@users.noreply.github.com> Date: Mon, 15 Jun 2026 16:06:57 +0930 Subject: [PATCH 01/14] Add card-script param declaration drift test Fails the build when a framework or effect class reads a card-script parameter it does not declare in OPTIONAL_PARAMS/REQUIRED_PARAMS, or declares one that appears nowhere in its source. The check is opt-in per class, so declarations can be added a few classes at a time. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../CardScriptParamDeclarationTest.java | 256 ++++++++++++++++++ 1 file changed, 256 insertions(+) create mode 100644 forge-game/src/test/java/forge/game/ability/CardScriptParamDeclarationTest.java diff --git a/forge-game/src/test/java/forge/game/ability/CardScriptParamDeclarationTest.java b/forge-game/src/test/java/forge/game/ability/CardScriptParamDeclarationTest.java new file mode 100644 index 000000000000..5bf76beec4c9 --- /dev/null +++ b/forge-game/src/test/java/forge/game/ability/CardScriptParamDeclarationTest.java @@ -0,0 +1,256 @@ +package forge.game.ability; + +import static org.testng.Assert.assertTrue; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Set; +import java.util.TreeSet; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import java.util.stream.Stream; + +import org.testng.annotations.Test; + +/** + * Guards the card-script parameter declarations against drift. A framework or effect class + * that reads card-script parameters declares them in OPTIONAL_PARAMS; effects may also group + * mutually-required params in REQUIRED_PARAMS. This test fails the build if such a class reads + * a parameter it does not declare, or declares one that appears nowhere in its source. + * + * Reads are detected from literal string arguments -- getParam("X"), the defined-resolution + * helpers, map lookups, and so on. A parameter read through a non-literal argument cannot be + * detected, so a passing run means "no undeclared literal read", not a proof of completeness. + * + * The check is opt-in: a class that declares neither field is skipped, so declarations can be + * added a few classes at a time. Once a class declares, removing it from the check requires + * deleting its whole declaration. + */ +public class CardScriptParamDeclarationTest { + + // The literal forms a card-script parameter read takes in source + private static final Pattern GETPARAM = Pattern.compile( + "(? MARKERS = Set.of("AB", "SP", "ST", "DB"); + + // Framework classes that own the shared base layers (their OPTIONAL_PARAMS form the base) + private static final String[] FRAMEWORK = { + "forge-game/src/main/java/forge/game/ability/SpellAbilityEffect.java", + "forge-game/src/main/java/forge/game/ability/AbilityFactory.java", + "forge-game/src/main/java/forge/game/ability/AbilityUtils.java", + "forge-game/src/main/java/forge/game/spellability/SpellAbility.java", + "forge-game/src/main/java/forge/game/spellability/SpellAbilityCondition.java", + "forge-game/src/main/java/forge/game/spellability/SpellAbilityRestriction.java", + "forge-game/src/main/java/forge/game/spellability/TargetRestrictions.java", + "forge-game/src/main/java/forge/game/CardTraitBase.java", + "forge-game/src/main/java/forge/game/cost/Cost.java", + "forge-ai/src/main/java/forge/ai/SpellAbilityAi.java", + }; + + // Cross-cutting readers (they read params owned by effects/triggers, so reads != owned): + // declared but NOT reads-gated. They still contribute their declared params to the base + // and are subject to the structural checks. + private static final Set READ_GATE_EXEMPT = Set.of("SpellAbilityAi"); + + @Test + public void declarationsDoNotDrift() throws IOException { + Path root = locateRoot(); + List errors = new ArrayList<>(); + + Set base = new TreeSet<>(); + for (String rel : FRAMEWORK) { + base.addAll(declared(read(root.resolve(rel)), "OPTIONAL_PARAMS")); + } + + for (String rel : FRAMEWORK) { + Path f = root.resolve(rel); + if (declares(read(f))) { + checkClass(f, base, false, errors); + } + } + + Path effects = root.resolve("forge-game/src/main/java/forge/game/ability/effects"); + try (Stream walk = Files.walk(effects)) { + for (Path p : (Iterable) walk.filter(f -> f.toString().endsWith(".java"))::iterator) { + if (declares(read(p))) { + checkClass(p, base, true, errors); + } + } + } + + assertTrue(errors.isEmpty(), + "Card-script param declarations are out of sync with the code:\n " + String.join("\n ", errors)); + } + + private void checkClass(Path file, Set base, boolean isEffect, List errors) throws IOException { + String src = read(file); + String name = file.getFileName().toString().replace(".java", ""); + Set optional = declared(src, "OPTIONAL_PARAMS"); + List> requiredGroups = requiredGroups(src); + Set requiredFlat = new TreeSet<>(); + requiredGroups.forEach(requiredFlat::addAll); + Set own = new TreeSet<>(optional); + own.addAll(requiredFlat); + + // (1) every param the class reads must be declared (own) or inherited (base) + if (!READ_GATE_EXEMPT.contains(name)) { + Set allowed = new TreeSet<>(own); + allowed.addAll(base); + for (String p : reads(src)) { + if (!allowed.contains(p)) { + errors.add(name + ": reads '" + p + "$' but it is not declared (add to OPTIONAL_PARAMS)"); + } + } + } + // (2) every declared param must appear as a literal somewhere in the source (no typos) + Set literals = literals(src); + for (String p : own) { + if (!literals.contains(p)) { + errors.add(name + ": declares '" + p + "$' but it appears nowhere in the source (typo?)"); + } + } + // (3) effects must not re-declare base params + if (isEffect) { + for (String p : own) { + if (base.contains(p)) { + errors.add(name + ": re-declares base param '" + p + "$' (it is already inherited)"); + } + } + } + // (4) required and optional are disjoint + for (String p : requiredFlat) { + if (optional.contains(p)) { + errors.add(name + ": '" + p + "$' is in both REQUIRED_PARAMS and OPTIONAL_PARAMS"); + } + } + // (5) no duplicate optional entries + List rawOpt = declaredList(src, "OPTIONAL_PARAMS"); + if (rawOpt.size() != new LinkedHashSet<>(rawOpt).size()) { + errors.add(name + ": OPTIONAL_PARAMS has duplicate entries"); + } + // (6) required groups are non-empty + for (Set g : requiredGroups) { + if (g.isEmpty()) { + errors.add(name + ": REQUIRED_PARAMS has an empty one-of group"); + } + } + // (7) every declared entry has a valid param-token shape + for (String p : own) { + if (!p.matches("[A-Za-z][A-Za-z0-9]*")) { + errors.add(name + ": '" + p + "' is not a valid param token"); + } + } + } + + private boolean declares(String src) { + return src.contains("OPTIONAL_PARAMS") || src.contains("REQUIRED_PARAMS"); + } + + private Set declared(String src, String field) { + return new TreeSet<>(declaredList(src, field)); + } + + private List declaredList(String src, String field) { + List out = new ArrayList<>(); + Matcher decl = Pattern.compile(field + "\\s*=\\s*\\{").matcher(src); + if (!decl.find()) { + return out; + } + int open = decl.end() - 1; + int close = src.indexOf("};", open); + if (close < 0) { + return out; + } + Matcher m = STRING_LITERAL.matcher(src.substring(open, close)); + while (m.find()) { + out.add(m.group(1)); + } + return out; + } + + /** REQUIRED_PARAMS is String[][]; return one set per inner { } group. */ + private List> requiredGroups(String src) { + List> groups = new ArrayList<>(); + Matcher decl = Pattern.compile("REQUIRED_PARAMS\\s*=\\s*\\{").matcher(src); + if (!decl.find()) { + return groups; + } + int open = decl.end() - 1; + int close = src.indexOf("};", open); + if (close < 0) { + return groups; + } + Matcher inner = Pattern.compile("\\{([^{}]*)\\}").matcher(src.substring(open + 1, close)); + while (inner.find()) { + Set g = new TreeSet<>(); + Matcher m = STRING_LITERAL.matcher(inner.group(1)); + while (m.find()) { + g.add(m.group(1)); + } + groups.add(g); + } + return groups; + } + + private Set reads(String src) { + Set out = new TreeSet<>(); + collect(GETPARAM, src, out); + collect(DEFINED, src, out); + collect(MAPGET, src, out); + collect(KEYVAR, src, out); + Matcher c = ADD_TO_COMBAT.matcher(src); + while (c.find()) { + Matcher m = STRING_LITERAL.matcher(c.group(1)); + while (m.find()) { + out.add(m.group(1)); + } + } + out.removeIf(p -> MARKERS.contains(p) || !p.matches("[A-Za-z][A-Za-z0-9]*")); + return out; + } + + private Set literals(String src) { + Set out = new TreeSet<>(); + collect(STRING_LITERAL, src, out); + return out; + } + + private void collect(Pattern p, String src, Set out) { + Matcher m = p.matcher(src); + while (m.find()) { + out.add(m.group(1)); + } + } + + private static Path locateRoot() { + Path d = Paths.get("").toAbsolutePath(); + for (int i = 0; i < 8 && d != null; i++) { + if (Files.isDirectory(d.resolve("forge-game/src/main/java"))) { + return d; + } + d = d.getParent(); + } + throw new IllegalStateException("could not locate repo root from " + Paths.get("").toAbsolutePath()); + } + + private static String read(Path p) throws IOException { + return new String(Files.readAllBytes(p), StandardCharsets.UTF_8); + } +} From b9cdcea05101a4239c7e4cd4b74c3019d85bf832 Mon Sep 17 00:00:00 2001 From: MostCromulent <201167372+MostCromulent@users.noreply.github.com> Date: Mon, 15 Jun 2026 16:06:57 +0930 Subject: [PATCH 02/14] Declare card-script params in SpellAbilityEffect Co-Authored-By: Claude Opus 4.8 (1M context) --- .../java/forge/game/ability/SpellAbilityEffect.java | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/forge-game/src/main/java/forge/game/ability/SpellAbilityEffect.java b/forge-game/src/main/java/forge/game/ability/SpellAbilityEffect.java index a364a67c8783..a52f8c526edb 100644 --- a/forge-game/src/main/java/forge/game/ability/SpellAbilityEffect.java +++ b/forge-game/src/main/java/forge/game/ability/SpellAbilityEffect.java @@ -41,6 +41,19 @@ */ public abstract class SpellAbilityEffect { + // Card-script parameters read by the framework, declared here so the set each class + // consumes is explicit and can be checked against the card scripts. Each framework class + // lists the shared (optional) base params it consumes; ability effects additionally + // declare REQUIRED_PARAMS (one-of groups) on top. + public static final String[] OPTIONAL_PARAMS = { + "AfterDescription", "Amount", "Announce", "AtEOTCondition", "AtEOTDesc", + "ConditionDescription", "Defined", "DefinedExiler", "Duration", + "ExiledWithEffectSource", "ForEach", "Forecast", "GiftDescription", + "IncludeAllComponentCards", "Named", "ReplaceDyingCondition", "ReplaceDyingDefined", + "ReplaceDyingExiledWith", "ReplaceDyingValid", "ReplaceDyingZone", "ReturnAbility", + "ReturnValid", "SpellDescription", "StackDescription", "StartingWith", + "ThisDefinedAndTgts", + }; public abstract void resolve(SpellAbility sa); From 92e56a95208d0aaf04adc3a24c3dbbbc01db02a4 Mon Sep 17 00:00:00 2001 From: MostCromulent <201167372+MostCromulent@users.noreply.github.com> Date: Mon, 15 Jun 2026 16:06:57 +0930 Subject: [PATCH 03/14] Declare card-script params in AbilityFactory Co-Authored-By: Claude Opus 4.8 (1M context) --- .../main/java/forge/game/ability/AbilityFactory.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/forge-game/src/main/java/forge/game/ability/AbilityFactory.java b/forge-game/src/main/java/forge/game/ability/AbilityFactory.java index 978fa0db4af7..aadb7b4bbcc8 100644 --- a/forge-game/src/main/java/forge/game/ability/AbilityFactory.java +++ b/forge-game/src/main/java/forge/game/ability/AbilityFactory.java @@ -46,6 +46,17 @@ * @version $Id$ */ public final class AbilityFactory { + public static final String[] OPTIONAL_PARAMS = { + "BidSubAbility", "CantChooseSubAbility", "Choices", "ChooseNumberSubAbility", + "ChooseSubAbility", "ChosenPile", "Cost", "Execute", "FallbackAbility", + "FalseSubAbility", "GiftAbility", "GuessCorrect", "GuessWrong", "HeadsSubAbility", + "Highest", "LoseSubAbility", "Lowest", "MatchedAbility", "NonBasicSpell", + "NotLowest", "Origin", "OtherwiseSubAbility", "PreventionSubAbility", + "RegenerationAbility", "RepeatSubAbility", "ResultSubAbilities", "ReturnAbility", + "SpellDescription", "SubAbility", "TailsSubAbility", "TrueSubAbility", + "UnchosenPile", "UnmatchedAbility", "ValidTgts", "VoteSubAbility", + "VoteTiedAbility", "WinSubAbility", + }; public static final List additionalAbilityKeys = Lists.newArrayList( "WinSubAbility", "OtherwiseSubAbility", // Clash From 9f26bbef175dc62e8cfbac32ea4f448901727df7 Mon Sep 17 00:00:00 2001 From: MostCromulent <201167372+MostCromulent@users.noreply.github.com> Date: Mon, 15 Jun 2026 16:06:57 +0930 Subject: [PATCH 04/14] Declare card-script params in AbilityUtils Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/main/java/forge/game/ability/AbilityUtils.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/forge-game/src/main/java/forge/game/ability/AbilityUtils.java b/forge-game/src/main/java/forge/game/ability/AbilityUtils.java index 780e3afb037b..6c09d0cdfb8c 100644 --- a/forge-game/src/main/java/forge/game/ability/AbilityUtils.java +++ b/forge-game/src/main/java/forge/game/ability/AbilityUtils.java @@ -50,6 +50,13 @@ import java.util.stream.IntStream; public class AbilityUtils { + public static final String[] OPTIONAL_PARAMS = { + "AbilityCount", "AnnounceMax", "Destination", "ETB", "ForgetOtherTargets", + "IncludeAllComponentCards", "LockInText", "RememberCostMana", "RememberTargets", + "Triggered", "UnlessColor", "UnlessCost", "UnlessPayer", "UnlessResolveSubs", + "UnlessSwitched", "UnlessUpTo", "XMax", "XMin", + }; + private final static ImmutableList cmpList = ImmutableList.of("LT", "LE", "EQ", "GE", "GT", "NE"); // should the three getDefined functions be merged into one? Or better to From 325176448eb0207126dd6bf084554e5ccba3c97e Mon Sep 17 00:00:00 2001 From: MostCromulent <201167372+MostCromulent@users.noreply.github.com> Date: Mon, 15 Jun 2026 16:06:58 +0930 Subject: [PATCH 05/14] Declare card-script params in SpellAbility Co-Authored-By: Claude Opus 4.8 (1M context) --- .../java/forge/game/spellability/SpellAbility.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/forge-game/src/main/java/forge/game/spellability/SpellAbility.java b/forge-game/src/main/java/forge/game/spellability/SpellAbility.java index 48bc00b702ea..22221089ddbf 100644 --- a/forge-game/src/main/java/forge/game/spellability/SpellAbility.java +++ b/forge-game/src/main/java/forge/game/spellability/SpellAbility.java @@ -74,6 +74,18 @@ * @version $Id$ */ public abstract class SpellAbility extends CardTraitBase implements ISpellAbility, IIdentifiable, Comparable { + public static final String[] OPTIONAL_PARAMS = { + "AlternateCost", "Amount", "Announce", "Boast", "CantCopy", "CloakUp", "CostDesc", + "CumulativeUpkeep", "DisguiseUp", "DividedAsYouChoose", "Exhaust", "Hidden", + "IsCurse", "ManaRestriction", "ManifestUp", "MaxTotalTargetCMC", + "MaxTotalTargetPower", "MorphUp", "Origin", "Planeswalker", "PowerUp", + "PrecostDesc", "SpellDescription", "TargetType", "TargetingPlayer", + "TargetingPlayerControls", "TargetsWithControllerProperty", + "TargetsWithDefinedController", "TargetsWithRelatedProperty", + "TargetsWithSharedCardType", "TargetsWithSharedTypes", "Unlock", "ValidAfterStack", + "WithoutManaCost", "XColor", + }; + private static int maxId = 0; private static int nextId() { return ++maxId; } From 82dcf10e584f459f69cd9d112d2b2b28cb817563 Mon Sep 17 00:00:00 2001 From: MostCromulent <201167372+MostCromulent@users.noreply.github.com> Date: Mon, 15 Jun 2026 16:06:58 +0930 Subject: [PATCH 06/14] Declare card-script params in SpellAbilityCondition Co-Authored-By: Claude Opus 4.8 (1M context) --- .../game/spellability/SpellAbilityCondition.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/forge-game/src/main/java/forge/game/spellability/SpellAbilityCondition.java b/forge-game/src/main/java/forge/game/spellability/SpellAbilityCondition.java index 5cd2c3e329e5..f08129d9ae05 100644 --- a/forge-game/src/main/java/forge/game/spellability/SpellAbilityCondition.java +++ b/forge-game/src/main/java/forge/game/spellability/SpellAbilityCondition.java @@ -46,6 +46,20 @@ * @since 1.0.15 */ public class SpellAbilityCondition extends SpellAbilityVariables { + public static final String[] OPTIONAL_PARAMS = { + "Condition", "ConditionActivationLimit", "ConditionCheckSVar", + "ConditionChosenColor", "ConditionCompare", "ConditionCompare2", "ConditionDefined", + "ConditionDefined2", "ConditionFirstCombat", "ConditionGameTypes", + "ConditionLifeAmount", "ConditionLifeTotal", "ConditionManaNotSpent", + "ConditionManaSpent", "ConditionNoDifferentColors", "ConditionNotPresent", + "ConditionOpponentTurn", "ConditionOptionalPaid", "ConditionPhases", + "ConditionPlayerContains", "ConditionPlayerDefined", "ConditionPlayerTurn", + "ConditionPresent", "ConditionPresent2", "ConditionSVarCompare", + "ConditionSorcerySpeed", "ConditionTargetValidTargeting", + "ConditionTargetsSingleTarget", "ConditionZone", "OrConditionCheckSVar", + "OrOtherConditionSVarCompare", + }; + // A class for handling SpellAbility Conditions. These restrictions include: // Zone, Phase, OwnTurn, Speed (instant/sorcery), Amount per Turn, Player, // Threshold, Metalcraft, LevelRange, etc From 55b148fec2e47b68c4f989b8477b4c5372773fe2 Mon Sep 17 00:00:00 2001 From: MostCromulent <201167372+MostCromulent@users.noreply.github.com> Date: Mon, 15 Jun 2026 16:06:58 +0930 Subject: [PATCH 07/14] Declare card-script params in SpellAbilityRestriction Co-Authored-By: Claude Opus 4.8 (1M context) --- .../game/spellability/SpellAbilityRestriction.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/forge-game/src/main/java/forge/game/spellability/SpellAbilityRestriction.java b/forge-game/src/main/java/forge/game/spellability/SpellAbilityRestriction.java index 21493f18e2a8..7899bcec685e 100644 --- a/forge-game/src/main/java/forge/game/spellability/SpellAbilityRestriction.java +++ b/forge-game/src/main/java/forge/game/spellability/SpellAbilityRestriction.java @@ -50,6 +50,16 @@ * @version $Id$ */ public class SpellAbilityRestriction extends SpellAbilityVariables { + public static final String[] OPTIONAL_PARAMS = { + "Activation", "ActivationAfterBlockers", "ActivationFirstCombat", + "ActivationGameTypes", "ActivationLifeAmount", "ActivationLifeTotal", + "ActivationLimit", "ActivationPhases", "ActivationZone", "Activator", + "AdditionalActivationZone", "Affected", "CheckSVar", "ClassLevel", + "GameActivationLimit", "InstantSpeed", "IsPresent", "OpponentTurn", "PlayerTurn", + "PresentCompare", "PresentDefined", "PresentZone", "SVarCompare", "SorcerySpeed", + "ValidSA", + }; + // A class for handling SpellAbility Restrictions. These restrictions include: // Zone, Phase, OwnTurn, Speed (instant/sorcery), Amount per Turn, Player, // Threshold, Metalcraft, LevelRange, etc From 0ac88f958a11b384b4c7cb5d908c9cd77df3cace Mon Sep 17 00:00:00 2001 From: MostCromulent <201167372+MostCromulent@users.noreply.github.com> Date: Mon, 15 Jun 2026 16:06:58 +0930 Subject: [PATCH 08/14] Declare card-script params in TargetRestrictions Co-Authored-By: Claude Opus 4.8 (1M context) --- .../forge/game/spellability/TargetRestrictions.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/forge-game/src/main/java/forge/game/spellability/TargetRestrictions.java b/forge-game/src/main/java/forge/game/spellability/TargetRestrictions.java index 943bcc4006c7..a5e4d0255ad5 100644 --- a/forge-game/src/main/java/forge/game/spellability/TargetRestrictions.java +++ b/forge-game/src/main/java/forge/game/spellability/TargetRestrictions.java @@ -43,6 +43,16 @@ * @version $Id$ */ public class TargetRestrictions { + public static final String[] OPTIONAL_PARAMS = { + "MaxTotalTargetCMC", "MaxTotalTargetPower", "RandomNumTargets", "TargetMax", + "TargetMin", "TargetUnique", "TargetValidTargeting", "TargetingPlayer", + "TargetsAtRandom", "TargetsForEachPlayer", "TargetsWithDifferentCMC", + "TargetsWithDifferentControllers", "TargetsWithDifferentNames", + "TargetsWithEqualToughness", "TargetsWithSameCardType", "TargetsWithSameController", + "TargetsWithSameCreatureType", "TargetsWithoutSameCreatureType", "TgtPrompt", + "TgtZone", "ValidTgts", "ValidTgtsDesc", + }; + // Target has two things happening: // Targeting restrictions (Creature, Min/Maxm etc) which are true for this // What this Object is restricted to targeting From f28ed9f105c6c5849697865bc3fd6b0c4bb6b811 Mon Sep 17 00:00:00 2001 From: MostCromulent <201167372+MostCromulent@users.noreply.github.com> Date: Mon, 15 Jun 2026 16:06:58 +0930 Subject: [PATCH 09/14] Declare card-script params in CardTraitBase Co-Authored-By: Claude Opus 4.8 (1M context) --- forge-game/src/main/java/forge/game/CardTraitBase.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/forge-game/src/main/java/forge/game/CardTraitBase.java b/forge-game/src/main/java/forge/game/CardTraitBase.java index 2bf2caed7e9b..2b416a8e5cf4 100644 --- a/forge-game/src/main/java/forge/game/CardTraitBase.java +++ b/forge-game/src/main/java/forge/game/CardTraitBase.java @@ -37,6 +37,16 @@ * */ public abstract class CardTraitBase implements GameObject, IHasCardView, IHasSVars { + public static final String[] OPTIONAL_PARAMS = { + "Adamant", "Blessing", "Bloodthirst", "CheckDefinedPlayer", "CheckSVar", + "CheckSecondSVar", "ClassLevel", "DayTime", "DefinedPlayerCompare", "Delirium", + "Desert", "FatefulHour", "Hellbent", "Invert", "IsPresent", "IsPresent2", + "LifeAmount", "LifeTotal", "ManaNotSpent", "ManaSpent", "Metalcraft", "Monarch", + "PresentCompare", "PresentCompare2", "PresentDefined", "PresentPlayer", + "PresentPlayer2", "PresentZone", "PresentZone2", "Revolt", "SVarCompare", + "SecondSVarCompare", "Secondary", "Threshold", "WerewolfTransformCondition", + "WerewolfUntransformCondition", + }; /** The host card. */ protected Card hostCard; From 0807d698b4a42953b9a4ce127538a629525a5b5d Mon Sep 17 00:00:00 2001 From: MostCromulent <201167372+MostCromulent@users.noreply.github.com> Date: Mon, 15 Jun 2026 16:06:58 +0930 Subject: [PATCH 10/14] Declare card-script params in Cost Co-Authored-By: Claude Opus 4.8 (1M context) --- forge-game/src/main/java/forge/game/cost/Cost.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/forge-game/src/main/java/forge/game/cost/Cost.java b/forge-game/src/main/java/forge/game/cost/Cost.java index b138995abd01..17e7db25fb32 100644 --- a/forge-game/src/main/java/forge/game/cost/Cost.java +++ b/forge-game/src/main/java/forge/game/cost/Cost.java @@ -47,6 +47,15 @@ * @version $Id$ */ public class Cost implements Serializable { + public static final String[] OPTIONAL_PARAMS = { + "AffectedZone", "Amount", "Announce", "Collected", "CollectedCards", "Color", + "Cost", "Exiled", "ExiledCards", "FirstForetell", "ForEachShard", "Foraged", + "ForagedCards", "IgnoreGeneric", "MinMana", "ModeCost", "OnlyFirstSpell", + "RaiseCost", "RaiseTo", "ReduceAmount", "ReduceCost", "Relative", + "SpellDescription", "TapCreaturesForMana", "Type", "UnlessValidTarget", "UpTo", + "ValidCard", "ValidSpell", "ValidTarget", + }; + /** * Serializables need a version ID. */ From f26a3229896187430ca07e824a1ef49d47b7c745 Mon Sep 17 00:00:00 2001 From: MostCromulent <201167372+MostCromulent@users.noreply.github.com> Date: Mon, 15 Jun 2026 16:06:59 +0930 Subject: [PATCH 11/14] Declare card-script params in SpellAbilityAi Co-Authored-By: Claude Opus 4.8 (1M context) --- forge-ai/src/main/java/forge/ai/SpellAbilityAi.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/forge-ai/src/main/java/forge/ai/SpellAbilityAi.java b/forge-ai/src/main/java/forge/ai/SpellAbilityAi.java index d2af9e0d1ce6..02fa3de8a479 100644 --- a/forge-ai/src/main/java/forge/ai/SpellAbilityAi.java +++ b/forge-ai/src/main/java/forge/ai/SpellAbilityAi.java @@ -37,6 +37,11 @@ * The three main methods are canPlayAI(), chkAIDrawback and doTriggerAINoCost. */ public abstract class SpellAbilityAi { + public static final String[] OPTIONAL_PARAMS = { + "AIActivateLast", "AIBidMax", "AICheckSVar", "AILifeThreshold", "AILogic", + "AIManaPref", "AIMaxTgtsCount", "AIPhyrexianPayment", "AIRespondsToOwnAbility", + "AISVarCompare", "AITgts", "AITgtsStrict", "AIXMax", "UnlessAI", + }; public Predicate CREATURE_OR_TAP_ABILITY = c -> { if (c.isCreature()) { From c97dff585f60dbb57665c4e7bb838d5618f104c6 Mon Sep 17 00:00:00 2001 From: MostCromulent <201167372+MostCromulent@users.noreply.github.com> Date: Mon, 15 Jun 2026 17:33:52 +0930 Subject: [PATCH 12/14] Remove redundant param-declaration comment from SpellAbilityEffect Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/main/java/forge/game/ability/SpellAbilityEffect.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/forge-game/src/main/java/forge/game/ability/SpellAbilityEffect.java b/forge-game/src/main/java/forge/game/ability/SpellAbilityEffect.java index a52f8c526edb..356910c5ff24 100644 --- a/forge-game/src/main/java/forge/game/ability/SpellAbilityEffect.java +++ b/forge-game/src/main/java/forge/game/ability/SpellAbilityEffect.java @@ -41,10 +41,6 @@ */ public abstract class SpellAbilityEffect { - // Card-script parameters read by the framework, declared here so the set each class - // consumes is explicit and can be checked against the card scripts. Each framework class - // lists the shared (optional) base params it consumes; ability effects additionally - // declare REQUIRED_PARAMS (one-of groups) on top. public static final String[] OPTIONAL_PARAMS = { "AfterDescription", "Amount", "Announce", "AtEOTCondition", "AtEOTDesc", "ConditionDescription", "Defined", "DefinedExiler", "Duration", From a2e244bf18609529bd05633bc6689dbcdcc66454 Mon Sep 17 00:00:00 2001 From: MostCromulent <201167372+MostCromulent@users.noreply.github.com> Date: Mon, 15 Jun 2026 18:09:36 +0930 Subject: [PATCH 13/14] Discover param declarers by walking the source tree Replace the hardcoded list of framework file paths with a walk over the module source roots, classifying each declarer as base or effect by location. Discovery keys on the field assignment (shared with the parse helpers so they can't drift), tolerating an explicit "new String[]" initializer, and a guard fails loudly if the walk finds nothing rather than passing vacuously. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../CardScriptParamDeclarationTest.java | 74 +++++++++++-------- 1 file changed, 45 insertions(+), 29 deletions(-) diff --git a/forge-game/src/test/java/forge/game/ability/CardScriptParamDeclarationTest.java b/forge-game/src/test/java/forge/game/ability/CardScriptParamDeclarationTest.java index 5bf76beec4c9..63b657615cc2 100644 --- a/forge-game/src/test/java/forge/game/ability/CardScriptParamDeclarationTest.java +++ b/forge-game/src/test/java/forge/game/ability/CardScriptParamDeclarationTest.java @@ -50,20 +50,22 @@ public class CardScriptParamDeclarationTest { // The ability declarator prefixes are not params private static final Set MARKERS = Set.of("AB", "SP", "ST", "DB"); - // Framework classes that own the shared base layers (their OPTIONAL_PARAMS form the base) - private static final String[] FRAMEWORK = { - "forge-game/src/main/java/forge/game/ability/SpellAbilityEffect.java", - "forge-game/src/main/java/forge/game/ability/AbilityFactory.java", - "forge-game/src/main/java/forge/game/ability/AbilityUtils.java", - "forge-game/src/main/java/forge/game/spellability/SpellAbility.java", - "forge-game/src/main/java/forge/game/spellability/SpellAbilityCondition.java", - "forge-game/src/main/java/forge/game/spellability/SpellAbilityRestriction.java", - "forge-game/src/main/java/forge/game/spellability/TargetRestrictions.java", - "forge-game/src/main/java/forge/game/CardTraitBase.java", - "forge-game/src/main/java/forge/game/cost/Cost.java", - "forge-ai/src/main/java/forge/ai/SpellAbilityAi.java", + // Module source roots walked to find declaring classes -- no per-file path list to maintain. + private static final String[] SOURCE_ROOTS = { + "forge-game/src/main/java", + "forge-ai/src/main/java", }; + // Effects own their own params; everything else that declares forms the shared base layer. + private static final String EFFECTS_DIR = "/ability/effects/"; + + // Assignment of an array initializer to a field -- tolerates an explicit "= new String[]{". + // Shared by discovery and parsing so the two can't drift apart. + private static final String ASSIGN = "\\s*=\\s*[^;]*?\\{"; + + // A class declares params iff one of these fields is assigned an array initializer. + private static final Pattern DECLARES = Pattern.compile("(?:OPTIONAL_PARAMS|REQUIRED_PARAMS)" + ASSIGN); + // Cross-cutting readers (they read params owned by effects/triggers, so reads != owned): // declared but NOT reads-gated. They still contribute their declared params to the base // and are subject to the structural checks. @@ -74,25 +76,35 @@ public void declarationsDoNotDrift() throws IOException { Path root = locateRoot(); List errors = new ArrayList<>(); - Set base = new TreeSet<>(); - for (String rel : FRAMEWORK) { - base.addAll(declared(read(root.resolve(rel)), "OPTIONAL_PARAMS")); + // Discover every declaring class by walking the source tree. + List declarers = new ArrayList<>(); + for (String rel : SOURCE_ROOTS) { + Path src = root.resolve(rel); + if (!Files.isDirectory(src)) { + continue; + } + try (Stream walk = Files.walk(src)) { + for (Path p : (Iterable) walk.filter(f -> f.toString().endsWith(".java"))::iterator) { + if (declares(read(p))) { + declarers.add(p); + } + } + } } - for (String rel : FRAMEWORK) { - Path f = root.resolve(rel); - if (declares(read(f))) { - checkClass(f, base, false, errors); + // Guard against a broken walk passing vacuously: the framework classes always declare. + assertTrue(!declarers.isEmpty(), "no param-declaring classes found -- source-tree discovery is broken"); + + // Base = declarers that aren't effects; their optional params are inherited by every effect. + Set base = new TreeSet<>(); + for (Path f : declarers) { + if (!isEffect(f)) { + base.addAll(declared(read(f), "OPTIONAL_PARAMS")); } } - Path effects = root.resolve("forge-game/src/main/java/forge/game/ability/effects"); - try (Stream walk = Files.walk(effects)) { - for (Path p : (Iterable) walk.filter(f -> f.toString().endsWith(".java"))::iterator) { - if (declares(read(p))) { - checkClass(p, base, true, errors); - } - } + for (Path f : declarers) { + checkClass(f, base, isEffect(f), errors); } assertTrue(errors.isEmpty(), @@ -160,7 +172,11 @@ private void checkClass(Path file, Set base, boolean isEffect, List declared(String src, String field) { @@ -169,7 +185,7 @@ private Set declared(String src, String field) { private List declaredList(String src, String field) { List out = new ArrayList<>(); - Matcher decl = Pattern.compile(field + "\\s*=\\s*\\{").matcher(src); + Matcher decl = Pattern.compile(field + ASSIGN).matcher(src); if (!decl.find()) { return out; } @@ -188,7 +204,7 @@ private List declaredList(String src, String field) { /** REQUIRED_PARAMS is String[][]; return one set per inner { } group. */ private List> requiredGroups(String src) { List> groups = new ArrayList<>(); - Matcher decl = Pattern.compile("REQUIRED_PARAMS\\s*=\\s*\\{").matcher(src); + Matcher decl = Pattern.compile("REQUIRED_PARAMS" + ASSIGN).matcher(src); if (!decl.find()) { return groups; } From a5fffaf32bdbe9a4074323f2bf256e0f20d288eb Mon Sep 17 00:00:00 2001 From: MostCromulent <201167372+MostCromulent@users.noreply.github.com> Date: Mon, 15 Jun 2026 20:00:04 +0930 Subject: [PATCH 14/14] Declare card-script params through the IHasForgeParams interface Implements the IHasForgeParams interface suggested in review: a class that reads card-script params implements it and lists them in its OPTIONAL_PARAMS field (effects also a REQUIRED_PARAMS), so the upcoming card-script linter can ask a class which params it accepts. getOptionalParams/getRequiredParams are default methods that read the runtime class's field via getClass().getField rather than returning it directly. Because the field is static, a direct return on a base class would hand subclasses the base's field; reading getClass()'s field gives each its own layer (or the inherited one when it declares none), so declarers need no per-class override and there is no wrong-layer hazard. The drift test discovers declarers by scanning the classpath for implementors instead of a hardcoded path list. forge-ai classes are off the forge-game test classpath (the dependency runs forge-ai -> forge-game), so its declarers are found by walking source; discovery fails loudly if it comes up empty or misses a known framework class. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../main/java/forge/ai/SpellAbilityAi.java | 3 +- .../main/java/forge/game/CardTraitBase.java | 3 +- .../forge/game/ability/AbilityFactory.java | 2 +- .../java/forge/game/ability/AbilityUtils.java | 2 +- .../forge/game/ability/IHasForgeParams.java | 31 +++++ .../game/ability/SpellAbilityEffect.java | 2 +- .../src/main/java/forge/game/cost/Cost.java | 3 +- .../spellability/SpellAbilityCondition.java | 3 +- .../spellability/SpellAbilityRestriction.java | 3 +- .../game/spellability/TargetRestrictions.java | 3 +- .../CardScriptParamDeclarationTest.java | 111 +++++++++++++----- 11 files changed, 128 insertions(+), 38 deletions(-) create mode 100644 forge-game/src/main/java/forge/game/ability/IHasForgeParams.java diff --git a/forge-ai/src/main/java/forge/ai/SpellAbilityAi.java b/forge-ai/src/main/java/forge/ai/SpellAbilityAi.java index 02fa3de8a479..5677b47dfcd8 100644 --- a/forge-ai/src/main/java/forge/ai/SpellAbilityAi.java +++ b/forge-ai/src/main/java/forge/ai/SpellAbilityAi.java @@ -11,6 +11,7 @@ import forge.card.ICardFace; import forge.card.mana.ManaCost; import forge.game.GameEntity; +import forge.game.ability.IHasForgeParams; import forge.game.card.Card; import forge.game.card.CardCopyService; import forge.game.card.CardState; @@ -36,7 +37,7 @@ *

* The three main methods are canPlayAI(), chkAIDrawback and doTriggerAINoCost. */ -public abstract class SpellAbilityAi { +public abstract class SpellAbilityAi implements IHasForgeParams { public static final String[] OPTIONAL_PARAMS = { "AIActivateLast", "AIBidMax", "AICheckSVar", "AILifeThreshold", "AILogic", "AIManaPref", "AIMaxTgtsCount", "AIPhyrexianPayment", "AIRespondsToOwnAbility", diff --git a/forge-game/src/main/java/forge/game/CardTraitBase.java b/forge-game/src/main/java/forge/game/CardTraitBase.java index 2b416a8e5cf4..d6ce13851a92 100644 --- a/forge-game/src/main/java/forge/game/CardTraitBase.java +++ b/forge-game/src/main/java/forge/game/CardTraitBase.java @@ -15,6 +15,7 @@ import forge.card.MagicColor; import forge.card.mana.ManaAtom; import forge.game.ability.AbilityUtils; +import forge.game.ability.IHasForgeParams; import forge.game.card.Card; import forge.game.card.CardCollection; import forge.game.card.CardLists; @@ -36,7 +37,7 @@ * Base class for Triggers,ReplacementEffects and StaticAbilities. * */ -public abstract class CardTraitBase implements GameObject, IHasCardView, IHasSVars { +public abstract class CardTraitBase implements GameObject, IHasCardView, IHasSVars, IHasForgeParams { public static final String[] OPTIONAL_PARAMS = { "Adamant", "Blessing", "Bloodthirst", "CheckDefinedPlayer", "CheckSVar", "CheckSecondSVar", "ClassLevel", "DayTime", "DefinedPlayerCompare", "Delirium", diff --git a/forge-game/src/main/java/forge/game/ability/AbilityFactory.java b/forge-game/src/main/java/forge/game/ability/AbilityFactory.java index aadb7b4bbcc8..50a2d6bde31f 100644 --- a/forge-game/src/main/java/forge/game/ability/AbilityFactory.java +++ b/forge-game/src/main/java/forge/game/ability/AbilityFactory.java @@ -45,7 +45,7 @@ * @author Forge * @version $Id$ */ -public final class AbilityFactory { +public final class AbilityFactory implements IHasForgeParams { public static final String[] OPTIONAL_PARAMS = { "BidSubAbility", "CantChooseSubAbility", "Choices", "ChooseNumberSubAbility", "ChooseSubAbility", "ChosenPile", "Cost", "Execute", "FallbackAbility", diff --git a/forge-game/src/main/java/forge/game/ability/AbilityUtils.java b/forge-game/src/main/java/forge/game/ability/AbilityUtils.java index 6c09d0cdfb8c..dfbc01b4efe6 100644 --- a/forge-game/src/main/java/forge/game/ability/AbilityUtils.java +++ b/forge-game/src/main/java/forge/game/ability/AbilityUtils.java @@ -49,7 +49,7 @@ import java.util.stream.Collectors; import java.util.stream.IntStream; -public class AbilityUtils { +public class AbilityUtils implements IHasForgeParams { public static final String[] OPTIONAL_PARAMS = { "AbilityCount", "AnnounceMax", "Destination", "ETB", "ForgetOtherTargets", "IncludeAllComponentCards", "LockInText", "RememberCostMana", "RememberTargets", diff --git a/forge-game/src/main/java/forge/game/ability/IHasForgeParams.java b/forge-game/src/main/java/forge/game/ability/IHasForgeParams.java new file mode 100644 index 000000000000..0ac7051a9281 --- /dev/null +++ b/forge-game/src/main/java/forge/game/ability/IHasForgeParams.java @@ -0,0 +1,31 @@ +package forge.game.ability; + +/** + * Implemented by classes that declare the card-script parameters they consume: the optional params + * in a {@code public static final String[] OPTIONAL_PARAMS} field, and (for effects) mutually-required + * groups in a {@code String[][] REQUIRED_PARAMS} field. The accessors expose those fields so the + * card-script linter can ask a class which params it accepts; CardScriptParamDeclarationTest discovers + * implementors by classpath scan and guards the declarations against drift. + * + * The accessors read the field of the runtime class ({@code getClass()}) rather than returning a field + * directly. Because the fields are {@code static}, a direct {@code return OPTIONAL_PARAMS} inherited by a + * subclass would return the superclass's field; reading {@code getClass()}'s field returns the subclass's + * own declaration (or the inherited one when it declares none), so no per-class override is needed. + */ +public interface IHasForgeParams { + default String[] getOptionalParams() { + try { + return (String[]) getClass().getField("OPTIONAL_PARAMS").get(null); + } catch (NoSuchFieldException | IllegalAccessException e) { + return new String[0]; + } + } + + default String[][] getRequiredParams() { + try { + return (String[][]) getClass().getField("REQUIRED_PARAMS").get(null); + } catch (NoSuchFieldException | IllegalAccessException e) { + return new String[0][]; + } + } +} diff --git a/forge-game/src/main/java/forge/game/ability/SpellAbilityEffect.java b/forge-game/src/main/java/forge/game/ability/SpellAbilityEffect.java index 356910c5ff24..198fd3778b7a 100644 --- a/forge-game/src/main/java/forge/game/ability/SpellAbilityEffect.java +++ b/forge-game/src/main/java/forge/game/ability/SpellAbilityEffect.java @@ -40,7 +40,7 @@ * @version $Id: AbilityFactoryAlterLife.java 17656 2012-10-22 19:32:56Z Max mtg $ */ -public abstract class SpellAbilityEffect { +public abstract class SpellAbilityEffect implements IHasForgeParams { public static final String[] OPTIONAL_PARAMS = { "AfterDescription", "Amount", "Announce", "AtEOTCondition", "AtEOTDesc", "ConditionDescription", "Defined", "DefinedExiler", "Duration", diff --git a/forge-game/src/main/java/forge/game/cost/Cost.java b/forge-game/src/main/java/forge/game/cost/Cost.java index 17e7db25fb32..3ed99d20ca91 100644 --- a/forge-game/src/main/java/forge/game/cost/Cost.java +++ b/forge-game/src/main/java/forge/game/cost/Cost.java @@ -21,6 +21,7 @@ import forge.card.CardType; import forge.card.mana.ManaCost; import forge.game.CardTraitBase; +import forge.game.ability.IHasForgeParams; import forge.game.card.Card; import forge.game.card.CounterEnumType; import forge.game.card.CounterType; @@ -46,7 +47,7 @@ * @author Forge * @version $Id$ */ -public class Cost implements Serializable { +public class Cost implements Serializable, IHasForgeParams { public static final String[] OPTIONAL_PARAMS = { "AffectedZone", "Amount", "Announce", "Collected", "CollectedCards", "Color", "Cost", "Exiled", "ExiledCards", "FirstForetell", "ForEachShard", "Foraged", diff --git a/forge-game/src/main/java/forge/game/spellability/SpellAbilityCondition.java b/forge-game/src/main/java/forge/game/spellability/SpellAbilityCondition.java index f08129d9ae05..b4e04609ce7d 100644 --- a/forge-game/src/main/java/forge/game/spellability/SpellAbilityCondition.java +++ b/forge-game/src/main/java/forge/game/spellability/SpellAbilityCondition.java @@ -24,6 +24,7 @@ import forge.game.GameObjectPredicates; import forge.game.GameType; import forge.game.ability.AbilityUtils; +import forge.game.ability.IHasForgeParams; import forge.game.card.Card; import forge.game.phase.PhaseHandler; import forge.game.phase.PhaseType; @@ -45,7 +46,7 @@ * @version $Id$ * @since 1.0.15 */ -public class SpellAbilityCondition extends SpellAbilityVariables { +public class SpellAbilityCondition extends SpellAbilityVariables implements IHasForgeParams { public static final String[] OPTIONAL_PARAMS = { "Condition", "ConditionActivationLimit", "ConditionCheckSVar", "ConditionChosenColor", "ConditionCompare", "ConditionCompare2", "ConditionDefined", diff --git a/forge-game/src/main/java/forge/game/spellability/SpellAbilityRestriction.java b/forge-game/src/main/java/forge/game/spellability/SpellAbilityRestriction.java index 7899bcec685e..9bf0259dc78f 100644 --- a/forge-game/src/main/java/forge/game/spellability/SpellAbilityRestriction.java +++ b/forge-game/src/main/java/forge/game/spellability/SpellAbilityRestriction.java @@ -28,6 +28,7 @@ import forge.game.GameObjectPredicates; import forge.game.GameType; import forge.game.ability.AbilityUtils; +import forge.game.ability.IHasForgeParams; import forge.game.card.*; import forge.game.keyword.Keyword; import forge.game.phase.PhaseType; @@ -49,7 +50,7 @@ * @author Forge * @version $Id$ */ -public class SpellAbilityRestriction extends SpellAbilityVariables { +public class SpellAbilityRestriction extends SpellAbilityVariables implements IHasForgeParams { public static final String[] OPTIONAL_PARAMS = { "Activation", "ActivationAfterBlockers", "ActivationFirstCombat", "ActivationGameTypes", "ActivationLifeAmount", "ActivationLifeTotal", diff --git a/forge-game/src/main/java/forge/game/spellability/TargetRestrictions.java b/forge-game/src/main/java/forge/game/spellability/TargetRestrictions.java index a5e4d0255ad5..9a9453903507 100644 --- a/forge-game/src/main/java/forge/game/spellability/TargetRestrictions.java +++ b/forge-game/src/main/java/forge/game/spellability/TargetRestrictions.java @@ -28,6 +28,7 @@ import forge.game.Game; import forge.game.GameEntity; import forge.game.ability.AbilityUtils; +import forge.game.ability.IHasForgeParams; import forge.game.card.Card; import forge.game.player.Player; import forge.game.zone.ZoneType; @@ -42,7 +43,7 @@ * @author Forge * @version $Id$ */ -public class TargetRestrictions { +public class TargetRestrictions implements IHasForgeParams { public static final String[] OPTIONAL_PARAMS = { "MaxTotalTargetCMC", "MaxTotalTargetPower", "RandomNumTargets", "TargetMax", "TargetMin", "TargetUnique", "TargetValidTargeting", "TargetingPlayer", diff --git a/forge-game/src/test/java/forge/game/ability/CardScriptParamDeclarationTest.java b/forge-game/src/test/java/forge/game/ability/CardScriptParamDeclarationTest.java index 63b657615cc2..2a25c3026bf8 100644 --- a/forge-game/src/test/java/forge/game/ability/CardScriptParamDeclarationTest.java +++ b/forge-game/src/test/java/forge/game/ability/CardScriptParamDeclarationTest.java @@ -3,6 +3,7 @@ import static org.testng.Assert.assertTrue; import java.io.IOException; +import java.lang.reflect.Field; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; @@ -16,6 +17,7 @@ import java.util.regex.Pattern; import java.util.stream.Stream; +import com.google.common.reflect.ClassPath; import org.testng.annotations.Test; /** @@ -28,9 +30,10 @@ * helpers, map lookups, and so on. A parameter read through a non-literal argument cannot be * detected, so a passing run means "no undeclared literal read", not a proof of completeness. * - * The check is opt-in: a class that declares neither field is skipped, so declarations can be - * added a few classes at a time. Once a class declares, removing it from the check requires - * deleting its whole declaration. + * Declarers are found by scanning the classpath for implementors of {@link IHasForgeParams}: a + * class is covered iff it implements that interface and lists its params, so there is no path or + * package list to maintain. Listing a param field without implementing the interface fails the + * test, so discovery and declaration cannot drift apart. */ public class CardScriptParamDeclarationTest { @@ -50,25 +53,33 @@ public class CardScriptParamDeclarationTest { // The ability declarator prefixes are not params private static final Set MARKERS = Set.of("AB", "SP", "ST", "DB"); - // Module source roots walked to find declaring classes -- no per-file path list to maintain. - private static final String[] SOURCE_ROOTS = { - "forge-game/src/main/java", - "forge-ai/src/main/java", - }; + // Package scanned for declarers (classes implementing IHasForgeParams) -- no path list to maintain + private static final String SCAN_PACKAGE = "forge.game"; - // Effects own their own params; everything else that declares forms the shared base layer. + // Where a discovered forge-game class's source lives, given its package path + private static final String GAME_SRC = "forge-game/src/main/java"; + + // Floor for the scan: a partial scan would silently drop declarers (a dropped reader stops + // gating its reads), so require these to be found and fail loudly on any shortfall. + private static final Set EXPECTED_FRAMEWORK = Set.of( + "AbilityFactory", "AbilityUtils", "SpellAbilityEffect", "CardTraitBase", "Cost", + "SpellAbility", "SpellAbilityCondition", "SpellAbilityRestriction", "TargetRestrictions"); + + // forge-ai declarers are found by walking this source root: the forge-game test classpath can't + // see forge-ai (the dependency runs forge-ai -> forge-game), so the classpath scan can't reach them. + private static final String AI_SRC = "forge-ai/src/main/java"; + + // Effects own their own params; everything else that declares forms the shared base layer private static final String EFFECTS_DIR = "/ability/effects/"; - // Assignment of an array initializer to a field -- tolerates an explicit "= new String[]{". - // Shared by discovery and parsing so the two can't drift apart. + // Assignment of an array initializer to a field -- tolerates an explicit "= new String[]{" private static final String ASSIGN = "\\s*=\\s*[^;]*?\\{"; - // A class declares params iff one of these fields is assigned an array initializer. + // A source file declares params iff it assigns one of these fields an array initializer private static final Pattern DECLARES = Pattern.compile("(?:OPTIONAL_PARAMS|REQUIRED_PARAMS)" + ASSIGN); - // Cross-cutting readers (they read params owned by effects/triggers, so reads != owned): - // declared but NOT reads-gated. They still contribute their declared params to the base - // and are subject to the structural checks. + // Cross-cutting readers read params owned by other classes, so their reads aren't gated; their + // declared params still join the base and face the structural checks. private static final Set READ_GATE_EXEMPT = Set.of("SpellAbilityAi"); @Test @@ -76,26 +87,55 @@ public void declarationsDoNotDrift() throws IOException { Path root = locateRoot(); List errors = new ArrayList<>(); - // Discover every declaring class by walking the source tree. + // Discover declarers by scanning the classpath for IHasForgeParams implementors List declarers = new ArrayList<>(); - for (String rel : SOURCE_ROOTS) { - Path src = root.resolve(rel); - if (!Files.isDirectory(src)) { + Set found = new TreeSet<>(); + ClassLoader cl = Thread.currentThread().getContextClassLoader(); + for (ClassPath.ClassInfo info : ClassPath.from(cl).getTopLevelClassesRecursive(SCAN_PACKAGE)) { + Class c; + boolean declares; + try { + c = Class.forName(info.getName(), false, cl); + declares = declaresOwnParams(c); + } catch (Throwable t) { + // A class that won't load/link can't be one of our plain declarers; skip it continue; } - try (Stream walk = Files.walk(src)) { - for (Path p : (Iterable) walk.filter(f -> f.toString().endsWith(".java"))::iterator) { - if (declares(read(p))) { - declarers.add(p); - } - } + if (!declares) { + continue; + } + if (!IHasForgeParams.class.isAssignableFrom(c)) { + errors.add(c.getSimpleName() + ": declares card-script params but does not implement IHasForgeParams"); + continue; + } + Path src = root.resolve(GAME_SRC).resolve(c.getName().replace('.', '/') + ".java"); + if (Files.exists(src)) { + declarers.add(src); + found.add(c.getSimpleName()); + } else { + errors.add(c.getName() + ": implements IHasForgeParams but its source was not found under " + GAME_SRC); } } - // Guard against a broken walk passing vacuously: the framework classes always declare. - assertTrue(!declarers.isEmpty(), "no param-declaring classes found -- source-tree discovery is broken"); + // Fail loudly on a partial scan instead of passing on a subset (see EXPECTED_FRAMEWORK) + Set missing = new TreeSet<>(EXPECTED_FRAMEWORK); + missing.removeAll(found); + assertTrue(missing.isEmpty(), "classpath scan missed expected declarers (partial/broken scan?): " + missing); + + // forge-ai declarers can't be reached by the classpath scan (see AI_SRC), so walk its source + Path aiSrc = root.resolve(AI_SRC); + assertTrue(Files.isDirectory(aiSrc), "forge-ai source root not found: " + AI_SRC); + int beforeAi = declarers.size(); + try (Stream walk = Files.walk(aiSrc)) { + for (Path p : (Iterable) walk.filter(f -> f.toString().endsWith(".java"))::iterator) { + if (declaresInSource(read(p))) { + declarers.add(p); + } + } + } + assertTrue(declarers.size() > beforeAi, "no forge-ai param declarers found under " + AI_SRC + " -- discovery is broken"); - // Base = declarers that aren't effects; their optional params are inherited by every effect. + // Base = declarers that aren't effects; their optional params are inherited by every effect Set base = new TreeSet<>(); for (Path f : declarers) { if (!isEffect(f)) { @@ -171,10 +211,23 @@ private void checkClass(Path file, Set base, boolean isEffect, List c) { + return hasOwnField(c, "OPTIONAL_PARAMS") || hasOwnField(c, "REQUIRED_PARAMS"); + } + + private boolean declaresInSource(String src) { return DECLARES.matcher(src).find(); } + private boolean hasOwnField(Class c, String name) { + for (Field f : c.getDeclaredFields()) { + if (f.getName().equals(name)) { + return true; + } + } + return false; + } + private boolean isEffect(Path file) { return file.toString().replace('\\', '/').contains(EFFECTS_DIR); }