feat(engine): roll-a-d20-for-each-player-keep-highest attack mechanic (Iron Mastiff #5928)

[shin-core]

Fixes #5928 (claimed there). Verified Iron Mastiff's Oracle text on Scryfall and reproduced the gap via a parse probe before building.

The feature

Iron Mastiff: "Whenever this creature attacks, roll a d20 for each player being attacked and ignore all but the highest roll. / 1—9 | ... deals damage equal to its power to you. / 10—19 | ... to defending player. / 20 | ... to each opponent." The Attacks trigger parsed, but its effect was Unimplemented("roll", "roll a d20 for each player being attacked and ignore all but the highest roll") and the three outcome rows landed as detached abilities — the roll-header parser deliberately bailed on any suffix other than "and add/subtract X", so try_parse_die_roll_table never attached the table.

Design (on the existing Effect::RollDie result-table infra)

• DieRollAggregate { EachIndependently, Highest } + a keep field on Effect::RollDie (serde default EachIndependently, skip-if-default so all existing d20 card-data stays byte-identical and every construction site keeps today's per-die semantics). add-engine-variant gate: Stage 1 DOES_NOT_EXIST, Stage 2 EXTEND_OK (leaf axis on result-lookup), Stage 3 WITHIN_SECTION (CR 706).
• Resolver: for Highest, roll all count dice (each still emits DieRolled + applies the modifier), then consult the result table exactly once for the maximum actual result — CR 706.6 ("the ignored rolls are treated as never having happened"). EachIndependently (every existing d20 card) is unchanged.
• Parser: a nom combinator recognizes the "for each player being attacked and ignore all but the highest roll" suffix so the ranged table attaches, lowering to RollDie { sides: 20, keep: Highest, count: PlayerCount { OpponentAttacked { subject: Source, scope: ThisCombat } } }. The count is the players this creature is attacking this combat (CR 508.6) — so a creature forced to attack multiple players honestly rolls N dice, not a hardcoded 1. Zero Unimplemented.

General building block — covers the "roll a d20 for each player attacked → outcome table, keep highest" dice-attack class, not just Iron Mastiff.

A rule correction worth noting

The plan I worked from cited "CR 706.4b" for keep-highest — that subrule doesn't exist in the CR. The correct rule is CR 706.6 ("If a player is instructed to ignore a roll, that roll is considered to have never happened..."), grep-verified and used throughout.

Tests (integration, real combat + seeded-roll pipeline; each revert-probed)

• parser shape: RollDie{sides:20, keep:Highest} with 3 branches (1-9 / 10-19 / 20), no detached abilities, each branch damage-bearing (not Unimplemented).
• low roll (1—9) → power to the controller; mid (10—19) → to the defending player; max (20) → to each opponent — each asserts the correct player's life dropped and the others' did not (life read before combat damage so the 4/4's own damage doesn't confound).
• keep-highest unit test: two distinct dice + a max-only branch → the table is consulted once for the max. Revert-probed: forcing per-die application makes it fire twice (2 vs 1), so the test genuinely guards the CR 706.6 semantic.
• regressions: Ancient Brass/Copper/Bronze Dragon, Vexing Puzzlebox, Bucknard's stay green.

Verification

Full engine lib: 17300 passed, 0 failed. Integration: 3589 passed, 0 failed. Clippy -D warnings clean; fmt clean; parser combinator gate PASS. Card-data regenerates Iron Mastiff with the correct RollDie shape and zero Unimplemented; the diff is feature-only (unrelated token-data regeneration churn reverted).

🤖 Generated with Claude Code

[superagent-security]

🚨 Contributor flagged. Click here for more info: Superagent Dashboard

[gemini-code-assist]

Code Review

This pull request implements support for the "keep-highest" die-rolling aggregation mechanism (CR 706.6), specifically addressing the parsing and resolution of abilities like Iron Mastiff's trigger ("roll a d20 for each player being attacked and ignore all but the highest roll"). It introduces the DieRollAggregate enum, updates the RollDie effect to handle both independent and keep-highest resolution, and adds comprehensive unit and integration tests. Feedback on the changes highlights a potential parsing bug in try_parse_roll_keep_highest_for_each where trimming trailing whitespace after punctuation matches is performed in the wrong order, which could cause valid inputs with trailing spaces to fail parsing.

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.

[gemini-code-assist]

Trimming the tail after calling trim_end_matches can cause parsing to fail if there is any trailing whitespace after the punctuation (e.g., ". " or ". "). Since trim_end_matches only strips from the very end of the string, the trailing whitespace prevents it from matching the punctuation, leaving the punctuation intact and causing the subsequent .trim().is_empty() check to return false (and thus returning None). Trimming the whitespace first resolves this robustly.

Suggested change

	if !tail.trim_end_matches(['.', ',', ';']).trim().is_empty() {
	return None;
	}
	if !tail.trim().trim_end_matches(['.', ',', ';']).is_empty() {
	return None;
	}

[matthewevans]

Blocked: the keep-highest path exposes ignored dice as real DieRolled events.

🔴 Blocker

[HIGH] resolve calls roll_die for every die (crates/engine/src/game/effects/roll_die.rs:65-69), and roll_die immediately pushes GameEvent::DieRolled (:16-29). The later Highest branch only suppresses result-table lookup (:114-122), so a multiplayer Iron Mastiff attack still triggers every “whenever you roll a die” ability for dice that were instructed to be ignored. CR 706.6 says an ignored roll is considered never to have happened and that no abilities trigger because of it. The new unit test codifies the opposite expectation by asserting two DieRolled events (:1147-1153); the integration fixture is two-player only (iron_mastiff_roll_d20_attack_5928.rs:45-100), so it never covers this path.

Please sample all dice without publishing roll events, then publish/state-stamp only the kept highest result for DieRollAggregate::Highest (while retaining the existing per-die behavior). Add a 3-player/multiple-defender regression that asserts exactly one numeric d20 roll event and that only the kept roll can trigger a die-roll witness.

✅ Clean

The reported trailing-whitespace parser concern does not reproduce through the public parser path: parse_effect_clause_inner normalizes with trim() before it reaches this recognizer (crates/engine/src/parser/oracle_effect/mod.rs:7316-7320). The Oracle text matches Scryfall’s current Iron Mastiff record.

Recommendation: request changes before enqueueing.

[github-actions]

Parse changes introduced by this PR · 1 card(s), 3 signature(s) (baseline: main 00241f221e92)

🟢 Added (1 signature)

• 1 card · ➕ ability/RollDie · added: RollDie (branches=3, count=# of each opponent this source attacked this combat, keep=highest, sides=20)
  • Affected (first 3): Iron Mastiff

🔴 Removed (2 signatures)

• 1 card · ➖ ability/roll · removed: roll
  • Affected (first 3): Iron Mastiff
• 1 card · ➖ ability/unknown · removed: unknown
  • Affected (first 3): Iron Mastiff

1 card(s) had Oracle-text changes (errata/reprint) — excluded as non-parser.