Skip to content

AbilityUtils.xCount: ThisTurnActivated passes the raw expression to doXMath (170 of 171 call sites pass expr) — extreme diagnostic noise, latent silent-drop #11613

Description

@johannesamee

Summary

AbilityUtils.xCount's ThisTurnActivated branch passes the raw count expression to doXMath where every other branch passes the extracted operator suffix. doXMath then tries to parse the valid-filter fragment as an arithmetic operand, fails, and prints

SVar 'Equip+YouCtrl' not defined in Card (Kíli the Resourceful)

The computed value is correct — the misparse's result is discarded — so this is a pure diagnostic-noise bug today. It is worth reporting anyway for two reasons: the volume is extreme (measured at 99.5% of stderr, ~7,185 lines per game), and the same line will silently drop a real operator suffix if any future card script uses one.

Verified against Forge 2.0.14; the line is unchanged on master today.

The code

forge-game/src/main/java/forge/game/ability/AbilityUtils.java#L2784-L2795
(that is master; the same block is at 2782–2793 in the 2.0.14 tag, and the faulty call is 2794 on
master, 2792 in 2.0.14
):

if (sq[0].startsWith("ThisTurnActivated")) {
    final String[] workingCopy = paidparts[0].split("_");
    final String validFilter = workingCopy[1];
    // use objectXCount ?
    int activated = CardUtil.getThisTurnActivated(validFilter, c, ctb, player).size();
    for (CostPaymentStack.Entry i : game.costPaymentStack) {
        if (i.payment().getAbility().isValid(validFilter, player, c, ctb)) {
            activated++;
        }
    }
    return doXMath(activated, s, c, ctb);   // <-- `s`, not `expr`
}

s is the whole expression, e.g. ThisTurnActivated_Activated.Equip+YouCtrl. expr is
CardFactoryUtil.extractOperators(s2), which returns the part after / and null when there is no /.

Inside doXMath, operators.split("\\.") splits ThisTurnActivated_Activated.Equip+YouCtrl into
["ThisTurnActivated_Activated", "Equip+YouCtrl"]. Length 2, so it tries Integer.parseInt("Equip+YouCtrl"),
throws, and falls into secondaryNum = calculateAmount(c, "Equip+YouCtrl", ctb) — which finds no such SVar,
prints the message, and returns 0. s[0] (ThisTurnActivated_Activated) then matches no operator token, so
doXMath falls through to return num and secondaryNum is discarded.

Passing expr gives the identical value with no warning, because doXMath returns num immediately on a
null operators argument.

This is the only call site in xCount that does this. Of 171 doXMath call sites between
AbilityUtils.java:1564 and :3190, 170 pass expr and this one passes s.

Impact

Noise. Every evaluation of the static ability emits one line. Measured over a 20-game AI-vs-AI Brawl
mirror with one Kíli the Resourceful per deck:

total stderr lines 144,416
SVar 'Equip+YouCtrl' not defined 143,704 (99.5%)
per game ~7,185

Affected scripts: 12 routed through the faulty line, 11 of them emitting the warning. Across all 33,587
card scripts, 12 use Count$ThisTurnActivated_. All 12 reach the faulty call; 11 emit the message and
one does not, because it is the only one whose expression contains a second ..

Please read the last column as derived, not observed. Only Kíli the Resourceful was watched emitting
this — that is the card I hit it on, and the 143,704 lines below are its. The other eleven rows are read off
the same code path: their SVars are quoted verbatim from cardsfolder.zip, and whether a given one emits
follows from the .-count alone, since doXMath reaches parseInt/calculateAmount only when the split
yields exactly two parts. I have not run a game with any of the other eleven.

Card SVar (complete) .-split emits warning
Advancing the Spirit Count$ThisTurnActivated_Activated.PowerUp+YouCtrl 2 yes
Bruenor Battlehammer Count$ThisTurnActivated_Activated.Equip+YouCtrl 2 yes
Elvish Refueler Count$ThisTurnActivated_Activated.Exhaust+YouCtrl 2 yes
Forge Anew Count$ThisTurnActivated_Activated.Equip+YouCtrl 2 yes
Gavi, Nest Warden Count$ThisTurnActivated_Activated.Cycling+YouCtrl 2 yes
Highway Reaver Count$ThisTurnActivated_Activated.Unearth+YouCtrl 2 yes
Kíli the Resourceful Count$ThisTurnActivated_Activated.Equip+YouCtrl 2 yes
Laboratory Drudge Count$ThisTurnActivated_Activated.YouCtrl+inZoneGraveyard 2 yes
Professor Hojo Count$ThisTurnActivated_Activated.YouCtrl+IsTargeting Valid Creature.YouCtrl~inZoneBattlefield 3 no
Spellpyre Phoenix Count$ThisTurnActivated_Activated.Cycling+YouCtrl 2 yes
Tezzeret, Betrayer of Flesh Count$ThisTurnActivated_Activated.Artifact+YouCtrl+inZoneBattlefield 2 yes
The Chain Veil Count$ThisTurnActivated_Activated.Loyalty+Planeswalker+YouCtrl 2 yes

Professor Hojo splits into three parts, so doXMath's if (s.length == 2) block is skipped entirely —
no parseInt, no calculateAmount, no message. It still takes the faulty path; it is just silent on it.

None of the twelve is currently computed wrongly. Checked on both routes by which it could be: none
carries a / operator suffix, and the pre-. segment is ThisTurnActivated_Activated in all twelve, which
contains none of doXMath's operator tokens — the complete set it tests is Plus, NMinus, Minus,
Twice, Thrice, HalfUp, HalfDown, ThirdUp, ThirdDown, Negative, Times, Pow,
DivideEvenlyUp, DivideEvenlyDown, Mod, Abs, LimitMax, LimitMin — so it always falls through to
return num.

The latent hazard, and Professor Hojo is half of it already shipping. When the split yields more than
two parts, secondaryNum stays 0 and no warning is printed at all — so if the pre-. segment ever did
match an operator, the result would be silently wrong. Professor Hojo proves that branch is reachable in a
shipped script today; only the operator-match half is hypothetical. Equally, a script writing
Count$ThisTurnActivated_Activated.Equip+YouCtrl/Times.2 would have its /Times.2 silently ignored. The
/Times.<SVar> idiom is in active use elsewhere (e.g. Desert Were-Worm's
Count$Valid Creature.attacking$CardPower/Times.TrigSwitch, unaffected because Count$Valid … takes a
different branch that passes expr), so this is a live trap for the next script author rather than a
hypothetical one.

Reproduction

Any game in which a player controls one of the eleven warning-emitting cards above and its static ability is
evaluated. Smallest version: put Kíli the Resourceful plus two Equipment on the battlefield (three
qualifying permanents gives the enduring story) and watch stderr.

Behaviour is correct throughout. Measured on a board built with the AITest/SimulationTest helpers
rather than in an AI-vs-AI game — Forge's AI never activated an equip ability in 20 logged games, since the
Equipment it plays arrives pre-attached by enter-the-battlefield triggers, so a constructed board was the
only way to exercise the cost. Activating two equip abilities in one turn with an enduring story, the first
paid {0} and the second paid its printed {3}; Count$ThisTurnActivated_… read 0, then 1, then 2, and
returned to 0 on the next turn. So the only observable symptom is the log volume.

Suggested fix

One line:

-    return doXMath(activated, s, c, ctb);
+    return doXMath(activated, expr, c, ctb);

Offered as a starting point rather than a patch: it matches what the other 170 call sites in the same method
do and it removes the warning without changing any of the twelve current values, but I have not built or run
Forge's test suite against it, and I have not checked whether any consumer depends on the current
ThisTurnActivated behaviour. The report is the reliable artifact here; treat the diff as a suggestion.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions