Fix Veteran Bodyguard / Weathered Bodyguards — dropped tap-gate + unblocked/combat source restriction on damage redirection

[rykerwilliams]

Summary

Fixes the "Replacement / prevention / 'instead' effect mis-modeled" misparse for Veteran Bodyguard and Weathered Bodyguards.

Both cards use the shape "As long as this creature is untapped, all [combat] damage that would be dealt to you [by unblocked creatures] is dealt to this creature instead." The parser was dropping three things: the leading "as long as untapped" condition gate (CR 604.2), the "by unblocked creatures" source-filter restriction (CR 509.1h), and the "combat damage" vs. plain "damage" scope distinction. Net effect before this fix: both cards redirected all damage to their controller, unconditionally, regardless of tapped state or source — strictly wrong per their printed text.

parse_damage_redirection_replacement (crates/engine/src/parser/oracle_replacement.rs) now parses all three, reusing pre-existing types verbatim — no new enum variants or struct fields anywhere, including types/ability.rs:

• ReplacementCondition::SourceTappedState { tapped: bool } for the tap-gate
• FilterProp::Unblocked for the "unblocked creatures" source restriction (via the pre-existing parse_damage_source_subject_filter → parse_type_phrase → parse_combat_status_prefix chain)
• CombatDamageScope::CombatOnly for Weathered Bodyguards' "combat damage" scope (via the pre-existing scan_combat_scope)

Palisade Giant is deliberately excluded from the source-filter class — its real Oracle text has no "unblocked" restriction — and has its own dedicated regression-guard test proving the exclusion.

Also renamed strip_as_long_as_prefix_for_prevention → strip_as_long_as_condition_prefix, since it is now shared between the prevention and redirection parsers (it was previously only used by the former).

Discovered pre-existing bug (out of scope)

While tracing the runtime path, I found that game/replacement.rs's damage_done_applier only reads the shield's redirect_target field inside its ShieldKind::Redirection branch (~line 1190-1193, CR 614.9). This entire card class — Pariah, Palisade Giant, Veteran Bodyguard, Weathered Bodyguards — parses to ShieldKind::Prevention with redirect_target: SelfRef as a separate field. The ShieldKind::Prevention branch (~line 1336-1349) never reads redirect_target and just returns ApplyResult::Prevented (~1382-1383). Net effect: this whole "damage is dealt to X instead" class only ever prevents damage to the controller — the "instead" creature never actually takes the redirected damage, can never die from it, and never triggers "dealt damage" abilities off it.

This is real, but it's in shared runtime resolver code affecting multiple already-shipped cards and needs its own dedicated plan+review cycle — it is not fixed by this PR, which is scoped to the parser only (tap-gate + source-filter + combat-scope, all upstream of ShieldKind dispatch and confirmed live/consumed at the replacement-candidate-eligibility gate regardless of this gap). The new tests document this in code comments and intentionally assert the life-total delta rather than damage_marked on the Bodyguard as their discriminator, so they don't silently mask the gap.

Files changed

• crates/engine/src/parser/oracle_replacement.rs
• crates/engine/tests/integration/main.rs
• crates/engine/tests/integration/veteran_bodyguard_tap_redirect.rs (new)
• docs/parser-misparse-backlog.md

Gate A

$ ./scripts/check-parser-combinators.sh
(exit 0, no output — no violations)

Anchored on

• crates/engine/src/parser/oracle_replacement.rs:8360 — existing parse_damage_prevention_replacement, the direct precedent this PR mirrors for adding a leading "as long as" tap-gate to a damage replacement parser.
• crates/engine/src/parser/oracle_replacement.rs:5937 — existing parse_damage_source_subject_filter, reused as-is (not reimplemented) to resolve the "unblocked creatures" source clause via its existing fallthrough to parse_type_phrase's combat-status recognition.
• crates/engine/src/parser/oracle_replacement.rs:6268 — existing scan_combat_scope, reused as-is to resolve the "combat damage" vs. plain "damage" scope distinction for Weathered Bodyguards.

CR references

• CR 604.2 — static ability's own "as long as" continuous-effect activation gate
• CR 614.9 — damage redirection effects
• CR 509.1h — blocked/unblocked creature definition
• CR 614.1a — "would deal damage... instead" replacement shape
• CR 615.1a — prevent + redirect combination
• CR 510.1c — blocked creature assigns damage to blockers (new trample discriminator test)
• CR 702.19b — trample excess damage assignment (new trample discriminator test)

Track

Developer

LLM

Model: claude-sonnet-5
Thinking: high

Verification

• cargo fmt --all — clean
• cargo clippy -p engine --all-targets -- -D warnings — clean, zero warnings (confirmed post-rebase)
• ./scripts/check-parser-combinators.sh — clean, exit 0
• cargo test -p engine --lib — 16041 passed, 0 failed, 6 ignored (confirmed post-rebase)
• cargo test -p engine --test integration -- veteran_bodyguard — 3 passed, 0 failed (confirmed post-rebase): tap-gate positive/negative pair plus a new discriminating test proving a blocked+trampling attacker's excess damage to the defending player is correctly not caught by the shield (the "unblocked creatures" source filter axis)
• Independent /review-impl — 2 rounds. Round 1 found zero blocking findings but flagged one non-blocking test-coverage gap (the source-filter and combat-scope axes were only proven at the parser-AST level, not through a full combat scenario). Addressed by adding the trample discriminator test. Round 2 (fresh reviewer, fresh context) confirmed clean, independently re-verified all CR citations and traced the new test's production path end-to-end.

Scope Expansion

None.

Validation Failures

None.

CI Failures

None.

[gemini-code-assist]

Code Review

This pull request implements parsing support for damage redirection replacement effects with optional source restrictions and tap-state conditions (e.g., Veteran Bodyguard and Weathered Bodyguards), along with corresponding integration and unit tests. Feedback highlights a need to correct a Comprehensive Rules citation for combat damage (CR 120.2a instead of CR 615) and to ensure that the extracted tap-state condition is not ignored in Pattern 3 redirection effects.

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]

The comment cites CR 615 (Prevention Effects) for the 'combat damage' qualifier, but combat damage is defined under CR 120.2a (or more generally CR 510). To adhere to rule R6, please update the citation to reference the correct section of the Comprehensive Rules.

Suggested change

	// CR 615: optional "combat damage" qualifier scopes to combat damage only.
	// CR 120.2a: optional "combat damage" qualifier scopes to combat damage only.

References

1. CR annotations are mandatory and verified (Rule R6). ^(link)

[gemini-code-assist]

The prefix_condition (extracted from the 'as long as' gate) is currently ignored in Pattern 3. If a card matching Pattern 3 has an 'as long as' condition, the condition will be silently discarded. Consider refactoring Pattern 3 to construct the ReplacementDefinition as a mutable variable and conditionally attach prefix_condition via .condition(cond) before returning.

[github-actions]

Parse changes introduced by this PR

✓ No card-parse changes detected.

[matthewevans]

Approving — this is high-quality work.

Verified:

• Card text Scryfall-verified verbatim for both Veteran Bodyguard and Weathered Bodyguards (and Palisade Giant's current Oracle, correctly noting its recipient-list half is a separate pre-existing bug).
• All six CR annotations grep-verified against the Comprehensive Rules: 604.2 (static continuous/replacement effects), 614.9 (damage-redirection replacement), 509.1h (attacker-with-blockers becomes blocked), 510.1c + 702.19b (trample assignment), 509.1g.
• Building-block reuse — the source clause delegates to the existing parse_damage_source_subject_filter → parse_type_phrase path (no new unblocked-detection), and the "as long as" gate lifter is shared cleanly via the strip_as_long_as_condition_prefix rename rather than duplicated.
• Both new-set fields are runtime-live: damage_source_filter (replacement.rs:4816/5339) and combat_scope (replacement.rs:4829-4834/5359-5363, CombatOnly if !is_combat) are consumed by the resolver — not inert. The blocked-trample integration test is a genuine end-to-end discriminator proving the Unblocked source filter gates at runtime.

On the disclosed gap: your honesty about redirect_target: SelfRef being dead data for the whole prevent-only class (ShieldKind::Prevention never redirects — Pariah/Palisade Giant included) is exactly right and appreciated. That's a pre-existing, class-wide runtime-resolver gap; this parser-scoped PR doesn't touch or worsen it, and the in-scope restrictions (source/combat/tap-gate) are all consumed and tested. Correct call to defer it as a separate follow-up.

Non-vacuous negative tests with a positive reach-guard, real Oracle text, delegated building blocks — thank you.