fix(parser): accept U+2019 apostrophe in mana-ability activation exemption#5217
Conversation
…ption MTGJSON oracle text carries the typographic apostrophe (U+2019), and there is no global apostrophe normalization in the parser pipeline — the `can't be activated` predicate combinators already dual-branch for this reason. But the CR 605.1a "unless they're mana abilities" exemption suffix was straight-apostrophe only, so a U+2019 printing silently lost the carve-out and the runtime wrongly BLOCKED mana abilities the rules require to stay activatable (Bound by Moonsilver, Faith's Fetters-shaped auras, Suppression Field; and the chosen-name predicate itself dropped Pithing Needle / Phyrexian Revoker / Sorcerous Spyglass under U+2019). - Add `parse_mana_ability_exemption_suffix`, the single dual-apostrophe authority, and route all three exemption-suffix sites through it (restriction.rs `parse_activation_exemption_suffix`, shared.rs `parse_activation_compound_tail`, dispatch.rs cost-modification). - Accept both glyphs on the chosen-name and type-list `can't be activated` predicates so Pithing Needle et al. parse under U+2019. The dual-apostrophe branches only ADD coverage — the ASCII path is unchanged. Tests: the shared combinator across both glyphs, and a U+2019 Pithing Needle through `parse_static_line` keeping the mana exemption. Closes phase-rs#4999
![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 updates the static oracle parser to support both ASCII (') and typographic (U+2019) apostrophes in 'can't be activated' predicates and 'unless they're mana abilities' exemption suffixes. It introduces a shared parse_mana_ability_exemption_suffix parser combinator to handle this dual-apostrophe matching across all relevant exemption sites and adds corresponding unit tests to verify the behavior. No review comments were provided, so there is no feedback to address.
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✓ No card-parse changes detected. |
matthewevans
left a comment
matthewevans
left a comment
There was a problem hiding this comment.
[MED] The U+2019 path still drops the mana-ability exemption for the attached/self-reference activation-prohibition branches. Evidence: crates/engine/src/parser/oracle_static/shared.rs:1305 and crates/engine/src/parser/oracle_static/dispatch.rs:2176 still gate on scan_contains(..., "activated abilities can't be activated"), and crates/engine/src/parser/oracle_static/restriction.rs:301 still scans only tag("can't be activated") before parsing the new dual-apostrophe suffix helper. Why it matters: the PR body names the Faith's Fetters / Bound by Moonsilver attached-permanent class, but MTGJSON-style can’t ... unless they’re mana abilities still either misses the compound branch or records ActivationExemption::None, so mana abilities remain blocked despite CR 605.1a. Suggested fix: route the predicate match through a shared dual-apostrophe combinator in every caller, including parse_cant_be_activated_exemption_in_text, and add a production-path test for an attached/self-reference line with U+2019 in both can’t and they’re.
…-apostrophe helper
Follow-up to the U+2019 mana-ability exemption fix: the self-reference
and compound-Aura activation-prohibition branches still gated on the
straight-apostrophe predicate, so a U+2019 printing failed the
`activated abilities can't be activated` scan (dropping the whole
static) or recorded `ActivationExemption::None` (wrongly blocking mana
abilities, CR 605.1a).
Add shared dual-apostrophe predicate authorities in `shared.rs`
(`parse_cant_be_activated_predicate`, `parse_activated_abilities_cant_be_activated`,
`contains_activated_abilities_cant_be_activated`) and route every caller
through them:
- `restriction.rs::parse_cant_be_activated_exemption_in_text` (exemption re-scan)
- `dispatch.rs` self-reference gate
- `shared.rs` compound-Aura gate + `parse_activation_compound_tail`
Test: a U+2019 self-reference line ("Its activated abilities can't be
activated unless they're mana abilities." with U+2019 in both `can't`
and `they're`) through `parse_static_line` keeps
`ActivationExemption::ManaAbilities`.
matthewevans
left a comment
matthewevans
left a comment
There was a problem hiding this comment.
[MED] Effect-form activation prohibitions still miss U+2019 predicates before the shared mana-exemption scan. Evidence: crates/engine/src/parser/oracle_effect/subject.rs:1373 and crates/engine/src/parser/oracle_effect/sequence.rs:2534. Why it matters: target creature’s activated abilities can’t be activated unless they’re mana abilities never reaches parse_cant_be_activated_exemption_in_text, so the same U+2019 mana-ability carve-out fixed for static lines still fails for temporary effect clauses. Suggested fix: route the effect subject splitter and conjunct-start predicate through the same dual-apostrophe activation predicate helper, then add a U+2019 effect-path test with the mana exemption.
The effect-form activation prohibitions (temporary clauses like "target creature's activated abilities can't be activated unless they're mana abilities") still matched the predicate with the straight ASCII apostrophe only, so a U+2019 printing never reached the shared `parse_cant_be_activated_exemption_in_text` scan and dropped the CR 605.1a mana-ability carve-out. - oracle_effect/subject.rs: split the effect subject on the predicate with either apostrophe glyph (`.or_else` on the U+2019 form). - oracle_effect/sequence.rs: route the possessive-anaphor conjunct-start predicate through the shared `parse_activated_abilities_cant_be_activated` dual-apostrophe combinator (re-exported from oracle_static). Test: a U+2019 effect clause through `parse_effect_chain` still yields a transient `CantBeActivated` static with `ActivationExemption::ManaAbilities`.
… guard
Root cause of the effect-path failure: the "supported unless clause"
guard that lets an activation-prohibition mana-exemption bypass the
unsupported-unless fallback matched "they're mana abilities" with the
straight apostrophe only. A U+2019 clause ("target creature's activated
abilities can't be activated unless they're mana abilities") fell through
to `Effect::unimplemented("Unsupported unless clause", …)` before the
restriction parser (and its dual-apostrophe split) ever ran.
Make the guard dual-apostrophe so the U+2019 exemption is recognized and
the clause routes to the restriction parser, which records
`ActivationExemption::ManaAbilities`.
matthewevans
left a comment
matthewevans
left a comment
There was a problem hiding this comment.
[MED] The current head still has unexplained parse-diff blast radius outside the U+2019 mana-ability exemption scope. Evidence: the current <!-- coverage-parse-diff --> comment reports 21 cards / 25 signatures changing, including Attach target rewrites, token creation removals/additions, Life and Limb, Mob Mentality, Dance with Calamity, Winter Blast, and multiple unrelated ability removals, while the code diff is scoped to activation-prohibition apostrophe handling. Why it matters: parser coverage can only be trusted if the card-level diff matches the claimed grammar class; these unrelated card changes may be stale contamination or unintended parser behavior. Suggested fix: rebase/update the branch so the parse-diff is regenerated against current main and reduce/explain the diff so only the activation-exemption class changes remain.
@matthewevans |
matthewevans
left a comment
matthewevans
left a comment
There was a problem hiding this comment.
Maintainer review: current head resolves the prior U+2019 predicate/suffix blockers, keeps the change at the shared parser seam, and the current parse-diff sticky shows no card-parse blast radius.
2 participants
Summary
Closes #4999
The CR 605.1a mana-ability exemption suffix "unless they're mana abilities" was parsed with the straight ASCII apostrophe only. MTGJSON oracle text carries the typographic apostrophe (U+2019), and there is no global apostrophe normalization in the parser pipeline — which is exactly why the
can't be activatedpredicate combinators already dual-branch. So a U+2019 printing silently lost the exemption (ActivationExemption::None), and the runtime then wrongly blocked the permanent's mana abilities — a rules violation, not just a coverage miss.Affected cards (a class)
…and its activated abilities can't be activated unless they're mana abilities.(compound → evasion path, dual-apostrophe predicate, straight-only suffix).Activated abilities cost {2} more to activate unless they're mana abilities.(cost-modification path).Activated abilities of sources with the chosen name can't be activated unless they're mana abilities.(chosen-name predicate was itself straight-only, so U+2019 dropped the whole static).Fix
parse_mana_ability_exemption_suffix— the single dual-apostrophe authority for the exemption suffix — and route all three suffix sites through it:restriction.rs::parse_activation_exemption_suffix,shared.rs::parse_activation_compound_tail, and thedispatch.rscost-modification path.can't be activatedpredicates too, so Pithing Needle et al. parse under U+2019 (mirroring the dual-branch already used inevasion.rs/parse_activation_compound_tail).Every added branch only adds U+2019 coverage — the ASCII path is untouched.
Tests
mana_ability_exemption_suffix_accepts_both_apostrophes— the shared combinator matches both glyphs, fully consumes, and rejects a non-mana carve-out.cant_be_activated_chosen_name_typographic_apostrophe_keeps_mana_exemption— a U+2019 Pithing Needle throughparse_static_linestill yieldsActivationExemption::ManaAbilities(revert-failing: straight-only tags make it parse toNoneor drop the exemption).CR
CR 605.1a — mana abilities remain activatable under a "can't be activated" static that carves them out, regardless of apostrophe glyph.