Add Baldin toughness damage static - #3147
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds support for parsing static abilities that apply toughness-based combat damage specifically during the controller's turn, along with a corresponding unit test. The review feedback correctly points out that the implementation violates repository style guidelines by using fragile verbatim string matching instead of modular nom-based combinators, and by omitting mandatory Comprehensive Rules (CR) annotations.
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.
| if let Some(rest_tp) = nom_tag_tp(&tp, "during your turn, ") { | ||
| if let Some(def) = parse_assigns_damage_from_toughness(rest_tp.lower, rest_tp.original) { | ||
| return Some( | ||
| def.condition(StaticCondition::DuringYourTurn) | ||
| .description(text.to_string()), | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
[HIGH] Missing mandatory CR annotation for new game-rule parser dispatch, and usage of fragile verbatim string matching for Oracle phrases.
Why it matters:
- Every rules-touching line of engine code must carry a verified
CR <number>: <description>comment to ensure strict fidelity to the MTG Comprehensive Rules. - Verbatim string equality (like matching "during your turn, " directly) bypasses the robust nom-based parser, creating fragile matches. Compound phrases should be decomposed into modular, reusable parsers.
Suggested fix: Add CR 510.1c and CR 611.3a annotations, and refactor the parser to use modular combinators instead of verbatim string matching.
// CR 510.1c: Doran-class effects that cause creatures to use toughness for combat damage.
// CR 611.3a: A continuous effect applies while its condition is true.
if let Ok((rest, cond)) = parse_condition(&tp) {
if let Some(def) = parse_assigns_damage_from_toughness(rest.lower, rest.original) {
return Some(
def.condition(cond)
.description(text.to_string()),
);
}
}References
- Rule R6: Every rules-touching line of engine code must carry a comment of the form
CR <number>: <description>. (link) - Avoid verbatim string equality for parsing Oracle phrases as it bypasses the robust nom-based parser and creates fragile matches. Instead, decompose compound phrases into modular, reusable parsers for constituent parts and compose them using idiomatic combinator aggregates.
matthewevans
left a comment
There was a problem hiding this comment.
Approved. This is the right seam for the Baldin wording: it reuses the existing Doran-class AssignDamageFromToughness static and composes the during-your-turn condition axis around that parser instead of adding a new runtime primitive. I verified CR 510.1c / CR 611.3a against docs/MagicCompRules.txt, checked the coverage-data wording, and ran the focused toughness-damage parser tests locally; GitHub CI is also green.
For future PRs, please sync with origin/main before opening/updating and use the /engine-implementer skill for non-trivial parser or engine work so the plan, implementation, and review all stay aligned with the project architecture.
f81cfaf to
09b8025
Compare
Summary
Adds the missing turn-gated form for Doran-style toughness combat damage statics:
During your turn, each creature assigns combat damage equal to its toughness rather than its power.parse_assigns_damage_from_toughnesspath and attachesStaticCondition::DuringYourTurnRules / implementation notes
AssignDamageFromToughnessmodification.Validation
cargo fmt --all./scripts/check-parser-combinators.shcargo test -p engine static_assigns_damage_from_toughness_all_creatures_during_your_turn --libcargo test -p engine static_assigns_damage_from_toughness --liboracle-gen --filter 'Baldin, Century Herdmaster'exportcoverage-reportover that filtered export showedBaldin, Century Herdmasterassupported=true,gap_count=0