feat(engine): journal effect-driven combat membership as CR733 resolved commands

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)]

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<CombatParticipation>,
+        found: Box<CombatParticipation>,
+    },
+}
 
 /// 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<ResolvedDelayedTriggerCommand>),
     ContinuousEffectInstall(Box<ResolvedContinuousEffectCommand>),
+    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<ResolvedCommandOrdinal, ResolvedRulesJournalError> {
+        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