From e8da580fb9f0406413e9adcbe49235d594cfad72 Mon Sep 17 00:00:00 2001 From: philluiz2323 Date: Wed, 15 Jul 2026 21:34:37 +0400 Subject: [PATCH 1/2] fix(engine): add active-voice "that dealt damage this turn" creature filter (#5615) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Red Guardian, Super-Soldier reads "When Red Guardian enters, destroy target creature an opponent controls that dealt damage this turn", but it could destroy a creature that had not dealt any damage. The parser only recognized the PASSIVE "that was dealt damage this turn" (`FilterProp::WasDealtDamageThisTurn` — the creature is the damage recipient). Red Guardian's clause is the ACTIVE voice — the creature is the damage SOURCE — which had no filter, so it was dropped and the target collapsed to "creature an opponent controls" (any). Add `FilterProp::DealtDamageThisTurn` (CR 120.1: "an object that deals damage is the source of that damage"). It reads the same per-turn ledger `state.damage_dealt_this_turn` the passive filter reads, matching on `source_id` instead of `target`. Wired through the parser verb-phrase table, the live and LKI filter evaluators, and every exhaustive classification site (read/write profile, projected-resource axis, AI memoization poison set, swallow-check, event-support, coverage emitter). The two form a damage-role pair; a doc-comment records that a third damage-role filter (e.g. "dealt combat damage") should fold the pair into `DamageThisTurn { role, combat_only }` per the sibling-cluster threshold. Verified: cargo test -p engine --lib -> 16675 passed, 0 failed. New tests cover the parser (active vs passive voice) and the filter evaluator (damage source matches, recipient does not, and the mirror for the passive filter). --- crates/engine/src/ai_support/filter.rs | 1 + crates/engine/src/game/ability_rw.rs | 2 + crates/engine/src/game/ability_scan.rs | 5 ++ crates/engine/src/game/coverage.rs | 3 +- crates/engine/src/game/filter.rs | 62 +++++++++++++++++++++++ crates/engine/src/parser/oracle_target.rs | 34 +++++++++++++ crates/engine/src/parser/swallow_check.rs | 1 + crates/engine/src/types/ability.rs | 12 +++++ crates/engine/src/types/events.rs | 1 + 9 files changed, 120 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/ai_support/filter.rs b/crates/engine/src/ai_support/filter.rs index d7e89ae616..8a1c592a7c 100644 --- a/crates/engine/src/ai_support/filter.rs +++ b/crates/engine/src/ai_support/filter.rs @@ -768,6 +768,7 @@ fn filterprop_reads_only_candidate_fp(p: &FilterProp) -> bool { | FilterProp::AttackingAlone | FilterProp::BlockingAlone | FilterProp::WasDealtDamageThisTurn + | FilterProp::DealtDamageThisTurn | FilterProp::EnteredThisTurn // CR 302.6: per-turn control-continuity marker — a turn/history-relative // predicate like the siblings here (POISON for memoization). diff --git a/crates/engine/src/game/ability_rw.rs b/crates/engine/src/game/ability_rw.rs index ce0190f39d..ff736a8b9b 100644 --- a/crates/engine/src/game/ability_rw.rs +++ b/crates/engine/src/game/ability_rw.rs @@ -2359,6 +2359,7 @@ fn legacy_filter_prop(p: &FilterProp) -> bool { | FilterProp::PowerExceedsBase | FilterProp::InAnyZone { .. } | FilterProp::WasDealtDamageThisTurn + | FilterProp::DealtDamageThisTurn | FilterProp::EnteredThisTurn | FilterProp::ControlledContinuouslySinceTurnBegan | FilterProp::ZoneChangedThisTurn { .. } @@ -2617,6 +2618,7 @@ fn member_bound_filter_prop(p: &FilterProp) -> bool { | FilterProp::PowerExceedsBase | FilterProp::InAnyZone { .. } | FilterProp::WasDealtDamageThisTurn + | FilterProp::DealtDamageThisTurn | FilterProp::EnteredThisTurn | FilterProp::ControlledContinuouslySinceTurnBegan | FilterProp::ZoneChangedThisTurn { .. } diff --git a/crates/engine/src/game/ability_scan.rs b/crates/engine/src/game/ability_scan.rs index d3b58f7f46..777a77b0bc 100644 --- a/crates/engine/src/game/ability_scan.rs +++ b/crates/engine/src/game/ability_scan.rs @@ -3246,6 +3246,11 @@ fn scan_filter_prop(x: &FilterProp) -> Axes { // has `damage_marked == 0` yet a persistent journal record, so gate (1) cannot // backstop this read — PROVEN projected, fail closed. FilterProp::WasDealtDamageThisTurn => Axes::CONSERVATIVE, + // CR 120.1: reads `state.damage_dealt_this_turn`, the same append-only + // per-turn journal a loop pumps and `project_out_resources` clears — a + // projected-resource read, PROVEN projected, fail closed (mirrors the + // passive `WasDealtDamageThisTurn` arm above). + FilterProp::DealtDamageThisTurn => Axes::CONSERVATIVE, // CR 400 / CR 603.6a: runtime eval reads `state.zone_changes_this_turn`, an // append-only event journal a loop pumps, cleared by `project_out_resources` // and strict-compared by nothing in gate (1). A flicker/blink loop keeps the diff --git a/crates/engine/src/game/coverage.rs b/crates/engine/src/game/coverage.rs index 3942f97bf8..72c4db0602 100644 --- a/crates/engine/src/game/coverage.rs +++ b/crates/engine/src/game/coverage.rs @@ -943,7 +943,8 @@ fn fmt_typed_filter(tf: &TypedFilter) -> String { }; parts.push(format!("{prefix} {name}{suffix}")); } - FilterProp::WasDealtDamageThisTurn => parts.push("dealt damage this turn".into()), + FilterProp::WasDealtDamageThisTurn => parts.push("was dealt damage this turn".into()), + FilterProp::DealtDamageThisTurn => parts.push("dealt damage this turn".into()), FilterProp::EnteredThisTurn => parts.push("entered this turn".into()), FilterProp::ControlledContinuouslySinceTurnBegan => { parts.push("controlled continuously since turn began".into()) diff --git a/crates/engine/src/game/filter.rs b/crates/engine/src/game/filter.rs index d3f914bfe5..db764d5a12 100644 --- a/crates/engine/src/game/filter.rs +++ b/crates/engine/src/game/filter.rs @@ -223,6 +223,7 @@ fn filter_prop_uses_object_population(prop: &FilterProp) -> bool { | FilterProp::NotHistoric | FilterProp::InAnyZone { .. } | FilterProp::WasDealtDamageThisTurn + | FilterProp::DealtDamageThisTurn | FilterProp::EnteredThisTurn | FilterProp::ControlledContinuouslySinceTurnBegan | FilterProp::ZoneChangedThisTurn { .. } @@ -462,6 +463,7 @@ fn entered_object_perturbs_filter_prop( | FilterProp::NotHistoric | FilterProp::InAnyZone { .. } | FilterProp::WasDealtDamageThisTurn + | FilterProp::DealtDamageThisTurn | FilterProp::EnteredThisTurn | FilterProp::ControlledContinuouslySinceTurnBegan | FilterProp::ZoneChangedThisTurn { .. } @@ -3329,6 +3331,7 @@ fn spell_record_matches_property(record: &SpellCastRecord, prop: &FilterProp) -> | FilterProp::DistinctFrom { .. } | FilterProp::SharesQuality { .. } | FilterProp::WasDealtDamageThisTurn + | FilterProp::DealtDamageThisTurn | FilterProp::EnteredThisTurn | FilterProp::ControlledContinuouslySinceTurnBegan | FilterProp::ZoneChangedThisTurn { .. } @@ -4363,6 +4366,14 @@ fn matches_filter_prop( .damage_dealt_this_turn .iter() .any(|record| matches!(record.target, TargetRef::Object(id) if id == object_id)), + // CR 120.1: active-voice counterpart — this object DEALT damage this turn, + // i.e. it was the source of a damage event (Red Guardian, Super-Soldier: + // "target creature ... that dealt damage this turn"). Reads the same + // per-turn ledger the passive arm above does, keyed by `source_id`. + FilterProp::DealtDamageThisTurn => state + .damage_dealt_this_turn + .iter() + .any(|record| record.source_id == object_id), // CR 400.7: Object entered the battlefield this turn. FilterProp::EnteredThisTurn => obj.entered_battlefield_turn == Some(state.turn_number), // CR 302.6 + CR 508.1a: controlled continuously since the controller's @@ -4856,6 +4867,14 @@ fn zone_change_record_matches_property( .damage_dealt_this_turn .iter() .any(|r| matches!(r.target, TargetRef::Object(id) if id == record.object_id)), + // CR 120.1: active-voice look-back — the object DEALT damage this turn. + // The `damage_dealt_this_turn` ledger is keyed by battlefield ObjectId and + // survives the object's zone change, so the LKI snapshot reads it by the + // record's `object_id`, mirroring the passive arm above. + FilterProp::DealtDamageThisTurn => state + .damage_dealt_this_turn + .iter() + .any(|r| r.source_id == record.object_id), // CR 110.5 + CR 110.5d + CR 608.2h: tap status is battlefield-only — once // the object has left its public zone it is neither tapped nor untapped, so // the live object can't answer a look-back "was tapped" rider (Brackish @@ -6362,6 +6381,49 @@ mod tests { )); } + // CR 120.1: `DealtDamageThisTurn` matches the damage SOURCE, not the + // recipient — the active-voice counterpart of `WasDealtDamageThisTurn` + // (Red Guardian, Super-Soldier). The two must not be confused: the same + // damage record makes the source match `DealtDamageThisTurn` and the target + // match `WasDealtDamageThisTurn`, never the reverse. + #[test] + fn dealt_damage_this_turn_matches_source_not_target() { + use crate::types::game_state::DamageRecord; + + let mut state = setup(); + let dealer = add_creature(&mut state, PlayerId(0), "Goblin Piker"); + let victim = add_creature(&mut state, PlayerId(1), "Grizzly Bears"); + state.damage_dealt_this_turn.push_back(DamageRecord { + source_id: dealer, + source_controller: PlayerId(0), + target: TargetRef::Object(victim), + target_controller: PlayerId(1), + amount: 2, + is_combat: true, + ..Default::default() + }); + + let dealt = TargetFilter::Typed( + TypedFilter::creature().properties(vec![FilterProp::DealtDamageThisTurn]), + ); + // The creature that dealt the damage matches; the one that received it does not. + assert!( + matches_target_filter(&state, dealer, &dealt, dealer), + "the damage source must satisfy DealtDamageThisTurn" + ); + assert!( + !matches_target_filter(&state, victim, &dealt, victim), + "the damage recipient must NOT satisfy DealtDamageThisTurn" + ); + + // Symmetry check against the passive filter: exactly the opposite. + let was_dealt = TargetFilter::Typed( + TypedFilter::creature().properties(vec![FilterProp::WasDealtDamageThisTurn]), + ); + assert!(matches_target_filter(&state, victim, &was_dealt, victim)); + assert!(!matches_target_filter(&state, dealer, &was_dealt, dealer)); + } + #[test] fn spell_record_matches_qualified_filter() { let record = SpellCastRecord { diff --git a/crates/engine/src/parser/oracle_target.rs b/crates/engine/src/parser/oracle_target.rs index 246c7966a3..798d383ffe 100644 --- a/crates/engine/src/parser/oracle_target.rs +++ b/crates/engine/src/parser/oracle_target.rs @@ -6485,6 +6485,9 @@ pub(crate) fn parse_that_clause_suffix<'a>( "was dealt damage this turn", FilterProp::WasDealtDamageThisTurn, ), + // CR 120.1: active voice — the creature dealt damage (was the source), + // distinct from the passive "was dealt damage" above (Red Guardian). + ("dealt damage this turn", FilterProp::DealtDamageThisTurn), ( "entered the battlefield this turn", FilterProp::EnteredThisTurn, @@ -13983,6 +13986,37 @@ mod tests { ); } + // CR 120.1: active-voice "that dealt damage this turn" (creature is the damage + // source) must parse to `DealtDamageThisTurn`, NOT the passive + // `WasDealtDamageThisTurn` (creature is the damage recipient). Red Guardian, + // Super-Soldier: "destroy target creature an opponent controls that dealt + // damage this turn." + #[test] + fn that_dealt_damage_this_turn_is_active_voice() { + let (filter, rest) = + parse_target("target creature an opponent controls that dealt damage this turn"); + let TargetFilter::Typed(ref tf) = filter else { + panic!("expected Typed filter, got {filter:?}"); + }; + assert!(tf.type_filters.contains(&TypeFilter::Creature)); + assert_eq!(tf.controller, Some(ControllerRef::Opponent)); + assert!( + tf.properties + .iter() + .any(|p| matches!(p, FilterProp::DealtDamageThisTurn)), + "Expected DealtDamageThisTurn (active), got: {:?}", + tf.properties + ); + assert!( + !tf.properties + .iter() + .any(|p| matches!(p, FilterProp::WasDealtDamageThisTurn)), + "must NOT collapse to the passive WasDealtDamageThisTurn: {:?}", + tf.properties + ); + assert!(rest.trim().is_empty(), "expected empty remainder: {rest:?}"); + } + #[test] fn that_entered_this_turn() { let (filter, rest) = parse_type_phrase("token you control that entered this turn"); diff --git a/crates/engine/src/parser/swallow_check.rs b/crates/engine/src/parser/swallow_check.rs index 6f5039408a..3bb5925b34 100644 --- a/crates/engine/src/parser/swallow_check.rs +++ b/crates/engine/src/parser/swallow_check.rs @@ -4017,6 +4017,7 @@ fn detect_duration_this_turn( matches!( x, FilterProp::WasDealtDamageThisTurn + | FilterProp::DealtDamageThisTurn | FilterProp::EnteredThisTurn | FilterProp::ZoneChangedThisTurn { .. } | FilterProp::AttackedThisTurn { .. } diff --git a/crates/engine/src/types/ability.rs b/crates/engine/src/types/ability.rs index 00f9a265f8..588b06e90f 100644 --- a/crates/engine/src/types/ability.rs +++ b/crates/engine/src/types/ability.rs @@ -3828,6 +3828,18 @@ pub enum FilterProp { /// CR 510.1: Object was dealt damage during this turn. /// Checks `damage_marked > 0` (damage persists until cleanup step). WasDealtDamageThisTurn, + /// CR 120.1: This object *dealt* damage during this turn — i.e. it was the + /// SOURCE of a damage event ("target creature ... that dealt damage this + /// turn", Red Guardian, Super-Soldier). The active-voice counterpart of + /// `WasDealtDamageThisTurn` (which reads the damage recipient). Evaluated by + /// scanning `state.damage_dealt_this_turn` for a record whose `source_id` + /// is this object. + /// + /// Parameterization note: these two form a damage-role pair. If a third + /// damage-role filter appears (e.g. "dealt combat damage this turn"), fold + /// the pair into `DamageThisTurn { role: {Dealt, Received}, combat_only }` + /// per the /add-engine-variant sibling-cluster threshold. + DealtDamageThisTurn, /// CR 400.7: Object entered the battlefield during this turn. /// Checks `entered_battlefield_turn == Some(current_turn)`. EnteredThisTurn, diff --git a/crates/engine/src/types/events.rs b/crates/engine/src/types/events.rs index e48a1e87f6..418cc78c26 100644 --- a/crates/engine/src/types/events.rs +++ b/crates/engine/src/types/events.rs @@ -592,6 +592,7 @@ impl EventObjectSnapshot { // ---- embedded per-turn history ---- FilterProp::WasDealtDamageThisTurn + | FilterProp::DealtDamageThisTurn | FilterProp::EnteredThisTurn | FilterProp::AttackedThisTurn { .. } | FilterProp::BlockedThisTurn From 944324d2bdcd06eb1dd44a4ac9f24886b291bc47 Mon Sep 17 00:00:00 2001 From: philluiz2323 Date: Wed, 15 Jul 2026 22:23:50 +0400 Subject: [PATCH 2/2] review: card-level target-legality test + drop cosmetic coverage-label diff (#5615) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses review on #5877: - Add a production-pipeline regression: Red Guardian's REAL parsed target filter is driven through `find_legal_targets`. An opponent creature that dealt damage this turn is a legal target; an otherwise-identical one that did not is not. Fails if the `DealtDamageThisTurn` FilterProp or its parser wiring is reverted (the filter would drop to "any opponent creature" and both would qualify). - Revert the passive `WasDealtDamageThisTurn` coverage label back to "dealt damage this turn" (now shared by both damage-role arms). This drops the 30 cosmetic label-only entries from the parse-diff, leaving only the genuine target-filter additions (Red Guardian, Avenging Arrow, and the bounce/pump cards) plus the two spurious `until end of turn` removals — which are correct: the old parser mis-read the bare "this turn" as a duration, and consuming "dealt damage this turn" as a filter removes it. --- crates/engine/src/game/coverage.rs | 9 +++- crates/engine/src/game/targeting.rs | 68 +++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/game/coverage.rs b/crates/engine/src/game/coverage.rs index 72c4db0602..b821abdde9 100644 --- a/crates/engine/src/game/coverage.rs +++ b/crates/engine/src/game/coverage.rs @@ -943,8 +943,13 @@ fn fmt_typed_filter(tf: &TypedFilter) -> String { }; parts.push(format!("{prefix} {name}{suffix}")); } - FilterProp::WasDealtDamageThisTurn => parts.push("was dealt damage this turn".into()), - FilterProp::DealtDamageThisTurn => parts.push("dealt damage this turn".into()), + // Both damage-role filters share this human coverage label (the AST + // variant carries the source-vs-recipient distinction); keeping the + // passive label unchanged avoids a cosmetic coverage-diff on every + // existing "was dealt damage this turn" card. + FilterProp::WasDealtDamageThisTurn | FilterProp::DealtDamageThisTurn => { + parts.push("dealt damage this turn".into()) + } FilterProp::EnteredThisTurn => parts.push("entered this turn".into()), FilterProp::ControlledContinuouslySinceTurnBegan => { parts.push("controlled continuously since turn began".into()) diff --git a/crates/engine/src/game/targeting.rs b/crates/engine/src/game/targeting.rs index c744c778d1..86fa81560c 100644 --- a/crates/engine/src/game/targeting.rs +++ b/crates/engine/src/game/targeting.rs @@ -2410,6 +2410,74 @@ mod tests { TargetFilter::Typed(TypedFilter::creature()) } + // CR 120.1 (#5615): Red Guardian, Super-Soldier — "destroy target creature an + // opponent controls that dealt damage this turn." This drives the card's + // REAL parsed target filter through the production `find_legal_targets` + // authority: an opponent creature that dealt damage this turn is a legal + // target; an otherwise-identical one that did not is not. Fails if the + // `DealtDamageThisTurn` FilterProp or its parser wiring is reverted (the + // filter would drop back to "any opponent creature" and both would qualify). + #[test] + fn red_guardian_targets_only_a_creature_that_dealt_damage_this_turn() { + use crate::types::ability::Effect; + use crate::types::game_state::DamageRecord; + + // Parse the card's actual Oracle text and pull the Destroy target filter. + let parsed = crate::parser::oracle::parse_oracle_text( + "When Red Guardian enters, destroy target creature an opponent controls that dealt damage this turn.", + "Red Guardian, Super-Soldier", + &[], + &["Creature".to_string()], + &[], + ); + let filter = parsed + .triggers + .iter() + .find_map(|t| match t.execute.as_deref()?.effect.as_ref() { + Effect::Destroy { target, .. } => Some(target.clone()), + _ => None, + }) + .expect("Red Guardian must parse a Destroy-target trigger"); + + // P0 controls Red Guardian; both candidate creatures are P1's (opponent's). + let (mut state, red_guardian, dealer) = setup_with_creatures(); + let bystander = create_object( + &mut state, + CardId(3), + PlayerId(1), + "Bystander".to_string(), + Zone::Battlefield, + ); + state + .objects + .get_mut(&bystander) + .unwrap() + .card_types + .core_types + .push(CoreType::Creature); + + // `dealer` (P1's Goblin from the helper) dealt damage this turn. + state.damage_dealt_this_turn.push_back(DamageRecord { + source_id: dealer, + source_controller: PlayerId(1), + target: TargetRef::Object(red_guardian), + target_controller: PlayerId(0), + amount: 1, + is_combat: true, + ..Default::default() + }); + + let legal = find_legal_targets(&state, &filter, PlayerId(0), red_guardian); + assert!( + legal.contains(&TargetRef::Object(dealer)), + "the opponent creature that dealt damage this turn must be targetable: {legal:?}" + ); + assert!( + !legal.contains(&TargetRef::Object(bystander)), + "an opponent creature that dealt NO damage must not be targetable: {legal:?}" + ); + } + #[test] fn post_replacement_source_controller_resolves_to_event_source_controller() { // CR 615.5 + CR 609.7: When `state.post_replacement_event_source` is