fix(parser): scope another-opponent attack triggers - #8919
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. 📝 WalkthroughWalkthroughThe parser now shares handling for “one of your opponents,” including an optional “another ” prefix. Parser and integration tests verify Karazikar’s attack-trigger scope in a three-player scenario. ChangesOpponent attack triggers
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The scoped parser and regression-test changes have no identified merge-blocking risk. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. (2 skipped: 2 too large.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Generated for head Parse changes introduced by this PR · 1 card(s), 2 signature(s) (baseline: main
|
|
@matthewevans Re-requesting review: all Rust shards are terminal and green, and the current-head parse-diff receipt is now published for |
matthewevans
left a comment
There was a problem hiding this comment.
Approved at ced328b84113c842b7db6917de5a623fa42604ac: the current parse-diff is limited to Karazikar's intended player attack-target and opponent-valid-target fields, and all required checks are green.
Summary
Karazikar's second ability lost its defending-player restriction because the shared attack-target grammar recognized
one of your opponentsbut notanother one of your opponents. Extend that composable noun phrase so these triggers retain both existing constraints: the attacker is an opponent, and the attacked player is also an opponent of the ability's controller.Add parser-shape and three-player production regressions covering both sides of the rule: an opponent attacking the controller does not trigger Karazikar, while that opponent attacking another opponent does. The separately unsupported compound effect body remains explicitly
Unimplementedand outside this change.Addresses #7189.
Verification
cargo fmt --all./scripts/check-parser-combinators.sh HEADcargo clippy --all-targets -- -D warningscargo test -p phase-engine(21,403 unit tests, 7,071 integration tests, doc tests)MTGJSON_SKIP_REFRESH=1 ./scripts/gen-card-data.sh(35,977 faces validated)anothergrammar makes the P1→P0 regression fail with one spurious trigger; restoring it makes both multiplayer cases passSummary by CodeRabbit
Bug Fixes
Tests