Skip to content

coverage-parse-diff is blind to activation_zone (fourth instance, in ability_details rather than effect_details) #7317

Description

@cuinhellcat

activation_zone appears zero times in crates/engine/src/game/coverage.rs, so an activated ability's parse-diff signature never renders the zone it functions from. ability_details (coverage.rs:3759) emits kind, duration, repeat_for, targeting, targets, conditional, timing and modal — and stops there.

Measured against the shipped export: 158 distinct detail keys across all cards including nested children, none containing the substring activat.

Bestial Bloodline's {4}{G}: Return this card from your graveyard to your hand. renders as:

{"category":"ability","label":"ChangeZone",
 "source_text":"{4}{G}: Return this card from your graveyard to your hand.",
 "supported":true,
 "details":[["from","graveyard"],["to","hand"],["target","self"],["kind","activated"]]}

from/to are the effect's ChangeZone origin and destination. The ability's own activation_zone — the field can_activate_ability_now (casting.rs:18415) reads to decide whether the ability is offered at all — is absent, and the signature is byte-identical whether it holds None, Graveyard, Hand, Exile or Command.

Why it matters

activation_zone is not cosmetic metadata. casting.rs:18415 and :18848 gate legality on activation_zone.unwrap_or(Zone::Battlefield), and candidates.rs:3791/:3829/:3864/:3898/:3956 key five separate enumeration loops off it. A change to this field moves cards between "offered" and "not offered" — and the PR gate cannot see it.

Surfaced on #7316 (CR 113.6m: activation zone derived from a self-ChangeZone's origin rather than its destination). That change moves 55 cards from activation_zone: null to Graveyard, withdrawing a battlefield offer and unlocking a graveyard one on each. Its parse-diff sticky is empty: zero clusters, zero SupportFlip, and coverage-data.json is comparable across the change.

That is the inverse of #5507's failure mode and arguably worse. There, removals with no compensating addition made a correct fix look like a regression — visibly wrong, so a reviewer investigates. Here a rules-behavior change across 55 cards is indistinguishable from a no-op. A reviewer who trusts the sticky concludes nothing happened. The only way to tell today is to grep coverage.rs for the field name, which is exactly the manual step #5507 called out.

Pattern

This is at least the fourth instance:

The distinguishing feature: every prior instance was an effect_details payload field, and #5507 closed by recommending exhaustive destructuring there. ability_details renders the ability shell — AbilityDefinition's own fields — and never received that treatment. It picks its keys by hand from a struct with well over thirty fields, so it is the same defect class in the sibling renderer that the earlier fixes did not reach.

Suggested fix

Emit the field unconditionally in ability_details:

// CR 113.6b + CR 113.6j + CR 113.6m: the zone this ability functions from.
// `None` is the CR 113.6 battlefield default and emits nothing.
if let Some(zone) = &def.activation_zone {
    d.push(("activates from".into(), fmt_zone(zone)));
}

Two mechanical notes, both verified:

  • The key must not be from. effect_details already emits from for a ChangeZone origin (coverage.rs:2743, :2803) and trigger_details emits it for a trigger origin (:3832), and build_ability_item silently drops duplicate keys (:4832). Reusing from would make the field invisible on precisely the abilities it exists to watch. activates from is free among the 158 existing keys.
  • Do not scope it the way repeat_for is scoped. activation_zone is a single Option<Zone> with four inhabited values on 937 abilities (Hand 656, Graveyard 279, Exile 1, Command 1). Scoping recreates the blindness.

Expect the landing diff to be purely additive: ~937 activates from ∅ → <zone> rows in at most four clusters, zero SupportFlip, zero ItemAdded/ItemRemoved. It should land on its own so those migration rows do not collapse into the clusters of whatever change ships beside it.

More usefully, and echoing #5507's closing recommendation: extend the exhaustive-destructuring discipline from effect_details to ability_details. The ability shell is load-bearing review infrastructure too, and it is picking fields by hand from a struct that keeps growing.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions