From 797b5c300c746d0d77323b9acf290ec78a4a28b7 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Sat, 25 Jul 2026 21:50:45 -0700 Subject: [PATCH] feat(engine): journal effect-driven combat membership as CR733 resolved commands Five authorities put an object into or out of combat by effect rather than by declaration -- combat::enter_attacking, combat::place_attacking_alongside, combat::place_blocking, combat::mark_attacker_blocked, and remove_from_combat::remove_object_from_combat -- and every one wrote its mutation raw. A retained-prefix replay had no record that a creature was ever attacking, blocking, or blocked by effect. Journaling happens INSIDE each authority, not at the 21 production call sites, so every caller is covered without touching one of them. One parameterized command, not five siblings -------------------------------------------- CLAUDE.md's categorical-boundary rule requires a parameterization axis to lie within a single CR section. A naive reading splits this three ways -- attacking is CR 508, blocking CR 509, removal CR 506.4 -- but the rules text says otherwise, and each citation was grep-verified against docs/MagicCompRules.txt: * CR 506.3a-g govern putting a permanent onto the battlefield "attacking or blocking" in ONE breath; 506.3e is the rule place_blocking already cites. * CR 506.4 governs removal. * The declaration-side rules delegate back: CR 509.1g ends "See rule 506.4." Effect-driven combat membership is therefore rooted at CR 506.3-506.4, a single section; CR 508.4 and CR 509.1g/h are entry points into it. Hence one ResolvedCombatMembershipCommand parameterized by a typed ResolvedCombatMembershipEdit { Attack, Block, MarkBlocked, Remove }. Turn-based declaration (CR 508.1 / CR 509.1) stays out of the family: it does not use the stack and validates before mutating. The defender must be recorded, never re-derived ----------------------------------------------- CR 508.4: "its controller chooses which defending player, planeswalker a defending player controls, or battle a defending player protects it's attacking." That is a choice the rules assign to a player. enter_attacking approximates it from AMBIENT state -- defending_player_for_enters_attacking reads the source's own attacker entry, then state.current_trigger_event, then a controller-scan of the live attacker list, then falls back to the first opponent. None of that is reconstructible at replay time. So apply_resolved_combat_membership never calls enter_attacking; it installs the recorded (defending_player, attack_target) pair verbatim. Re-deriving would both desynchronize replay and re-decide a settled choice -- wrong quietly, which is the worst failure mode. Notes ----- prune_object_from_combat is extracted as the single structural authority shared by the live removal and the replay applier, so a replayed prune cannot drift. The Remove receipt carries damage_assignments explicitly because CombatState's hand-written PartialEq compares only 10 of its 14 fields, skipping damage_assignments, damage_step_index, pending_damage, and regular_damage_done -- a check leaning on whole-struct equality would be blind to exactly the bookkeeping this authority prunes. Tests compare fields individually for the same reason. Tests drive the real pipeline with verbatim Oracle text (Kaalia of the Vast, Dazzling Beauty, Mirror Match), carry reach guards that fail before the journal assertions, and were each watched go red with only the record_* calls neutered. --- crates/engine/src/game/combat.rs | 358 +++++++++- .../src/game/effects/remove_from_combat.rs | 63 +- crates/engine/src/types/resolved_commands.rs | 154 ++++ .../cr733_resolved_combat_membership.rs | 655 ++++++++++++++++++ .../integration/cr733_resolved_commands_p2.rs | 4 + .../tests/integration/cr733_resolved_draw.rs | 3 + crates/engine/tests/integration/main.rs | 1 + 7 files changed, 1204 insertions(+), 34 deletions(-) create mode 100644 crates/engine/tests/integration/cr733_resolved_combat_membership.rs diff --git a/crates/engine/src/game/combat.rs b/crates/engine/src/game/combat.rs index 0126b4f0dc..a5c8ccc4f3 100644 --- a/crates/engine/src/game/combat.rs +++ b/crates/engine/src/game/combat.rs @@ -14,6 +14,10 @@ use crate::types::identifiers::{ObjectId, ObjectIncarnationRef}; use crate::types::keywords::Keyword; use crate::types::mana::ManaColor; use crate::types::player::PlayerId; +use crate::types::resolved_commands::{ + ResolvedCombatMembershipCommand, ResolvedCombatMembershipEdit, + ResolvedCombatMembershipReplayInvariantError, +}; use crate::types::statics::{ AttackDefenderScope, BlockExceptionKind, CombatAloneAction, CombatAloneRequirement, StaticMode, StaticModeKind, @@ -340,6 +344,115 @@ pub enum DamageTarget { Player(PlayerId), } +/// CR 506.4: The exact combat roles one object held at a single moment. +/// +/// Recorded by the CR 733 removal command so a replay can verify it is pruning +/// the same edges the live removal pruned. `damage_assignments` is carried +/// explicitly because `CombatState`'s hand-written `PartialEq` does NOT compare +/// that field — a check that leaned on whole-struct equality would be blind to +/// exactly the bookkeeping this authority prunes. +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct CombatParticipation { + /// The object's own attacker entry, when it was an attacking creature. + pub attacking: Option, + /// CR 509.1g: attacking creatures this object was blocking. + pub blocking: Vec, + /// CR 509.1h: blocking creatures assigned to this object as an attacker. + pub blocked_by: Vec, + /// CR 510.1: combat damage this object had already been assigned to deal. + pub damage_assignments: Vec, +} + +impl CombatParticipation { + /// Reads every combat role `oid` currently holds. + pub fn capture(state: &GameState, oid: ObjectId) -> Self { + let Some(combat) = state.combat.as_ref() else { + return Self::default(); + }; + Self { + attacking: combat + .attackers + .iter() + .find(|a| a.object_id == oid) + .cloned(), + blocking: combat + .blocker_to_attacker + .get(&oid) + .cloned() + .unwrap_or_default(), + blocked_by: combat + .blocker_assignments + .get(&oid) + .cloned() + .unwrap_or_default(), + damage_assignments: combat + .damage_assignments + .get(&oid) + .cloned() + .unwrap_or_default(), + } + } + + /// CR 506.4: whether the object held no combat role at all, so removing it + /// would prune nothing. + pub fn is_empty(&self) -> bool { + self.attacking.is_none() + && self.blocking.is_empty() + && self.blocked_by.is_empty() + && self.damage_assignments.is_empty() + } +} + +/// CR 506.4: Drops every combat edge that names `oid`, in the live authority's +/// order. Returns whether `oid` was an attacking creature, which is the only +/// case that can change a Layer 6 `FilterProp::Attacking` grant. +/// +/// Single structural authority shared by the live `remove_object_from_combat` +/// and the CR 733 replay applier, so a replayed prune cannot drift from the +/// prune the game actually performed. +pub(crate) fn prune_object_from_combat(state: &mut GameState, oid: ObjectId) -> bool { + let Some(combat) = state.combat.as_mut() else { + return false; + }; + let attackers_before = combat.attackers.len(); + combat.attackers.retain(|a| a.object_id != oid); + let attacker_removed = combat.attackers.len() != attackers_before; + // Drop attacker-keyed forward assignments (oid was an attacker with blockers + // assigned to it). + combat.blocker_assignments.remove(&oid); + // Remove as blocker from all remaining attacker assignments. + for blockers in combat.blocker_assignments.values_mut() { + blockers.retain(|b| *b != oid); + } + // Remove reverse lookup when oid was a blocker. + combat.blocker_to_attacker.remove(&oid); + // Prune oid from every blocker's attacker list (oid was an attacker). + combat.blocker_to_attacker.retain(|_, attackers| { + attackers.retain(|id| *id != oid); + !attackers.is_empty() + }); + // CR 510.1: remove any pending damage assignments for this object. + combat.damage_assignments.remove(&oid); + attacker_removed +} + +/// CR 733: Journals one settled combat-membership edit through its owning family. +fn record_combat_membership_edit( + state: &mut GameState, + object: ObjectIncarnationRef, + edit: ResolvedCombatMembershipEdit, +) { + let cause = state.current_or_begin_rules_execution_node(); + state + .resolved_rules_journal + .record_combat_membership(ResolvedCombatMembershipCommand { + object, + edit, + cause, + }) + .expect("resolved combat membership must have a live journal cause"); +} + /// CR 508.4: Place a permanent onto the battlefield attacking. /// The creature is not "declared as an attacker" — attack triggers do not fire. /// Determines the defending player from: (1) source creature's combat info, @@ -355,6 +468,26 @@ pub fn enter_attacking( let (defending_player, attack_target) = defending_player_for_enters_attacking(state, source_id, controller); + push_attacker_and_journal(state, object_id, defending_player, attack_target); +} + +/// CR 508.4 + CR 733: seat `object_id` as an attacking creature against an +/// already-decided defender and journal the settled pair. +/// +/// Shared by `enter_attacking` (which derives the pair from ambient state) and +/// `place_attacking_alongside` (which is handed the pair by its caller), so both +/// entry points record through one authority. +fn push_attacker_and_journal( + state: &mut GameState, + object_id: ObjectId, + defending_player: PlayerId, + attack_target: AttackTarget, +) { + let reference = state + .objects + .get(&object_id) + .map(ObjectIncarnationRef::from_object); + if let Some(combat) = state.combat.as_mut() { combat.attackers.push(AttackerInfo::new( object_id, @@ -365,6 +498,20 @@ pub fn enter_attacking( // attacking is an attacking creature; re-evaluate Layer 6 // FilterProp::Attacking { defender: None } grants immediately. state.layers_dirty.mark_full(); + + // CR 733 + CR 508.4: the defending player and attack target are a CHOICE + // the rules assign to the controller; record the settled pair so replay + // installs it instead of re-deriving from ambient state. + if let Some(reference) = reference { + record_combat_membership_edit( + state, + reference, + ResolvedCombatMembershipEdit::Attack { + resulting_defending_player: defending_player, + resulting_attack_target: attack_target, + }, + ); + } } } @@ -478,17 +625,12 @@ pub fn place_attacking_alongside( // at entry time, not re-entry). obj.summoning_sick = true; } - if let Some(combat) = state.combat.as_mut() { - combat.attackers.push(AttackerInfo::new( - object_id, - attack_target, - defending_player, - )); - // CR 702.49c + CR 702.190b + CR 506.4 + CR 613.1f: Ninjutsu/Sneak place a - // creature already attacking; re-evaluate Layer 6 FilterProp::Attacking { defender: None } - // grants. - state.layers_dirty.mark_full(); - } + // CR 702.49c + CR 702.190b + CR 506.4 + CR 613.1f: Ninjutsu/Sneak place a + // creature already attacking; re-evaluate Layer 6 FilterProp::Attacking { defender: None } + // grants. CR 733: journals through the same attacker-seating authority as + // `enter_attacking` — the caller already chose the defender, so the recorded + // pair is its argument rather than an ambient derivation. + push_attacker_and_journal(state, object_id, defending_player, attack_target); } /// CR 509.1g + CR 506.3e + CR 509.1h: Put a permanent onto the battlefield as a @@ -504,9 +646,11 @@ pub fn place_attacking_alongside( /// trigger, so no `BlockersDeclared` event is emitted; combat damage reads the /// recorded assignments directly. Returns `true` when the block was established. pub fn place_blocking(state: &mut GameState, blocker_id: ObjectId, attacker_id: ObjectId) -> bool { - let Some(blocker_controller) = state.objects.get(&blocker_id).map(|o| o.controller) else { + let Some(blocker) = state.objects.get(&blocker_id) else { return false; }; + let blocker_controller = blocker.controller; + let reference = ObjectIncarnationRef::from_object(blocker); let Some(combat) = state.combat.as_mut() else { return false; }; @@ -524,6 +668,8 @@ pub fn place_blocking(state: &mut GameState, blocker_id: ObjectId, attacker_id: return false; } // CR 509.1h: an attacking creature with one or more blockers becomes blocked. + // The bit is sticky, so its prior value is recorded rather than recomputed. + let expected_attacker_blocked = info.blocked; info.blocked = true; // CR 509.1g: the creature becomes a blocking creature for the chosen attacker. combat @@ -541,6 +687,18 @@ pub fn place_blocking(state: &mut GameState, blocker_id: ObjectId, attacker_id: // CR 506.4 + CR 613.1f: a new blocking creature can satisfy Layer 6 // `FilterProp::Blocking` grants; re-evaluate continuous effects. state.layers_dirty.mark_full(); + // CR 733: journal the settled block. All four writes above (the sticky + // blocked bit, both blocker maps, and the per-turn blocked set) follow + // structurally from this blocker/attacker pair, so the pair plus the prior + // blocked bit is the whole receipt. + record_combat_membership_edit( + state, + reference, + ResolvedCombatMembershipEdit::Block { + resulting_attacker: attacker_id, + expected_attacker_blocked, + }, + ); true } @@ -551,18 +709,194 @@ pub fn place_blocking(state: &mut GameState, blocker_id: ObjectId, attacker_id: /// damage. Emits no event (the caller decides whether the CR 509.3c precondition /// is met). Returns `false` if `oid` is not a current attacker. pub fn mark_attacker_blocked(state: &mut GameState, oid: ObjectId) -> bool { + let reference = state + .objects + .get(&oid) + .map(ObjectIncarnationRef::from_object); let Some(combat) = state.combat.as_mut() else { return false; }; let Some(info) = combat.attackers.iter_mut().find(|a| a.object_id == oid) else { return false; }; + // CR 509.1h: the bit is sticky, so re-marking an already-blocked attacker + // mutates nothing and is not journaled. + let already_blocked = info.blocked; info.blocked = true; // CR 613.1f: `FilterProp::Blocked` grants may now apply; re-evaluate layers. state.layers_dirty.mark_full(); + // CR 733: journal only the false-to-true transition, so the applier can + // require the bit is still clear before installing it. + if let Some(reference) = reference.filter(|_| !already_blocked) { + record_combat_membership_edit(state, reference, ResolvedCombatMembershipEdit::MarkBlocked); + } true } +/// Installs one already-resolved CR 506.3 / CR 506.4 combat-membership edit +/// verbatim. +/// +/// Deliberately re-runs NONE of the resolve-time derivation. In particular it +/// never calls `enter_attacking`: that authority picks the defending player from +/// ambient state (`state.current_trigger_event`, the source's attacker entry, a +/// controller-scan of the live attacker list, then an opponent fallback), none +/// of which is reconstructible during replay. CR 508.4 makes the defender a +/// choice the controller owns, so re-deriving it would both desynchronize replay +/// and re-decide a settled choice. The recorded pair is installed as-is. +/// +/// Legality gates are likewise not re-run: CR 506.3b/c/e and CR 508.4a decided +/// at resolve time whether the creature ever became attacking or blocking, and a +/// recorded command exists only because it did. +pub fn apply_resolved_combat_membership( + state: &mut GameState, + command: &ResolvedCombatMembershipCommand, +) -> Result<(), ResolvedCombatMembershipReplayInvariantError> { + let object_id = command.object.object_id; + let object = state.objects.get(&object_id).ok_or( + ResolvedCombatMembershipReplayInvariantError::UnknownObject(object_id), + )?; + // CR 400.7: a re-entered object is a new object and must not satisfy a + // command recorded against its predecessor incarnation. + let found = ObjectIncarnationRef::from_object(object); + if found != command.object { + return Err(ResolvedCombatMembershipReplayInvariantError::StaleObject { + expected: command.object, + found, + }); + } + + // Every edit in this family reads the recorded expectation against live + // state BEFORE any mutation, so a rejected command leaves no partial edit. + match &command.edit { + ResolvedCombatMembershipEdit::Attack { + resulting_defending_player, + resulting_attack_target, + } => { + let combat = state + .combat + .as_mut() + .ok_or(ResolvedCombatMembershipReplayInvariantError::NoCombat)?; + if combat.attackers.iter().any(|a| a.object_id == object_id) { + return Err( + ResolvedCombatMembershipReplayInvariantError::AlreadyAttacking(object_id), + ); + } + // CR 508.4: install the RECORDED defender and attack target. + combat.attackers.push(AttackerInfo::new( + object_id, + *resulting_attack_target, + *resulting_defending_player, + )); + } + ResolvedCombatMembershipEdit::Block { + resulting_attacker, + expected_attacker_blocked, + } => { + let combat = state + .combat + .as_mut() + .ok_or(ResolvedCombatMembershipReplayInvariantError::NoCombat)?; + let info = combat + .attackers + .iter_mut() + .find(|a| a.object_id == *resulting_attacker) + .ok_or(ResolvedCombatMembershipReplayInvariantError::NotAttacking( + *resulting_attacker, + ))?; + if info.blocked != *expected_attacker_blocked { + return Err( + ResolvedCombatMembershipReplayInvariantError::BlockedPreconditionMismatch { + attacker: *resulting_attacker, + expected: *expected_attacker_blocked, + found: info.blocked, + }, + ); + } + if combat + .blocker_to_attacker + .get(&object_id) + .is_some_and(|attackers| attackers.contains(resulting_attacker)) + { + return Err( + ResolvedCombatMembershipReplayInvariantError::DuplicateBlock { + attacker: *resulting_attacker, + blocker: object_id, + }, + ); + } + // CR 509.1h then CR 509.1g: the same four writes the live authority + // performed, in the same order. + info.blocked = true; + combat + .blocker_to_attacker + .entry(object_id) + .or_default() + .push(*resulting_attacker); + combat + .blocker_assignments + .entry(*resulting_attacker) + .or_default() + .push(object_id); + state.creatures_blocked_this_turn.insert(object_id); + } + ResolvedCombatMembershipEdit::MarkBlocked => { + let combat = state + .combat + .as_mut() + .ok_or(ResolvedCombatMembershipReplayInvariantError::NoCombat)?; + let info = combat + .attackers + .iter_mut() + .find(|a| a.object_id == object_id) + .ok_or(ResolvedCombatMembershipReplayInvariantError::NotAttacking( + object_id, + ))?; + // CR 509.1h: recorded only on a false-to-true transition, so a + // still-unblocked attacker is the only state this can install into. + if info.blocked { + return Err( + ResolvedCombatMembershipReplayInvariantError::BlockedPreconditionMismatch { + attacker: object_id, + expected: false, + found: true, + }, + ); + } + info.blocked = true; + } + ResolvedCombatMembershipEdit::Remove { + expected_participation, + } => { + let found = CombatParticipation::capture(state, object_id); + if found != *expected_participation { + return Err( + ResolvedCombatMembershipReplayInvariantError::ParticipationMismatch { + object: object_id, + expected: Box::new(expected_participation.clone()), + found: Box::new(found), + }, + ); + } + // CR 506.4: the prune itself is a structural consequence of the + // verified participation, so it re-runs the live authority. + // + // CR 613.1f: mirror the live authority's narrower marking — only a + // removed ATTACKER can change a `FilterProp::Attacking` grant, so + // pruning a pure blocker leaves the layer system alone. + if prune_object_from_combat(state, object_id) { + state.layers_dirty.mark_full(); + } + return Ok(()); + } + } + + // CR 506.4 + CR 613.1f: seating an attacker or establishing a block drives + // Layer 6 `FilterProp::Attacking` / `Blocking` / `Blocked` grants; + // re-evaluate exactly as the live authorities do. + state.layers_dirty.mark_full(); + Ok(()) +} + /// Validate attacker declarations per CR 508.1. pub fn validate_attackers(state: &GameState, attacker_ids: &[ObjectId]) -> Result<(), String> { let active = state.active_player; diff --git a/crates/engine/src/game/effects/remove_from_combat.rs b/crates/engine/src/game/effects/remove_from_combat.rs index b290ae7bd4..61d3921c13 100644 --- a/crates/engine/src/game/effects/remove_from_combat.rs +++ b/crates/engine/src/game/effects/remove_from_combat.rs @@ -1,6 +1,11 @@ +use crate::game::combat::CombatParticipation; use crate::types::ability::{Effect, EffectError, EffectKind, ResolvedAbility, TargetFilter}; use crate::types::events::GameEvent; use crate::types::game_state::GameState; +use crate::types::identifiers::ObjectIncarnationRef; +use crate::types::resolved_commands::{ + ResolvedCombatMembershipCommand, ResolvedCombatMembershipEdit, +}; /// CR 506.4: Remove a creature from combat — it stops being an attacking, /// blocking, blocked, and/or unblocked creature. @@ -46,29 +51,20 @@ pub fn resolve( /// Reusable building block for any code that needs to remove a permanent from combat /// (regeneration, effect resolution, controller change, etc.). pub fn remove_object_from_combat(state: &mut GameState, oid: crate::types::identifiers::ObjectId) { - let mut attacker_removed = false; - if let Some(ref mut combat) = state.combat { - // Remove as attacker - let attackers_before = combat.attackers.len(); - combat.attackers.retain(|a| a.object_id != oid); - attacker_removed = combat.attackers.len() != attackers_before; - // Drop attacker-keyed forward assignments (oid was blocking nobody as a key, - // but was an attacker with blockers assigned to it). - combat.blocker_assignments.remove(&oid); - // Remove as blocker from all remaining attacker assignments - for blockers in combat.blocker_assignments.values_mut() { - blockers.retain(|b| *b != oid); - } - // Remove reverse lookup when oid was a blocker - combat.blocker_to_attacker.remove(&oid); - // Prune oid from every blocker's attacker list (oid was an attacker) - combat.blocker_to_attacker.retain(|_, attackers| { - attackers.retain(|id| *id != oid); - !attackers.is_empty() - }); - // Remove any pending damage assignments for this object - combat.damage_assignments.remove(&oid); + // CR 733: read the exact roles being pruned BEFORE the prune, so the journal + // records what this removal actually did. An object holding no combat role + // prunes nothing and is not recorded. + let participation = CombatParticipation::capture(state, oid); + if participation.is_empty() { + return; } + let reference = state + .objects + .get(&oid) + .map(ObjectIncarnationRef::from_object); + + let attacker_removed = crate::game::combat::prune_object_from_combat(state, oid); + // CR 506.4 + CR 613.1f: a creature removed from combat stops being attacking, // so a granted "while attacking" keyword (deathtouch/lifelink via // FilterProp::Attacking { defender: None }, Layer 6) must be revoked immediately. Mark dirty only @@ -77,6 +73,29 @@ pub fn remove_object_from_combat(state: &mut GameState, oid: crate::types::ident if attacker_removed { state.layers_dirty.mark_full(); } + + if let Some(reference) = reference { + record_combat_membership_removal(state, reference, participation); + } +} + +/// CR 733: Journals one settled CR 506.4 removal through its owning family. +fn record_combat_membership_removal( + state: &mut GameState, + object: ObjectIncarnationRef, + expected_participation: CombatParticipation, +) { + let cause = state.current_or_begin_rules_execution_node(); + state + .resolved_rules_journal + .record_combat_membership(ResolvedCombatMembershipCommand { + object, + edit: ResolvedCombatMembershipEdit::Remove { + expected_participation, + }, + cause, + }) + .expect("resolved combat removal must have a live journal cause"); } #[cfg(test)] diff --git a/crates/engine/src/types/resolved_commands.rs b/crates/engine/src/types/resolved_commands.rs index a421544ddd..e4de6fd1ea 100644 --- a/crates/engine/src/types/resolved_commands.rs +++ b/crates/engine/src/types/resolved_commands.rs @@ -7,6 +7,7 @@ use std::collections::HashSet; use serde::{Deserialize, Deserializer, Serialize, Serializer}; +use crate::game::combat::{AttackTarget, CombatParticipation}; use crate::game::game_object::AttachTarget; use crate::game::triggers::{ConsumedTriggerEventOccurrence, PendingTriggerContext}; @@ -323,6 +324,108 @@ pub enum ResolvedContinuousEffectReplayInvariantError { )] TimestampAboveHighWater { timestamp: u64, high_water: u64 }, } +/// One exact effect-driven combat-membership edit (CR 506.3 / CR 506.4). +/// +/// The five production authorities — `enter_attacking`, +/// `place_attacking_alongside`, `place_blocking`, `mark_attacker_blocked`, and +/// `remove_object_from_combat` — all edit the one membership structure +/// (`CombatState.attackers` plus the two blocker maps), so they share a single +/// parameterized command rather than five sibling variants. +/// +/// The parameterization axis stays inside one CR section, as the categorical +/// boundary rule requires: CR 506.3a-g govern putting a permanent onto the +/// battlefield "attacking or blocking" in one breath, CR 506.4 governs removal, +/// and the declaration-side rules delegate back to it — CR 509.1g ends with +/// "See rule 506.4." CR 508.4 and CR 509.1g/h are the entry points; CR 506 is +/// the section that actually defines membership. +/// +/// Turn-based declaration (CR 508.1 / CR 509.1) is deliberately NOT part of this +/// family: it does not use the stack and validates before mutating. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ResolvedCombatMembershipCommand { + pub object: ObjectIncarnationRef, + pub edit: ResolvedCombatMembershipEdit, + pub cause: RulesExecutionNodeRef, +} + +/// Which membership edit one [`ResolvedCombatMembershipCommand`] settled. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub enum ResolvedCombatMembershipEdit { + /// CR 508.4: the object became an attacking creature. + /// + /// CR 508.4 assigns the defender to a CHOICE ("its controller chooses which + /// defending player, planeswalker a defending player controls, or battle a + /// defending player protects it's attacking"). The resolve-time authority + /// derives it from ambient state — `state.current_trigger_event`, the + /// source's own attacker entry, and a controller-scan of the live attacker + /// list. None of that is reconstructible at replay time, so both halves of + /// the chosen pair are RECORDED and installed verbatim. Re-deriving would + /// silently seat the creature against a different defender. + Attack { + resulting_defending_player: PlayerId, + resulting_attack_target: AttackTarget, + }, + /// CR 509.1g + CR 506.3e: the object became a blocking creature for the + /// recorded attacker, which becomes blocked per CR 509.1h. + /// + /// `expected_attacker_blocked` pins the attacker's sticky blocked bit as it + /// stood before this block, so a replay installing a second blocker onto an + /// already-blocked attacker is distinguishable from the first one. + Block { + resulting_attacker: ObjectId, + expected_attacker_blocked: bool, + }, + /// CR 509.1h: the object became a blocked creature purely by effect, with no + /// blocking creature assigned. Recorded only on a false-to-true transition, + /// so the applier can require the bit is still clear. + MarkBlocked, + /// CR 506.4: the object stopped being an attacking, blocking, and/or blocked + /// creature. Records the exact roles it held so the applier can verify it is + /// pruning the same edges, then re-runs the structural prune — which is a + /// consequence of the recorded participation, not a re-selection. + Remove { + expected_participation: CombatParticipation, + }, +} + +/// Typed failure while applying one already-resolved combat-membership edit. +#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] +pub enum ResolvedCombatMembershipReplayInvariantError { + #[error("combat-membership command references an unknown object {0:?}")] + UnknownObject(ObjectId), + #[error("combat-membership occurrence mismatch: expected {expected:?}, found {found:?}")] + StaleObject { + expected: ObjectIncarnationRef, + found: ObjectIncarnationRef, + }, + #[error("combat-membership command applies to a state with no combat")] + NoCombat, + #[error("combat-membership command would attack twice with {0:?}")] + AlreadyAttacking(ObjectId), + #[error("combat-membership command references a non-attacking creature {0:?}")] + NotAttacking(ObjectId), + #[error( + "combat-membership blocked precondition mismatch for {attacker:?}: expected {expected}, found {found}" + )] + BlockedPreconditionMismatch { + attacker: ObjectId, + expected: bool, + found: bool, + }, + #[error("combat-membership command would repeat the block of {attacker:?} by {blocker:?}")] + DuplicateBlock { + attacker: ObjectId, + blocker: ObjectId, + }, + #[error( + "combat-membership participation mismatch for {object:?}: expected {expected:?}, found {found:?}" + )] + ParticipationMismatch { + object: ObjectId, + expected: Box, + found: Box, + }, +} /// One exact CR 110.2a + CR 603.6a "under your control" battlefield-entry /// controller override. @@ -764,6 +867,7 @@ pub enum ResolvedRulesCommand { Attachment(ResolvedAttachmentCommand), DelayedTriggerInstall(Box), ContinuousEffectInstall(Box), + CombatMembership(ResolvedCombatMembershipCommand), ControllerOverride(ResolvedControllerOverrideCommand), EntryProvenance(ResolvedEntryProvenanceCommand), ObjectCease(ResolvedObjectCeaseCommand), @@ -1736,6 +1840,17 @@ impl ResolvedRulesJournal { ResolvedRulesCommand::ContinuousEffectInstall(Box::new(command)), ) } + /// Records one exact CR 506.3 / CR 506.4 combat-membership edit under its + /// causal node. + pub fn record_combat_membership( + &mut self, + command: ResolvedCombatMembershipCommand, + ) -> Result { + self.append_command( + command.cause, + ResolvedRulesCommand::CombatMembership(command), + ) + } /// Records one exact CR 110.2a controller override under its causal node. pub fn record_controller_override( @@ -2010,6 +2125,7 @@ impl ResolvedRulesJournal { | ResolvedRulesCommand::Attachment(_) | ResolvedRulesCommand::DelayedTriggerInstall(_) | ResolvedRulesCommand::ContinuousEffectInstall(_) + | ResolvedRulesCommand::CombatMembership(_) | ResolvedRulesCommand::ControllerOverride(_) | ResolvedRulesCommand::EntryProvenance(_) | ResolvedRulesCommand::ObjectCease(_) @@ -2315,6 +2431,44 @@ impl ResolvedRulesJournal { )); } } + ResolvedRulesCommand::CombatMembership(command) => { + if entry.node != command.cause { + return Err(ResolvedRulesJournalError::InvalidSerializedAuthority( + "combat-membership command has an unrelated cause".to_string(), + )); + } + // CR 400.7: combat membership is per-incarnation, so a re-entered + // object must never satisfy a command recorded for its predecessor. + if command.object.incarnation == LEGACY_INCARNATION { + return Err(ResolvedRulesJournalError::InvalidSerializedAuthority( + "combat-membership command cannot use a legacy object identity".to_string(), + )); + } + match &command.edit { + // CR 509.1a: a creature is chosen to block an attacking + // creature, which is never itself. + ResolvedCombatMembershipEdit::Block { + resulting_attacker, .. + } if *resulting_attacker == command.object.object_id => { + return Err(ResolvedRulesJournalError::InvalidSerializedAuthority( + "combat-membership command blocks its own blocker".to_string(), + )); + } + // CR 506.4: removing an object that held no combat role + // pruned nothing, so it is not a removal that ever happened. + ResolvedCombatMembershipEdit::Remove { + expected_participation, + } if expected_participation.is_empty() => { + return Err(ResolvedRulesJournalError::InvalidSerializedAuthority( + "combat-membership removal prunes no combat role".to_string(), + )); + } + ResolvedCombatMembershipEdit::Attack { .. } + | ResolvedCombatMembershipEdit::Block { .. } + | ResolvedCombatMembershipEdit::MarkBlocked + | ResolvedCombatMembershipEdit::Remove { .. } => {} + } + } ResolvedRulesCommand::ControllerOverride(command) => { // CR 110.2a: an override that leaves both the derived and the // pinned controller exactly as they were retagged nothing, so it diff --git a/crates/engine/tests/integration/cr733_resolved_combat_membership.rs b/crates/engine/tests/integration/cr733_resolved_combat_membership.rs new file mode 100644 index 0000000000..40f3c6619a --- /dev/null +++ b/crates/engine/tests/integration/cr733_resolved_combat_membership.rs @@ -0,0 +1,655 @@ +//! CR733 coverage for the effect-driven combat-membership family. +//! +//! Five authorities put an object into or out of combat by effect rather than by +//! declaration — `combat::enter_attacking`, `combat::place_attacking_alongside`, +//! `combat::place_blocking`, `combat::mark_attacker_blocked`, and +//! `remove_from_combat::remove_object_from_combat` — and every one of them wrote +//! its mutation raw. A retained-prefix replay therefore had no record that a +//! creature was ever attacking, blocking, or blocked by effect. +//! +//! They share ONE parameterized command rather than five sibling variants. The +//! axis stays inside a single CR section, as the categorical boundary rule +//! requires: CR 506.3a-g govern putting a permanent onto the battlefield +//! "attacking or blocking" in one breath, CR 506.4 governs removal, and the +//! declaration-side rules delegate back to it (CR 509.1g ends "See rule 506.4."). +//! +//! The load-bearing reason the defender must be RECORDED rather than re-derived +//! is CR 508.4: "its controller chooses which defending player, planeswalker a +//! defending player controls, or battle a defending player protects it's +//! attacking." That is a choice the rules assign to a player. The resolve-time +//! authority approximates it from ambient state — +//! `defending_player_for_enters_attacking` reads the source's own attacker +//! entry, then `state.current_trigger_event`, then a controller-scan of the live +//! attacker list, then falls back to the first opponent. None of that is +//! reconstructible at replay time, so re-deriving would silently seat the +//! creature against a different defender. `replay_installs_recorded_defender_ +//! when_ambient_derivation_diverges` is the test that pins it, and it carries a +//! probe proving the ambient path really would answer differently. + +use engine::game::combat::{apply_resolved_combat_membership, AttackTarget, CombatParticipation}; +use engine::game::scenario::{GameRunner, GameScenario, P0, P1}; +use engine::types::ability::TargetRef; +use engine::types::actions::GameAction; +use engine::types::events::GameEvent; +use engine::types::game_state::GameState; +use engine::types::game_state::WaitingFor; +use engine::types::identifiers::ObjectId; +use engine::types::phase::Phase; +use engine::types::player::PlayerId; +use engine::types::resolved_commands::{ + ResolvedCombatMembershipCommand, ResolvedCombatMembershipEdit, + ResolvedCombatMembershipReplayInvariantError, ResolvedRulesCommand, +}; + +/// Verbatim Oracle text (Scryfall, 2026-07). A paraphrase risks taking a +/// different parser branch than the real card. +const KAALIA_ORACLE: &str = "Flying\nWhenever Kaalia attacks an opponent, you may put an Angel, Demon, or Dragon creature card from your hand onto the battlefield tapped and attacking that opponent."; + +/// Every combat-membership command recorded after `journal_start`, in journal +/// (execution) order. +fn membership_commands( + state: &GameState, + journal_start: usize, +) -> Vec { + state + .resolved_rules_journal + .entries() + .iter() + .skip(journal_start) + .filter_map(|entry| entry.command.clone()) + .filter_map(|command| match command { + ResolvedRulesCommand::CombatMembership(command) => Some(command), + _ => None, + }) + .collect() +} + +fn attacker_entry(state: &GameState, oid: ObjectId) -> Option<(PlayerId, AttackTarget, bool)> { + state.combat.as_ref().and_then(|combat| { + combat + .attackers + .iter() + .find(|a| a.object_id == oid) + .map(|a| (a.defending_player, a.attack_target, a.blocked)) + }) +} + +/// Advance from a main-phase priority to the declare-attackers step. +fn advance_to_declare_attackers(runner: &mut GameRunner, attacker: PlayerId) { + runner.state_mut().active_player = attacker; + runner.state_mut().priority_player = attacker; + runner.state_mut().waiting_for = WaitingFor::Priority { player: attacker }; + + for _ in 0..40 { + match runner.state().waiting_for.clone() { + WaitingFor::DeclareAttackers { .. } => return, + WaitingFor::Priority { .. } => { + runner + .act(GameAction::PassPriority) + .expect("priority pass should advance toward declare attackers"); + } + WaitingFor::OrderTriggers { triggers, .. } => { + let order = (0..triggers.len()).collect(); + runner + .act(GameAction::OrderTriggers { order }) + .expect("ordering combat triggers should succeed"); + } + other => panic!("unexpected waiting_for advancing to declare attackers: {other:?}"), + } + } + panic!("expected DeclareAttackers"); +} + +/// A Kaalia attack that has resolved its "put a creature onto the battlefield +/// tapped and attacking that opponent" trigger. +struct KaaliaAttack { + runner: GameRunner, + kaalia: ObjectId, + angel: ObjectId, + /// A bystander creature used by the L1 probe to observe what the LIVE + /// ambient derivation answers in the divergent replay state. + control: ObjectId, + journal_start: usize, +} + +/// Drives the REAL pipeline: declare attackers, then let Kaalia's attack trigger +/// resolve through the production stack so `change_zone` reaches +/// `combat::enter_attacking` exactly as it does in a game. +fn kaalia_attack() -> KaaliaAttack { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + + let kaalia = scenario + .add_creature_from_oracle(P0, "Kaalia of the Vast", 2, 2, KAALIA_ORACLE) + .id(); + let angel = scenario + .add_creature_to_hand(P0, "Serra Angel", 4, 4) + .with_subtypes(vec!["Angel"]) + .id(); + let control = scenario.add_vanilla(P0, 1, 1); + + let mut runner = scenario.build(); + advance_to_declare_attackers(&mut runner, P0); + + let journal_start = runner.state().resolved_rules_journal.entries().len(); + + runner + .act(GameAction::DeclareAttackers { + attacks: vec![(kaalia, AttackTarget::Player(P1))], + bands: Vec::new(), + }) + .expect("Kaalia can attack the only opponent"); + + // Resolve the attack trigger, answering the "you may" and the card choice. + for _ in 0..40 { + match runner.state().waiting_for.clone() { + WaitingFor::OrderTriggers { triggers, .. } => { + let order = (0..triggers.len()).collect(); + runner + .act(GameAction::OrderTriggers { order }) + .expect("ordering the attack trigger should succeed"); + } + WaitingFor::OptionalEffectChoice { .. } => { + runner + .act(GameAction::DecideOptionalEffect { accept: true }) + .expect("accepting Kaalia's may-clause should succeed"); + } + WaitingFor::Priority { .. } => { + if runner.state().stack.is_empty() { + break; + } + runner + .act(GameAction::PassPriority) + .expect("priority pass should resolve the attack trigger"); + } + other => panic!("unexpected waiting_for resolving Kaalia's trigger: {other:?}"), + } + } + + KaaliaAttack { + runner, + kaalia, + angel, + control, + journal_start, + } +} + +/// Headline: a creature put onto the battlefield attacking is journaled as an +/// exact resolved command, and replaying that command reproduces the attacking +/// entry against the RECORDED defender. +#[test] +fn enters_attacking_journals_and_replays_its_recorded_defender() { + let attack = kaalia_attack(); + let state = attack.runner.state(); + + // Reach guards. Without these the journal assertion below could pass + // vacuously on a trigger that never resolved, or on an Angel that entered + // the battlefield without ever becoming an attacking creature. + assert_eq!( + state.objects[&attack.angel].zone, + engine::types::zones::Zone::Battlefield, + "the trigger must have put the Angel onto the battlefield" + ); + let (defender, target, _) = attacker_entry(state, attack.angel) + .expect("CR 508.4: the Angel must be an attacking creature"); + assert_eq!( + defender, P1, + "CR 508.4: the Angel attacks the opponent Kaalia attacked" + ); + assert_eq!(target, AttackTarget::Player(P1)); + + // Discriminating assertion: the membership edit is journaled. A raw + // mutation records nothing here. + let commands: Vec<_> = membership_commands(state, attack.journal_start) + .into_iter() + .filter(|command| command.object.object_id == attack.angel) + .collect(); + assert_eq!( + commands.len(), + 1, + "the attacking authority must journal exactly one resolved edit for the Angel" + ); + let command = &commands[0]; + assert_eq!( + command.edit, + ResolvedCombatMembershipEdit::Attack { + resulting_defending_player: P1, + resulting_attack_target: AttackTarget::Player(P1), + }, + "the recorded pair is the CR 508.4 choice the authority settled" + ); + + // Replay exactness. Fields are compared individually: `CombatState`'s + // hand-written `PartialEq` skips `damage_assignments`, `damage_step_index`, + // `pending_damage`, and `regular_damage_done`, so a whole-struct equality + // assertion would be blind to divergence in exactly those fields. + // + // The predecessor is the post-resolution state with ONLY this family's edit + // undone. It cannot be the pre-declaration state: the Angel moved from hand + // to battlefield, and CR 400.7 makes that a NEW object, so the recorded + // incarnation would legitimately not match (the applier rejects that with + // `StaleObject`, which the fail-closed test relies on). + let mut replay = state.clone(); + if let Some(combat) = replay.combat.as_mut() { + combat.attackers.retain(|a| a.object_id != attack.angel); + } + apply_resolved_combat_membership(&mut replay, command) + .expect("the recorded edit must replay against a state without the Angel attacking"); + let (replay_defender, replay_target, replay_blocked) = attacker_entry(&replay, attack.angel) + .expect("replay must reinstate the Angel as an attacking creature"); + assert_eq!( + replay_defender, P1, + "replay installs the recorded defending player" + ); + assert_eq!( + replay_target, + AttackTarget::Player(P1), + "replay installs the recorded attack target" + ); + assert!( + !replay_blocked, + "CR 509.1h: a creature entering attacking is not blocked" + ); +} + +/// The point of the unit (landmine L1). `enter_attacking` picks its defender +/// from AMBIENT state, so the applier must never call it. This drives replay +/// into a state whose ambient derivation demonstrably answers DIFFERENTLY, and +/// proves the recorded defender still wins. +/// +/// CR 508.4 makes this a rules question, not just a determinism question: the +/// defender is a choice the controller already made, and re-deriving re-decides +/// a settled choice. +#[test] +fn replay_installs_recorded_defender_when_ambient_derivation_diverges() { + let attack = kaalia_attack(); + let state = attack.runner.state(); + let command = membership_commands(state, attack.journal_start) + .into_iter() + .find(|command| command.object.object_id == attack.angel) + .expect("the Angel's attacking entry must be journaled"); + + // Build a replay state where every ambient source disagrees with the + // record: Kaalia is no longer an attacker (defeating both the source lookup + // and the controller-scan), and the current trigger event names P0. + let mut replay = state.clone(); + if let Some(combat) = replay.combat.as_mut() { + combat + .attackers + .retain(|a| a.object_id != attack.angel && a.object_id != attack.kaalia); + } + replay.current_trigger_event = Some(GameEvent::DamageDealt { + source_id: attack.kaalia, + target: TargetRef::Player(P0), + amount: 1, + is_combat: false, + excess: 0, + }); + + // Non-vacuity probe, VERIFIED APPLIED: run the live authority in this exact + // state on a control creature. If it still derived P1 the test below would + // be vacuous — a re-deriving applier would coincidentally look correct. + let mut probe = replay.clone(); + engine::game::combat::enter_attacking(&mut probe, attack.control, attack.kaalia, P0); + let (probe_defender, _, _) = attacker_entry(&probe, attack.control) + .expect("the probe creature must be seated as an attacker"); + assert_eq!( + probe_defender, P0, + "probe: ambient derivation in this state must answer P0, otherwise the \ + divergence assertion below is vacuous" + ); + + // The discriminating assertion: replay ignores ambient state entirely. + apply_resolved_combat_membership(&mut replay, &command) + .expect("the recorded edit must replay regardless of ambient combat state"); + let (replay_defender, replay_target, _) = attacker_entry(&replay, attack.angel) + .expect("replay must reinstate the Angel as an attacking creature"); + assert_eq!( + replay_defender, P1, + "CR 508.4: replay installs the RECORDED defender, not the ambient one" + ); + assert_eq!( + replay_target, + AttackTarget::Player(P1), + "replay installs the recorded attack target, not the ambient one" + ); +} + +/// Fail-closed: an `expected_*` that no longer describes live state must return +/// a typed error instead of installing the `resulting_*` anyway. CR 506.3c and +/// CR 508.4a are the rules reason — a creature whose recorded defender is gone +/// is never an attacking creature, so silently seating it elsewhere is wrong. +#[test] +fn replay_fails_closed_when_the_recorded_precondition_no_longer_holds() { + let attack = kaalia_attack(); + let state = attack.runner.state(); + let command = membership_commands(state, attack.journal_start) + .into_iter() + .find(|command| command.object.object_id == attack.angel) + .expect("the Angel's attacking entry must be journaled"); + + // The Angel is ALREADY attacking in this state, so the command's + // "not yet an attacker" precondition is violated. + let mut replay = state.clone(); + assert!( + attacker_entry(&replay, attack.angel).is_some(), + "reach guard: the Angel must already be attacking for this to be a real conflict" + ); + let error = apply_resolved_combat_membership(&mut replay, &command) + .expect_err("replaying onto an already-attacking creature must fail closed"); + assert_eq!( + error, + ResolvedCombatMembershipReplayInvariantError::AlreadyAttacking(attack.angel), + "the failure is typed, not a silent duplicate attacker" + ); + // The rejection left no partial edit. + assert_eq!( + replay.combat.as_ref().map(|combat| combat + .attackers + .iter() + .filter(|a| a.object_id == attack.angel) + .count()), + Some(1), + "a rejected command must not have pushed a second attacker entry" + ); +} + +/// CR 506.4 removal: the authority records the exact roles it pruned, replay +/// reproduces the prune, and a participation mismatch fails closed. +/// +/// `damage_assignments` is carried in the receipt precisely because +/// `CombatState`'s `PartialEq` does not compare it. +#[test] +fn removal_journals_exact_participation_and_replays_it() { + let attack = kaalia_attack(); + let mut runner = attack.runner; + + let before_removal = runner.state().clone(); + let journal_start = before_removal.resolved_rules_journal.entries().len(); + let expected = CombatParticipation::capture(&before_removal, attack.angel); + + // Reach guard: the object really is in combat, so the removal below prunes + // something and the journal assertion cannot pass vacuously. + assert!( + !expected.is_empty(), + "reach guard: the Angel must hold a combat role before removal" + ); + assert!( + expected.attacking.is_some(), + "reach guard: the Angel is the attacking creature being pruned" + ); + + engine::game::effects::remove_from_combat::remove_object_from_combat( + runner.state_mut(), + attack.angel, + ); + + assert!( + attacker_entry(runner.state(), attack.angel).is_none(), + "CR 506.4: the removed creature stops being an attacking creature" + ); + + let commands: Vec<_> = membership_commands(runner.state(), journal_start) + .into_iter() + .filter(|command| command.object.object_id == attack.angel) + .collect(); + assert_eq!( + commands.len(), + 1, + "the removal authority must journal exactly one resolved edit" + ); + assert_eq!( + commands[0].edit, + ResolvedCombatMembershipEdit::Remove { + expected_participation: expected.clone(), + }, + "the receipt records the exact roles the removal pruned" + ); + + // Replay exactness against the captured predecessor. + let mut replay = before_removal.clone(); + apply_resolved_combat_membership(&mut replay, &commands[0]) + .expect("the recorded removal must replay against its captured predecessor"); + assert!( + attacker_entry(&replay, attack.angel).is_none(), + "replay reproduces the prune" + ); + assert_eq!( + CombatParticipation::capture(&replay, attack.angel), + CombatParticipation::default(), + "replay leaves the object holding no combat role at all" + ); + + // Fail-closed: the same command against a state where the object no longer + // participates must be rejected rather than pruning nothing silently. + let error = apply_resolved_combat_membership(&mut replay, &commands[0]) + .expect_err("re-applying a removal to an unparticipating object must fail closed"); + assert!( + matches!( + error, + ResolvedCombatMembershipReplayInvariantError::ParticipationMismatch { .. } + ), + "the failure is a typed participation mismatch, got {error:?}" + ); +} + +/// Verbatim Oracle text (Scryfall, 2026-07). +const DAZZLING_BEAUTY: &str = "Cast this spell only during the declare blockers step.\nTarget unblocked attacking creature becomes blocked."; + +/// Verbatim Oracle text (Scryfall, 2026-07). +const MIRROR_MATCH: &str = "Cast this spell only during the declare blockers step.\nFor each creature attacking you or a planeswalker you control, create a token that's a copy of that creature and that's blocking that creature. Exile those tokens at end of combat."; + +/// Drive P0's attack into the declare-blockers step and hand priority to P1 so +/// the defending player can cast during that step (CR 509). +fn advance_to_declare_blockers_and_give_priority(runner: &mut GameRunner, caster: PlayerId) { + for _ in 0..20 { + if runner.state().phase == Phase::DeclareBlockers { + let state = runner.state_mut(); + state.priority_player = caster; + state.waiting_for = WaitingFor::Priority { player: caster }; + return; + } + let wf = runner.state().waiting_for.clone(); + let acted = match wf { + WaitingFor::Priority { .. } => runner.act(GameAction::PassPriority), + WaitingFor::DeclareBlockers { .. } => runner.act(GameAction::DeclareBlockers { + assignments: vec![], + }), + other => panic!("unexpected waiting state before declare-blockers: {other:?}"), + }; + acted.expect("combat advance action"); + } + panic!("did not reach the declare-blockers step"); +} + +/// CR 509.1h via `mark_attacker_blocked`: an attacker made blocked purely by +/// effect, with no blocker assigned. Driven through a real Dazzling Beauty cast. +#[test] +fn become_blocked_journals_a_mark_blocked_edit_and_replays_it() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let attacker = scenario.add_creature(P0, "Charging Ox", 3, 3).id(); + let spell = scenario + .add_spell_to_hand_from_oracle(P1, "Dazzling Beauty", true, DAZZLING_BEAUTY) + .id(); + + let mut runner = scenario.build(); + runner.advance_to_combat(); + runner + .declare_attackers(&[(attacker, AttackTarget::Player(P1))]) + .expect("declare attackers"); + advance_to_declare_blockers_and_give_priority(&mut runner, P1); + + // Reach guard: the attacker must be an UNBLOCKED attacker before the cast, + // otherwise the mark below is a no-op and is never journaled. + let (_, _, blocked_before) = + attacker_entry(runner.state(), attacker).expect("the Ox must be attacking"); + assert!( + !blocked_before, + "reach guard: the attacker must start unblocked" + ); + + let journal_start = runner.state().resolved_rules_journal.entries().len(); + let before = runner.state().clone(); + runner.cast(spell).target_objects(&[attacker]).resolve(); + + // Reach guard: the effect actually took hold. + let (_, _, blocked_after) = + attacker_entry(runner.state(), attacker).expect("the Ox must still be attacking"); + assert!( + blocked_after, + "CR 509.1h: the resolved spell must make the attacker blocked" + ); + + let commands: Vec<_> = membership_commands(runner.state(), journal_start) + .into_iter() + .filter(|command| command.object.object_id == attacker) + .collect(); + assert_eq!( + commands.len(), + 1, + "the mark-blocked authority must journal exactly one resolved edit" + ); + assert_eq!( + commands[0].edit, + ResolvedCombatMembershipEdit::MarkBlocked, + "CR 509.1h: the recorded edit is the effect-driven blocked mark" + ); + + // Replay exactness against the pre-cast state. + let mut replay = before; + apply_resolved_combat_membership(&mut replay, &commands[0]) + .expect("the recorded mark must replay against its captured predecessor"); + let (_, _, replayed_blocked) = + attacker_entry(&replay, attacker).expect("replay keeps the Ox attacking"); + assert!(replayed_blocked, "replay installs the blocked bit"); + // CR 509.1h: marking blocked assigns NO blocker, so the maps stay empty. + let participation = CombatParticipation::capture(&replay, attacker); + assert!( + participation.blocked_by.is_empty(), + "CR 510.1c: an effect-blocked attacker has no creatures blocking it" + ); + + // Fail-closed: the bit is sticky, so re-applying must be rejected rather + // than silently re-marking. + let error = apply_resolved_combat_membership(&mut replay, &commands[0]) + .expect_err("re-marking an already-blocked attacker must fail closed"); + assert!( + matches!( + error, + ResolvedCombatMembershipReplayInvariantError::BlockedPreconditionMismatch { .. } + ), + "the failure is a typed blocked-precondition mismatch, got {error:?}" + ); +} + +/// CR 509.1g + CR 506.3e via `place_blocking`: a token put onto the battlefield +/// already blocking. Driven through a real Mirror Match cast. +/// +/// L3: the authority writes FOUR places — the attacker's sticky `blocked` bit, +/// `blocker_to_attacker`, `blocker_assignments`, and `creatures_blocked_this_turn` +/// (which lives on `GameState`, not `CombatState`). All four are asserted after +/// replay, individually. +#[test] +fn place_blocking_journals_a_block_edit_and_replays_all_four_writes() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let attacker = scenario.add_creature(P0, "Charging Ox", 3, 3).id(); + let spell = scenario + .add_spell_to_hand_from_oracle(P1, "Mirror Match", true, MIRROR_MATCH) + .id(); + + let mut runner = scenario.build(); + runner.advance_to_combat(); + runner + .declare_attackers(&[(attacker, AttackTarget::Player(P1))]) + .expect("declare attackers"); + advance_to_declare_blockers_and_give_priority(&mut runner, P1); + + let journal_start = runner.state().resolved_rules_journal.entries().len(); + runner.cast(spell).resolve(); + runner.advance_until_stack_empty(); + + // Reach guard: a copy token exists and is genuinely blocking the attacker. + let state = runner.state(); + let combat = state.combat.as_ref().expect("combat is live"); + let blockers = combat + .blocker_assignments + .get(&attacker) + .cloned() + .unwrap_or_default(); + assert_eq!( + blockers.len(), + 1, + "CR 509.1g: Mirror Match must put exactly one copy token in as a blocker" + ); + let token = blockers[0]; + assert!( + combat + .blocker_to_attacker + .get(&token) + .is_some_and(|a| a.contains(&attacker)), + "reach guard: the reverse lookup must name the attacker" + ); + + let commands: Vec<_> = membership_commands(state, journal_start) + .into_iter() + .filter(|command| command.object.object_id == token) + .collect(); + assert_eq!( + commands.len(), + 1, + "the blocking authority must journal exactly one resolved edit for the token" + ); + assert_eq!( + commands[0].edit, + ResolvedCombatMembershipEdit::Block { + resulting_attacker: attacker, + expected_attacker_blocked: false, + }, + "CR 509.1h: the token is the FIRST blocker, so the sticky bit was clear" + ); + + // Replay exactness: undo only this family's edit, then reinstall it. + let mut replay = state.clone(); + if let Some(combat) = replay.combat.as_mut() { + combat.blocker_assignments.remove(&attacker); + combat.blocker_to_attacker.remove(&token); + for info in combat.attackers.iter_mut() { + if info.object_id == attacker { + info.blocked = false; + } + } + } + replay.creatures_blocked_this_turn.remove(&token); + + apply_resolved_combat_membership(&mut replay, &commands[0]) + .expect("the recorded block must replay against its predecessor"); + + // All four writes, asserted individually. `CombatState`'s hand-written + // `PartialEq` omits four fields, so a whole-struct equality check here + // would be blind to exactly the bookkeeping this family edits. + let replayed = replay.combat.as_ref().expect("combat is live after replay"); + assert!( + replayed + .attackers + .iter() + .any(|a| a.object_id == attacker && a.blocked), + "write 1 — CR 509.1h: the attacker's sticky blocked bit" + ); + assert_eq!( + replayed.blocker_to_attacker.get(&token), + Some(&vec![attacker]), + "write 2 — CR 509.1g: the blocker -> attacker reverse lookup" + ); + assert_eq!( + replayed.blocker_assignments.get(&attacker), + Some(&vec![token]), + "write 3 — CR 509.1g: the attacker -> blocker forward assignment" + ); + assert!( + replay.creatures_blocked_this_turn.contains(&token), + "write 4 — CR 509.1a: the per-turn blocked-this-turn set on GameState" + ); +} diff --git a/crates/engine/tests/integration/cr733_resolved_commands_p2.rs b/crates/engine/tests/integration/cr733_resolved_commands_p2.rs index 96995efa58..41ffe0d375 100644 --- a/crates/engine/tests/integration/cr733_resolved_commands_p2.rs +++ b/crates/engine/tests/integration/cr733_resolved_commands_p2.rs @@ -91,6 +91,9 @@ fn apply_semantic_command(state: &mut GameState, command: &ResolvedRulesCommand) .apply_resolved_continuous_effect(command.as_ref()) .unwrap(); } + ResolvedRulesCommand::CombatMembership(command) => { + engine::game::combat::apply_resolved_combat_membership(state, command).unwrap(); + } ResolvedRulesCommand::ControllerOverride(command) => { engine::game::zones::apply_resolved_controller_override(state, command).unwrap(); } @@ -216,6 +219,7 @@ fn exact_mana_spend_rejects_a_second_removal() { | ResolvedRulesCommand::Attachment(_) | ResolvedRulesCommand::DelayedTriggerInstall(_) | ResolvedRulesCommand::ContinuousEffectInstall(_) + | ResolvedRulesCommand::CombatMembership(_) | ResolvedRulesCommand::ControllerOverride(_) | ResolvedRulesCommand::EntryProvenance(_) | ResolvedRulesCommand::ObjectCease(_) diff --git a/crates/engine/tests/integration/cr733_resolved_draw.rs b/crates/engine/tests/integration/cr733_resolved_draw.rs index a9c6e99b6f..6d5f671110 100644 --- a/crates/engine/tests/integration/cr733_resolved_draw.rs +++ b/crates/engine/tests/integration/cr733_resolved_draw.rs @@ -56,6 +56,9 @@ fn apply_semantic_command(state: &mut GameState, command: &ResolvedRulesCommand) .apply_resolved_continuous_effect(command.as_ref()) .unwrap(); } + ResolvedRulesCommand::CombatMembership(command) => { + engine::game::combat::apply_resolved_combat_membership(state, command).unwrap(); + } ResolvedRulesCommand::ControllerOverride(command) => { engine::game::zones::apply_resolved_controller_override(state, command).unwrap(); } diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index b964caec95..3153af2edf 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -92,6 +92,7 @@ mod counter_double_redirect_choice; mod counter_spell_zone_redirect; mod court_of_cunning_multi_target_mill; mod cr733_resolved_attachment; +mod cr733_resolved_combat_membership; mod cr733_resolved_commands_p0; mod cr733_resolved_commands_p1; mod cr733_resolved_commands_p2;