fix(parser): recognize "land types" in the loses-all enumeration (Alpine Moon, Lithoform Blight, Ultima) - #6271
Conversation
…ine Moon, Lithoform Blight, Ultima) Closes phase-rs#6270 A continuous static of the form "<subject> loses all land types and abilities [and has/gain <ability>]" silently dropped the entire type/ability-removal clause: the "loses all <list>" enumeration (LossMember in oracle_static/grammar.rs) only recognized abilities/card types/creature types, so "land types" matched none of the three alt arms, separated_list1 failed on the first token, and scan_loss_enumeration returned an empty Vec for the whole clause. Adds LossMember::LandTypes + a fourth `tag("land types")` alt arm in grammar.rs, and the corresponding RemoveAllSubtypes{Land} match arm in keyword_grant.rs's parse_continuous_modifications loss loop — the same shared composing authority already generalized for the sibling color (PR phase-rs#5807) and additive-type (PR phase-rs#6081) composition gaps. The engine runtime already fully supports SubtypeSet::Land removal (game/layers.rs), so this is a pure parser-wiring fix. Also explains and closes phase-rs#4222 (Ultima's blight counters doing nothing), whose own investigation independently reached the same root cause for Ultima specifically.
There was a problem hiding this comment.
Code Review
This pull request adds support for parsing and handling the loss of land types (e.g., 'loses all land types and abilities') by introducing the LossMember::LandTypes enum variant, updating the parser to recognize 'land types' using nom combinators, and mapping it to the RemoveAllSubtypes modification for the Land subtype set. All changes are properly annotated with the relevant Comprehensive Rules (CR 205.3i). There are no review comments, and I have no feedback to provide.
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.
Parse changes introduced by this PR · 3 card(s), 3 signature(s) (baseline: main
|
matthewevans
left a comment
There was a problem hiding this comment.
Maintainer review complete: current-head parser change is at the existing LossMember authority; the current parse-diff is limited to Alpine Moon, Lithoform Blight, and Ultima.
Closes #6270. Also explains and closes #4222.
Summary
A continuous static of the form
<subject> loses all land types and abilities [and has/gain "<ability>"]silently dropped the entire type/ability-removal clause, keeping only the granted ability. Alpine Moon, Lithoform Blight, and Ultima, Origin of Oblivion all use this exact "loses all land types and abilities" phrasing, and none of them removed the affected land's original type/abilities before this fix.Files changed
crates/engine/src/parser/oracle_static/grammar.rscrates/engine/src/parser/oracle_static/keyword_grant.rscrates/engine/src/parser/oracle_static/tests.rsCR references
Root cause
oracle_static/grammar.rs'sLossMemberenum models the "loses all<list>" enumeration as a closed 3-member set (Abilities/CardTypes/CreatureTypes).parse_loss_enumeration'sseparated_list1requires every item in the comma-and list to match one of the threealtarms; "land types" matches none of them, so the list parse fails at the first token andscan_loss_enumerationreturns an emptyVecfor every scan window.parse_continuous_modifications's loss loop (keyword_grant.rs) then pushes nothing for the whole clause — the card still reports as fully parsed (noUnimplementedresidual) because the trailing granted-ability clause parses fine on its own, so the drop is silent.parse_continuous_modificationsis the same shared composing authority already generalized twice for this exact bug shape — a compound predicate where a narrow enumeration/dispatch gap lets one clause vanish while the rest of the line still "succeeds": the color-defining static fix (#5807,SetColordropped when composed with a keyword) and the additive-type-clause fix (#6081, a granted quoted ability dropped the additive type). This is the same function, the same missing-enumeration-member shape, on a different axis (CR 205.3i land types).The engine runtime side needed zero changes —
SubtypeSet::Landalready exists (crates/engine/src/types/card_type.rs) andgame/layers.rsalready has a workingRemoveAllSubtypes { set: Land }arm. This is a pure parser-wiring fix.Fix
grammar.rs: addLossMember::LandTypesand a fourthvalue(LossMember::LandTypes, tag("land types"))arm to the enumerationalt(no ordering hazard — none of the four tags share a prefix).keyword_grant.rs: add the correspondingLossMember::LandTypes => modifications.push(ContinuousModification::RemoveAllSubtypes { set: SubtypeSet::Land })arm, mirroring the existingCreatureTypesarm.tests.rs: a building-block test (scan_loss_enumeration_recognizes_land_types) mirroring the existingCreatureTypescoverage, plus two production-path regression tests parsing Lithoform Blight's and Alpine Moon's actual Oracle text and assertingRemoveAllSubtypes { Land }+RemoveAllAbilities+ the grantedGrantAbility(s) all compose.Parse diff (Lithoform Blight,
Enchanted land loses all land types and abilities and has "{T}: Add {C}" and "{T}, Pay 1 life: Add one mana of any color.")Before — the type/ability removal is dropped:
After:
Anchored on
crates/engine/src/parser/oracle_static/grammar.rs(LossMember/parse_loss_enumeration/scan_loss_enumeration) — the existing 3-member loss-enumeration seam this extends to 4.CreatureTypesis the direct structural sibling: sameRemoveAllSubtypesruntime, same enumeration position, same test-authoring pattern (static_nonlegendary_creatures_enchanted_player_controls_base_pt_and_lose_types, Curse of Conformity).crates/engine/src/parser/oracle_static/keyword_grant.rs(parse_continuous_modifications's loss loop) — mirrors the existingCreatureTypes => RemoveAllSubtypes { set: Creature }arm one line above the newLandTypesarm.crates/engine/src/game/layers.rs(RemoveAllSubtypes { set } => match set { ... SubtypeSet::Land => ... }) — confirmed the runtime arm forLandalready exists and is exercised elsewhere, so no engine change was needed, only the parser wiring.Claimed parse impact
<subject> loses all land types and abilities [and has/gain <ability>]classVerification
I could not run
cargo test/cargo clippy/cargo buildlocally in this session — this environment's Rust toolchain has no usable linker for thex86_64-pc-windows-msvctarget (no MSVC Build Tools or Windows SDK installed, and no WSL/Docker/Tilt available as a fallback; confirmed the linker gap is total, not just aPATHshadowing issue, by checking forkernel32.libetc. anywhere on the machine).cargo fmt --alldoes work (no linking involved) and ran clean over this diff with no other changes.In place of running the suite, I verified by hand, tracing the actual source at the cited line numbers:
LossMember's only exhaustivematch(keyword_grant.rs) is updated with the new arm; the one other reference (type_change.rs, a non-exhaustivematches!(m, LossMember::CardTypes)) is unaffected by the new variant.SubtypeSet::Landand itsRemoveAllSubtypesruntime arm ingame/layers.rsalready exist and are already exercised by other tests, so the new modification is not a dead end at resolution time.TargetFilter::And,TargetFilter::Typed,TypeFilter::Land,ControllerRef::Opponent,TargetFilter::HasChosenName,ContinuousModification::RemoveAllSubtypes/RemoveAllAbilities/GrantAbility) was copied from currently-existing, already-passing sibling tests in the same file (static_nonlegendary_creatures_enchanted_player_controls_base_pt_and_lose_types/ Curse of Conformity for the loss-enumeration shape;lands_with_chosen_name_grant_quoted_ability/ Petrified Hamlet for theAnd[Typed(Land), HasChosenName]subject shape), not invented.data/mtgjson/AtomicCards.jsonand cross-checked byte-for-byte against the quotes in this PR and in parser: "loses all land types and abilities" enumeration doesn't recognize "land types" (Alpine Moon, Lithoform Blight, Ultima) #6270.I'd appreciate CI (and/or a maintainer with a working local toolchain) confirming the actual
cargo test -p engine --lib parser::/cargo clippyrun, since I was not able to produce that signal myself this session.Track
Developer
LLM
Model: claude-sonnet-5