diff --git a/client/src/adapter/types.ts b/client/src/adapter/types.ts index 3871dfc3d0..bec16e8da5 100644 --- a/client/src/adapter/types.ts +++ b/client/src/adapter/types.ts @@ -2433,7 +2433,9 @@ export type PlayerActionKind = | "CollectEvidence" | "ShuffledLibrary" | "Proliferate" - | "Investigate"; + | "Investigate" + | "Draw" + | "Forage"; export type GameEvent = | { type: "GameStarted" } diff --git a/client/src/adapter/ws-adapter.ts b/client/src/adapter/ws-adapter.ts index b74a1f155d..7f9b37e4ec 100644 --- a/client/src/adapter/ws-adapter.ts +++ b/client/src/adapter/ws-adapter.ts @@ -203,6 +203,7 @@ export class NativeEngineVersionMismatchError extends Error { * `crates/server-core/src/protocol.rs`. Bump in lockstep when either side * adds, removes, renames, or changes the type of a protocol variant field. * + * 30 — Serialized player-action completion provenance and modal continuations. * 29 — Added requester-correlated ResolveAllRejected response frames. * 28 — Added native ResolveAll request/result frames. * 27 — Added DraftKind.Sealed, serialized by draft WebSocket messages. @@ -236,7 +237,7 @@ export class NativeEngineVersionMismatchError extends Error { * into a MulliganDecisionPhase::BottomCards sub-phase on * WaitingFor::MulliganDecision. */ -export const PROTOCOL_VERSION = 29; +export const PROTOCOL_VERSION = 30; /** * Lowest server protocol version this client will accept in the handshake. diff --git a/client/src/network/__tests__/protocol.test.ts b/client/src/network/__tests__/protocol.test.ts index a38afacc21..541a869842 100644 --- a/client/src/network/__tests__/protocol.test.ts +++ b/client/src/network/__tests__/protocol.test.ts @@ -36,8 +36,8 @@ const viewerInteractionWithProducedMana = { } as never; describe("encodeWireMessage / decodeWireMessage", () => { - it("pins the P2P wire protocol to v18", () => { - expect(WIRE_PROTOCOL_VERSION).toBe(19); + it("pins the P2P wire protocol to v20", () => { + expect(WIRE_PROTOCOL_VERSION).toBe(20); }); it("defaults shortcut actions for a legacy payload created before the additive field", () => { diff --git a/client/src/network/protocol.ts b/client/src/network/protocol.ts index bd37dbf74f..9f70d390a3 100644 --- a/client/src/network/protocol.ts +++ b/client/src/network/protocol.ts @@ -80,6 +80,7 @@ export function legalActionsFromWire(wire: LegalActionsWire): LegalActionsResult * of silently corrupting state. * * Bumps to date: + * 20 — Serialized player-action completion provenance and modal continuations. * 19 — Added an action_noop acknowledgement for accepted transport no-ops. * 18 — DebugCardEntries added a serialized, private resolution frame for * multi-card sandbox battlefield entries that pause for replacement or @@ -111,7 +112,7 @@ export function legalActionsFromWire(wire: LegalActionsWire): LegalActionsResult * sub-phase on WaitingFor::MulliganDecision; the MulliganBottomCards * variant was removed */ -export const WIRE_PROTOCOL_VERSION = 19 as const; +export const WIRE_PROTOCOL_VERSION = 20 as const; export type P2PMessage = P2PAuthorityWire & ( | { type: "guest_deck"; deckData: unknown; displayName?: string; reservationToken?: string } diff --git a/crates/engine/src/analysis/ability_graph.rs b/crates/engine/src/analysis/ability_graph.rs index 800e9de790..19c31dedc6 100644 --- a/crates/engine/src/analysis/ability_graph.rs +++ b/crates/engine/src/analysis/ability_graph.rs @@ -1016,6 +1016,7 @@ fn effect_projection(effect: &Effect) -> Projection { | Effect::Adapt { .. } | Effect::Learn | Effect::Forage + | Effect::CompletePlayerAction { .. } | Effect::Harness | Effect::CollectEvidence { .. } | Effect::Endure { .. } diff --git a/crates/engine/src/game/ability_rw.rs b/crates/engine/src/game/ability_rw.rs index 3d6b8a59b6..1100cb0ac4 100644 --- a/crates/engine/src/game/ability_rw.rs +++ b/crates/engine/src/game/ability_rw.rs @@ -3421,6 +3421,7 @@ fn legacy_effect(x: &Effect) -> bool { | Effect::Learn | Effect::NoteManaSpent | Effect::Forage + | Effect::CompletePlayerAction { .. } | Effect::Harness | Effect::CollectEvidence { .. } | Effect::Specialize @@ -4758,6 +4759,7 @@ fn rw_effect( p.writes_membership_external_zones.merge(ZoneSpan::Any); (p, None) } + Effect::CompletePlayerAction { .. } => (RwProfile::conservative(), None), Effect::Connive { target, count } => { let (mut p, sc) = obj(StateKind::ObjectCounters, target); p.writes_external.set(StateKind::HandLibrary); diff --git a/crates/engine/src/game/ability_scan.rs b/crates/engine/src/game/ability_scan.rs index 15d91485f9..8757a312bb 100644 --- a/crates/engine/src/game/ability_scan.rs +++ b/crates/engine/src/game/ability_scan.rs @@ -1734,6 +1734,7 @@ fn scan_effect(x: &Effect, mode: ScanMode) -> Axes { } Effect::Learn => Axes::NONE, Effect::Forage => Axes::NONE, + Effect::CompletePlayerAction { .. } => Axes::NONE, Effect::Harness => Axes::NONE, Effect::CollectEvidence { amount: _ } => Axes::NONE, Effect::Endure { amount, subject } => { @@ -5568,6 +5569,7 @@ fn effect_target_ctx(e: &Effect, mode: ScanMode) -> FilterReadContext { | Effect::Adapt { .. } | Effect::Learn | Effect::Forage + | Effect::CompletePlayerAction { .. } | Effect::Harness | Effect::CollectEvidence { .. } | Effect::Endure { .. } @@ -5946,6 +5948,7 @@ fn effect_census_role(e: &Effect) -> CensusRole { | Effect::Adapt { .. } | Effect::Learn | Effect::Forage + | Effect::CompletePlayerAction { .. } | Effect::Harness | Effect::Endure { .. } | Effect::BlightEffect { .. } @@ -6206,6 +6209,7 @@ pub(crate) fn effect_is_randomness_bearing(e: &Effect) -> bool { | Effect::Adapt { .. } | Effect::Learn | Effect::Forage + | Effect::CompletePlayerAction { .. } | Effect::Harness | Effect::CollectEvidence { .. } | Effect::Endure { .. } diff --git a/crates/engine/src/game/contraptions.rs b/crates/engine/src/game/contraptions.rs index 776065dbd0..c6ea9645d1 100644 --- a/crates/engine/src/game/contraptions.rs +++ b/crates/engine/src/game/contraptions.rs @@ -245,6 +245,7 @@ pub(crate) fn continue_assemble_batch( parent_targets: Vec::new(), context: crate::types::ability::SpellContext::default(), replacement_applied: Default::default(), + continuation: None, players: vec![player], }, ); @@ -421,6 +422,7 @@ fn prompt_reassemble_sprocket_choice( parent_targets: ability.targets.clone(), context: ability.context.clone(), replacement_applied: ability.replacement_applied.clone(), + continuation: None, players: vec![ability.controller], }, ); diff --git a/crates/engine/src/game/coverage.rs b/crates/engine/src/game/coverage.rs index 2be6093fb7..0a934aa01c 100644 --- a/crates/engine/src/game/coverage.rs +++ b/crates/engine/src/game/coverage.rs @@ -3712,6 +3712,7 @@ fn effect_details(effect: &Effect) -> Vec<(String, String)> { | Effect::ChangeTargets { .. } | Effect::ExchangeControl { .. } | Effect::Forage + | Effect::CompletePlayerAction { .. } | Effect::Harness | Effect::Learn | Effect::NoteManaSpent @@ -6586,6 +6587,7 @@ fn visit_direct_effect_ability_payloads<'a>( | Effect::Adapt { .. } | Effect::Learn | Effect::Forage + | Effect::CompletePlayerAction { .. } | Effect::Harness | Effect::CollectEvidence { .. } | Effect::Endure { .. } diff --git a/crates/engine/src/game/effects/change_zone.rs b/crates/engine/src/game/effects/change_zone.rs index 06162bf02a..5116c43f3c 100644 --- a/crates/engine/src/game/effects/change_zone.rs +++ b/crates/engine/src/game/effects/change_zone.rs @@ -4,8 +4,8 @@ use crate::game::game_object::AttachTarget; #[cfg(test)] use crate::game::zones; use crate::types::ability::{ - ControllerRef, Duration, Effect, EffectError, EffectKind, FilterProp, LibraryPosition, - QuantityExpr, ResolvedAbility, TargetChoiceTiming, TargetFilter, TargetRef, + ControllerRef, Duration, Effect, EffectError, EffectKind, EffectResolutionResult, FilterProp, + LibraryPosition, QuantityExpr, ResolvedAbility, TargetChoiceTiming, TargetFilter, TargetRef, TargetSelectionMode, TypeFilter, TypedFilter, }; #[cfg(test)] @@ -410,7 +410,8 @@ pub fn resolve( state: &mut GameState, ability: &ResolvedAbility, events: &mut Vec, -) -> Result<(), EffectError> { +) -> Result, EffectError> { + let events_before = events.len(); let ( origin, dest_zone, @@ -482,6 +483,11 @@ pub fn resolve( _ => return Err(EffectError::MissingParam("Destination".to_string())), }; + let completed_result = |count| { + super::this_way_cause_for_zone(dest_zone) + .map(|cause| EffectResolutionResult { cause, count }) + }; + let mut origin = origin; let parsed_target = match &ability.effect { @@ -605,7 +611,7 @@ pub fn resolve( source_id: ability.source_id, subject: None, }); - return Ok(()); + return Ok(completed_result(0)); } // CR 400.7: SelfRef resolves only to the exact source or, for a departure @@ -618,7 +624,7 @@ pub fn resolve( source_id: ability.source_id, subject: None, }); - return Ok(()); + return Ok(completed_result(0)); } // CR 400.7 + CR 603.7c: a delayed ability whose pinned referent became a @@ -639,7 +645,7 @@ pub fn resolve( source_id: ability.source_id, subject: None, }); - return Ok(()); + return Ok(completed_result(0)); } // CR 701.23b + CR 401.2: Interactive library-step fail-to-find guard. @@ -678,7 +684,7 @@ pub fn resolve( source_id: ability.source_id, subject: None, }); - return Ok(()); + return Ok(completed_result(0)); } // CR 608.2c: A tracked-set filter ("from among the milled cards" / "X @@ -757,7 +763,7 @@ pub fn resolve( source_id: ability.source_id, subject: None, }); - return Ok(()); + return Ok(completed_result(0)); } if eligible.is_empty() { @@ -769,7 +775,7 @@ pub fn resolve( source_id: ability.source_id, subject: None, }); - return Ok(()); + return Ok(completed_result(0)); } if matches!(ability.target_selection_mode, TargetSelectionMode::Random) @@ -844,9 +850,9 @@ pub fn resolve( ability.source_id, ); crate::game::replacement::park_waiting_for(state, player); - return Ok(()); + return Ok(None); } - ZoneMoveResult::NeedsAuraAttachmentChoice => return Ok(()), + ZoneMoveResult::NeedsAuraAttachmentChoice => return Ok(None), } // CR 614.13a: single-pick entry completed (Done branch) — clear the @@ -865,7 +871,11 @@ pub fn resolve( source_id: ability.source_id, subject: None, }); - return Ok(()); + return Ok(completed_result(count_selected_zone_arrivals( + &events[events_before..], + &[chosen], + dest_zone, + ))); } if eligible.len() == 1 && !choice_up_to && choice_count == 1 { @@ -936,9 +946,9 @@ pub fn resolve( ability.source_id, ); crate::game::replacement::park_waiting_for(state, player); - return Ok(()); + return Ok(None); } - ZoneMoveResult::NeedsAuraAttachmentChoice => return Ok(()), + ZoneMoveResult::NeedsAuraAttachmentChoice => return Ok(None), } // CR 614.13a: single-pick entry completed (Done branch) — clear the @@ -957,7 +967,11 @@ pub fn resolve( source_id: ability.source_id, subject: None, }); - return Ok(()); + return Ok(completed_result(count_selected_zone_arrivals( + &events[events_before..], + &[chosen], + dest_zone, + ))); } state.waiting_for = WaitingFor::EffectZoneChoice { @@ -998,7 +1012,7 @@ pub fn resolve( }; // EffectResolved is emitted by the EffectZoneChoice handler after the player chooses // (matching the DiscardChoice pattern — single authority for the event). - return Ok(()); + return Ok(None); } let ctx = ChangeZoneIterationCtx { @@ -1155,7 +1169,7 @@ pub fn resolve( effect_kind: EffectKind::from(&ability.effect), }, ); - return Ok(()); + return Ok(None); } crate::game::zone_pipeline::ZoneMoveTerminalResult::NeedsChoice(player) => { // CR 614.12b + CR 614.1c + CR 614.13: stash the unprocessed targets @@ -1220,7 +1234,7 @@ pub fn resolve( crate::game::replacement::park_waiting_for(state, player); // EffectResolved is emitted by the drain after the loop completes — // do NOT emit here. - return Ok(()); + return Ok(None); } } } @@ -1256,7 +1270,30 @@ pub fn resolve( subject: None, }); - Ok(()) + Ok(completed_result(count_selected_zone_arrivals( + &events[events_before..], + &targeted_objects, + dest_zone, + ))) +} + +/// CR 614.6 + CR 608.2c: Count only selected members that actually arrived in +/// the requested destination during this operation's exact event slice. +pub(crate) fn count_selected_zone_arrivals( + events: &[GameEvent], + selected: &[ObjectId], + destination: Zone, +) -> usize { + events + .iter() + .filter(|event| { + matches!( + event, + GameEvent::ZoneChanged { object_id, to, .. } + if *to == destination && selected.contains(object_id) + ) + }) + .count() } /// CR 122.1 + CR 614.1c: Merge unconditional and conditional entry-time counters diff --git a/crates/engine/src/game/effects/choose_one_of.rs b/crates/engine/src/game/effects/choose_one_of.rs index 9f113286b6..5cca407265 100644 --- a/crates/engine/src/game/effects/choose_one_of.rs +++ b/crates/engine/src/game/effects/choose_one_of.rs @@ -50,6 +50,7 @@ pub fn resolve( branches, parent_targets: ability.targets.clone(), context: ability.context.clone(), + continuation: ability.sub_ability.clone(), replacement_applied: ability.replacement_applied.clone(), players, }, @@ -69,6 +70,7 @@ pub(crate) struct PromptRequest { pub branches: Vec, pub parent_targets: Vec, pub context: crate::types::ability::SpellContext, + pub continuation: Option>, pub replacement_applied: HashSet, pub players: Vec, } @@ -80,6 +82,7 @@ pub(crate) fn prompt_next(state: &mut GameState, request: PromptRequest) { branches, parent_targets, context, + continuation, replacement_applied, mut players, } = request; @@ -96,6 +99,7 @@ pub(crate) fn prompt_next(state: &mut GameState, request: PromptRequest) { branch_descriptions, parent_targets, context, + continuation, replacement_applied, remaining_players: players, }; @@ -125,6 +129,7 @@ pub(crate) fn resume_pending(state: &mut GameState, _events: &mut Vec branches: pending.branches, parent_targets: pending.parent_targets, context: pending.context, + continuation: pending.continuation, replacement_applied: pending.replacement_applied, players: pending.remaining_players, }, @@ -138,6 +143,7 @@ pub(crate) struct BranchSelection { pub branches: Vec, pub parent_targets: Vec, pub context: crate::types::ability::SpellContext, + pub continuation: Option>, pub replacement_applied: HashSet, pub remaining_players: Vec, pub index: usize, @@ -155,6 +161,7 @@ pub(crate) fn resolve_branch( branches, parent_targets, context, + continuation, replacement_applied, remaining_players, index, @@ -164,14 +171,16 @@ pub(crate) fn resolve_branch( "ChooseOneOf branch index {index} out of range" ))); }; + let is_final_chooser = remaining_players.is_empty(); - if !remaining_players.is_empty() { + if !is_final_chooser { state.push_choose_one_of(PendingChooseOneOf { controller, source_id, branches: branches.clone(), parent_targets: parent_targets.clone(), context: context.clone(), + continuation: continuation.clone(), replacement_applied: replacement_applied.clone(), remaining_players, }); @@ -190,6 +199,16 @@ pub(crate) fn resolve_branch( resolved.targets.push(TargetRef::Player(player)); } + // CR 608.2c + CR 701.55d: Instructions after a multi-player branch choice + // run once, after the final chooser's selected branch. Keeping the runtime + // continuation in the typed choice carrier preserves its exact resolved + // context without converting it back into an AbilityDefinition. + if is_final_chooser { + if let Some(continuation) = continuation { + crate::game::ability_utils::append_to_sub_chain(&mut resolved, *continuation); + } + } + super::resolve_ability_chain(state, &resolved, events, 1)?; resume_pending(state, events); // NOTE: the token-choice applied seed is intentionally NOT cleared here. @@ -468,7 +487,16 @@ mod tests { Vec::new(), source, PlayerId(0), - ); + ) + .sub_ability(ResolvedAbility::new( + Effect::GainLife { + amount: QuantityExpr::Fixed { value: 10 }, + player: TargetFilter::Controller, + }, + Vec::new(), + source, + PlayerId(0), + )); let mut events = Vec::new(); super::resolve(&mut state, &ability, &mut events).expect("first opponent is prompted"); @@ -498,7 +526,10 @@ mod tests { }, ) .expect("resolving the pause must drain its branch continuation"); - assert_eq!(state.players[0].life, 21); + assert_eq!( + state.players[0].life, 21, + "the outer tail must not run until the paused first branch and every chooser finish" + ); assert!(matches!( state.waiting_for, WaitingFor::ChooseOneOfBranch { @@ -517,11 +548,72 @@ mod tests { }, ) .expect("second branch pause resolves"); - assert_eq!(state.players[0].life, 22); + assert_eq!( + state.players[0].life, 32, + "two branch gains plus exactly one outer tail after the final paused branch" + ); assert!(matches!(state.waiting_for, WaitingFor::Priority { .. })); assert!(state.resolution_stack.is_empty()); } + #[test] + fn multi_chooser_runtime_tail_runs_once_after_final_choice() { + // CR 701.55d + CR 608.2c: each instructed player resolves a branch in + // APNAP order, then the instruction following the whole choice runs + // once. It must not run after every player's branch. + let mut state = GameState::new(FormatConfig::standard(), 3, 42); + let branch = AbilityDefinition::new( + AbilityKind::Spell, + Effect::GainLife { + amount: QuantityExpr::Fixed { value: 1 }, + player: TargetFilter::Controller, + }, + ); + let tail = ResolvedAbility::new( + Effect::GainLife { + amount: QuantityExpr::Fixed { value: 10 }, + player: TargetFilter::Controller, + }, + Vec::new(), + ObjectId(1), + PlayerId(0), + ); + let ability = ResolvedAbility::new( + Effect::ChooseOneOf { + chooser: PlayerFilter::Opponent, + branches: vec![branch], + }, + Vec::new(), + ObjectId(1), + PlayerId(0), + ) + .sub_ability(tail); + let mut events = Vec::new(); + + resolve(&mut state, &ability, &mut events).expect("first opponent is prompted"); + apply_as_current(&mut state, GameAction::ChooseBranch { index: 0 }) + .expect("first opponent chooses a branch"); + assert_eq!( + state.players[0].life, 21, + "the outer tail cannot run before the final chooser" + ); + assert!(matches!( + state.waiting_for, + WaitingFor::ChooseOneOfBranch { + player: PlayerId(2), + .. + } + )); + + apply_as_current(&mut state, GameAction::ChooseBranch { index: 0 }) + .expect("final opponent chooses a branch"); + assert_eq!( + state.players[0].life, 32, + "two branch gains plus exactly one ten-life outer tail" + ); + assert!(matches!(state.waiting_for, WaitingFor::Priority { .. })); + } + #[test] fn token_branches_without_descriptions_get_create_labels() { let food = AbilityDefinition::new( diff --git a/crates/engine/src/game/effects/complete_player_action.rs b/crates/engine/src/game/effects/complete_player_action.rs new file mode 100644 index 0000000000..5651fa63eb --- /dev/null +++ b/crates/engine/src/game/effects/complete_player_action.rs @@ -0,0 +1,164 @@ +use crate::types::ability::{Effect, EffectError, ResolvedAbility}; +use crate::types::events::GameEvent; +use crate::types::game_state::GameState; + +/// CR 608.2c: A player action completes only when the immediately preceding +/// operation produced the exact typed result required by this continuation. +pub(crate) fn succeeded(ability: &ResolvedAbility) -> bool { + let Effect::CompletePlayerAction { + required_result, .. + } = &ability.effect + else { + return false; + }; + ability.context.prior_effect_result.as_ref() == Some(required_result) +} + +/// CR 603.2 + CR 608.2c: Publish the completed action after its final +/// operation result is known. The `EffectResolved` event is deliberately +/// adjacent and first, matching the engine's trigger ordering contract. +pub(crate) fn resolve( + _state: &mut GameState, + ability: &ResolvedAbility, + events: &mut Vec, +) -> Result<(), EffectError> { + let Effect::CompletePlayerAction { + parent_kind, + action, + .. + } = &ability.effect + else { + return Err(EffectError::InvalidParam( + "complete_player_action resolver requires CompletePlayerAction".to_string(), + )); + }; + + events.push(GameEvent::EffectResolved { + kind: *parent_kind, + source_id: ability.source_id, + subject: None, + }); + if succeeded(ability) { + events.push(GameEvent::PlayerPerformedAction { + player_id: ability.controller, + action: *action, + look_count: None, + scry_bottom_count: None, + scry_top_count: None, + }); + } + + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::types::ability::{EffectKind, EffectResolutionResult, ThisWayCause}; + use crate::types::events::PlayerActionKind; + use crate::types::identifiers::ObjectId; + use crate::types::player::PlayerId; + + fn completion() -> ResolvedAbility { + ResolvedAbility::new( + Effect::CompletePlayerAction { + parent_kind: EffectKind::Forage, + action: PlayerActionKind::Forage, + required_result: EffectResolutionResult { + cause: ThisWayCause::Sacrificed, + count: 1, + }, + }, + Vec::new(), + ObjectId(7), + PlayerId(0), + ) + } + + #[test] + fn publishes_action_only_for_exact_direct_result() { + let mut state = GameState::new_two_player(1); + let mut ability = completion(); + let mut events = Vec::new(); + + resolve(&mut state, &ability, &mut events).unwrap(); + assert_eq!(events.len(), 1); + + // `succeeded` compares the whole `EffectResolutionResult`, so each field + // is load-bearing on its own. Without these two near misses a resolver + // that checked only `cause`, or only `count`, would still pass — the + // matching case below differs from the `None` case above in both fields + // at once, so neither field is exercised in isolation. + for near_miss in [ + EffectResolutionResult { + cause: ThisWayCause::Sacrificed, + count: 2, + }, + EffectResolutionResult { + cause: ThisWayCause::Exiled, + count: 1, + }, + ] { + ability.context.prior_effect_result = Some(near_miss); + events.clear(); + resolve(&mut state, &ability, &mut events).unwrap(); + assert_eq!( + events.len(), + 1, + "{near_miss:?} does not equal the required result, so the action \ + must not be published" + ); + } + + ability.context.prior_effect_result = Some(EffectResolutionResult { + cause: ThisWayCause::Sacrificed, + count: 1, + }); + events.clear(); + resolve(&mut state, &ability, &mut events).unwrap(); + assert!(matches!( + events.as_slice(), + [ + GameEvent::EffectResolved { + kind: EffectKind::Forage, + .. + }, + GameEvent::PlayerPerformedAction { + action: PlayerActionKind::Forage, + .. + } + ] + )); + } + + #[test] + fn direct_result_round_trips_only_on_completion_node() { + let mut ability = completion(); + ability.sub_ability = Some(Box::new(ResolvedAbility::new( + Effect::NoOp, + Vec::new(), + ObjectId(7), + PlayerId(0), + ))); + ability.set_prior_effect_result_for_immediate_node(EffectResolutionResult { + cause: ThisWayCause::Sacrificed, + count: 1, + }); + + let serialized = serde_json::to_string(&ability).expect("completion node serializes"); + let restored: ResolvedAbility = + serde_json::from_str(&serialized).expect("completion node deserializes"); + + assert!(succeeded(&restored)); + assert_eq!( + restored + .sub_ability + .as_deref() + .expect("grandchild retained") + .context + .prior_effect_result, + None, + "the one-hop result must not leak into a grandchild" + ); + } +} diff --git a/crates/engine/src/game/effects/endure.rs b/crates/engine/src/game/effects/endure.rs index 134063bb75..a9c2eee9c6 100644 --- a/crates/engine/src/game/effects/endure.rs +++ b/crates/engine/src/game/effects/endure.rs @@ -101,6 +101,7 @@ pub fn resolve( parent_targets: ability.targets.clone(), context: ability.context.clone(), replacement_applied: ability.replacement_applied.clone(), + continuation: None, players: vec![enduring_controller], }, ); diff --git a/crates/engine/src/game/effects/forage.rs b/crates/engine/src/game/effects/forage.rs index 3eb654bbfe..237e1e7e88 100644 --- a/crates/engine/src/game/effects/forage.rs +++ b/crates/engine/src/game/effects/forage.rs @@ -16,11 +16,11 @@ //! Food — so a mode is offered only when it can be performed in full. If //! neither mode is performable, foraging does nothing. -use crate::game::ability_utils::build_resolved_from_def; +use crate::game::ability_utils::{append_to_sub_chain, build_resolved_from_def}; use crate::types::ability::{ AbilityDefinition, AbilityKind, Comparator, ControllerRef, Effect, EffectError, EffectKind, - FilterProp, MultiTargetSpec, PlayerFilter, QuantityExpr, ResolvedAbility, TargetChoiceTiming, - TargetFilter, TargetRef, TypedFilter, + EffectResolutionResult, FilterProp, MultiTargetSpec, PlayerFilter, QuantityExpr, + ResolvedAbility, TargetChoiceTiming, TargetFilter, TargetRef, ThisWayCause, TypedFilter, }; use crate::types::events::{GameEvent, PlayerActionKind}; use crate::types::game_state::GameState; @@ -31,6 +31,12 @@ use crate::types::zones::Zone; /// CR 701.61a: "exile three cards from your graveyard". const FORAGE_EXILE_COUNT: usize = 3; +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum ForageMode { + ExileThree, + SacrificeFood, +} + fn graveyard_size(state: &GameState, player: PlayerId) -> usize { state .players @@ -47,6 +53,32 @@ fn controls_food(state: &GameState, player: PlayerId, source_id: ObjectId) -> bo super::player_control_count_compares(state, player, &filter, Comparator::GE, 1, source_id) } +fn available_modes(state: &GameState, player: PlayerId, source_id: ObjectId) -> Vec { + let mut modes = Vec::with_capacity(2); + if graveyard_size(state, player) >= FORAGE_EXILE_COUNT { + modes.push(ForageMode::ExileThree); + } + if controls_food(state, player, source_id) { + modes.push(ForageMode::SacrificeFood); + } + modes +} + +pub(crate) fn can_forage(state: &GameState, ability: &ResolvedAbility) -> bool { + !available_modes(state, ability.controller, ability.source_id).is_empty() +} + +fn completion(cause: ThisWayCause, count: usize) -> AbilityDefinition { + AbilityDefinition::new( + AbilityKind::Spell, + Effect::CompletePlayerAction { + parent_kind: EffectKind::Forage, + action: PlayerActionKind::Forage, + required_result: EffectResolutionResult { cause, count }, + }, + ) +} + /// CR 701.61a (exile mode): exile three chosen cards from the forager's /// graveyard. `Owned { You }` scopes the scan to the forager's own graveyard; /// `MultiTargetSpec::fixed(3, 3)` forces exactly three (eligibility is checked @@ -78,6 +110,7 @@ fn exile_three_branch() -> AbilityDefinition { )) .target_choice_timing(TargetChoiceTiming::Resolution) .description("Exile three cards from your graveyard.".to_string()) + .sub_ability(completion(ThisWayCause::Exiled, FORAGE_EXILE_COUNT)) } /// CR 701.61a (Food mode): sacrifice a Food the forager controls. @@ -95,6 +128,7 @@ fn sacrifice_food_branch() -> AbilityDefinition { }, ) .description("Sacrifice a Food.".to_string()) + .sub_ability(completion(ThisWayCause::Sacrificed, 1)) } /// CR 701.61a: resolve a "forage" instruction. Offers only the performable @@ -106,25 +140,42 @@ pub(crate) fn resolve( events: &mut Vec, ) -> Result<(), EffectError> { let controller = ability.controller; - - let mut branches: Vec = Vec::new(); - if graveyard_size(state, controller) >= FORAGE_EXILE_COUNT { - branches.push(exile_three_branch()); - } - if controls_food(state, controller, ability.source_id) { - branches.push(sacrifice_food_branch()); + let modes = available_modes(state, controller, ability.source_id); + let mut branches: Vec = modes + .iter() + .map(|mode| match mode { + ForageMode::ExileThree => exile_three_branch(), + ForageMode::SacrificeFood => sacrifice_food_branch(), + }) + .collect(); + let mut tail = ability.sub_ability.as_deref().cloned(); + if let Some(tail) = tail.as_mut() { + tail.clear_prior_effect_result_recursive(); } - let foraged = !branches.is_empty(); match branches.len() { // CR 701.61a: neither mode performable — foraging does nothing. - 0 => {} + 0 => { + events.push(GameEvent::EffectResolved { + kind: EffectKind::Forage, + source_id: ability.source_id, + subject: None, + }); + if let Some(mut tail) = tail { + tail.set_optional_effect_performed_recursive(false); + super::resolve_ability_chain(state, &tail, events, 1)?; + } + } // Exactly one performable mode — perform it directly (no modal prompt). 1 => { let branch = branches.pop().expect("len checked == 1"); let mut resolved = build_resolved_from_def(&branch, ability.source_id, controller); resolved.context = ability.context.clone(); + resolved.clear_prior_effect_result_recursive(); resolved.set_scoped_player_recursive(controller); + if let Some(tail) = tail { + append_to_sub_chain(&mut resolved, tail); + } // Depth 1, not 0: `forage::resolve` already runs inside a resolution, // so a depth-0 re-entry would re-run the depth-0 prelude mid-resolution // (clearing chain-scoped state, re-bumping counters). Matches the @@ -143,35 +194,24 @@ pub(crate) fn resolve( controller, ); choose.context = ability.context.clone(); + choose.clear_prior_effect_result_recursive(); + choose.sub_ability = tail.map(Box::new); super::choose_one_of::resolve(state, &choose, events)?; } } - if foraged { - events.push(GameEvent::PlayerPerformedAction { - player_id: controller, - action: PlayerActionKind::Forage, - look_count: None, - scry_bottom_count: None, - scry_top_count: None, - }); - } - - events.push(GameEvent::EffectResolved { - kind: EffectKind::Forage, - source_id: ability.source_id, - subject: None, - }); - Ok(()) } #[cfg(test)] mod tests { use super::*; + use crate::game::engine::apply; use crate::game::zones::create_object; + use crate::types::ability::{AbilityCondition, EffectOutcomeSignal, SubAbilityLink}; + use crate::types::actions::GameAction; use crate::types::card_type::CoreType; - use crate::types::game_state::WaitingFor; + use crate::types::game_state::{PendingContinuation, WaitingFor}; use crate::types::identifiers::{CardId, ObjectId}; fn forage_ability(controller: PlayerId, source: ObjectId) -> ResolvedAbility { @@ -235,6 +275,42 @@ mod tests { ); } + /// CR 608.2c + CR 609.3: a zero-mode Forage is a failed action for an + /// `IfYouDo` rider, while a separate unconditional printed instruction + /// remains independent and still resolves. + #[test] + fn zero_mode_gates_if_you_do_but_runs_unconditional_sibling() { + let source = ObjectId(49); + let tail = |condition| { + let mut tail = ResolvedAbility::new( + Effect::GainLife { + amount: QuantityExpr::Fixed { value: 1 }, + player: TargetFilter::Controller, + }, + Vec::new(), + source, + PlayerId(0), + ); + tail.condition = condition; + tail.sub_link = SubAbilityLink::SequentialSibling; + tail + }; + + let mut gated_state = GameState::new_two_player(1); + let gated = forage_ability(PlayerId(0), source).sub_ability(tail(Some( + AbilityCondition::EffectOutcome { + signal: EffectOutcomeSignal::OptionalEffectPerformed, + }, + ))); + resolve(&mut gated_state, &gated, &mut Vec::new()).unwrap(); + assert_eq!(gated_state.players[0].life, 20); + + let mut independent_state = GameState::new_two_player(1); + let independent = forage_ability(PlayerId(0), source).sub_ability(tail(None)); + resolve(&mut independent_state, &independent, &mut Vec::new()).unwrap(); + assert_eq!(independent_state.players[0].life, 21); + } + /// CR 701.61a (exile mode): three graveyard cards and no Food prompts an /// exile-three-from-your-graveyard selection (Graveyard -> Exile, count 3). #[test] @@ -335,4 +411,110 @@ mod tests { state.waiting_for ); } + + /// CR 608.2c + CR 701.55d: when both Forage modes are legal, the runtime + /// printed tail is carried by the modal choice and attached only after the + /// selected branch's completion node. A successful Food branch therefore + /// runs an `IfYouDo` tail exactly once. + #[test] + fn two_mode_choice_runs_success_gated_runtime_tail_once() { + let mut state = GameState::new_two_player(1); + for n in 1..=3 { + add_graveyard_card(&mut state, PlayerId(0), n); + } + let food = add_food(&mut state, PlayerId(0)); + let source = ObjectId(54); + let mut tail = ResolvedAbility::new( + Effect::GainLife { + amount: QuantityExpr::Fixed { value: 1 }, + player: TargetFilter::Controller, + }, + Vec::new(), + source, + PlayerId(0), + ); + tail.condition = Some(AbilityCondition::EffectOutcome { + signal: EffectOutcomeSignal::OptionalEffectPerformed, + }); + tail.sub_link = SubAbilityLink::SequentialSibling; + let ability = forage_ability(PlayerId(0), source).sub_ability(tail); + let mut events = Vec::new(); + + resolve(&mut state, &ability, &mut events).unwrap(); + let serialized = serde_json::to_string(&state).expect("modal Forage state serializes"); + state = serde_json::from_str(&serialized) + .expect("modal Forage state with runtime tail deserializes"); + let food_branch = match &state.waiting_for { + WaitingFor::ChooseOneOfBranch { branches, .. } => branches + .iter() + .position(|branch| matches!(branch.effect.as_ref(), Effect::Sacrifice { .. })) + .expect("Food branch must be offered"), + other => panic!("expected modal Forage choice, got {other:?}"), + }; + assert_eq!(state.players[0].life, 20); + + let result = apply( + &mut state, + PlayerId(0), + GameAction::ChooseBranch { index: food_branch }, + ) + .expect("choose Food forage branch"); + + assert_eq!(state.objects[&food].zone, Zone::Graveyard); + assert_eq!(state.players[0].life, 21); + assert_eq!( + result + .events + .iter() + .filter(|event| matches!( + event, + GameEvent::PlayerPerformedAction { + action: PlayerActionKind::Forage, + .. + } + )) + .count(), + 1 + ); + } + + /// CR 608.2c: a synchronous sacrifice returns its result to its own direct + /// child. It must not stamp an already-active, same-source completion frame + /// merely because that unrelated frame asks for the same cause and count. + #[test] + fn synchronous_food_result_does_not_stamp_unrelated_same_source_continuation() { + let mut state = GameState::new_two_player(1); + add_food(&mut state, PlayerId(0)); + let source = ObjectId(55); + let unrelated = build_resolved_from_def( + &completion(ThisWayCause::Sacrificed, 1), + source, + PlayerId(0), + ); + let pending = PendingContinuation::new(Box::new(unrelated), &state); + state.park_ability_continuation(pending); + let sacrifice = build_resolved_from_def(&sacrifice_food_branch(), source, PlayerId(0)); + let mut events = Vec::new(); + + let result = + crate::game::effects::sacrifice::resolve(&mut state, &sacrifice, &mut events).unwrap(); + + assert_eq!( + result, + Some(EffectResolutionResult { + cause: ThisWayCause::Sacrificed, + count: 1, + }) + ); + assert_eq!( + state + .active_ability_continuation() + .expect("hostile continuation remains parked") + .chain + .context + .prior_effect_result, + None, + "a synchronous operation cannot stamp an unrelated active frame" + ); + } } diff --git a/crates/engine/src/game/effects/mod.rs b/crates/engine/src/game/effects/mod.rs index 03c7d6aaf8..57fdf2a3c5 100644 --- a/crates/engine/src/game/effects/mod.rs +++ b/crates/engine/src/game/effects/mod.rs @@ -11,9 +11,9 @@ use crate::game::speed::has_max_speed; use crate::types::ability::{ AbilityCondition, AbilityCost, AbilityKind, CardPlayMode, CardTypeSetSource, ControllerRef, CopyRetargetPermission, CostPaidObjectSnapshot, EachDamageRecipient, Effect, EffectError, - EffectKind, EffectOutcomeSignal, EffectScope, FilterProp, OpponentMayScope, PlayerFilter, - PlayerScope, QuantityExpr, QuantityRef, RepeatContinuation, ResolvedAbility, - RevealUntilDisposition, SacrificeCost, SacrificeRequirement, SharedQuality, + EffectKind, EffectOutcomeSignal, EffectResolutionResult, EffectScope, FilterProp, + OpponentMayScope, PlayerFilter, PlayerScope, QuantityExpr, QuantityRef, RepeatContinuation, + ResolvedAbility, RevealUntilDisposition, SacrificeCost, SacrificeRequirement, SharedQuality, SharedQualityRelation, SiblingCondition, SubAbilityLink, TapStateChange, TargetChoiceTiming, TargetFilter, TargetRef, ThisWayCause, }; @@ -71,6 +71,7 @@ pub mod choose_one_of; pub mod clash; pub mod cleanup; pub mod collect_evidence; +pub mod complete_player_action; pub mod conjure; pub mod connive; pub mod control_next_turn; @@ -1434,6 +1435,17 @@ fn drain_pending_change_zone_iteration(state: &mut GameState, events: &mut Vec bool { + state.active_ability_continuation().is_some_and(|pending| { + pending.chain.source_id == source_id + && matches!( + &pending.chain.effect, + Effect::CompletePlayerAction { + required_result, + .. + } if required_result.cause == cause + ) + }) +} + fn prepend_to_pending_continuation(state: &mut GameState, mut head: ResolvedAbility) { if state .resolution_stack @@ -2467,6 +2519,25 @@ fn apply_parent_chain_context( // one immediate looping child. Ordinary hand-offs must not let it leak to // a later grandchild with a different instruction scope. child.context.parent_target_iteration_members = None; + // CR 608.2c: A completed effect result is a one-hop input. Clear the + // inherited copy first, then stamp it only onto the same-source immediate + // `CompletePlayerAction` child whose requirement names the same producer + // action. Count mismatches are intentionally retained so the completion + // node can resolve as a failed action rather than observing no result. + let prior_effect_result = child.context.prior_effect_result.take(); + if child.source_id == parent.source_id { + if let ( + Some(result), + Effect::CompletePlayerAction { + required_result, .. + }, + ) = (prior_effect_result, &child.effect) + { + if result.cause == required_result.cause { + child.set_prior_effect_result_for_immediate_node(result); + } + } + } // CR 701.9a + CR 608.2c: A discard result is visible only to the direct // contingent child. Every ordinary hand-off clears it, preventing a later // grandchild (or an unrelated chain branch) from reading stale provenance. @@ -3034,6 +3105,9 @@ fn effect_manages_own_outcome_flag(effect: &Effect) -> bool { without_paying_mana_cost: false, .. } + // CR 608.2c: this node derives success from its exact one-hop + // operation result and propagates that boolean to its printed tail. + | Effect::CompletePlayerAction { .. } ) } @@ -4209,7 +4283,7 @@ pub fn resolve_effect( state: &mut GameState, ability: &ResolvedAbility, events: &mut Vec, -) -> Result<(), EffectError> { +) -> Result, EffectError> { match &ability.effect { Effect::StartYourEngines { .. } => speed_effects::resolve_start(state, ability, events), Effect::ChangeSpeed { .. } => speed_effects::resolve_change_speed(state, ability, events), @@ -4238,7 +4312,7 @@ pub fn resolve_effect( // dispatched inside `resolve_set_tap_state`. Effect::SetTapState { .. } => tap_untap::resolve_set_tap_state(state, ability, events), Effect::RemoveCounter { .. } => counters::resolve_remove(state, ability, events), - Effect::Sacrifice { .. } => sacrifice::resolve(state, ability, events), + Effect::Sacrifice { .. } => return sacrifice::resolve(state, ability, events), Effect::DiscardCard { .. } => discard::resolve(state, ability, events), Effect::Mill { .. } => mill::resolve(state, ability, events), Effect::Scry { .. } => scry::resolve(state, ability, events), @@ -4246,7 +4320,7 @@ pub fn resolve_effect( Effect::DamageAll { .. } => deal_damage::resolve_all(state, ability, events), Effect::DamageEachPlayer { .. } => deal_damage::resolve_each_player(state, ability, events), Effect::DestroyAll { .. } => destroy::resolve_all(state, ability, events), - Effect::ChangeZone { .. } => change_zone::resolve(state, ability, events), + Effect::ChangeZone { .. } => return change_zone::resolve(state, ability, events), Effect::ChangeZoneAll { .. } => change_zone::resolve_all(state, ability, events), Effect::Dig { .. } => dig::resolve(state, ability, events), Effect::GainControl { .. } => gain_control::resolve(state, ability, events), @@ -4558,6 +4632,9 @@ pub fn resolve_effect( Effect::BlightEffect { .. } => blight::resolve(state, ability, events), Effect::Endure { .. } => endure::resolve(state, ability, events), Effect::Forage => forage::resolve(state, ability, events), + Effect::CompletePlayerAction { .. } => { + complete_player_action::resolve(state, ability, events) + } Effect::Harness => harness::resolve(state, ability, events), Effect::CollectEvidence { .. } => collect_evidence::resolve(state, ability, events), Effect::SetLifeTotal { .. } => life::resolve_set_life_total(state, ability, events), @@ -4619,7 +4696,8 @@ pub fn resolve_effect( } Ok(()) } - } + }?; + Ok(None) } /// Returns true if the given effect has a handler in the engine. @@ -5090,26 +5168,18 @@ fn affected_objects_with_causes( /// effect kind (and its declared destination), so it is independent of any /// replacement that later redirects the members' landing zone. pub(crate) fn this_way_cause_for_effect(effect: &Effect) -> Option { - use crate::types::zones::Zone; - // CR 400.7: a generic zone change names a "this way" verb only for the - // destinations a consumer references — Exile (exiled), Battlefield - // (returned/put onto the battlefield), Hand (bounced/returned to hand). - let cause_for_zone = |destination: Zone| match destination { - Zone::Exile => Some(ThisWayCause::Exiled), - Zone::Battlefield => Some(ThisWayCause::Returned), - Zone::Hand => Some(ThisWayCause::Bounced), - _ => None, - }; match effect { Effect::Destroy { .. } | Effect::DestroyAll { .. } => Some(ThisWayCause::Destroyed), Effect::Sacrifice { .. } => Some(ThisWayCause::Sacrificed), Effect::Mill { .. } => Some(ThisWayCause::Milled), Effect::Discard { .. } | Effect::DiscardCard { .. } => Some(ThisWayCause::Discarded), Effect::ChangeZone { destination, .. } | Effect::ChangeZoneAll { destination, .. } => { - cause_for_zone(*destination) + this_way_cause_for_zone(*destination) } // CR 611.2c: mass-bounce destination defaults to Hand. - Effect::BounceAll { destination, .. } => cause_for_zone(destination.unwrap_or(Zone::Hand)), + Effect::BounceAll { destination, .. } => { + this_way_cause_for_zone(destination.unwrap_or(Zone::Hand)) + } Effect::ExileTop { .. } | Effect::ExileFromTopUntil { .. } => Some(ThisWayCause::Exiled), // CR 608.2c: a coercion (mass MustAttack) names no "ed this way" set — // "those creatures" is a bare frozen population, so its members carry no @@ -5122,6 +5192,17 @@ pub(crate) fn this_way_cause_for_effect(effect: &Effect) -> Option } } +/// CR 400.7 + CR 608.2c: Map a requested one-shot zone destination to the +/// producer-action vocabulary used by typed "this way" and completion data. +pub(crate) fn this_way_cause_for_zone(destination: Zone) -> Option { + match destination { + Zone::Exile => Some(ThisWayCause::Exiled), + Zone::Battlefield => Some(ThisWayCause::Returned), + Zone::Hand => Some(ThisWayCause::Bounced), + _ => None, + } +} + fn affected_objects_from_events( state: &GameState, ability: &ResolvedAbility, @@ -5994,6 +6075,9 @@ fn optional_effect_is_infeasible(state: &GameState, ability: &ResolvedAbility) - Effect::RemoveCounter { .. } => { counters::remove_counter_optional_is_infeasible(state, ability) } + // CR 701.61a + CR 608.2d: A player cannot choose to forage unless at + // least one complete forage mode is currently available. + Effect::Forage => !forage::can_forage(state, ability), Effect::CastFromZone { mode, target, @@ -7690,6 +7774,7 @@ pub(crate) enum PendingPlayerScopeSacrificeOutcome { Completed { events_before_sacrifice: usize, events_after_sacrifice: usize, + sacrificed_count: usize, }, } @@ -7698,6 +7783,7 @@ enum PlayerScopeSacrificePerformOutcome { Completed { events_before_sacrifice: usize, events_after_sacrifice: usize, + sacrificed_count: usize, }, } @@ -7960,6 +8046,7 @@ fn perform_player_scope_sacrifices( } } } + let sacrificed_count = completion.sacrificed.len(); events.push(GameEvent::EffectResolved { kind: completion .effect_kind @@ -7973,6 +8060,7 @@ fn perform_player_scope_sacrifices( Ok(PlayerScopeSacrificePerformOutcome::Completed { events_before_sacrifice, events_after_sacrifice, + sacrificed_count, }) } @@ -8130,9 +8218,11 @@ pub(crate) fn perform_collected_player_scope_sacrifices_with_completion( PlayerScopeSacrificePerformOutcome::Completed { events_before_sacrifice, events_after_sacrifice, + sacrificed_count, } => Ok(PendingPlayerScopeSacrificeOutcome::Completed { events_before_sacrifice, events_after_sacrifice, + sacrificed_count, }), } } @@ -8231,9 +8321,11 @@ pub(crate) fn advance_pending_player_scope_sacrifice_choice( PlayerScopeSacrificePerformOutcome::Completed { events_before_sacrifice, events_after_sacrifice, + sacrificed_count, } => Ok(PendingPlayerScopeSacrificeOutcome::Completed { events_before_sacrifice, events_after_sacrifice, + sacrificed_count, }), } } @@ -8260,9 +8352,11 @@ pub(crate) fn drain_pending_player_scope_sacrifice_after_replacement( PlayerScopeSacrificePerformOutcome::Completed { events_before_sacrifice, events_after_sacrifice, + sacrificed_count, } => Ok(PendingPlayerScopeSacrificeOutcome::Completed { events_before_sacrifice, events_after_sacrifice, + sacrificed_count, }), } } @@ -9899,6 +9993,7 @@ fn resolve_chain_body( // CR 603.7: Snapshot event count so we can detect objects moved by this effect. let events_before = events.len(); + let mut immediate_effect_result = None; // Skip no-op unimplemented/runtime-handled effects, and a random // `Effect::Choose` already resolved above by `resolve_random_in_chain`. @@ -10166,7 +10261,11 @@ fn resolve_chain_body( // resolution, mirroring the drain-path resume at depth 1. let _ = resolve_ability_chain(state, iter_effective, events, depth.max(1)); } else { - let _ = resolve_effect(state, iter_effective, events); + if let Ok(result) = resolve_effect(state, iter_effective, events) { + if iterations == 1 { + immediate_effect_result = result; + } + } } // CR 608.2c + CR 109.5: When the inner effect enters an // interactive WaitingFor (e.g. SearchChoice), stash the @@ -10231,6 +10330,22 @@ fn resolve_chain_body( } // end shares_quality_failed else } + // CR 701.61a + CR 608.2c: Forage owns the nested B→C→tail chain it + // constructs at runtime. Its nested completion node is the sole publisher + // of the action and its ledgers; the outer frame must not walk or publish + // the same tail a second time. + if matches!(ability.effect, Effect::Forage) { + return Ok(()); + } + // CR 608.2c + CR 701.55d: ChooseOneOf snapshots its runtime tail in the + // typed branch-choice carrier and attaches it only to the final selected + // branch. The generic walker must not also park the same sub-ability. + if matches!(ability.effect, Effect::ChooseOneOf { .. }) + && matches!(state.waiting_for, WaitingFor::ChooseOneOfBranch { .. }) + { + return Ok(()); + } + if matches!(ability.effect, Effect::ChangeZone { .. }) && matches!(state.waiting_for, WaitingFor::Priority { .. }) && state.active_change_zone_frame().is_none() @@ -10281,6 +10396,15 @@ fn resolve_chain_body( _ => None, }) .collect(); + let result_context_owned; + let ability = if let Some(result) = immediate_effect_result { + let mut owned = ability.clone(); + owned.context.prior_effect_result = Some(result); + result_context_owned = owned; + &result_context_owned + } else { + ability + }; // CR 701.9a + CR 608.2c + CR 400.7: Capture the result from the active, // operation-owned discard frame. Unlike an event-slice or global ledger, // the frame remains exact through replacement redirection and cannot be @@ -10345,6 +10469,19 @@ fn resolve_chain_body( } } + // CR 608.2c: Normalize the actual completion outcome before the printed + // tail is evaluated. A count/cause mismatch remains a resolved no-op and + // therefore keeps `WhenYouDo` / `IfYouDo` descendants false. + let completion_outcome_owned; + let ability = if matches!(ability.effect, Effect::CompletePlayerAction { .. }) { + let mut owned = ability.clone(); + owned.set_optional_effect_performed_recursive(complete_player_action::succeeded(ability)); + completion_outcome_owned = owned; + &completion_outcome_owned + } else { + ability + }; + // CR 603.7: Record the objects affected by this effect as a tracked set so // downstream sub-abilities can resolve "this way" references (pronouns, // `TrackedSetSize`, `TrackedSet` filters). The signal event depends on the diff --git a/crates/engine/src/game/effects/sacrifice.rs b/crates/engine/src/game/effects/sacrifice.rs index ce77d9455d..1c08c07fc3 100644 --- a/crates/engine/src/game/effects/sacrifice.rs +++ b/crates/engine/src/game/effects/sacrifice.rs @@ -1,7 +1,7 @@ use crate::game::quantity::resolve_quantity_with_targets; use crate::types::ability::{ - ControllerRef, Effect, EffectError, EffectKind, QuantityExpr, ResolvedAbility, TargetFilter, - TargetRef, + ControllerRef, Effect, EffectError, EffectKind, EffectResolutionResult, QuantityExpr, + ResolvedAbility, TargetFilter, TargetRef, ThisWayCause, }; use crate::types::events::GameEvent; use crate::types::game_state::{GameState, PendingPlayerScopeSacrificeCompletion, WaitingFor}; @@ -12,12 +12,12 @@ use crate::types::zones::Zone; /// Resolve the set of players whose permanents are eligible for a sacrifice /// effect, derived from the target filter's `ControllerRef`. /// -/// CR 701.17a: A player can only sacrifice a permanent they control. +/// CR 701.21a: A player can only sacrifice a permanent they control. /// /// - `You` (or no controller clause): only the ability controller sacrifices /// (the historical default). /// - `Opponent`: each player other than the ability controller may be asked to -/// sacrifice. Per CR 701.17a, each affected player can only sacrifice their +/// sacrifice. Per CR 701.21a, each affected player can only sacrifice their /// own permanent; this resolver handles the single-opponent two-player case /// by routing both filter scope and chooser to that opponent. /// - `ScopedPlayer`: an event-context player such as the active player for @@ -126,12 +126,18 @@ fn trigger_event_scoped_player(state: &GameState, ability: &ResolvedAbility) -> }) } -/// CR 701.17a: To sacrifice a permanent, its controller moves it to its owner's graveyard. +/// CR 701.21a: To sacrifice a permanent, its controller moves it to its owner's graveyard. pub fn resolve( state: &mut GameState, ability: &ResolvedAbility, events: &mut Vec, -) -> Result<(), EffectError> { +) -> Result, EffectError> { + let completed_result = |count| { + Some(EffectResolutionResult { + cause: ThisWayCause::Sacrificed, + count, + }) + }; // CR 609.3: Resolve the dynamic sacrifice count through // `resolve_quantity_with_targets` before attempting the sacrifice so // mandatory effects can do as much as possible against the rebound @@ -180,7 +186,7 @@ pub fn resolve( source_id: ability.source_id, subject: None, }); - return Ok(()); + return Ok(completed_result(0)); } let scoped_ability; let ability = if matches!( @@ -207,9 +213,8 @@ pub fn resolve( // which resolves a player scope and would make the controller sacrifice a // DIFFERENT permanent (`resolve_sacrifice_scope`, CR 701.21a: "To sacrifice // a permanent, its controller moves it from the battlefield directly to its - // owner's graveyard"). Note this file elsewhere cites CR 701.17a for - // sacrifice; 701.17a is mill (CR 701.21 is Sacrifice). That pre-existing - // cluster is left alone here so the correction lands as one auditable pass. + // owner's graveyard"). The surrounding sacrifice annotations use the + // verified CR 701.21a rule; CR 701.17 is mill. // // Emits EffectResolved first, matching the shipped CR 400.7 SelfRef guard // above, which this guard is the direct extension of. @@ -219,7 +224,7 @@ pub fn resolve( source_id: ability.source_id, subject: None, }); - return Ok(()); + return Ok(completed_result(0)); } let live_targets = ability.live_object_targets(state); @@ -241,7 +246,7 @@ pub fn resolve( }; if targeted_objects.is_empty() { - // CR 701.17a: Derive the player(s) whose permanents are in scope from + // CR 701.21a: Derive the player(s) whose permanents are in scope from // the target filter's ControllerRef. Defaults to `[ability.controller]` // when no controller clause is present (historical "you sacrifice" // default). For `Opponent` / `TargetPlayer`, each affected player is @@ -298,7 +303,7 @@ pub fn resolve( source_id: ability.source_id, subject: None, }); - return Ok(()); + return Ok(completed_result(0)); } if eligible.is_empty() { @@ -310,10 +315,10 @@ pub fn resolve( source_id: ability.source_id, subject: None, }); - return Ok(()); + return Ok(completed_result(0)); } - // CR 701.17a + CR 609.3: When the resolved count is at least the + // CR 701.21a + CR 609.3: When the resolved count is at least the // eligible pool and the sacrifice is mandatory, sacrifice every // eligible permanent — the effect does as much as possible. Fast-path // this rather than round-tripping through EffectZoneChoice. @@ -327,7 +332,7 @@ pub fn resolve( propagate_parent_context: state.active_ability_continuation().is_some(), ..Default::default() }; - let _ = super::perform_collected_player_scope_sacrifices_with_completion( + let outcome = super::perform_collected_player_scope_sacrifices_with_completion( state, ability.source_id, ability.controller, @@ -335,10 +340,16 @@ pub fn resolve( completion, events, )?; - return Ok(()); + return Ok(match outcome { + super::PendingPlayerScopeSacrificeOutcome::Completed { + sacrificed_count, .. + } => completed_result(sacrificed_count), + super::PendingPlayerScopeSacrificeOutcome::WaitingForNextChoice + | super::PendingPlayerScopeSacrificeOutcome::PausedForReplacement => None, + }); } - // CR 701.17a: "Sacrifice N permanents" — the affected player picks + // CR 701.21a: "Sacrifice N permanents" — the affected player picks // which `count` permanents out of the eligible pool. Clamped to pool // size for safety; the branch above handles the mandatory-all case. let choice_count = count.min(eligible.len()); @@ -371,7 +382,7 @@ pub fn resolve( // EffectResolved is emitted by the EffectZoneChoice handler after the player chooses // (matching the DiscardChoice pattern — single authority for the event). - return Ok(()); + return Ok(None); } let mut selections = Vec::new(); @@ -386,17 +397,17 @@ pub fn resolve( continue; } - // CR 701.17a: A player can't sacrifice something that isn't a permanent. + // CR 701.21a: A player can't sacrifice something that isn't a permanent. if obj.zone != Zone::Battlefield { continue; } - // CR 701.17a: Defense-in-depth — a player can only sacrifice permanents + // CR 701.21a: Defense-in-depth — a player can only sacrifice permanents // they control. The primary fix is that Sacrifice no longer creates // target slots (see extract_target_filter_from_effect), but if this // path is ever reached, enforce controller ownership. // - // CR 701.17a: "To sacrifice a permanent, its controller moves it..." — for an + // CR 701.21a: "To sacrifice a permanent, its controller moves it..." — for an // explicit anaphoric target (ParentTarget/ParentTargetSlot, e.g. Animate // Dead's "that creature's controller sacrifices it"), the acting player is // the object's OWN current controller, unconditionally, even if control @@ -427,7 +438,7 @@ pub fn resolve( effect_kind: Some(EffectKind::from(&ability.effect)), ..Default::default() }; - let _ = super::perform_collected_player_scope_sacrifices_with_completion( + let outcome = super::perform_collected_player_scope_sacrifices_with_completion( state, ability.source_id, ability.controller, @@ -436,7 +447,13 @@ pub fn resolve( events, )?; - Ok(()) + Ok(match outcome { + super::PendingPlayerScopeSacrificeOutcome::Completed { + sacrificed_count, .. + } => completed_result(sacrificed_count), + super::PendingPlayerScopeSacrificeOutcome::WaitingForNextChoice + | super::PendingPlayerScopeSacrificeOutcome::PausedForReplacement => None, + }) } #[cfg(test)] @@ -617,7 +634,7 @@ mod tests { /// OTHER creature, the mandatory sacrifice auto-resolves onto it and the /// source survives. /// - /// CR 701.17a: sacrifice moves the chosen permanent to its owner's + /// CR 701.21a: sacrifice moves the chosen permanent to its owner's /// graveyard. `FilterProp::Another` is evaluated via /// `FilterContext::from_ability` (source excluded). The paired negative /// (source survives) is made non-vacuous by asserting the OTHER creature was @@ -868,7 +885,7 @@ mod tests { assert!(state.cost_payment_failed_flag); } - // CR 701.17a: When the target filter scopes sacrifice to opponents + // CR 701.21a: When the target filter scopes sacrifice to opponents // (ControllerRef::Opponent) or a target player (ControllerRef::TargetPlayer), // the affected player — not the ability controller — both provides the // eligible permanent pool and makes the choice. @@ -1129,7 +1146,7 @@ mod tests { } } - /// CR 701.17a: Even if the targeted path is reached (defense-in-depth), + /// CR 701.21a: Even if the targeted path is reached (defense-in-depth), /// sacrifice must skip permanents not controlled by the ability controller. #[test] fn targeted_path_skips_opponent_permanents() { @@ -1545,7 +1562,7 @@ mod tests { /// planeswalker with the greatest mana value among creatures and /// planeswalkers they control." /// - /// CR 202.3 + CR 608.2h + CR 701.17a: each opponent's eligible pool must + /// CR 202.3 + CR 608.2h + CR 701.21a: each opponent's eligible pool must /// be restricted to *that opponent's* permanents tied for the greatest /// mana value among their own creatures/planeswalkers — never a global /// battlefield maximum. diff --git a/crates/engine/src/game/effects/stickers.rs b/crates/engine/src/game/effects/stickers.rs index bcfd93795c..b4627ff5eb 100644 --- a/crates/engine/src/game/effects/stickers.rs +++ b/crates/engine/src/game/effects/stickers.rs @@ -211,6 +211,7 @@ fn resolve_put_sticker( parent_targets: ability.targets.clone(), context: ability.context.clone(), replacement_applied: ability.replacement_applied.clone(), + continuation: None, players: vec![ability.controller], }, ); @@ -277,6 +278,7 @@ fn prompt_count_choice( parent_targets: ability.targets.clone(), context: ability.context.clone(), replacement_applied: ability.replacement_applied.clone(), + continuation: None, players: vec![ability.controller], }, ); diff --git a/crates/engine/src/game/engine.rs b/crates/engine/src/game/engine.rs index 87050a07e4..41eb5b0b26 100644 --- a/crates/engine/src/game/engine.rs +++ b/crates/engine/src/game/engine.rs @@ -16106,9 +16106,14 @@ mod stage2_injector_tests { // shifts combine with #6958's paid-cast outcome exclusion and // #6976's conditional-branch exclusions. None creates an // `OptionalEffect` prompt. Re-pinned against the merged source. - "game/effects/mod.rs:6306".to_string(), - "game/effects/mod.rs:6383".to_string(), - "game/effects/mod.rs:9578".to_string(), + // #7221 adds the typed player-action completion seam above all three + // producers: `:6306/:6383/:9578 => :6390/:6467/:9672`. The first + // two move by +84; the third also includes ten lines added inside + // `resolve_chain_body`. The census and partition assertions above + // remain unchanged, and each producer remains in its named function. + "game/effects/mod.rs:6390".to_string(), + "game/effects/mod.rs:6467".to_string(), + "game/effects/mod.rs:9672".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/game/engine_phase_trigger_regression_tests.rs b/crates/engine/src/game/engine_phase_trigger_regression_tests.rs index 6f3835cb45..2970b8bfed 100644 --- a/crates/engine/src/game/engine_phase_trigger_regression_tests.rs +++ b/crates/engine/src/game/engine_phase_trigger_regression_tests.rs @@ -3020,6 +3020,7 @@ fn choose_one_of_branch_resolves_selected_branch_with_original_controller() { branch_descriptions: vec!["Gain 3 life.".to_string(), "Lose 3 life.".to_string()], parent_targets: vec![], context: Default::default(), + continuation: None, replacement_applied: Default::default(), remaining_players: vec![], }; diff --git a/crates/engine/src/game/engine_replacement.rs b/crates/engine/src/game/engine_replacement.rs index 31f035a23f..7c74bad679 100644 --- a/crates/engine/src/game/engine_replacement.rs +++ b/crates/engine/src/game/engine_replacement.rs @@ -987,6 +987,11 @@ pub(super) fn handle_replacement_choice( // CR 101.4: a simultaneous each-player sacrifice paused by a // CR 616.1 replacement choice resumes the already-announced // remaining sacrifices before any parked continuation can run. + let sacrifice_source_id = state + .pending_player_scope_sacrifice_choice + .as_ref() + .map(|pending| pending.ability.source_id) + .expect("active sacrifice choice owns its source identity"); match effects::drain_pending_player_scope_sacrifice_after_replacement(state, events) .map_err(|error| EngineError::InvalidAction(error.to_string()))? { @@ -994,7 +999,18 @@ pub(super) fn handle_replacement_choice( effects::PendingPlayerScopeSacrificeOutcome::PausedForReplacement => { waiting_for = state.waiting_for.clone(); } - effects::PendingPlayerScopeSacrificeOutcome::Completed { .. } => { + effects::PendingPlayerScopeSacrificeOutcome::Completed { + sacrificed_count, + .. + } => { + effects::stamp_active_player_action_completion( + state, + sacrifice_source_id, + crate::types::ability::EffectResolutionResult { + cause: crate::types::ability::ThisWayCause::Sacrificed, + count: sacrificed_count, + }, + ); effects::drain_pending_continuation(state, events); if !matches!(state.waiting_for, WaitingFor::Priority { .. }) { waiting_for = state.waiting_for.clone(); @@ -6864,6 +6880,7 @@ mod tests { branch_descriptions: Vec::new(), parent_targets: Vec::new(), context: Default::default(), + continuation: None, replacement_applied: Default::default(), remaining_players: Vec::new(), }; diff --git a/crates/engine/src/game/engine_resolution_choices.rs b/crates/engine/src/game/engine_resolution_choices.rs index 74bdab9971..4c56bee841 100644 --- a/crates/engine/src/game/engine_resolution_choices.rs +++ b/crates/engine/src/game/engine_resolution_choices.rs @@ -585,6 +585,16 @@ fn batch_or_drain_observer_triggers( || !matches!(ev, GameEvent::ZoneChanged { .. })) }) .cloned() + .chain( + // CR 603.2 + CR 608.2c: a typed completion continuation may + // publish the player action that the interactive move just + // completed. Include that semantic event without widening the + // owner-bounded zone slice to continuation-produced zone moves. + events[event_slice_end..] + .iter() + .filter(|event| matches!(event, GameEvent::PlayerPerformedAction { .. })) + .cloned(), + ) .collect(); super::triggers::collect_triggers_into_deferred(state, &trigger_events); if let Some(wf) = super::triggers::drain_deferred_trigger_queue(state, events) { @@ -604,6 +614,15 @@ fn batch_or_drain_observer_triggers( || !matches!(ev, GameEvent::ZoneChanged { .. })) }) .cloned() + .chain( + // CR 603.2 + CR 608.2c: see the settled branch above. Park a + // completion action across a further continuation prompt, but + // do not fold that continuation's zone changes into this owner. + events[event_slice_end..] + .iter() + .filter(|event| matches!(event, GameEvent::PlayerPerformedAction { .. })) + .cloned(), + ) .collect(); super::triggers::collect_triggers_into_deferred(state, &trigger_events); None @@ -4377,6 +4396,7 @@ pub(super) fn handle_resolution_choice( branch_descriptions: _, parent_targets, context, + continuation, replacement_applied, remaining_players, }, @@ -4392,6 +4412,7 @@ pub(super) fn handle_resolution_choice( branches, parent_targets, context, + continuation, replacement_applied, remaining_players, index, @@ -4942,7 +4963,16 @@ pub(super) fn handle_resolution_choice( effects::PendingPlayerScopeSacrificeOutcome::Completed { events_before_sacrifice, events_after_sacrifice, + sacrificed_count, } => { + effects::stamp_active_player_action_completion( + state, + source_id, + crate::types::ability::EffectResolutionResult { + cause: crate::types::ability::ThisWayCause::Sacrificed, + count: sacrificed_count, + }, + ); // CR 614.12a + CR 614.13a: a direct sacrifice selection can be the // complete body of a paused post-replacement dispatch // (Devour). Retire that exact resident before its outer @@ -5093,6 +5123,22 @@ pub(super) fn handle_resolution_choice( source_id, subject: None, }); + let result = match effect_kind { + EffectKind::Sacrifice => Some(crate::types::ability::EffectResolutionResult { + cause: crate::types::ability::ThisWayCause::Sacrificed, + count: 0, + }), + EffectKind::ChangeZone => destination + .and_then(effects::this_way_cause_for_zone) + .map(|cause| crate::types::ability::EffectResolutionResult { + cause, + count: 0, + }), + _ => None, + }; + if let Some(result) = result { + effects::stamp_active_player_action_completion(state, source_id, result); + } set_priority(state, player); resume_with_error_propagation(state, events)?; return Ok(ResolutionChoiceOutcome::WaitingFor( @@ -5130,7 +5176,16 @@ pub(super) fn handle_resolution_choice( effects::PendingPlayerScopeSacrificeOutcome::Completed { events_before_sacrifice, events_after_sacrifice, + sacrificed_count, } => { + effects::stamp_active_player_action_completion( + state, + source_id, + crate::types::ability::EffectResolutionResult { + cause: crate::types::ability::ThisWayCause::Sacrificed, + count: sacrificed_count, + }, + ); // CR 614.12a + CR 614.13a: see the matching player-scope // sacrifice completion above. This EffectZoneChoice // path can complete a Devour drain without an @@ -5169,6 +5224,10 @@ pub(super) fn handle_resolution_choice( ) })?; let chosen_ids: Vec<_> = chosen.to_vec(); + let completion_cause = effects::this_way_cause_for_zone(dest_zone); + let tracks_player_action_completion = completion_cause.is_some_and(|cause| { + effects::active_player_action_completion_requires(state, source_id, cause) + }); let mut logical_zone_change_group = crate::game::triggers::allocate_logical_zone_change_group( state, @@ -5291,7 +5350,16 @@ pub(super) fn handle_resolution_choice( conditional_enter_with_counters.clone(), duration: ctx.duration.clone(), track_exiled_by_source: ctx.track_exiled_by_source, - moved_count: None, + moved_count: tracks_player_action_completion.then(|| { + i32::try_from( + effects::change_zone::count_selected_zone_arrivals( + &events[events_before_effect..], + &chosen_ids, + dest_zone, + ), + ) + .expect("selected zone arrivals fit in i32") + }), // CR 708.2a + CR 708.3: preserve the // face-down profile across a further pause. face_down_profile: ctx.face_down_profile.clone(), @@ -5364,7 +5432,16 @@ pub(super) fn handle_resolution_choice( conditional_enter_with_counters.clone(), duration: ctx.duration.clone(), track_exiled_by_source: ctx.track_exiled_by_source, - moved_count: None, + moved_count: tracks_player_action_completion.then(|| { + i32::try_from( + effects::change_zone::count_selected_zone_arrivals( + &events[events_before_effect..], + &chosen_ids, + dest_zone, + ), + ) + .expect("selected zone arrivals fit in i32") + }), // CR 708.2a + CR 708.3: preserve the // face-down profile across a further pause. face_down_profile: ctx.face_down_profile.clone(), @@ -5888,6 +5965,20 @@ pub(super) fn handle_resolution_choice( false, ); } + if matches!(effect_kind, EffectKind::ChangeZone) { + if let Some(cause) = destination.and_then(effects::this_way_cause_for_zone) { + let count = effects::change_zone::count_selected_zone_arrivals( + &events[events_before_effect..], + &chosen, + destination.expect("ChangeZone destination checked above"), + ); + effects::stamp_active_player_action_completion( + state, + source_id, + crate::types::ability::EffectResolutionResult { cause, count }, + ); + } + } state.last_effect_count = Some(chosen.len() as i32); events.push(GameEvent::EffectResolved { kind: effect_kind, diff --git a/crates/engine/src/game/resolution_prompt.rs b/crates/engine/src/game/resolution_prompt.rs index 0df0fa0bef..a995e32e86 100644 --- a/crates/engine/src/game/resolution_prompt.rs +++ b/crates/engine/src/game/resolution_prompt.rs @@ -209,7 +209,7 @@ fn quantity_offers_up_to_choice(q: &QuantityExpr) -> bool { fn effect_offers_choice(e: &Effect) -> bool { match e { // Engine-set from the activation-payment snapshot, never a player prompt. - Effect::NoteManaSpent => false, + Effect::NoteManaSpent | Effect::CompletePlayerAction { .. } => false, // ---- SCOPE FILTER. DESTRUCTURED WITHOUT `..` on every arm, exactly as // HEAD's three allow arms are, so a new field on any of them forces // a re-audit of whether the class is still in scope. diff --git a/crates/engine/src/game/trigger_index.rs b/crates/engine/src/game/trigger_index.rs index 2a268d2794..b067abf8ca 100644 --- a/crates/engine/src/game/trigger_index.rs +++ b/crates/engine/src/game/trigger_index.rs @@ -962,6 +962,7 @@ fn keys_from_effect_kind(kind: EffectKind, push: &mut impl FnMut(TriggerEventKey | EffectKind::RuntimeHandled | EffectKind::Learn | EffectKind::Forage + | EffectKind::CompletePlayerAction | EffectKind::Harness | EffectKind::CollectEvidence | EffectKind::Endure diff --git a/crates/engine/src/game/visibility.rs b/crates/engine/src/game/visibility.rs index eb87e158a3..6e124753f4 100644 --- a/crates/engine/src/game/visibility.rs +++ b/crates/engine/src/game/visibility.rs @@ -143,6 +143,13 @@ pub fn filter_state_for_viewer(state: &GameState, viewer: PlayerId) -> GameState // payloads; the separately projected `WaitingFor` prompt is the complete // viewer-facing interaction surface. filtered.resolution_stack = Default::default(); + // ChooseOneOf retains its runtime tail inside the authoritative prompt so + // resolution can resume after the branch selection. Like every other + // resolved continuation, that carrier can contain private object IDs and + // last-known information; clients need only the branch presentation. + if let WaitingFor::ChooseOneOfBranch { continuation, .. } = &mut filtered.waiting_for { + *continuation = None; + } // The provenance journal contains exact source identities, restrictions, // and cost-recipient relationships. It is server authority and must not // expose one player's mana history to another viewer. @@ -1975,6 +1982,53 @@ mod tests { use crate::types::zones::{ExileCostSourceZone, Zone}; use rand::RngCore; + #[test] + fn choose_one_prompt_redacts_runtime_continuation_for_every_viewer() { + let mut state = GameState::new_two_player(42); + let hidden = create_object( + &mut state, + CardId(1), + PlayerId(1), + "Private Card".to_string(), + Zone::Hand, + ); + state.waiting_for = WaitingFor::ChooseOneOfBranch { + player: PlayerId(0), + controller: PlayerId(0), + source_id: ObjectId(9), + branches: Vec::new(), + branch_descriptions: Vec::new(), + parent_targets: Vec::new(), + context: Default::default(), + continuation: Some(Box::new(ResolvedAbility::new( + Effect::NoOp, + vec![crate::types::ability::TargetRef::Object(hidden)], + ObjectId(9), + PlayerId(0), + ))), + replacement_applied: Default::default(), + remaining_players: Vec::new(), + }; + + for viewer in [PlayerId(0), PlayerId(1)] { + let filtered = filter_state_for_viewer(&state, viewer); + assert!(matches!( + filtered.waiting_for, + WaitingFor::ChooseOneOfBranch { + continuation: None, + .. + } + )); + } + assert!(matches!( + state.waiting_for, + WaitingFor::ChooseOneOfBranch { + continuation: Some(_), + .. + } + )); + } + #[test] fn priority_passing_preferences_are_visible_only_to_their_owner() { use crate::types::game_state::PriorityPassingMode; diff --git a/crates/engine/src/parser/oracle_effect/sequence.rs b/crates/engine/src/parser/oracle_effect/sequence.rs index 12e3980cae..5ec20c3971 100644 --- a/crates/engine/src/parser/oracle_effect/sequence.rs +++ b/crates/engine/src/parser/oracle_effect/sequence.rs @@ -6365,6 +6365,7 @@ pub(super) fn clause_is_dig_lookback_transparent(effect: &Effect) -> bool { | Effect::Learn | Effect::NoteManaSpent | Effect::Forage + | Effect::CompletePlayerAction { .. } | Effect::Harness | Effect::CollectEvidence { .. } | Effect::Endure { .. } diff --git a/crates/engine/src/parser/oracle_ir/doc.rs b/crates/engine/src/parser/oracle_ir/doc.rs index bcad65ee19..138ee242bb 100644 --- a/crates/engine/src/parser/oracle_ir/doc.rs +++ b/crates/engine/src/parser/oracle_ir/doc.rs @@ -1646,6 +1646,7 @@ fn stamp_effect_printed_slot(effect: &mut Effect, slot: usize, kind: PrintedItem Effect::Adapt { .. } => {} Effect::Learn => {} Effect::Forage => {} + Effect::CompletePlayerAction { .. } => {} Effect::Harness => {} Effect::CollectEvidence { .. } => {} Effect::Endure { .. } => {} diff --git a/crates/engine/src/parser/oracle_trigger.rs b/crates/engine/src/parser/oracle_trigger.rs index 4d7a7702f0..22c4509637 100644 --- a/crates/engine/src/parser/oracle_trigger.rs +++ b/crates/engine/src/parser/oracle_trigger.rs @@ -8839,7 +8839,10 @@ fn continues_player_action_list(after_comma: &str) -> bool { .next() .unwrap_or(trimmed) .trim(); - if parse_player_action_phrase(candidate).is_some() { + if all_consuming(parse_player_action_phrase_nom) + .parse(candidate) + .is_ok() + { return true; } // Avatar crossover: a comma-separated bending-verb disjunction @@ -15884,63 +15887,68 @@ fn is_bare_discover_subject(text: &str) -> bool { } fn parse_player_action_list(text: &str) -> Option> { - let normalized = text - .replace(", or ", "|") - .replace(" or ", "|") - .replace(", ", "|"); - let parts: Vec<_> = normalized.split('|').collect(); - if parts.is_empty() { - return None; - } - - let mut actions = Vec::with_capacity(parts.len()); - for part in parts { - actions.push(parse_player_action_phrase(part.trim())?); - } - Some(actions) -} - -fn parse_player_action_phrase(text: &str) -> Option { - if let Ok(("", action)) = parse_proliferate_player_action(text) { - return Some(action); - } - if let Ok(("", action)) = parse_forage_player_action(text) { - return Some(action); - } - match text { - "search your library" | "searches their library" => Some(PlayerActionKind::SearchedLibrary), - "scry" | "scries" => Some(PlayerActionKind::Scry), - "surveil" | "surveils" => Some(PlayerActionKind::Surveil), - // CR 701.59a: Collect evidence — exile cards from your graveyard with total mana value N or more. - "collect evidence" | "collects evidence" => Some(PlayerActionKind::CollectEvidence), - // CR 701.16a: Investigate — create a Clue artifact token. - "investigate" | "investigates" => Some(PlayerActionKind::Investigate), - "shuffle your library" - | "shuffles their library" - | "shuffle their library" - | "shuffles his or her library" - | "shuffle his or her library" - | "shuffles a library" - | "shuffle a library" => Some(PlayerActionKind::ShuffledLibrary), - _ => None, - } + all_consuming(terminated( + separated_list1( + alt((tag(", or "), tag(" or "), tag(", "))), + parse_player_action_phrase_nom, + ), + opt(one_of(".;")), + )) + .parse(text) + .ok() + .map(|(_, actions)| actions) } -fn parse_proliferate_player_action(input: &str) -> OracleResult<'_, PlayerActionKind> { - // CR 701.34a: Proliferate — choose permanents/players with counters. - all_consuming(alt(( - value(PlayerActionKind::Proliferate, tag("proliferate")), - value(PlayerActionKind::Proliferate, tag("proliferates")), - ))) - .parse(input) -} +fn parse_player_action_phrase_nom(input: &str) -> OracleResult<'_, PlayerActionKind> { + let search = value( + PlayerActionKind::SearchedLibrary, + ( + alt((tag("searches"), tag("search"))), + space1, + alt((tag("his or her"), tag("your"), tag("their"), tag("a"))), + space1, + tag("library"), + ), + ); + let shuffle = value( + PlayerActionKind::ShuffledLibrary, + ( + alt((tag("shuffles"), tag("shuffle"))), + space1, + alt((tag("his or her"), tag("your"), tag("their"), tag("a"))), + space1, + tag("library"), + ), + ); -fn parse_forage_player_action(input: &str) -> OracleResult<'_, PlayerActionKind> { - // CR 701.61a: Forage — exile three cards from your graveyard or sacrifice a Food. - all_consuming(alt(( - value(PlayerActionKind::Forage, tag("forage")), - value(PlayerActionKind::Forage, tag("forages")), - ))) + alt(( + search, + shuffle, + value( + PlayerActionKind::CollectEvidence, + alt((tag("collects evidence"), tag("collect evidence"))), + ), + value( + PlayerActionKind::Investigate, + alt((tag("investigates"), tag("investigate"))), + ), + // CR 701.34a: Proliferate — choose permanents/players with counters. + value( + PlayerActionKind::Proliferate, + alt((tag("proliferates"), tag("proliferate"))), + ), + value( + PlayerActionKind::Surveil, + alt((tag("surveils"), tag("surveil"))), + ), + value(PlayerActionKind::Scry, alt((tag("scries"), tag("scry")))), + // CR 701.61a: Forage — exile three cards from your graveyard or + // sacrifice a Food. + value( + PlayerActionKind::Forage, + alt((tag("forages"), tag("forage"))), + ), + )) .parse(input) } diff --git a/crates/engine/src/parser/oracle_trigger_tests.rs b/crates/engine/src/parser/oracle_trigger_tests.rs index 3e8134ed58..1e48219cf4 100644 --- a/crates/engine/src/parser/oracle_trigger_tests.rs +++ b/crates/engine/src/parser/oracle_trigger_tests.rs @@ -12018,7 +12018,7 @@ fn trigger_you_proliferate() { } #[test] -fn trigger_you_forage() { +fn trigger_you_forage_uses_generic_player_action_path() { let def = parse_trigger_line( "Whenever you forage, put a +1/+1 counter on this creature.", "Corpseberry Cultivator", @@ -12028,6 +12028,36 @@ fn trigger_you_forage() { assert_eq!(def.player_actions, Some(vec![PlayerActionKind::Forage])); } +#[test] +fn trigger_opponent_forages_preserves_player_scope() { + let def = parse_trigger_line( + "Whenever an opponent forages, draw a card.", + "Synthetic Forage Observer", + ); + assert_eq!(def.mode, TriggerMode::PlayerPerformedAction); + assert_eq!( + def.valid_target, + Some(TargetFilter::Typed( + TypedFilter::default().controller(ControllerRef::Opponent), + )) + ); + assert_eq!(def.player_actions, Some(vec![PlayerActionKind::Forage])); +} + +#[test] +fn trigger_player_action_list_includes_forage_without_prefix_matches() { + assert_eq!( + parse_player_action_list("scries, surveils, or forages"), + Some(vec![ + PlayerActionKind::Scry, + PlayerActionKind::Surveil, + PlayerActionKind::Forage, + ]) + ); + assert_eq!(parse_player_action_list("forager"), None); + assert_eq!(parse_player_action_list("forageable"), None); +} + #[test] fn trigger_you_scry_or_surveil() { let def = parse_trigger_line( diff --git a/crates/engine/src/types/ability.rs b/crates/engine/src/types/ability.rs index db49f3f1ff..527ede27d8 100644 --- a/crates/engine/src/types/ability.rs +++ b/crates/engine/src/types/ability.rs @@ -13885,6 +13885,14 @@ pub enum Effect { Learn, /// CR 701.61a: Forage — exile three cards from your graveyard or sacrifice a Food. Forage, + /// CR 608.2c + CR 603.2: Publish a player action only when the immediately + /// preceding operation completed the required typed result. This is a + /// parameterized completion seam, not a Forage-specific special case. + CompletePlayerAction { + parent_kind: EffectKind, + action: PlayerActionKind, + required_result: EffectResolutionResult, + }, /// CR 701.64a: Harness [this permanent] — if the source permanent isn't /// harnessed, it becomes harnessed. A unit keyword action (no parameters): /// it always designates the source permanent, mirroring `Forage`'s @@ -15726,6 +15734,7 @@ impl Effect { | Effect::Adapt { .. } | Effect::Learn | Effect::Forage + | Effect::CompletePlayerAction { .. } | Effect::Harness | Effect::CollectEvidence { .. } | Effect::Endure { .. } @@ -16195,7 +16204,7 @@ impl Effect { Effect::Behold { .. } => false, // CR 701.61a: forage exiles from a graveyard or sacrifices a Food. - Effect::Forage => false, + Effect::Forage | Effect::CompletePlayerAction { .. } => false, // CR 701.59a: "To 'collect evidence N' means to exile any number of // cards from your GRAVEYARD with total mana value N or greater." @@ -17097,6 +17106,7 @@ impl Effect { | Effect::Specialize | Effect::Learn | Effect::Forage + | Effect::CompletePlayerAction { .. } | Effect::Harness | Effect::CollectEvidence { .. } | Effect::BlightEffect { .. } @@ -17334,6 +17344,7 @@ impl Effect { | Effect::FlipCoin { .. } | Effect::FlipCoinUntilLose { .. } | Effect::Forage + | Effect::CompletePlayerAction { .. } | Effect::Harness | Effect::FreeCastFromZones { .. } | Effect::GiftDelivery { .. } @@ -17595,6 +17606,7 @@ impl Effect { | Effect::FlipCoin { .. } | Effect::FlipCoinUntilLose { .. } | Effect::Forage + | Effect::CompletePlayerAction { .. } | Effect::Harness | Effect::FreeCastFromZones { .. } | Effect::GiftDelivery { .. } @@ -17874,6 +17886,7 @@ pub fn effect_variant_name(effect: &Effect) -> &str { }, Effect::Learn => "Learn", Effect::Forage => "Forage", + Effect::CompletePlayerAction { .. } => "CompletePlayerAction", Effect::Harness => "Harness", Effect::CollectEvidence { .. } => "CollectEvidence", Effect::Endure { .. } => "Endure", @@ -18120,6 +18133,7 @@ pub enum EffectKind { RuntimeHandled, Learn, Forage, + CompletePlayerAction, Harness, CollectEvidence, Endure, @@ -18404,6 +18418,7 @@ impl From<&Effect> for EffectKind { Effect::RuntimeHandled { .. } => EffectKind::RuntimeHandled, Effect::Learn => EffectKind::Learn, Effect::Forage => EffectKind::Forage, + Effect::CompletePlayerAction { .. } => EffectKind::CompletePlayerAction, Effect::Harness => EffectKind::Harness, Effect::CollectEvidence { .. } => EffectKind::CollectEvidence, Effect::Endure { .. } => EffectKind::Endure, @@ -20531,6 +20546,15 @@ pub struct DiscardedCardResult { pub final_zone: Zone, } +/// CR 608.2c: The typed result of one immediately preceding effect, retained +/// only for its direct continuation. `cause` identifies the producer action; +/// `count` is the number of members for which that action completed. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +pub struct EffectResolutionResult { + pub cause: ThisWayCause, + pub count: usize, +} + /// Casting-time facts that flow with a spell from casting through resolution. /// Conditions in the sub_ability chain are evaluated against this context. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default)] @@ -20551,6 +20575,11 @@ pub struct SpellContext { /// iteration universe instead of an unqualified battlefield census. #[serde(default, skip_serializing_if = "Option::is_none")] pub parent_target_iteration_members: Option>, + /// CR 608.2c: Result of the immediately preceding effect. Ordinary + /// parent-to-child handoffs clear this field; only the producer's direct + /// completion node may consume it. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub prior_effect_result: Option, /// CR 601.2c + CR 115.1: For a target slot announced by "an opponent's /// choice", the opponent the spell's controller chose to make that choice. /// In a multiplayer game the controller picks which opponent announces; @@ -25376,6 +25405,29 @@ impl ResolvedAbility { } } + /// CR 608.2c: Stamp one completed effect result onto exactly the deferred + /// direct child. Descendants and alternate branches are cleared so the + /// result cannot leak beyond its typed completion node. + pub fn set_prior_effect_result_for_immediate_node(&mut self, result: EffectResolutionResult) { + self.context.prior_effect_result = Some(result); + if let Some(sub) = self.sub_ability.as_mut() { + sub.clear_prior_effect_result_recursive(); + } + if let Some(else_branch) = self.else_ability.as_mut() { + else_branch.clear_prior_effect_result_recursive(); + } + } + + pub(crate) fn clear_prior_effect_result_recursive(&mut self) { + self.context.prior_effect_result = None; + if let Some(sub) = self.sub_ability.as_mut() { + sub.clear_prior_effect_result_recursive(); + } + if let Some(else_branch) = self.else_ability.as_mut() { + else_branch.clear_prior_effect_result_recursive(); + } + } + /// CR 701.47c + CR 608.2c: Stamp the Army chosen by an amass instruction /// across every continuation branch in this resolution. pub fn set_amassed_army_object_recursive(&mut self, snapshot: CostPaidObjectSnapshot) { diff --git a/crates/engine/src/types/ability_visit.rs b/crates/engine/src/types/ability_visit.rs index c25ed87f9d..c324f20045 100644 --- a/crates/engine/src/types/ability_visit.rs +++ b/crates/engine/src/types/ability_visit.rs @@ -990,6 +990,7 @@ where | Effect::Adapt { .. } | Effect::Learn | Effect::Forage + | Effect::CompletePlayerAction { .. } | Effect::Harness | Effect::CollectEvidence { .. } | Effect::Endure { .. } diff --git a/crates/engine/src/types/game_state.rs b/crates/engine/src/types/game_state.rs index 5db8d5e62a..5f4e38bf2e 100644 --- a/crates/engine/src/types/game_state.rs +++ b/crates/engine/src/types/game_state.rs @@ -3837,6 +3837,10 @@ pub struct PendingChooseOneOf { pub parent_targets: Vec, #[serde(default)] pub context: super::ability::SpellContext, + /// CR 608.2c: Runtime tail retained until the final queued chooser selects + /// a branch. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub continuation: Option>, /// CR 614.5 + CR 616.1f: replacement effects already applied to the event /// that produced this queued branch choice. #[serde( @@ -10349,6 +10353,11 @@ pub enum WaitingFor { parent_targets: Vec, #[serde(default)] context: super::ability::SpellContext, + /// CR 608.2c: Runtime continuation that follows the complete modal + /// choice. It is attached only to the final selected branch, after all + /// CR 701.55d choosers have resolved their instances. + #[serde(default, skip_serializing_if = "Option::is_none")] + continuation: Option>, /// CR 614.5 + CR 616.1f: replacement effects already applied to the /// event that produced this choice. #[serde( @@ -27180,6 +27189,7 @@ mod tests { branch_descriptions: vec!["Draw a card.".to_string()], parent_targets: vec![], context: crate::types::ability::SpellContext::default(), + continuation: None, replacement_applied: Default::default(), remaining_players: vec![], })); diff --git a/crates/engine/src/types/resolution.rs b/crates/engine/src/types/resolution.rs index 7f8963726c..150f794c50 100644 --- a/crates/engine/src/types/resolution.rs +++ b/crates/engine/src/types/resolution.rs @@ -5525,6 +5525,7 @@ mod tests { branches: Vec::new(), parent_targets: Vec::new(), context: SpellContext::default(), + continuation: None, replacement_applied: HashSet::new(), remaining_players: Vec::new(), }, diff --git a/crates/engine/tests/integration/issue_7221_forage_trigger.rs b/crates/engine/tests/integration/issue_7221_forage_trigger.rs index 5472931087..2817274a3c 100644 --- a/crates/engine/tests/integration/issue_7221_forage_trigger.rs +++ b/crates/engine/tests/integration/issue_7221_forage_trigger.rs @@ -1,62 +1,538 @@ +//! Regression coverage for issue #7221: a completed non-cost Forage action +//! must publish the generic player-action event that drives "Whenever you +//! forage" triggers. Declining or failing to perform the action must not. + use engine::game::scenario::{GameRunner, GameScenario, P0}; +use engine::types::ability::{ + AbilityDefinition, AbilityKind, Effect, EffectScope, ReplacementDefinition, TapStateChange, + TargetFilter, +}; use engine::types::actions::GameAction; use engine::types::counter::CounterType; +use engine::types::events::{GameEvent, PlayerActionKind}; use engine::types::game_state::WaitingFor; use engine::types::identifiers::ObjectId; use engine::types::phase::Phase; +use engine::types::replacements::ReplacementEvent; +use engine::types::zones::{EtbTapState, Zone}; + +const CORPSEBERRY_ORACLE: &str = "At the beginning of combat on your turn, you may forage. (Exile three cards from your graveyard or sacrifice a Food.)\nWhenever you forage, put a +1/+1 counter on this creature."; -const CORPSEBERRY_CULTIVATOR: &str = "At the beginning of combat on your turn, you may forage. \ -(Exile three cards from your graveyard or sacrifice a Food.)\n\ -Whenever you forage, put a +1/+1 counter on this creature."; +fn destination_redirect_replacement( + from: Zone, + to: Zone, + description: &str, +) -> ReplacementDefinition { + ReplacementDefinition::new(ReplacementEvent::Moved) + .destination_zone(from) + .execute(AbilityDefinition::new( + AbilityKind::Spell, + Effect::ChangeZone { + origin: None, + destination: to, + target: TargetFilter::SelfRef, + owner_library: false, + enter_transformed: false, + enters_under: None, + enter_tapped: EtbTapState::Unspecified, + enters_attacking: false, + up_to: false, + enter_with_counters: Vec::new(), + conditional_enter_with_counters: Vec::new(), + face_down_profile: None, + enters_modified_if: None, + }, + )) + .description(description.to_string()) +} + +fn zone_tap_state_replacement( + destination: Zone, + state: TapStateChange, + description: &str, +) -> ReplacementDefinition { + ReplacementDefinition::new(ReplacementEvent::Moved) + .destination_zone(destination) + .execute(AbilityDefinition::new( + AbilityKind::Spell, + Effect::SetTapState { + target: TargetFilter::SelfRef, + scope: EffectScope::Single, + state, + }, + )) + .description(description.to_string()) +} + +fn corpseberry_board( + with_food: bool, + graveyard_cards: usize, +) -> (GameRunner, ObjectId, Option) { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let corpseberry = scenario + .add_creature_from_oracle(P0, "Corpseberry Cultivator", 2, 3, CORPSEBERRY_ORACLE) + .id(); + let food = with_food.then(|| { + scenario + .add_creature(P0, "Food", 0, 0) + .as_artifact() + .with_subtypes(vec!["Food"]) + .id() + }); + let names: Vec = (0..graveyard_cards) + .map(|index| format!("Graveyard Card {index}")) + .collect(); + let refs: Vec<&str> = names.iter().map(String::as_str).collect(); + scenario.with_graveyard(P0, &refs); + (scenario.build(), corpseberry, food) +} -fn p1p1(runner: &GameRunner, id: ObjectId) -> u32 { - runner.state().objects[&id] - .counters - .get(&CounterType::Plus1Plus1) +fn plus_one_counters(runner: &GameRunner, object_id: ObjectId) -> u32 { + runner + .state() + .objects + .get(&object_id) + .and_then(|object| object.counters.get(&CounterType::Plus1Plus1)) .copied() .unwrap_or(0) } -fn resolve_until_counter(runner: &mut GameRunner, cultivator: ObjectId) { - for _ in 0..200 { - if p1p1(runner, cultivator) > 0 { - return; - } - match &runner.state().waiting_for { - WaitingFor::OptionalEffectChoice { .. } => { - runner - .act(GameAction::DecideOptionalEffect { accept: true }) - .expect("accept the forage trigger"); +fn reach_optional_forage(runner: &mut GameRunner) -> Vec { + runner.pass_both_players(); + assert_eq!(runner.state().phase, Phase::BeginCombat); + + let mut events = Vec::new(); + for _ in 0..40 { + match runner.state().waiting_for.clone() { + WaitingFor::OptionalEffectChoice { player, .. } => { + assert_eq!(player, P0); + return events; + } + WaitingFor::OrderTriggers { .. } => { + engine::game::triggers::drain_order_triggers_with_identity(runner.state_mut()); + } + WaitingFor::Priority { .. } => { + events.extend( + runner + .act(GameAction::PassPriority) + .expect("advance Corpseberry begin-combat trigger") + .events, + ); } - WaitingFor::EffectZoneChoice { cards, count, .. } => { - let cards = cards.iter().take(*count).copied().collect(); - runner - .act(GameAction::SelectCards { cards }) - .expect("exile three cards to forage"); + other => panic!("expected Corpseberry optional forage, got {other:?}"), + } + } + panic!("Corpseberry optional forage did not surface"); +} + +fn drain_stack(runner: &mut GameRunner, events: &mut Vec) { + for _ in 0..80 { + match runner.state().waiting_for.clone() { + WaitingFor::OrderTriggers { .. } => { + engine::game::triggers::drain_order_triggers_with_identity(runner.state_mut()); } - _ => { - runner - .act(GameAction::PassPriority) - .expect("advance the game"); + WaitingFor::Priority { .. } if runner.state().stack.is_empty() => return, + WaitingFor::Priority { .. } => { + events.extend( + runner + .act(GameAction::PassPriority) + .expect("drain Corpseberry trigger stack") + .events, + ); } + other => panic!("unexpected prompt while draining Corpseberry stack: {other:?}"), } } - panic!("forage trigger did not resolve"); + panic!("Corpseberry trigger stack did not settle"); +} + +fn forage_action_count(events: &[GameEvent]) -> usize { + events + .iter() + .filter(|event| { + matches!( + event, + GameEvent::PlayerPerformedAction { + player_id: P0, + action: PlayerActionKind::Forage, + .. + } + ) + }) + .count() +} + +fn assert_ordered_forage_completion(events: &[GameEvent], source_id: ObjectId) { + assert!(events.windows(2).any(|pair| { + matches!( + pair, + [ + GameEvent::EffectResolved { + kind: engine::types::ability::EffectKind::Forage, + source_id: event_source, + .. + }, + GameEvent::PlayerPerformedAction { + player_id: P0, + action: PlayerActionKind::Forage, + .. + } + ] if *event_source == source_id + ) + })); +} + +/// CR 701.61a + CR 603.2: sacrificing the only Food completes Forage once, +/// publishes the action after completion, and fires Corpseberry's trigger once. +#[test] +fn corpseberry_food_forage_fires_trigger_once() { + let (mut runner, corpseberry, food) = corpseberry_board(true, 0); + let food = food.expect("Food fixture"); + let mut events = reach_optional_forage(&mut runner); + + events.extend( + runner + .act(GameAction::DecideOptionalEffect { accept: true }) + .expect("accept optional forage") + .events, + ); + drain_stack(&mut runner, &mut events); + + assert_eq!(runner.state().objects[&food].zone, Zone::Graveyard); + assert_eq!(forage_action_count(&events), 1); + assert_ordered_forage_completion(&events, corpseberry); + assert_eq!(plus_one_counters(&runner, corpseberry), 1); + assert_eq!( + runner + .state() + .player_actions_this_turn + .iter() + .filter(|(player, action)| *player == P0 && *action == PlayerActionKind::Forage) + .count(), + 1, + "the outer Forage frame must not duplicate the nested completion ledger" + ); +} + +/// CR 701.61a + CR 603.2: when several Foods are available, selecting one is +/// part of the forage instruction. Completion is published only after the +/// selected Food is sacrificed through the real EffectZoneChoice resume path. +#[test] +fn corpseberry_food_choice_fires_after_selected_sacrifice() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let corpseberry = scenario + .add_creature_from_oracle(P0, "Corpseberry Cultivator", 2, 3, CORPSEBERRY_ORACLE) + .id(); + let foods = ["Food A", "Food B"].map(|name| { + scenario + .add_creature(P0, name, 0, 0) + .as_artifact() + .with_subtypes(vec!["Food"]) + .id() + }); + let mut runner = scenario.build(); + let mut events = reach_optional_forage(&mut runner); + + events.extend( + runner + .act(GameAction::DecideOptionalEffect { accept: true }) + .expect("accept Food forage") + .events, + ); + let selected = match runner.state().waiting_for.clone() { + WaitingFor::EffectZoneChoice { + cards, + count: 1, + effect_kind: engine::types::ability::EffectKind::Sacrifice, + .. + } => cards[0], + other => panic!("expected Food sacrifice choice, got {other:?}"), + }; + assert_eq!(forage_action_count(&events), 0); + events.extend( + runner + .act(GameAction::SelectCards { + cards: vec![selected], + }) + .expect("select Food to sacrifice") + .events, + ); + drain_stack(&mut runner, &mut events); + + assert_eq!(runner.state().objects[&selected].zone, Zone::Graveyard); + assert_eq!( + foods + .iter() + .filter(|food| runner.state().objects[food].zone == Zone::Battlefield) + .count(), + 1 + ); + assert_eq!(forage_action_count(&events), 1); + assert_ordered_forage_completion(&events, corpseberry); + assert_eq!(plus_one_counters(&runner, corpseberry), 1); } +/// CR 608.2d: declining the optional instruction performs no Forage action and +/// therefore cannot trigger Corpseberry's second ability. +#[test] +fn corpseberry_decline_does_not_forage() { + let (mut runner, corpseberry, food) = corpseberry_board(true, 0); + let food = food.expect("Food fixture"); + let mut events = reach_optional_forage(&mut runner); + + events.extend( + runner + .act(GameAction::DecideOptionalEffect { accept: false }) + .expect("decline optional forage") + .events, + ); + drain_stack(&mut runner, &mut events); + + assert_eq!(runner.state().objects[&food].zone, Zone::Battlefield); + assert_eq!(forage_action_count(&events), 0); + assert_eq!(plus_one_counters(&runner, corpseberry), 0); +} + +/// CR 608.2d: when neither complete Forage mode is possible, the optional +/// instruction is infeasible and must not open an acceptance prompt. +#[test] +fn corpseberry_impossible_optional_is_not_offered() { + let (mut runner, corpseberry, _) = corpseberry_board(false, 2); + runner.pass_both_players(); + assert_eq!(runner.state().phase, Phase::BeginCombat); + let mut events = Vec::new(); + + drain_stack(&mut runner, &mut events); + + assert_eq!(forage_action_count(&events), 0); + assert_eq!(plus_one_counters(&runner, corpseberry), 0); + assert!(matches!( + runner.state().waiting_for, + WaitingFor::Priority { .. } + )); +} + +/// CR 701.61a + CR 608.2c: the exile mode does not publish completion at its +/// selection prompt. It completes only after exactly three chosen graveyard +/// cards arrive in exile, then fires Corpseberry once. #[test] fn foraging_from_graveyard_triggers_corpseberry_cultivator() { + let (mut runner, corpseberry, _) = corpseberry_board(false, 4); + let mut events = reach_optional_forage(&mut runner); + events.extend( + runner + .act(GameAction::DecideOptionalEffect { accept: true }) + .expect("accept optional forage") + .events, + ); + + let cards = match runner.state().waiting_for.clone() { + WaitingFor::EffectZoneChoice { + cards, + count: 3, + zone: Zone::Graveyard, + destination: Some(Zone::Exile), + .. + } => cards.into_iter().take(3).collect::>(), + other => panic!("expected exile-three selection, got {other:?}"), + }; + assert_eq!(forage_action_count(&events), 0); + events.extend( + runner + .act(GameAction::SelectCards { + cards: cards.clone(), + }) + .expect("select three graveyard cards for forage") + .events, + ); + drain_stack(&mut runner, &mut events); + + assert!(cards + .iter() + .all(|card| runner.state().objects[card].zone == Zone::Exile)); + assert_eq!(runner.state().players[P0.0 as usize].graveyard.len(), 1); + assert_eq!(forage_action_count(&events), 1); + assert_ordered_forage_completion(&events, corpseberry); + assert_eq!(plus_one_counters(&runner, corpseberry), 1); +} + +/// CR 701.61a + CR 616.1: replacement ordering can pause each member of the +/// exile-three operation. The exact moved-count and completion continuation +/// must survive serialization and publish Forage only after all three cards +/// have actually arrived in exile. +#[test] +fn corpseberry_exile_replacement_pause_completes_after_restore() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let corpseberry = scenario + .add_creature_from_oracle(P0, "Corpseberry Cultivator", 2, 3, CORPSEBERRY_ORACLE) + .id(); + scenario.with_graveyard(P0, &["A", "B", "C"]); + for (name, tap_state) in [ + ("Synthetic Exile Tap", TapStateChange::Tap), + ("Synthetic Exile Untap", TapStateChange::Untap), + ] { + scenario + .add_creature(P0, name, 0, 0) + .as_enchantment() + .with_replacement_definition(zone_tap_state_replacement( + Zone::Exile, + tap_state, + "If a card would be exiled, modify that event.", + )); + } + let mut runner = scenario.build(); + let mut events = reach_optional_forage(&mut runner); + events.extend( + runner + .act(GameAction::DecideOptionalEffect { accept: true }) + .expect("accept exile-mode forage") + .events, + ); + let chosen = match runner.state().waiting_for.clone() { + WaitingFor::EffectZoneChoice { cards, .. } => cards, + other => panic!("expected exile selection, got {other:?}"), + }; + events.extend( + runner + .act(GameAction::SelectCards { + cards: chosen.clone(), + }) + .expect("select cards for replacement-paused forage") + .events, + ); + assert!(matches!( + runner.state().waiting_for, + WaitingFor::ReplacementChoice { .. } + )); + assert_eq!(forage_action_count(&events), 0); + + let serialized = serde_json::to_string(runner.state()) + .expect("replacement-paused exile Forage state serializes"); + *runner.state_mut() = serde_json::from_str(&serialized) + .expect("replacement-paused exile Forage state deserializes"); + + for _ in 0..chosen.len() { + assert!(matches!( + runner.state().waiting_for, + WaitingFor::ReplacementChoice { .. } + )); + events.extend( + runner + .act(GameAction::ChooseReplacement { index: 0 }) + .expect("order exile replacement") + .events, + ); + } + drain_stack(&mut runner, &mut events); + + assert!(chosen + .iter() + .all(|card| runner.state().objects[card].zone == Zone::Exile)); + assert_eq!(forage_action_count(&events), 1); + assert_ordered_forage_completion(&events, corpseberry); + assert_eq!(plus_one_counters(&runner, corpseberry), 1); +} + +/// CR 701.21a + CR 614.6: a Food remains sacrificed when a replacement sends +/// it to exile instead of its owner's graveyard. The paused operation result +/// and its completion continuation also survive a serialized state round trip. +#[test] +fn corpseberry_food_redirected_to_exile_still_forges_after_restore() { let mut scenario = GameScenario::new(); scenario.at_phase(Phase::PreCombatMain); - let cultivator = scenario - .add_creature_from_oracle(P0, "Corpseberry Cultivator", 2, 3, CORPSEBERRY_CULTIVATOR) + let corpseberry = scenario + .add_creature_from_oracle(P0, "Corpseberry Cultivator", 2, 3, CORPSEBERRY_ORACLE) .id(); - for _ in 0..3 { - scenario.add_creature_to_graveyard(P0, "Fodder", 1, 1); + let food = scenario + .add_creature(P0, "Food", 0, 0) + .as_artifact() + .with_subtypes(vec!["Food"]) + .id(); + for name in ["Rest in Peace", "Leyline of the Void"] { + scenario + .add_creature(P0, name, 0, 0) + .as_enchantment() + .with_replacement_definition(destination_redirect_replacement( + Zone::Graveyard, + Zone::Exile, + "If a card would be put into a graveyard, exile it instead.", + )); } + let mut runner = scenario.build(); + let mut events = reach_optional_forage(&mut runner); + events.extend( + runner + .act(GameAction::DecideOptionalEffect { accept: true }) + .expect("accept Food forage under redirects") + .events, + ); + assert!(matches!( + runner.state().waiting_for, + WaitingFor::ReplacementChoice { .. } + )); + assert_eq!(forage_action_count(&events), 0); + + let serialized = + serde_json::to_string(runner.state()).expect("replacement-paused Forage state serializes"); + *runner.state_mut() = + serde_json::from_str(&serialized).expect("replacement-paused Forage state deserializes"); + events.extend( + runner + .act(GameAction::ChooseReplacement { index: 0 }) + .expect("choose graveyard redirect") + .events, + ); + drain_stack(&mut runner, &mut events); + + assert_eq!(runner.state().objects[&food].zone, Zone::Exile); + assert_eq!(forage_action_count(&events), 1); + assert_ordered_forage_completion(&events, corpseberry); + assert_eq!(plus_one_counters(&runner, corpseberry), 1); +} +/// CR 614.6: the exile mode completes only for cards that actually arrive in +/// exile. Redirecting every selected card to hand resolves the instruction but +/// does not perform the Forage action or fire Corpseberry. +#[test] +fn corpseberry_redirected_exile_does_not_forage() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let corpseberry = scenario + .add_creature_from_oracle(P0, "Corpseberry Cultivator", 2, 3, CORPSEBERRY_ORACLE) + .id(); + scenario.with_graveyard(P0, &["A", "B", "C"]); + scenario + .add_creature(P0, "Synthetic Exile Redirect", 0, 0) + .as_enchantment() + .with_replacement_definition(destination_redirect_replacement( + Zone::Exile, + Zone::Hand, + "If a card would be exiled, put it into its owner's hand instead.", + )); let mut runner = scenario.build(); - resolve_until_counter(&mut runner, cultivator); + let mut events = reach_optional_forage(&mut runner); + events.extend( + runner + .act(GameAction::DecideOptionalEffect { accept: true }) + .expect("accept exile-mode forage") + .events, + ); + let chosen = match runner.state().waiting_for.clone() { + WaitingFor::EffectZoneChoice { cards, .. } => cards, + other => panic!("expected exile selection, got {other:?}"), + }; + events.extend( + runner + .act(GameAction::SelectCards { cards: chosen }) + .expect("select redirected exile cards") + .events, + ); + drain_stack(&mut runner, &mut events); - assert_eq!(p1p1(&runner, cultivator), 1); + assert_eq!(runner.state().players[P0.0 as usize].hand.len(), 3); + assert_eq!(forage_action_count(&events), 0); + assert_eq!(plus_one_counters(&runner, corpseberry), 0); } diff --git a/crates/lobby-broker/src/protocol.rs b/crates/lobby-broker/src/protocol.rs index 755042bc15..e00e578a68 100644 --- a/crates/lobby-broker/src/protocol.rs +++ b/crates/lobby-broker/src/protocol.rs @@ -43,6 +43,7 @@ pub enum ServerErrorCode { /// handshake. When making such changes, plan a deprecation window where /// both the old and new variants coexist, then bump and remove the old. /// +/// 30 — Serialized player-action completion provenance and modal continuations. /// 29 — Added requester-correlated `ResolveAllRejected` response frames. /// 28 — Added native `ResolveAll` request/result frames. /// 27 — Added `DraftKind::Sealed`, serialized by draft WebSocket messages. @@ -76,7 +77,7 @@ pub enum ServerErrorCode { /// payload; mulligan bottoming folded into a /// `MulliganDecisionPhase::BottomCards` sub-phase on /// `WaitingFor::MulliganDecision`. -pub const PROTOCOL_VERSION: u32 = 29; +pub const PROTOCOL_VERSION: u32 = 30; /// Minimum protocol version accepted by lobby-only brokers at the hello /// handshake. Lobby traffic has a one-version rollout window; full game servers @@ -413,12 +414,12 @@ mod tests { #[test] fn protocol_version_tracks_full_game_wire_additions() { - assert_eq!(PROTOCOL_VERSION, 29); + assert_eq!(PROTOCOL_VERSION, 30); // Lobby keeps its one-version rollout window; full-game servers stay // current-only (`server_core::MIN_SUPPORTED_PROTOCOL == PROTOCOL_VERSION`), // which is what refuses an older full-game peer whose GameState cannot // understand a success acknowledgment the submitting client awaits. - assert_eq!(MIN_SUPPORTED_PROTOCOL, 28); + assert_eq!(MIN_SUPPORTED_PROTOCOL, 29); } #[test] diff --git a/crates/phase-ai/src/policies/effect_classify.rs b/crates/phase-ai/src/policies/effect_classify.rs index a46e90fec2..dee30eec3d 100644 --- a/crates/phase-ai/src/policies/effect_classify.rs +++ b/crates/phase-ai/src/policies/effect_classify.rs @@ -304,6 +304,7 @@ pub(crate) fn effect_polarity(effect: &Effect) -> EffectPolarity { | Effect::FlipCoins { .. } | Effect::FlipCoinUntilLose { .. } | Effect::Forage + | Effect::CompletePlayerAction { .. } | Effect::ForceAttack { .. } | Effect::ForEachCategory { .. } | Effect::FreeCastFromZones { .. } diff --git a/crates/phase-ai/src/policies/redundancy_avoidance.rs b/crates/phase-ai/src/policies/redundancy_avoidance.rs index 6e90e9d54d..309fb9add2 100644 --- a/crates/phase-ai/src/policies/redundancy_avoidance.rs +++ b/crates/phase-ai/src/policies/redundancy_avoidance.rs @@ -568,6 +568,7 @@ fn redundancy_delta( | Effect::Adapt { .. } | Effect::Learn | Effect::Forage + | Effect::CompletePlayerAction { .. } | Effect::Harness | Effect::CollectEvidence { .. } | Effect::Endure { .. } diff --git a/crates/server-core/src/protocol.rs b/crates/server-core/src/protocol.rs index cb0275725f..acbf5b5fea 100644 --- a/crates/server-core/src/protocol.rs +++ b/crates/server-core/src/protocol.rs @@ -2320,8 +2320,8 @@ mod tests { } #[test] - fn protocol_version_is_29() { - assert_eq!(PROTOCOL_VERSION, 29); + fn protocol_version_is_30() { + assert_eq!(PROTOCOL_VERSION, 30); } /// The bump alone is inert — a version number nobody enforces prevents no @@ -2331,7 +2331,7 @@ mod tests { /// understand. /// /// REVERT-PROBE: relax to `PROTOCOL_VERSION - 1` — the exact regression - /// this guards — and this test reds while `protocol_version_is_29` stays + /// this guards — and this test reds while `protocol_version_is_30` stays /// green, which is why the two are separate assertions. #[test] fn full_game_floor_is_current_only_not_a_rollout_window() { diff --git a/scripts/check-protocol-version.mjs b/scripts/check-protocol-version.mjs index 433b71dbda..3fad9222e1 100644 --- a/scripts/check-protocol-version.mjs +++ b/scripts/check-protocol-version.mjs @@ -3,7 +3,7 @@ import { dirname, resolve } from "node:path"; import { fileURLToPath } from "node:url"; const root = resolve(dirname(fileURLToPath(import.meta.url)), ".."); -const EXPECTED_PROTOCOL_VERSION = 29; +const EXPECTED_PROTOCOL_VERSION = 30; function extractVersion(source, pattern, label) { const match = source.match(pattern);