feat(engine): dynamic CDA power/toughness counted quantities (distinct subtypes, turns taken) (CR 604.3/613.4a)

[real-venus]

Summary

Extends the existing characteristic-defining P/T (CDA) framework with two counted-quantity references so */* CDA cards resolve (CR 604.3, layer 7a per CR 613.4a):

• Subgoyf (*/1+*) — new QuantityRef::DistinctSubtypes { source: CardTypeSetSource, exclude: SubtypeExclusion }, the subtype-axis peer of the existing DistinctCardTypes (reuses its CardTypeSetSource scope axis). Counts distinct non-creature subtypes across all graveyards; toughness is that count plus 1 (the cda.rs "and its toughness is equal to that number plus 1" override now applies to the both branch, not just power_only).
• Control Win Condition (*/*) — new parser arm mapping "number of turns you've taken this game" to the pre-existing QuantityRef::TurnsTaken (resolver already worked; its coverage flag is flipped to Handled).

Both resolve live through layer 7a — the P/T recomputes on each layer pass (not a snapshot). Three cards are honestly gapped (coverage stays red, no green-washing): Graveyard Busybody (printed flavor text is not ingested by the card DB), Wood Elemental and Minion of the Wastes (ETB-snapshot quantities — "as it entered" — a fundamentally different mechanism from live recompute; a live recount would wrongly read 0).

Files changed

• crates/engine/src/types/ability.rs (new SubtypeExclusion enum + DistinctSubtypes variant)
• crates/engine/src/parser/oracle_nom/quantity.rs (two nested nom combinators), oracle_quantity.rs (parser tests), oracle_static/cda.rs (toughness +N offset on the both branch), oracle_static/tests.rs, parser/oracle.rs
• crates/engine/src/game/{quantity.rs (resolver), layers.rs, triggers.rs, ability_rw.rs, ability_scan.rs, ability_utils.rs, effects/mod.rs, coverage.rs} (exhaustive-match arms + coverage)
• crates/engine/tests/integration/cda_counted_quantities_pt.rs (new) + main.rs

CR references

CR 604.3, CR 613.4a, CR 208.2, CR 205.2, CR 205.3, CR 205.3m, CR 400.1, CR 404.1, CR 500, CR 109.2

Track

Developer

LLM

Model: claude-opus-4-8
Thinking: high

Verification

• cargo fmt --all — clean
• cargo clippy -p engine --all-targets --features proptest -- -D warnings — clean
• cargo test -p engine --lib — 15157 passed / 0 failed
• cargo test -p engine --test integration — 1760 passed / 0 failed (incl. 2 new layer-7a runtime tests)
• scripts/check-parser-combinators.sh — exit 0
• Discriminating tests are revert-failing: Subgoyf toughness = N+1 (2/3, 0/1, 3/4 — flips to N/N if the offset is reverted); Control Win Condition proves live recompute (turns 3→4) and per-controller scoping (opponent turns don't count); negatives confirm the subtype arm doesn't steal "card types among…" and the gapped cards still parse to None
• CR-annotation gate: zero UNVERIFIED

Scope Expansion

None. (Defensive DistinctSubtypes arms were added at three non-exhaustive match sites mirroring the parallel DistinctCardTypes behavior, to prevent a silent gap if the new variant is ever constructed with those sources — additive parity, no behavior change for existing variants.)

Validation Failures

None. (cargo semantic-audit / cargo coverage could not run in this worktree — jq is not installed and card-data.json is absent, so gen-card-data.sh fails. The change is purely additive; Oracle text for both cards was verified directly against MTGJSON AtomicCards + Scryfall. Recommend running both audits pre-merge in a worktree that has card-data.json.)

CI Failures

None.

[gemini-code-assist]

Code Review

This pull request implements support for characteristic-defining abilities (CDAs) that count distinct subtypes (e.g., Subgoyf) and player turns taken (e.g., Control Win Condition). It introduces the DistinctSubtypes and TurnsTaken quantity references, adds corresponding parser support, and updates the layer system to evaluate these values dynamically. Feedback on the changes highlights three key improvements: optimizing the creature type exclusion lookup in quantity.rs using a HashSet to avoid linear scans in a hot path, replacing pointer arithmetic with safe string length calculations in the parser, and adding a mandatory CR 613.4a rule annotation in layers.rs to comply with the repository's style guide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

[HIGH] Optimize creature type exclusion lookup. Evidence: crates/engine/src/game/quantity.rs:2226-2237.

Why it matters: Performing a linear scan (.iter().any(...)) over state.all_creature_types (which contains hundreds of creature types) for every subtype of every card in a hot path like layer 7a recomputation is highly inefficient.

Suggested fix: Collect state.all_creature_types into a temporary HashSet<&str> once before the loop to achieve O(1) lookups.

            let exclude_creature = matches!(exclude, SubtypeExclusion::CreatureTypes);
            let creature_types: HashSet<&str> = if exclude_creature {
                state.all_creature_types.iter().map(|s| s.as_str()).collect()
            } else {
                HashSet::new()
            };
            let mut seen: HashSet<&str> = HashSet::new();
            for obj_id in &obj_ids {
                if let Some(obj) = state.objects.get(obj_id) {
                    for sub in &obj.card_types.subtypes {
                        if exclude_creature && creature_types.contains(sub.as_str()) {
                            continue;
                        }
                        seen.insert(sub.as_str());
                    }
                }
            }

[gemini-code-assist]

[MEDIUM] Avoid pointer arithmetic for consumed length. Evidence: crates/engine/src/parser/oracle_nom/quantity.rs:1783.

Why it matters: Subtracting pointers in safe Rust is unnecessary and less idiomatic when the length of the consumed string can be computed directly.

Suggested fix: Use type_text.len() - remainder.len() to safely calculate the consumed length.

Suggested change

	let consumed = remainder.as_ptr() as usize - input.as_ptr() as usize;
	let consumed = type_text.len() - remainder.len();

[gemini-code-assist]

[MEDIUM] Missing CR annotation. Evidence: crates/engine/src/game/layers.rs:2060-2061.

Why it matters: The repository style guide requires every rules-touching line of engine code to carry a verified CR <number>: <description> comment.

Suggested fix: Add a CR 613.4a annotation to the layer dependency check.

Suggested change

	// Distinct subtypes read `zone` when sourced from that zone's cards
	// (Subgoyf: different subtypes among cards in all graveyards).
	// CR 613.4a: Distinct subtypes read `zone` when sourced from that zone's cards
	// (Subgoyf: different subtypes among cards in all graveyards).

References

1. Every rules-touching line of engine code must carry a comment of the form CR : . ^(link)

[real-venus]

Now I am working on it.

[matthewevans]

Thanks for the update. I found one production correctness issue that the current tests do not cover, plus a required proof gap before this can move forward.

• [HIGH] TurnsTaken CDA values can stay stale during real turn advancement. The new test mutates turns_taken and then manually marks layers dirty before evaluating layers, but production turn start increments turns_taken without marking layers dirty. flush_layers can then no-op when the layer dirty state is clean, so a CDA like Control Win Condition may not recompute from the new turn count in the actual game path. Please mark layers dirty when turns_taken changes, or add an equivalent targeted dependency, and add a regression that advances through the production turn path instead of manually forcing a full layer evaluation.

• [MED] The DistinctSubtypes counting path does a repeated state.all_creature_types.iter().any(...) scan for every subtype on every counted object. This runs in the layer/CDA evaluation path, so please build a temporary lookup set for the creature-type exclusion case rather than scanning the catalog repeatedly.

There are also merge conflicts on the current head, and this PR still needs the parser/card-level proof comment for the parse-affecting changes. I am holding approval/enqueue until the production invalidation path, conflict, and proof gap are resolved.

[matthewevans]

Thanks for the follow-up. The subtype-count hot path is fixed on this head, but the production TurnsTaken invalidation blocker is still open.

start_next_turn still increments turns_taken without marking layers dirty, so a clean layer cache can remain clean after the active player's turn count changes. The new regression does not exercise that path: it mutates turns_taken directly and calls a helper that forces layers_dirty.mark_full() before evaluate_layers. That proves the quantity resolver works once re-evaluation is forced, but not that the real turn-advance path invalidates a Control Win Condition-style CDA.

Please mark layers dirty, or add an equivalent targeted dependency, when turns_taken changes in the production turn-start path, and add a regression that advances through start_next_turn/the normal turn path instead of manually forcing a full layer recompute.

Still holding approval/enqueue until that production path is covered and the parser/card-data proof is present for this head.

[github-actions]

Parse changes introduced by this PR · 2 card(s), 2 signature(s) (baseline: main c8198e7f56c7)

2 card(s) · static/Continuous · added: Continuous (CDA=yes, affects=self, mods=dynamic power, dynamic toughness)

Examples: Control Win Condition, Subgoyf

2 card(s) · ability/static_structure · removed: static_structure

Examples: Control Win Condition, Subgoyf

1 card(s) had Oracle-text changes (errata/reprint) — excluded as non-parser.

[real-venus]

@matthewevans
Done.

[matthewevans]

Thanks for the follow-up. The previous TurnsTaken production-path blocker is fixed on this head: start_next_turn now dirties layers after incrementing turns_taken, and the new regression exercises that path.

I still can't approve this head because the Subgoyf side has the same production invalidation gap for normal graveyard churn.

[HIGH] Subgoyf-style CDA values can stay stale after a card enters or leaves a graveyard without another layer-dirty event. Evidence: crates/engine/src/game/layers.rs now teaches quantity_ref_reads_zone that QuantityRef::DistinctSubtypes { source: Zone { zone: Graveyard, .. } } reads graveyard membership, but the actual graveyard mutation gate in crates/engine/src/game/zones.rs only calls mark_layers_full when any_active_static_reads_zone_membership(state, Zone::Graveyard) is true. That helper currently checks StaticDefinition.condition only, not StaticMode::Continuous modifications such as SetDynamicPower / SetDynamicToughness whose QuantityExpr reads the graveyard. The new Subgoyf test also calls the local recompute() helper before each assertion, so it proves the resolver works once re-evaluation is forced, but not that the normal move_to_zone path dirties layers when a graveyard subtype count changes.

Please extend the live graveyard membership dirty check to include active continuous modifications whose dynamic quantity reads that zone, and add a regression that starts from a clean layer cache, moves/mills a card with a new non-creature subtype into a graveyard through the production zone-move path, and asserts the layer cache was dirtied before recomputing. That should make the Subgoyf coverage production-path equivalent to the new start_next_turn regression.