Seasoned Pyromancer — Discarded 2 nonland cards, but only created one token instead of two

[matthewevans]

Reported in Discord: https://discord.com/channels/1485498006781427802/1507054922871476284/1507054922871476284

Thread: Seasoned Pyromancer
Cards: none detected
Parser status: fully_parsed
Extraction confidence: 0.90

Summary

Discarded 2 nonland cards, but only created one token instead of two

Oracle text (verified from client/public/card-data.json)

No card names were detected for this report.

---

_{report_id: discord:1507054922871476284:1507054922871476284:0 · discord: 1507054922871476284/1507054922871476284}

[matthewevans]

🤖 Implementation Plan — #740 (Seasoned Pyromancer)

Investigated by Claude Code; not yet implemented.

Oracle: When this creature enters, discard two cards, then draw two cards. For each nonland card discarded this way, create a 1/1 red Elemental creature token.

Parse is correct. The ETB lowers (via triggers[0]) to a Discard{2} → sub Draw{2} → sub Token{count: Fixed 1, repeat_for: Ref(FilteredTrackedSetSize{ filter: Typed[Card, Non Land], caused_by: Discarded })} chain. The token count is right (1 token × N iterations), and the repeat_for quantity is the correct AST (FilteredTrackedSetSize scoped to caused_by: Discarded). So the bug is runtime, not parser.

---

1. Root cause

The repeat_for count is QuantityRef::FilteredTrackedSetSize, which is resolved by reading the most-recently-published tracked set:

crates/engine/src/game/quantity.rs:1687

QuantityRef::FilteredTrackedSetSize { filter, caused_by } => {
    let Some((set_id, ids)) = state.tracked_object_sets.iter().max_by_key(|(id, _)| id.0)
    else { return 0; };
    let count = ids.iter().filter(|&&oid| {
        let cause_ok = match caused_by {
            None => true,
            Some(cause) => state.tracked_set_member_causes.get(set_id)
                .and_then(|causes| causes.get(&oid))
                .is_some_and(|member_cause| member_cause == cause),   // == Discarded
        };
        cause_ok && matches_target_filter(state, oid, filter, &filter_ctx)
    }).count();
    ...
}

For this to return 2, the Discard{2} effect must have published the two discarded cards into tracked_object_sets and stamped each with ThisWayCause::Discarded in tracked_set_member_causes. That stamping happens through publish_tracked_set_with_causes (effects/mod.rs:3029), driven by the chain-resolution gate next_sub_needs_tracked_set (effects/mod.rs:4280 and :5081).

The gate is the bug. next_sub_needs_tracked_set → ability_or_branch_references_tracked_set (effects/mod.rs:2459-2477) walks ability.effect, ability.sub_ability, and ability.else_ability, and effect_references_tracked_set (:2484-2503) inspects each effect's own quantity fields (Effect::Token { count, .. } => quantity_hits_tracked(count)). For Seasoned Pyromancer the Token's count is Fixed 1 — the tracked-set reference lives in the Token's repeat_for field, which is a field on ResolvedAbility, not on Effect, and is never inspected by either function.

Walk for this card:

• next_sub_needs_tracked_set(Discard) → recurse into sub Draw → effect_references_tracked_set(Draw{count: Fixed 2}) = false → recurse into sub Token → effect_references_tracked_set(Token{count: Fixed 1}) = false (target filter absent), and Token.repeat_for is not checked → returns false.

Because the gate returns false, the Discard's publish_tracked_set_with_causes at effects/mod.rs:5087 (and the scoped twin at :4281) never fires on the non-interactive discard path → tracked_set_member_causes has no Discarded entries → FilteredTrackedSetSize filters down to 0 (or a stale prior set), and the repeat_for loop runs the wrong number of iterations.

Why "2 discarded → 1 token" specifically (interactive path): The interactive WaitingFor::DiscardChoice handler publishes the set unconditionally and correctly (engine_resolution_choices.rs:2295-2306, stamps every graveyard-bound member Some(Discarded)), so a hand-of-3+ discard should yield 2. The single-token symptom is the non-interactive discard branches in effects/discard.rs that bypass the chooser entirely and fall straight through to EffectResolved without publishing:

crates/engine/src/game/effects/discard.rs

• :288 random-discard branch (discard two cards at random-style / AI) — loops discard_caused_by_effect_with_source, then :348 EffectResolved. No publish.
• :312 forced-all branch (hand_cards.len() <= count, e.g. exactly 2 cards in hand) — loops discard, then :348 EffectResolved. No publish.

Both rely on the chain gate at effects/mod.rs:5081 to publish — which is dead for this card because of the repeat_for blind spot. So on the forced/AI path the set is empty → FilteredTrackedSetSize = 0; the observed "1" is from a stale max-id set or a partial publish. Either way the count is unscoped to this resolution's discards. The interactive 3+-card hand is the only path that currently works.

---

2. File-set

File	Change
crates/engine/src/game/effects/mod.rs	Primary. effect_references_tracked_set / ability_or_branch_references_tracked_set must inspect ability.repeat_for (a QuantityExpr) via the existing quantity_expr_references_tracked_set (:2580). This makes the gate fire so publish_tracked_set_with_causes runs for the Discard.
crates/engine/tests/seasoned_pyromancer_740.rs	New. Discriminating runtime test (see §5).
crates/engine/src/game/effects/discard.rs	Verify only (likely no change). The non-interactive branches at :288/:312 must not regress; the fix at the gate level covers them. Only touch if a defense-in-depth publish is wanted — but prefer the single gate fix to avoid double-publish with the interactive path.

No parser, no card-data.json, no AST changes — the AST is already correct.

---

3. Approach

Single fix, at the gate. The repeat_for quantity is part of the ability and must participate in tracked-set dependency detection exactly like effect.count does. In ability_or_branch_references_tracked_set (effects/mod.rs:2459), add a repeat_for check:

fn ability_or_branch_references_tracked_set(ability: &ResolvedAbility) -> bool {
    let consumes = matches!(&ability.effect,
        Effect::CreateDelayedTrigger { uses_tracked_set: true, .. } | Effect::ChooseFromZone { .. }
    ) || effect_references_tracked_set(&ability.effect)
    // CR 608.2c: a `repeat_for` count may itself reference the chain's "this way"
    // tracked set (e.g. "for each nonland card discarded this way" — Seasoned
    // Pyromancer). The producing ancestor (Discard) must publish so this loop reads it.
    || ability.repeat_for.as_ref().is_some_and(quantity_expr_references_tracked_set);

    consumes
        || ability.sub_ability.as_deref().is_some_and(ability_or_branch_references_tracked_set)
        || ability.else_ability.as_deref().is_some_and(ability_or_branch_references_tracked_set)
}

This is the building-block fix: it generalizes to every repeat_for-over-tracked-set card (the for-each-X-this-way class), not just this one. quantity_expr_references_tracked_set (:2580) already returns true for both TrackedSetSize and FilteredTrackedSetSize (:2586), so no new variant is needed.

Once the gate fires, the existing publish_tracked_set_with_causes(state, affected_with_causes) at effects/mod.rs:5087 publishes the discarded cards with ThisWayCause::Discarded (via this_way_cause_for_effect → Effect::Discard => Some(Discarded), :2708). FilteredTrackedSetSize then counts exactly the nonland cards discarded this resolution (scoped by caused_by: Discarded + the Card, Non Land filter), and drive_repeat_for_outermost (:3304, base_iterations = resolve_quantity_with_targets(repeat_for)) runs the loop the right number of times → 1 token per nonland discarded.

Scoping note: the max_by_key set is the chain set; chain_tracked_set_id keeps the Discard publish as the live set across the Draw (Draw publishes nothing — confirmed, no publish_* calls in effects/draw.rs), so the Token's repeat_for reads the Discard set, not the draw. Land-vs-nonland discrimination is the Typed[Card, Non Land] filter evaluated per-member in quantity.rs:1709.

---

4. CR annotations (grep-verified vs docs/MagicCompRules.txt)

⚠️ Brief correction: the issue brief cited "CR 701.8 Discard" and "CR 107.3 for-each" — both are wrong. 701.8 is Destroy; 107.3 is X placeholders. Use the verified numbers below. The codebase already annotates this subsystem with 701.9a / 608.2c.

$ grep -nE "^701\.9|^701\.8" docs/MagicCompRules.txt
3313:701.8. Destroy
3315:701.8a To destroy a permanent, move it from the battlefield to its owner's graveyard.
3321:701.9. Discard
3323:701.9a To discard a card, move it from its owner's hand to that player's graveyard.
3325:701.9b By default, effects that cause a player to discard a card allow the affected player to choose which card to discard. Some effects, however, require a random discard...

$ grep -nE "^608\.2c" docs/MagicCompRules.txt
2789:608.2c The controller of the spell or ability follows its instructions in the order written...
        read the whole text and apply the rules of English to the text.

$ grep -nE "^107\.3\b" docs/MagicCompRules.txt
464:107.3. Many objects use the letter X as a placeholder for a number...   # ← NOT for-each; do not cite

Annotation to add at the repeat_for gate line:

// CR 701.9a: discarding moves the card to the graveyard. CR 608.2c: the
// instruction order ("discard ... then draw ... For each nonland card discarded
// this way, ...") binds the for-each count to THIS resolution's discard set, so
// the producing Discard must publish its tracked set with the `Discarded` cause
// before the `repeat_for` loop reads it.

(Existing nearby annotations already use CR 608.2c + CR 614.6 for the cause-stamp side map at effects/mod.rs:3012-3047 and quantity.rs:1695 — leave those; they're correct.)

---

5. Discriminating runtime test

crates/engine/tests/seasoned_pyromancer_740.rs — drive the actual chain (parse → build_resolved_from_def → resolve_ability_chain + apply for any DiscardChoice), mirroring tests/aclazotz_attack_discard_multi_opponent.rs. Must exercise both the count and the land/nonland discrimination, and must use the forced-all (non-interactive) path since that is the broken one.

Body under test (the ETB tail, post-trigger): "discard two cards, then draw two cards. For each nonland card discarded this way, create a 1/1 red Elemental creature token."

• Case A — 2 nonland discarded → 2 tokens. Hand = exactly 2 nonland cards (e.g. two Lightning Bolt), forcing the discard.rs:312 forced-all path (no chooser). Library stocked so Draw 2 succeeds. Assert: 2 cards in graveyard, 2 Elemental tokens on the battlefield (count battlefield objects named Elemental / Creature Elemental 1/1 red). Fails today: 0 or 1 tokens.
• Case B — 1 land + 1 nonland discarded → 1 token. Hand = exactly 1 Forest + 1 Lightning Bolt (forced-all path again). Assert: exactly 1 Elemental token (the land discard contributes 0). Guards the Non Land filter, not just the raw set size.
• (Optional) Case C — interactive 3-card hand, discard 2 nonland → 2 tokens. Hand = 3 nonland; resolve the WaitingFor::DiscardChoice via apply(GameAction::DiscardCards(...)) choosing 2. Confirms the interactive path still works and the gate fix doesn't double-publish.

Assert on the token count delta, not on AST shape — this is a pipeline/runtime test, per card-test conventions. Place a one-line //! header citing issue #740 and the root cause.

---

6. Risks + class

• Class: for-each-this-way over a repeat_for count. Every card of the form "for each <filter> <verb>-ed this way, <do Y>" where the multiplier is repeat_for: (Filtered)TrackedSetSize is affected — not just Seasoned Pyromancer. The fix is the building block (gate inspects repeat_for), so the whole class is unblocked at once. Examples sharing the shape: discard/mill/sacrifice/exile "this way" → per-member fan-out.
• Risk — double publish. The interactive DiscardChoice handler already publishes unconditionally (engine_resolution_choices.rs:2305). After the gate fix, the chain path at effects/mod.rs:5087 would also publish for the same Discard. publish_tracked_set extends the chain set (:2998), so a double publish risks duplicated member ids inflating the count. Mitigation: the interactive path returns from resolve at discard.rs:344 before reaching the chain-gate publish, so the two paths are mutually exclusive — the gate publish only runs on the auto/forced paths that don't publish themselves. Verify this with Case C; if a dup appears, dedup in publish_tracked_set (affected_ids extend → .dedup() after sort) or guard the chain publish on "not already published this chain". Confidence: medium-high that they're exclusive (the return Ok(()) at :344 is the seam).
• Risk — broadening the gate fires extra publishes. Adding repeat_for to the gate makes more abilities publish tracked sets. This is correct (those abilities do read the set), and publish_tracked_set_with_causes is cheap; no behavioral change for abilities whose repeat_for doesn't reference a tracked set (is_some_and short-circuits). Low risk.
• Confidence: High on the root cause (gate omits repeat_for; verified by reading ability_or_branch_references_tracked_set and effect_references_tracked_set — neither walks repeat_for). Medium-high on the double-publish seam (rests on the discard.rs:344 early return).

---

7. ROI

• One-line gate fix (add a repeat_for clause to an existing predicate) + one new test file. No parser, no AST, no card-data regen.
• Unblocks the entire for-each-discarded/milled/sacrificed-this-way-via-repeat_for class, not a single card.
• Verification: cargo fmt --all, then Tilt test-engine (new test) + clippy. No card-data rebuild needed.

---

Pins: quantity.rs:1687 (count read) · effects/mod.rs:2459-2503 (gate blind to repeat_for) · effects/mod.rs:5081-5087 & :4280-4281 (skipped publish) · effects/discard.rs:288,312,348 (non-interactive no-publish branches) · engine_resolution_choices.rs:2295-2306 (working interactive publish) · effects/mod.rs:3304 (drive_repeat_for_outermost).