fix(parser): UNSUPPORTED full-set one-off: Bad Wolf Bay#5079
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements the CantEnterBattlefieldFrom game restriction to support floating continuous effects that prohibit objects from entering the battlefield from specific zones (such as Bad Wolf Bay's chaos ability). The feedback recommends fully supporting all RestrictionExpiry variants (such as untap-anchored durations) during untap and lowering, rather than hardcoding bypasses, to adhere to the composable building blocks principle (R3/L1).
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.
| // Not untap-anchored — CantEnterBattlefieldFrom expires at cleanup | ||
| // (CR 514.2), handled in the end-of-turn retain below. | ||
| GameRestriction::DamagePreventionDisabled { .. } | ||
| | GameRestriction::CantEnterBattlefieldFrom { .. } => true, |
There was a problem hiding this comment.
[MEDIUM] Support all RestrictionExpiry variants for CantEnterBattlefieldFrom during untap.
Why it matters: Hardcoding CantEnterBattlefieldFrom to always return true (never expiring on untap) violates the composable building blocks principle (R3/L1) and will cause bugs if a future card uses this restriction with an untap-anchored duration like UntilPlayerNextTurn.
Suggested fix: Match on the restriction's expiry field to correctly handle untap-anchored expirations.
| // Not untap-anchored — CantEnterBattlefieldFrom expires at cleanup | |
| // (CR 514.2), handled in the end-of-turn retain below. | |
| GameRestriction::DamagePreventionDisabled { .. } | |
| | GameRestriction::CantEnterBattlefieldFrom { .. } => true, | |
| GameRestriction::CantEnterBattlefieldFrom { expiry, .. } => { | |
| !matches!(expiry, RestrictionExpiry::UntilPlayerNextTurn { player } if *player == active) | |
| } | |
| GameRestriction::DamagePreventionDisabled { .. } => true, |
References
- R3. Parameterize, don't proliferate. Before a new sibling enum variant lands, the variant must NOT be a leaf-level parameterization of an existing variant's structural axis. (link)
- L1. Class vs single case. Does the change cover a class of cases or just one? If only one example exists, the change is a special case dressed as a building block — flag it. (link)
| // CR 611.2a: the parser hardcodes `EndOfTurn` ("this turn") for | ||
| // CantEnterBattlefieldFrom, so there is no duration to lower — same as | ||
| // DamagePreventionDisabled. | ||
| GameRestriction::DamagePreventionDisabled { .. } | ||
| | GameRestriction::CantEnterBattlefieldFrom { .. } => {} |
There was a problem hiding this comment.
[MEDIUM] Support lowering of RestrictionExpiry for CantEnterBattlefieldFrom.
Why it matters: Hardcoding CantEnterBattlefieldFrom to bypass expiry lowering violates the composable building blocks principle (R3/L1) and will leave the player ID uninitialized if a future card uses an untap-anchored duration.
Suggested fix: Group CantEnterBattlefieldFrom with ProhibitActivity in the expiry lowering match block (which is just above this hunk) so that its expiry is correctly initialized.
References
- R3. Parameterize, don't proliferate. Before a new sibling enum variant lands, the variant must NOT be a leaf-level parameterization of an existing variant's structural axis. (link)
- L1. Class vs single case. Does the change cover a class of cases or just one? If only one example exists, the change is a special case dressed as a building block — flag it. (link)
matthewevans
left a comment
matthewevans
left a comment
There was a problem hiding this comment.
[MED] CantEnterBattlefieldFrom accepts arbitrary trailing duration text but always lowers to EndOfTurn. Evidence: crates/engine/src/parser/oracle_effect/mod.rs:2623 ignores the nom remainder from cant_enter_battlefield_from_zone, and crates/engine/src/parser/oracle_effect/mod.rs:2639 unconditionally stores RestrictionExpiry::EndOfTurn; the added test at crates/engine/src/parser/oracle_effect/tests.rs:38709 explicitly treats trailing " this turn" as harmless leftover instead of parsing/validating the duration. Why it matters: this new parser arm will also accept phrases like cards can't enter from exile until your next turn or any other trailing text after the origin zone and silently give them the wrong cleanup timing, while the lifecycle code only arms/prunes ProhibitActivity for the non-EOT RestrictionExpiry variants. Suggested fix: either require the remainder to be empty or exactly " this turn" for this PR, or parse/thread the duration and make the restriction lifecycle handle every supported RestrictionExpiry variant for CantEnterBattlefieldFrom.
Separately, the current head still lacks the required <!-- coverage-parse-diff --> evidence for this parser/engine surface, so I can't treat it as queue-ready even after the code issue is addressed.
Parse changes introduced by this PR · 1 card(s), 2 signature(s) (baseline: main
|
2 participants
Summary
Fixes a parser misparse affecting 1 card(s) in the Doctor Who Commander precons.
Root cause: UNSUPPORTED full-set one-off: Bad Wolf Bay
Cards corrected
Fix
Implemented cluster 90 (Bad Wolf Bay, WHO Plane) exactly per the approved plan. The sole misparse was the chaos ability's leaf "cards can't enter from exile this turn," which lowered to Effect::Unimplemented. Built the missing class-level primitive as a new floating GameRestriction variant across all layers.
WHAT WAS BUILT (new engine variant, add-engine-variant checklist followed):
=> falsearm (not a damage restriction).RUNTIME PROOF + tests (building-block level): parser test asserts the combinator yields correct zone+type across bare/any-card, type-scoped, and trailing-"this turn" forms; full-card test asserts 0 Unimplemented and the Planeswalk SequentialSibling chain survives; zones.rs runtime card-test drives real move_to_zone -> block-from-exile, then execute_cleanup prune -> move succeeds (CR 611.2a/614.1d/514.2). All 3 pass. Emergent correct interaction: the card's own Trigger-1 flicker return is also blocked when chaos ensued, via the same gate.
VERIFICATION: cargo fmt clean; clippy --all-targets -D warnings exit 0; cargo test -p engine exit 0 (incl. 3 new tests); re-parse shows 0 Unimplemented/0 Unknown with the exact intended AST; gen-card-data shows the card supported; semantic-audit reports 0 findings for the card; client pnpm type-check passes. Parser combinator gate: my added lines contain no string-dispatch (the only .find is an iterator method inside a #[test]); check-parser-combinators.sh exit=1 is pre-existing baseline noise from files I never touched (it diffs against an ancient ancestor commit).
NOTES: (1) Restored crates/engine/data/known-tokens.toml, which gen-card-data.sh regenerated with a 9113-line token-data regression (this worktree lacks fetched token-set data for some cards) — that is a generation side effect, not my source fix, and would be harmful if committed. (2) crates/engine/data/oracle-subtypes.json carries a pre-existing single-line change ("Shi-Ar") that was already modified at task start, before any of my actions; left untouched per multi-agent safety. No commits made. No stop-and-return items; no CR ambiguity.
Files changed
CR references
Verification
cargo fmt --all— passcheck-parser-combinators.sh (merge-base 184c1304b)— passcargo clippy -p engine --all-targets -- -D warnings— passcargo test -p engine— passoracle-gen data --filter 'bad wolf bay'— passCards confirmed re-parsed correctly: Bad Wolf Bay
🤖 Generated with Claude Code