feat(parser): parse mixed variable+fixed P/T grant (Cranial Ram +X/+1)#5447
Conversation
parse_variable_pt_pattern restricted each P/T axis to `x`/`0`, so a mixed grant such as Cranial Ram's "Equipped creature gets +X/+1, where X is the number of artifacts you control" failed the pattern outright and the whole equip static was dropped. Generalize each axis to carry either the variable X (dynamic) or a fixed integer magnitude. parse_dynamic_pt_in_text now emits the constant modification (AddPower/AddToughness) for a fixed nonzero axis alongside the X-valued modification for the dynamic axis. A fixed 0 axis still contributes nothing, preserving +X/+0, -X/-X and the other existing forms (covered by the unchanged dynamic-pt tests).
![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 parser to support mixed dynamic and fixed power/toughness modifications (such as "+X/+1") under CR 613.4c. It refactors parse_variable_pt_pattern to return optional magnitudes for each axis and updates the parser logic to apply either dynamic or constant modifications accordingly. Feedback suggests using nom::combinator::map_res instead of map with unwrap_or(0) to properly propagate integer parsing errors instead of swallowing them.
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 · 1 card(s), 2 signature(s) (baseline: main
|
matthewevans
left a comment
matthewevans
left a comment
There was a problem hiding this comment.
Maintainer review clean: the mixed X/fixed P/T parser change is at the shared dynamic-P/T seam, Gemini’s stale map_res finding is fixed at head, parse-diff is scoped to Cranial Ram, and required checks are green.
2 participants
Problem
parse_variable_pt_patternrestricted each P/T axis of a variable grant toxor0. A mixed grant — one dynamic axis and one fixed nonzero axis — failed the pattern outright, so the whole static was dropped.The clearest example is Cranial Ram:
Equipped creature gets +X/+1, where X is the number of artifacts you control.The+1toughness could not be represented, so the equip static produced no P/T modification at all.Fix
parse_variable_pt_pattern(grammar.rs) now parses each axis as either the variable X (dynamic, returned asNone) or a fixed integer magnitude (Some(n)).parse_dynamic_pt_in_text(anthem.rs) emits a constantAddPower/AddToughnessfor a fixed nonzero axis alongside theAddDynamicPower/AddDynamicToughnessfor the dynamic axis (CR 613.4c layer 7c).0axis still contributes nothing, so+X/+0,-X/-X,+0/+X, etc. are unchanged.Tests
dynamic_pt_in_text_mixed_dynamic_power_fixed_toughnessasserts+X/+1emits both the X power modification and a constantAddToughness { value: 1 }(and that the fixed axis is not dynamic).+X/+0,-X/-0,-X/-X,+0/+X, cost-X defaults) remain green — the fixed-0path preserves their behavior.parser::oracle_staticmodule: 1070 passed.cargo fmt, clippy (-D warnings), and the parser-combinator / engine-authorities gates are clean.