diff --git a/crates/engine/src/game/effects/mod.rs b/crates/engine/src/game/effects/mod.rs index b649049d30..51625c8f82 100644 --- a/crates/engine/src/game/effects/mod.rs +++ b/crates/engine/src/game/effects/mod.rs @@ -9473,18 +9473,22 @@ fn resolve_chain_body( .collect(); if let Some(first) = opponent_order.first().copied() { let remaining = opponent_order.split_off(1); - state.push_optional_effect_frame(OptionalEffectFrame { - ability: Box::new(ability_with_event_context_targets(state, ability)), - trigger_event: state.current_trigger_event.clone(), - trigger_events: state.current_trigger_events.clone(), - trigger_match_count: state.current_trigger_match_count, - }); - state.waiting_for = WaitingFor::OpponentMayChoice { - player: first, - source_id: ability.source_id, - description, - remaining, - }; + state + .install_direct_choice_frame( + ResolutionFrame::OptionalEffect(OptionalEffectFrame { + ability: Box::new(ability_with_event_context_targets(state, ability)), + trigger_event: state.current_trigger_event.clone(), + trigger_events: state.current_trigger_events.clone(), + trigger_match_count: state.current_trigger_match_count, + }), + WaitingFor::OpponentMayChoice { + player: first, + source_id: ability.source_id, + description, + remaining, + }, + ) + .map_err(|error| EffectError::InvalidParam(error.to_string()))?; } return Ok(()); } @@ -9549,30 +9553,34 @@ fn resolve_chain_body( return Ok(()); } } - state.push_optional_effect_frame(OptionalEffectFrame { - ability: Box::new(ability_with_event_context_targets(state, ability)), - // CR 608.2: capture the triggering event in lockstep with the stashed - // ability while `current_trigger_event` is still live (we are inside - // `execute_effect`). Restored when the optional decision resumes so an - // optional ("may") trigger's effect resolves `TriggeringPlayer` and - // other event-context refs exactly as a non-optional trigger would. - trigger_event: state.current_trigger_event.clone(), - // CR 603.2c + CR 608.2: capture the PLURAL event batch in lockstep so - // a "you may" reproduction (Captain Marvel, Apex Avenger) folds every - // `CounterAdded` occurrence when the decision resumes. - trigger_events: state.current_trigger_events.clone(), - // CR 603.2c + CR 608.2: mirror the batched-trigger subject count so a - // "you may" sub-ability of a batched trigger (Ur-Dragon's optional - // permanent-from-hand sub-effect) resumes with the same - // `EventContextAmount` the pre-pause resolution observed. - trigger_match_count: state.current_trigger_match_count, - }); - state.waiting_for = WaitingFor::OptionalEffectChoice { - player: prompt_player, - source_id: ability.source_id, - description, - may_trigger_key, - }; + state + .install_direct_choice_frame( + ResolutionFrame::OptionalEffect(OptionalEffectFrame { + ability: Box::new(ability_with_event_context_targets(state, ability)), + // CR 608.2: capture the triggering event in lockstep with the stashed + // ability while `current_trigger_event` is still live (we are inside + // `execute_effect`). Restored when the optional decision resumes so an + // optional ("may") trigger's effect resolves `TriggeringPlayer` and + // other event-context refs exactly as a non-optional trigger would. + trigger_event: state.current_trigger_event.clone(), + // CR 603.2c + CR 608.2: capture the PLURAL event batch in lockstep so a + // "you may" reproduction (Captain Marvel, Apex Avenger) folds every + // `CounterAdded` occurrence when the decision resumes. + trigger_events: state.current_trigger_events.clone(), + // CR 603.2c + CR 608.2: mirror the batched-trigger subject count so a + // "you may" sub-ability of a batched trigger (Ur-Dragon's optional + // permanent-from-hand sub-effect) resumes with the same + // `EventContextAmount` the pre-pause resolution observed. + trigger_match_count: state.current_trigger_match_count, + }), + WaitingFor::OptionalEffectChoice { + player: prompt_player, + source_id: ability.source_id, + description, + may_trigger_key, + }, + ) + .map_err(|error| EffectError::InvalidParam(error.to_string()))?; return Ok(()); } diff --git a/crates/engine/src/game/engine.rs b/crates/engine/src/game/engine.rs index 276f2590d7..ec3c030253 100644 --- a/crates/engine/src/game/engine.rs +++ b/crates/engine/src/game/engine.rs @@ -15916,7 +15916,16 @@ mod stage2_injector_tests { } if test_file || spans.iter().any(|(a, b)| (*a..=*b).contains(&n)) { in_test += 1; - } else if line.contains("waiting_for = ") || line.contains("Ok(Some(") { + } else if line.contains("waiting_for = ") + || line.contains("Ok(Some(") + // `install_direct_choice_frame` owns the actual + // `state.waiting_for` write. Its typed prompt argument is + // still a production mint, not a reader; the call sits + // within this bounded argument expression. + || lines[n.saturating_sub(32)..n] + .iter() + .any(|prior| prior.contains(".install_direct_choice_frame(")) + { producers.push(format!("{rel}:{}", n + 1)); } else { readers.push(format!("{rel}:{}", n + 1)); @@ -16037,7 +16046,7 @@ mod stage2_injector_tests { // `OptionalEffect` prompt. Re-pinned against the merged source. "game/effects/mod.rs:6300".to_string(), "game/effects/mod.rs:6377".to_string(), - "game/effects/mod.rs:9570".to_string(), + "game/effects/mod.rs:9576".to_string(), // UNMOVED across the rebase, and that is itself evidence the SET did not // move: a census that had gained or lost a producer would not leave this // entry both byte-identical AND at the same coordinate. diff --git a/crates/engine/src/types/game_state.rs b/crates/engine/src/types/game_state.rs index 6804f91cab..6aab8ba753 100644 --- a/crates/engine/src/types/game_state.rs +++ b/crates/engine/src/types/game_state.rs @@ -45,7 +45,7 @@ use super::replacements::ReplacementEvent; #[cfg(debug_assertions)] use super::resolution::debug_assert_runtime_resolution_invariants; use super::resolution::{ - AbilityContinuationFrame, ChangeZoneFrame, MultiDrawFrame, OptionalEffectFrame, + AbilityContinuationFrame, ChangeZoneFrame, FrameGate, MultiDrawFrame, OptionalEffectFrame, PendingCoinFlip, PendingMutateMerge, PendingProliferateActions, RepeatedOptionalPaymentFrame, ResolutionFrame, ResolutionStack, ResolutionStackError, ResolutionStateWire, }; @@ -18767,6 +18767,38 @@ impl GameState { Ok(command) } + /// Atomically installs a direct-choice owner and the prompt it is allowed to + /// consume. A direct-choice frame may never be visible with an unrelated + /// `WaitingFor`, nor may it be buried below another direct-choice owner. + pub fn install_direct_choice_frame( + &mut self, + frame: ResolutionFrame, + waiting_for: WaitingFor, + ) -> Result<(), ResolutionStackError> { + if !matches!(frame.gate(), FrameGate::DirectChoice(_)) { + return Err(ResolutionStackError::InvalidPayload { + frame: frame.kind(), + message: "direct-choice installation requires a direct-choice frame".to_string(), + }); + } + + let transition = ResolvedFrameTransition::Push { + frame: frame.clone(), + }; + let mut candidate = (*self.resolution_stack).clone(); + candidate.push_inner(frame); + candidate.validate(&waiting_for)?; + + let previous_waiting_for = std::mem::replace(&mut self.waiting_for, waiting_for); + if let Err(error) = self.resolve_and_apply_frame_transition(transition) { + self.waiting_for = previous_waiting_for; + return Err(match error { + ResolvedFrameTransitionReplayInvariantError::Stack(error) => error, + }); + } + Ok(()) + } + /// Applies and journals one already-resolved player resource edit. /// /// Replacements and dynamic quantities must be settled before this boundary; diff --git a/crates/engine/tests/integration/cr733_resolved_frame_transition.rs b/crates/engine/tests/integration/cr733_resolved_frame_transition.rs index a5c3c362cc..8cfd768cd5 100644 --- a/crates/engine/tests/integration/cr733_resolved_frame_transition.rs +++ b/crates/engine/tests/integration/cr733_resolved_frame_transition.rs @@ -50,6 +50,15 @@ fn coin_flip_frame() -> ResolutionFrame { }) } +fn optional_effect_frame(state: &GameState) -> ResolutionFrame { + ResolutionFrame::OptionalEffect(OptionalEffectFrame { + ability: pending_continuation(state).chain, + trigger_event: None, + trigger_events: Vec::new(), + trigger_match_count: None, + }) +} + fn frames(state: &GameState) -> Vec { state.resolution_stack.iter().cloned().collect() } @@ -179,6 +188,42 @@ fn optional_effect_frame_cannot_survive_into_search_choice_parent_insertion() { assert_eq!(state.resolution_stack, before); } +/// Regression for #6867: direct-choice ownership and its live prompt are one +/// atomic state transition. A second optional owner is rejected at installation +/// and leaves the first player-facing prompt untouched. +#[test] +fn direct_choice_install_rejects_a_second_optional_owner_atomically() { + let mut state = GameState::new_two_player(98); + let first_prompt = WaitingFor::OptionalEffectChoice { + player: PlayerId(0), + source_id: ObjectId(100), + description: None, + may_trigger_key: None, + }; + let first_frame = optional_effect_frame(&state); + state + .install_direct_choice_frame(first_frame, first_prompt.clone()) + .expect("first optional prompt installs"); + let before_stack = state.resolution_stack.clone(); + let before_waiting_for = state.waiting_for.clone(); + + let second_frame = optional_effect_frame(&state); + assert_eq!( + state.install_direct_choice_frame( + second_frame, + WaitingFor::OptionalEffectChoice { + player: PlayerId(1), + source_id: ObjectId(101), + description: None, + may_trigger_key: None, + }, + ), + Err(ResolutionStackError::MultipleDirectChoiceOwners) + ); + assert_eq!(state.resolution_stack, before_stack); + assert_eq!(state.waiting_for, before_waiting_for); +} + /// Missing parents and wrong active kinds are typed failures; neither failure /// changes the existing stack. #[test]