diff --git a/crates/engine/src/game/effects/draw.rs b/crates/engine/src/game/effects/draw.rs index c72049b30a..a4696f82fe 100644 --- a/crates/engine/src/game/effects/draw.rs +++ b/crates/engine/src/game/effects/draw.rs @@ -4,7 +4,7 @@ use crate::game::quantity::resolve_quantity_with_targets; use crate::game::replacement::{self, ReplacementResult}; use crate::game::static_abilities::prohibition_scope_matches_player; use crate::types::ability::{Effect, EffectError, EffectKind, ResolvedAbility}; -use crate::types::events::GameEvent; +use crate::types::events::{GameEvent, PlayerActionKind}; use crate::types::game_state::{DrawSequenceOrigin, GameState}; use crate::types::proposed_event::{AppliedReplacementKey, ProposedEvent}; use crate::types::statics::StaticMode; @@ -446,6 +446,26 @@ fn resume_draw_sequence_outcome( return DrawSequenceOutcome::Parked(ReplacementResult::Prevented); }; state.last_effect_count = Some(frame.accumulated as i32); + // Record the drawing player exactly once per + // settled draw INSTRUCTION — the emission granularity is the whole draw, not + // the per-card unit that `apply_draw_after_replacement` settles. `frame.player` + // is the concrete drawer, so during a `player_scope: Opponent` fan-out (Cut a + // Deal) each scoped opponent's own instruction records that opponent, without + // relying on `ability.controller` rebinding. Gated on `frame.accumulated > 0` + // so an instruction that delivered no card (empty library, or every unit + // replaced away) records nothing because that player did not draw. The generic + // post-effect scan in `effects/mod.rs` folds this + // event into `player_actions_this_way` (a set — dedups the drawer for a + // multi-card draw) and `player_actions_this_turn` (a Vec — now one entry per + // draw event, not per card). + if frame.accumulated > 0 { + events.push(GameEvent::PlayerPerformedAction { + player_id: frame.player, + action: PlayerActionKind::Draw, + look_count: None, + scry_bottom_count: None, + }); + } match frame.origin { DrawSequenceOrigin::Plain => { // Intentionally no `EffectResolved { Draw }`: no trigger matcher consumes @@ -668,6 +688,16 @@ pub fn apply_draw_after_replacement( .expect("empty-library draw bookkeeping must have a live player and journal cause"); } + // CR 121.1 + CR 608.2c + CR 109.5: The `PlayerPerformedAction { Draw }` + // ledger emission is NOT made here. This helper settles ONE draw unit — the + // sequence driver (`resume_draw_sequence_outcome`) calls it once per card + // (count = 1), and the resumed-choice path in `engine_replacement` settles a + // single paused unit too — so recording here would push one event per card. + // Instead the drawing player is recorded exactly once per settled draw + // INSTRUCTION, at frame completion, gated on the instruction's true total. + // That keeps `player_actions_this_way` (a set) counting players who drew and + // makes `player_actions_this_turn` (a Vec) count draw events, not cards, so a + // future `PlayerActionsThisTurn { Draw }` consumer measures draws not cards. drawn_count } @@ -1687,3 +1717,113 @@ mod tranche4_draw_pipeline_tests { assert!(state.players[0].graveyard.contains(&drawn)); } } + +/// CR 121.1 + CR 608.2c + CR 109.5: The `PlayerPerformedAction { Draw }` ledger +/// emission fires once per settled draw INSTRUCTION, at draw-sequence completion +/// (`resume_draw_sequence_outcome`). These tests drive the REAL production driver +/// (`start_draw_sequence`), which internally delivers a multi-card draw +/// unit-by-unit (count = 1 per card) — the exact shape production uses — and +/// assert the emission granularity is per instruction, not per card. A direct +/// `apply_draw_after_replacement` call with `count: 2` is deliberately NOT used: +/// production never settles a multi-card draw in a single such call, so it would +/// exercise a shape the engine doesn't drive. +#[cfg(test)] +mod draw_this_way_ledger_tests { + use super::*; + use crate::game::scenario::{GameScenario, P0}; + + fn drew_action_events(events: &[GameEvent]) -> usize { + events + .iter() + .filter(|event| { + matches!( + event, + GameEvent::PlayerPerformedAction { + action: PlayerActionKind::Draw, + .. + } + ) + }) + .count() + } + + fn card_drawn_events(events: &[GameEvent]) -> usize { + events + .iter() + .filter(|event| matches!(event, GameEvent::CardDrawn { .. })) + .count() + } + + /// CR 121.1 + CR 608.2c: A TWO-card draw driven by the production sequence + /// (`start_draw_sequence(.., 2, ..)`) delivers two cards (two `CardDrawn` + /// events) but records the drawing player with exactly ONE + /// `PlayerPerformedAction { Draw }` — the emission is per settled draw + /// instruction, not per card. Revert-failing anchor: moving the emit back + /// into the per-unit `apply_draw_after_replacement` makes `drew_action_events` + /// == 2 (one per card) and fails the final assertion. This is the emission + /// side of ruling #2 ("if an opponent drew more than one card this way … you + /// still draw only one card for that player"); the `player_actions_this_way` + /// `HashSet` is the second line of defence, validated end-to-end in the + /// `cut_a_deal_draw_this_way_count` integration suite. + #[test] + fn multi_card_instruction_records_player_once_via_sequence() { + let mut sc = GameScenario::new(); + sc.add_card_to_library_top(P0, "Island"); + sc.add_card_to_library_top(P0, "Mountain"); + let mut state = sc.state; + + let mut events = Vec::new(); + start_draw_sequence(&mut state, P0, 2, &mut events); + + assert_eq!( + card_drawn_events(&events), + 2, + "the two-card instruction must deliver two cards (per-card CardDrawn)" + ); + assert_eq!( + drew_action_events(&events), + 1, + "but the draw-action ledger event must fire exactly once per instruction, \ + not once per card (CR 608.2c ruling #2)" + ); + } + + /// CR 121.1: A draw instruction that delivers no card (empty library — no top + /// card enters the hand, so no draw occurs) emits no + /// `PlayerPerformedAction { Draw }`, so a player who doesn't draw is never + /// counted (CR 608.2c ruling #1). This is the `frame.accumulated > 0` gate. + #[test] + fn empty_library_instruction_records_nothing_via_sequence() { + let sc = GameScenario::new(); + let mut state = sc.state; + + let mut events = Vec::new(); + start_draw_sequence(&mut state, P0, 1, &mut events); + + assert_eq!( + card_drawn_events(&events), + 0, + "empty library delivers no card" + ); + assert_eq!( + drew_action_events(&events), + 0, + "a draw that delivers nothing must not record the player (CR 608.2c ruling #1)" + ); + } + + /// CR 121.1: Baseline — a normal one-card draw instruction records the drawing + /// player once, so ordinary draws still populate the ledger. + #[test] + fn single_card_instruction_records_player_once_via_sequence() { + let mut sc = GameScenario::new(); + sc.add_card_to_library_top(P0, "Plains"); + let mut state = sc.state; + + let mut events = Vec::new(); + start_draw_sequence(&mut state, P0, 1, &mut events); + + assert_eq!(card_drawn_events(&events), 1); + assert_eq!(drew_action_events(&events), 1); + } +} diff --git a/crates/engine/src/game/effects/mod.rs b/crates/engine/src/game/effects/mod.rs index d6580b949f..23ec37deb2 100644 --- a/crates/engine/src/game/effects/mod.rs +++ b/crates/engine/src/game/effects/mod.rs @@ -3622,8 +3622,24 @@ fn detach_after_player_scope_local_chain( // "Each opponent may X and Y" makes the whole same-sentence X/Y clause // optional for that opponent. Keep the continuation inside the scoped // template so accepting the offer performs both instructions. - let next_is_optional_clause_continuation = - node.optional && next.sub_link == SubAbilityLink::ContinuationStep; + // + // CR 608.2c + CR 109.5: EXCEPT a child that carries its OWN distinct + // `player_scope` (a different population than this fan-out's) is a SEPARATE + // scoped instruction, not a continuation of this optional clause — e.g. + // Kwain, Itinerant Meddler's "each player may draw a card, then each player + // who drew a card this way gains 1 life": the GainLife is scoped to the + // drawers (`PerformedActionThisWay`), not to this "each player" (`All`) + // fan-out. Keeping it co-scoped would re-enter the fan-out driver once per + // outer iteration and re-count the incrementally growing this-way ledger, + // over-applying to the earlier-iterated players (a triangular over-gain). It + // must detach and resolve ONCE over its own population after the parent + // fan-out completes. + let next_is_optional_clause_continuation = node.optional + && next.sub_link == SubAbilityLink::ContinuationStep + && next + .player_scope + .as_ref() + .is_none_or(|child_scope| child_scope == scope); // CR 701.23i + CR 701.24a: A shuffle explicitly scoped to players who // searched this way is a once-after-all-searches tail, not the ordinary // per-player SearchLibrary → ChangeZone → Shuffle continuation. Keep the diff --git a/crates/engine/src/game/engine.rs b/crates/engine/src/game/engine.rs index 337f56f683..fa50b0e638 100644 --- a/crates/engine/src/game/engine.rs +++ b/crates/engine/src/game/engine.rs @@ -15961,9 +15961,11 @@ mod stage2_injector_tests { // and is offered as a follow-up rather than taken unannounced mid-review. // #6812 noted-mana support inserts two lines above all three producers: // `:6210/:6287/:9475 => :6212/:6289/:9477`. The producers remain byte-identical. - "game/effects/mod.rs:6212".to_string(), - "game/effects/mod.rs:6289".to_string(), - "game/effects/mod.rs:9477".to_string(), + // #7018 adds the 16-line distinct-player-scope continuation gate above all + // three producers: `:6212/:6289/:9477 => :6228/:6305/:9493`. + "game/effects/mod.rs:6228".to_string(), + "game/effects/mod.rs:6305".to_string(), + "game/effects/mod.rs:9493".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/log.rs b/crates/engine/src/game/log.rs index 1c9be9be3a..a5847d770b 100644 --- a/crates/engine/src/game/log.rs +++ b/crates/engine/src/game/log.rs @@ -404,6 +404,15 @@ fn should_exclude_event(event: &GameEvent, state: &GameState) -> bool { { true } + // PlayerPerformedAction { Draw } is an internal ledger signal consumed by + // "for each player who drew a card this way" counting and + // the player-action trigger index), not a user-facing event. Unlike + // CardDrawn, which remains available as a HiddenInformation diagnostic, + // excluding it keeps the visible log from narrating internal ledger events. + GameEvent::PlayerPerformedAction { + action: crate::types::events::PlayerActionKind::Draw, + .. + } => true, // StackPushed/StackResolved are low-signal bookkeeping — // the meaningful info is in SpellCast/AbilityActivated and EffectResolved GameEvent::StackPushed { .. } | GameEvent::StackResolved { .. } => true, @@ -1816,6 +1825,42 @@ mod tests { )); } + #[test] + fn draw_player_action_is_excluded_but_other_actions_are_logged() { + use crate::types::events::PlayerActionKind; + + let state = GameState::new_two_player(42); + // The Draw ledger signal must not reach the visible log — + // this assertion flips (entries.len() == 1) if the exclusion is reverted. + let draw_event = GameEvent::PlayerPerformedAction { + player_id: PlayerId(0), + action: PlayerActionKind::Draw, + look_count: None, + scry_bottom_count: None, + }; + let draw_entries = resolve_log_entries(&[draw_event], &state, &state); + assert!( + draw_entries.is_empty(), + "PlayerPerformedAction {{ Draw }} is a ledger-only signal and must be excluded from the log" + ); + + // Reach-guard against an over-broad exclusion: a non-Draw player action + // (Scry) must still produce a log entry. Fails if someone excludes all + // PlayerPerformedAction variants instead of just Draw. + let scry_event = GameEvent::PlayerPerformedAction { + player_id: PlayerId(0), + action: PlayerActionKind::Scry, + look_count: Some(1), + scry_bottom_count: Some(0), + }; + let scry_entries = resolve_log_entries(&[scry_event], &state, &state); + assert_eq!( + scry_entries.len(), + 1, + "Non-Draw player actions must remain visible in the log" + ); + } + #[test] fn damage_dealt_non_combat_is_life_category() { let event = GameEvent::DamageDealt { diff --git a/crates/engine/src/parser/oracle_effect/lower.rs b/crates/engine/src/parser/oracle_effect/lower.rs index 71b0f47a4f..bd2f46bd12 100644 --- a/crates/engine/src/parser/oracle_effect/lower.rs +++ b/crates/engine/src/parser/oracle_effect/lower.rs @@ -4424,6 +4424,20 @@ pub(super) fn strip_each_player_subject(text: &str) -> (Option, St return (Some(attr_scope), deconjugated); } + // CR 608.2c + CR 109.5: A "who [verb]ed … this way" relative clause after + // "each player" / "each opponent" restricts the affected set to the players + // who performed the tracked action during THIS resolution (Kwain, Itinerant + // Meddler: "each player who drew a card this way gains 1 life" — only players + // who actually drew gain the life, so an opponent who declined the optional + // draw or had an empty library is excluded). Like the "who controls" / + // attribute clauses above, the relative clause MUST be consumed and reflected + // in the scope; dropping it would over-apply the effect to every player. + if let Some((action_scope, after_clause)) = strip_performed_action_this_way_clause(&scope, rest) + { + let deconjugated = subject::deconjugate_verb(&after_clause); + return (Some(action_scope), deconjugated); + } + // CR 508.6 + CR 104.3e: A "[source] attacked this turn" relative clause after // "each player" / "each opponent" restricts the affected set to the players // the ability source creature attacked this turn — Angel of Destiny: "each @@ -5068,6 +5082,67 @@ fn strip_player_attribute_clause( )) } +/// CR 608.2c + CR 109.5: Strip a "who [verb]ed … this way" relative clause after +/// an "each opponent"/"each player" subject. Returns +/// `PlayerFilter::PerformedActionThisWay` (carrying the base subject's relation +/// and the performed action, keyed at runtime on the `player_actions_this_way` +/// ledger that each settled search/investigate/draw populates) and the +/// verb-phrase remainder. Returns `None` when no such clause is present. +/// +/// The this-way verb table is delegated whole to `parse_who_action_this_way` +/// (oracle_quantity.rs) — the same authority the quantity path +/// (`parse_action_this_way`) uses — so search, investigate, and draw stay one +/// building block across both the quantity and subject scopes. This function +/// adds only the subject-path concerns: deriving the relation from the base +/// subject and enforcing a non-empty verb-phrase residual. Kwain, Itinerant +/// Meddler ("each player who drew a card this way gains 1 life") is the +/// subject-scope sibling of Cut a Deal's quantity-path "for each opponent who +/// drew a card this way". +fn strip_performed_action_this_way_clause( + base: &PlayerFilter, + rest: &str, +) -> Option<(PlayerFilter, String)> { + use crate::types::ability::PlayerRelation; + let relation = match base { + PlayerFilter::Opponent => PlayerRelation::Opponent, + PlayerFilter::All => PlayerRelation::All, + PlayerFilter::Controller + | PlayerFilter::DefendingPlayer + | PlayerFilter::OpponentLostLife + | PlayerFilter::OpponentGainedLife + | PlayerFilter::HasLostTheGame + | PlayerFilter::OpponentDealtDamage { .. } + | PlayerFilter::OpponentAttacked { .. } + | PlayerFilter::OpponentAttackingEnchantedPlayer + | PlayerFilter::AllExcept { .. } + | PlayerFilter::HighestSpeed + | PlayerFilter::ZoneChangedThisWay + | PlayerFilter::PerformedActionThisWay { .. } + | PlayerFilter::OwnersOfCardsExiledBySource + | PlayerFilter::TriggeringPlayer + | PlayerFilter::OpponentOtherThanTriggering + | PlayerFilter::OpponentOfTriggeringPlayer + | PlayerFilter::OpponentOfTriggeringPlayerNotAttacked + | PlayerFilter::VotedFor { .. } + | PlayerFilter::ParentObjectTargetController + | PlayerFilter::ControlsCount { .. } + | PlayerFilter::PlayerAttribute { .. } + | PlayerFilter::ChosenPlayer { .. } + | PlayerFilter::ParentObjectTargetOwner + | PlayerFilter::TrackedSetPossessor { .. } => return None, + }; + let (remainder, action) = + crate::parser::oracle_quantity::parse_who_action_this_way(rest).ok()?; + let verb_phrase = remainder.trim_start(); + if verb_phrase.is_empty() { + return None; + } + Some(( + PlayerFilter::PerformedActionThisWay { relation, action }, + verb_phrase.to_string(), + )) +} + fn strip_linked_exile_owner_subject(text: &str) -> (Option, String) { let lower = text.to_lowercase(); let scope_rest = nom_on_lower(text, &lower, |i| { diff --git a/crates/engine/src/parser/oracle_quantity.rs b/crates/engine/src/parser/oracle_quantity.rs index eb423db4b3..3eb9590366 100644 --- a/crates/engine/src/parser/oracle_quantity.rs +++ b/crates/engine/src/parser/oracle_quantity.rs @@ -2649,10 +2649,9 @@ fn target_hand_card_filter( /// searched and failed to find). /// /// Nesting: the population word ("opponent(s)"/"player(s)") fixes the relation, -/// then the shared `"who "` prefix dispatches on the verb arm. The search arm -/// carries an object-noun ("searched their library"); the investigate arm is -/// object-less ("investigated"). Composed entirely from `alt`/`value`/`tag` — -/// no permutation enumeration. +/// then the shared `"who … this way"` tail (`parse_who_action_this_way`) +/// dispatches on the verb arm. Composed entirely from `alt`/`value`/`tag` — no +/// permutation enumeration. fn parse_action_this_way( input: &str, ) -> nom::IResult<&str, (PlayerRelation, PlayerActionKind), OracleError<'_>> { @@ -2663,10 +2662,32 @@ fn parse_action_this_way( value(PlayerRelation::All, tag("player ")), )) .parse(input)?; + let (input, action) = parse_who_action_this_way(input)?; + Ok((input, (relation, action))) +} + +/// CR 608.2c + CR 109.5: The population-agnostic `"who [verb] this way"` +/// relative-clause tail shared by both this-way callers. Matches the `"who "` +/// prefix, the verb arm (search carries an object-noun "searched their library"; +/// investigate is object-less; draw carries "drew a card"), and the `" this way"` +/// anaphor terminator, returning the performed action and the residual after the +/// terminator. Composed entirely from `alt`/`value`/`tag`. +/// +/// Single authority for the this-way verb table across two scopes: +/// - `parse_action_this_way` (this module) prepends the population relation for +/// the QUANTITY path ("the number of opponents who drew a card this way"). +/// - `strip_performed_action_this_way_clause` (oracle_effect/lower.rs) supplies +/// the relation from the already-stripped "each player "/"each opponent " +/// subject prefix for the player-SCOPE SUBJECT path (Kwain, Itinerant Meddler: +/// "each player who drew a card this way gains 1 life"). +pub(crate) fn parse_who_action_this_way( + input: &str, +) -> nom::IResult<&str, PlayerActionKind, OracleError<'_>> { let (input, _) = tag("who ").parse(input)?; - let (input, action) = alt((parse_searched_arm, parse_investigated_arm)).parse(input)?; + let (input, action) = + alt((parse_searched_arm, parse_investigated_arm, parse_drew_arm)).parse(input)?; let (input, _) = tag(" this way").parse(input)?; - Ok((input, (relation, action))) + Ok((input, action)) } /// "searches/searched a/their library" → `SearchedLibrary` (Tempting Offer cycle). @@ -2687,6 +2708,21 @@ fn parse_investigated_arm(input: &str) -> nom::IResult<&str, PlayerActionKind, O .parse(input) } +/// "draws/drew/draw a card" → `Draw`. The tense axis is one `alt`; the +/// object noun "a card" is fixed. The `" this way"` terminator is consumed by +/// `parse_who_action_this_way`, so "drew a card this turn" cannot reach this arm. +/// +/// Reached from both this-way callers via that shared tail: the QUANTITY path +/// (Cut a Deal: "for each opponent who drew a card this way") through +/// `parse_action_this_way`, and the player-SCOPE SUBJECT path (Kwain, Itinerant +/// Meddler: "each player who drew a card this way gains 1 life") through +/// `strip_performed_action_this_way_clause` in oracle_effect/lower.rs. +fn parse_drew_arm(input: &str) -> nom::IResult<&str, PlayerActionKind, OracleError<'_>> { + let (input, _) = alt((tag("draws"), tag("drew"), tag("draw"))).parse(input)?; + let (input, _) = tag(" a card").parse(input)?; + Ok((input, PlayerActionKind::Draw)) +} + /// Normalize the two existing " this way" tails to a /// common `(filter, cause)` pair. This CALLS them; it does not restate either /// verb table, so both keep their single authority over their own verbs. @@ -6628,6 +6664,92 @@ mod tests { ); } + /// CR 121.1 + CR 608.2c + CR 109.5: Cut a Deal — "you draw a card for each + /// opponent who drew a card this way" must count the PLAYERS who drew, via + /// `PerformedActionThisWay { Opponent, Draw }`, NOT the object-count + /// `TrackedSetSize` fallback that the misparse produced. Revert-failing + /// anchor: without the `parse_drew_arm` in `parse_action_this_way`, this + /// clause falls through to `TrackedSetSize`. + #[test] + fn for_each_opponent_who_drew_a_card_this_way_is_player_count() { + let qty = parse_for_each_clause("opponent who drew a card this way").unwrap(); + assert_eq!( + qty, + QuantityRef::PlayerCount { + filter: PlayerFilter::PerformedActionThisWay { + relation: PlayerRelation::Opponent, + action: PlayerActionKind::Draw, + }, + } + ); + } + + /// CR 121.1 + CR 608.2c: The present-tense / all-players sibling ("each + /// player who draws a card this way", Kwain class) shares the same combinator + /// and only differs on the relation axis. + #[test] + fn for_each_player_who_draws_a_card_this_way_is_all_player_count() { + let qty = parse_for_each_clause("player who draws a card this way").unwrap(); + assert_eq!( + qty, + QuantityRef::PlayerCount { + filter: PlayerFilter::PerformedActionThisWay { + relation: PlayerRelation::All, + action: PlayerActionKind::Draw, + }, + } + ); + } + + /// CR 121.1: Reach-guard — `parse_action_this_way` (the shared authority for + /// both quantity dispatch sites) recognizes the population-scoped "drew a + /// card this way" form and binds the correct relation. + #[test] + fn parse_action_this_way_binds_drew_arm() { + assert_eq!( + parse_action_this_way("opponent who drew a card this way"), + Ok(("", (PlayerRelation::Opponent, PlayerActionKind::Draw))) + ); + } + + #[test] + fn parse_action_this_way_binds_plural_draw_arm() { + assert_eq!( + parse_action_this_way("players who draw a card this way"), + Ok(("", (PlayerRelation::All, PlayerActionKind::Draw))) + ); + } + + /// CR 121.1 + CR 608.2c: Negative — "drew a card this turn" is a per-turn + /// attribute, NOT the CR 608.2c "this way" anaphor. The `" this way"` + /// terminator in `parse_action_this_way` rejects the "this turn" tail, so the + /// Draw arm is unreachable and the clause never becomes a + /// `PerformedActionThisWay` draw count. Paired with the positive above, this + /// proves the arm is gated on the "this way" anaphor, not on the verb alone. + #[test] + fn opponent_who_drew_a_card_this_turn_is_not_performed_action_draw() { + assert!(parse_action_this_way("opponent who drew a card this turn").is_err()); + let this_way = QuantityRef::PlayerCount { + filter: PlayerFilter::PerformedActionThisWay { + relation: PlayerRelation::Opponent, + action: PlayerActionKind::Draw, + }, + }; + assert_ne!( + parse_for_each_clause("opponent who drew a card this turn"), + Some(this_way) + ); + } + + /// CR 121.1: Negative — a bare "cards drawn this way" object phrase has no + /// "[population] who" prefix, so `parse_action_this_way` cannot match and the + /// arm stays unreachable; such clauses keep falling through to the + /// object-count `TrackedSetSize` path unchanged. + #[test] + fn cards_drawn_this_way_has_no_population_who_prefix() { + assert!(parse_action_this_way("cards drawn this way").is_err()); + } + /// CR 109.1 + CR 122.1: "[type] you control with a [counter] counter on it" /// lowers to `ObjectCount` over a filter that includes `FilterProp::Counters`, /// not `CountersOnSelf` over a bogus counter-type string. Inspiring Call class. diff --git a/crates/engine/src/types/events.rs b/crates/engine/src/types/events.rs index dd2b3d4201..3fb50c6b35 100644 --- a/crates/engine/src/types/events.rs +++ b/crates/engine/src/types/events.rs @@ -145,6 +145,16 @@ pub enum PlayerActionKind { Proliferate, /// CR 701.16a: A player investigated (created a Clue token). Investigate, + /// A player completed a draw instruction that delivered at least + /// one card. Emitted once per settled draw INSTRUCTION (at draw-sequence + /// completion), not once per card — so a multi-card draw records a single + /// event. Recorded so "for each opponent who drew a card this way" (Cut a + /// Deal) resolves via `PlayerFilter::PerformedActionThisWay` — a count over + /// players, not objects — and so `PlayerActionsThisTurn { Draw }` would count + /// draw events rather than cards. `player_actions_this_way` (a set) counts the + /// drawing player once; a draw that delivered no card (empty library, or every + /// unit replaced away) emits nothing because that player did not draw. + Draw, } /// CR 701.30d: Result of a clash — whether the controller won, lost, or tied. diff --git a/crates/engine/tests/integration/cut_a_deal_draw_this_way_count.rs b/crates/engine/tests/integration/cut_a_deal_draw_this_way_count.rs new file mode 100644 index 0000000000..39e363f618 --- /dev/null +++ b/crates/engine/tests/integration/cut_a_deal_draw_this_way_count.rs @@ -0,0 +1,344 @@ +//! Integration tests for Cut a Deal's second-draw count (CR 121.1 + CR 608.2c + +//! CR 109.5). +//! +//! Oracle text (verbatim): "Each opponent draws a card, then you draw a card for +//! each opponent who drew a card this way." +//! +//! The misparse this fixes: the second draw's "for each opponent who drew a card +//! this way" count parsed to `QuantityRef::TrackedSetSize`, but the preceding +//! opponent-scoped Draw publishes no tracked object set, so it resolved to 0 (or +//! a stale set) instead of counting the opponents who drew. The corrected parse +//! is `PlayerCount { PerformedActionThisWay { Opponent, Draw } }`, resolved from +//! the `player_actions_this_way` ledger that each settled draw now populates. +//! +//! This mirrors `tempt_with_discovery.rs` / `wernog_riders_chaplain_investigate_count.rs` +//! — the identical "each opponent does X, then you do X once per opponent who did +//! it this way" machinery — but for Draw instead of Search/Investigate, and with +//! a MANDATORY first clause (no `may`), so no `OptionalEffectChoice` prompts: the +//! opponents draw automatically and the whole chain resolves in one call. + +use engine::game::ability_utils::build_resolved_from_def; +use engine::game::effects::resolve_ability_chain; +use engine::game::zones::create_object; +use engine::parser::oracle::parse_oracle_text; +use engine::types::ability::{ + Effect, PlayerFilter, PlayerRelation, QuantityExpr, QuantityRef, ResolvedAbility, TargetFilter, +}; +use engine::types::events::PlayerActionKind; +use engine::types::format::FormatConfig; +use engine::types::game_state::GameState; +use engine::types::identifiers::{CardId, ObjectId}; +use engine::types::player::PlayerId; +use engine::types::zones::Zone; + +const CUT_A_DEAL_ORACLE: &str = + "Each opponent draws a card, then you draw a card for each opponent who drew a card this way."; + +/// Parse Cut a Deal and build its resolved Spell ability (controller = P0). +fn make_game_and_ability(num_players: u8) -> (GameState, ResolvedAbility) { + let parsed = parse_oracle_text( + CUT_A_DEAL_ORACLE, + "Cut a Deal", + &[], + &["Sorcery".to_string()], + &[], + ); + let ability = build_resolved_from_def(&parsed.abilities[0], ObjectId(9000), PlayerId(0)); + let state = GameState::new(FormatConfig::standard(), num_players, 42); + (state, ability) +} + +fn seed_library(state: &mut GameState, owner: PlayerId, count: u64, base_id: u64) { + for i in 0..count { + create_object( + state, + CardId(base_id + i), + owner, + format!("Card {owner:?}-{i}"), + Zone::Library, + ); + } +} + +fn hand_size(state: &GameState, player: PlayerId) -> usize { + state + .players + .iter() + .find(|p| p.id == player) + .expect("player exists") + .hand + .len() +} + +/// CR 121.1 + CR 608.2c + CR 109.5: The parsed AST must fix ONLY the second +/// draw's count. The first clause ("each opponent draws a card") stays a +/// `Draw { Fixed(1), Controller }` fanned out over `player_scope: Opponent`; the +/// second draw's count becomes `PlayerCount { PerformedActionThisWay { Opponent, +/// Draw } }` instead of the object-count `TrackedSetSize` misparse. +/// +/// Revert-failing: reverting the `parse_drew_arm` addition makes the second +/// count fall back to `TrackedSetSize`, flipping the final `assert_eq!`. +#[test] +fn cut_a_deal_parses_second_draw_as_player_count_over_droppers() { + let parsed = parse_oracle_text( + CUT_A_DEAL_ORACLE, + "Cut a Deal", + &[], + &["Sorcery".to_string()], + &[], + ); + assert!( + !parsed.abilities.is_empty(), + "Cut a Deal must produce a Spell ability, got {:?}", + parsed.abilities + ); + let def = &parsed.abilities[0]; + + // First clause is unchanged: each opponent draws one card. + assert!( + matches!( + &*def.effect, + Effect::Draw { + count: QuantityExpr::Fixed { value: 1 }, + target: TargetFilter::Controller, + } + ), + "outer clause must stay 'each opponent draws a card', got {:?}", + def.effect + ); + assert_eq!( + def.player_scope, + Some(PlayerFilter::Opponent), + "the first clause fans out over opponents" + ); + + // Second clause: you draw one card per opponent who drew this way. + let sub = def + .sub_ability + .as_ref() + .expect("Cut a Deal has a second-draw sub_ability"); + let Effect::Draw { count, target } = &*sub.effect else { + panic!("second clause must be a Draw, got {:?}", sub.effect); + }; + assert_eq!(*target, TargetFilter::Controller, "you draw"); + assert_eq!( + *count, + QuantityExpr::Ref { + qty: QuantityRef::PlayerCount { + filter: PlayerFilter::PerformedActionThisWay { + relation: PlayerRelation::Opponent, + action: PlayerActionKind::Draw, + }, + }, + }, + "the second draw must count opponents who drew a card this way \ + (the misparse produced TrackedSetSize)" + ); +} + +/// CR 121.1 + CR 608.2c + CR 109.5: Happy path — 3 players (P0 controller, P1 + +/// P2 opponents). Both opponents draw one card each (mandatory first clause), +/// each recording itself in `player_actions_this_way`; then the controller's +/// detached draw resolves `PlayerCount { PerformedActionThisWay { Opponent, +/// Draw } }` = 2 and draws exactly two cards. +/// +/// Revert-failing on BOTH halves of the fix: reverting the parser change leaves +/// the count as `TrackedSetSize` (0/stale → P0 draws 0); reverting the draw +/// emission leaves the ledger empty (`PlayerCount` = 0 → P0 draws 0). Either way +/// the `hand_size(P0) == 2` assertion fails. +#[test] +fn cut_a_deal_controller_draws_one_per_opponent_who_drew() { + let (mut state, ability) = make_game_and_ability(3); + seed_library(&mut state, PlayerId(0), 5, 100); + seed_library(&mut state, PlayerId(1), 2, 200); + seed_library(&mut state, PlayerId(2), 2, 300); + + let mut events = Vec::new(); + resolve_ability_chain(&mut state, &ability, &mut events, 0).unwrap(); + + // Both opponents drew this way and are recorded against themselves (the + // scoped opponent, not the controller). + assert!( + state + .player_actions_this_way + .contains(&(PlayerId(1), PlayerActionKind::Draw)), + "P1 drew a card this way and must be recorded, got {:?}", + state.player_actions_this_way + ); + assert!( + state + .player_actions_this_way + .contains(&(PlayerId(2), PlayerActionKind::Draw)), + "P2 drew a card this way and must be recorded, got {:?}", + state.player_actions_this_way + ); + + assert_eq!( + hand_size(&state, PlayerId(1)), + 1, + "each opponent draws exactly one card" + ); + assert_eq!(hand_size(&state, PlayerId(2)), 1); + assert_eq!( + hand_size(&state, PlayerId(0)), + 2, + "controller draws one card per opponent who drew this way (2); a wrong \ + count means the second draw resolved TrackedSetSize (0/stale) or the \ + draw emission never populated the ledger" + ); +} + +/// CR 121.1 + CR 109.5: Boundary — 2 players (one opponent). The single opponent +/// draws one card; the controller's detached draw counts exactly one opponent +/// who drew this way and draws one card. The controller's OWN detached draw also +/// enters the ledger, but the `Opponent` relation excludes it from its own count, +/// so P0 draws 1 and not 2. +#[test] +fn cut_a_deal_two_players_controller_draws_one() { + let (mut state, ability) = make_game_and_ability(2); + seed_library(&mut state, PlayerId(0), 5, 100); + seed_library(&mut state, PlayerId(1), 2, 200); + + let mut events = Vec::new(); + resolve_ability_chain(&mut state, &ability, &mut events, 0).unwrap(); + + assert!( + state + .player_actions_this_way + .contains(&(PlayerId(1), PlayerActionKind::Draw)), + "the single opponent drew this way and must be recorded" + ); + assert_eq!(hand_size(&state, PlayerId(1)), 1); + assert_eq!( + hand_size(&state, PlayerId(0)), + 1, + "controller counts only the one opponent who drew — its own detached \ + draw is excluded by the Opponent relation, so P0 draws 1, not 2" + ); +} + +/// CR 121.1 + CR 608.2c: Ruling #1 — an opponent who doesn't draw is not counted. +/// 3 players; P2's library is empty, so P2's mandatory draw delivers no card and +/// records nothing this way (the `drawn_count > 0` emission gate). P1 draws one +/// card and is recorded; the controller draws exactly one (only P1 counted). +#[test] +fn cut_a_deal_opponent_who_cannot_draw_is_not_counted() { + let (mut state, ability) = make_game_and_ability(3); + seed_library(&mut state, PlayerId(0), 5, 100); + seed_library(&mut state, PlayerId(1), 2, 200); + // P2 has an EMPTY library — its draw delivers no card. + + let mut events = Vec::new(); + resolve_ability_chain(&mut state, &ability, &mut events, 0).unwrap(); + + assert!( + state + .player_actions_this_way + .contains(&(PlayerId(1), PlayerActionKind::Draw)), + "P1 drew a card this way and must be recorded" + ); + assert!( + !state + .player_actions_this_way + .contains(&(PlayerId(2), PlayerActionKind::Draw)), + "P2 drew no card (empty library) and must NOT be recorded, got {:?}", + state.player_actions_this_way + ); + assert_eq!( + hand_size(&state, PlayerId(2)), + 0, + "P2's empty-library draw delivers no card" + ); + assert_eq!( + hand_size(&state, PlayerId(0)), + 1, + "only the one opponent who actually drew (P1) counts toward the \ + controller's draw (CR 608.2c ruling #1)" + ); +} + +/// CR 121.1: A bare `Effect::Draw` (no `player_scope`, no count reference) still +/// draws the right number and leaves a harmless `(controller, Draw)` ledger +/// entry — the unconditional emission does not perturb ordinary draws. +#[test] +fn bare_draw_leaves_harmless_this_way_ledger_entry() { + let mut state = GameState::new(FormatConfig::standard(), 2, 42); + seed_library(&mut state, PlayerId(0), 3, 100); + + let ability = ResolvedAbility::new( + Effect::Draw { + count: QuantityExpr::Fixed { value: 1 }, + target: TargetFilter::Controller, + }, + vec![], + ObjectId(9000), + PlayerId(0), + ); + + let mut events = Vec::new(); + resolve_ability_chain(&mut state, &ability, &mut events, 0).unwrap(); + + assert_eq!( + hand_size(&state, PlayerId(0)), + 1, + "a bare draw must draw exactly one card" + ); + assert!( + state + .player_actions_this_way + .contains(&(PlayerId(0), PlayerActionKind::Draw)), + "the unconditional ledger emit records the drawing player" + ); +} + +/// CR 121.1: A single multi-card draw instruction records ONE draw event in both +/// ledgers — the `player_actions_this_turn` Vec (which preserves repeated actions +/// for count-style consumers) gets exactly one `(P0, Draw)` entry for a two-card +/// draw, not one per card. This is the production-path guard for the latent +/// over-count: a future `QuantityRef::PlayerActionsThisTurn { action: Draw }` +/// consumer must measure draw EVENTS, not cards drawn. +/// +/// Revert-failing: emitting `PlayerPerformedAction { Draw }` per card (the old +/// per-unit shape) makes the Vec hold two `(P0, Draw)` entries and fails the +/// count assertion. +#[test] +fn multi_card_draw_records_one_turn_ledger_entry() { + let mut state = GameState::new(FormatConfig::standard(), 2, 42); + seed_library(&mut state, PlayerId(0), 5, 100); + + let ability = ResolvedAbility::new( + Effect::Draw { + count: QuantityExpr::Fixed { value: 2 }, + target: TargetFilter::Controller, + }, + vec![], + ObjectId(9000), + PlayerId(0), + ); + + let mut events = Vec::new(); + resolve_ability_chain(&mut state, &ability, &mut events, 0).unwrap(); + + assert_eq!( + hand_size(&state, PlayerId(0)), + 2, + "the two-card draw instruction delivers two cards" + ); + let turn_draws = state + .player_actions_this_turn + .iter() + .filter(|(player, action)| *player == PlayerId(0) && *action == PlayerActionKind::Draw) + .count(); + assert_eq!( + turn_draws, 1, + "a two-card draw is ONE draw event: the turn-scoped Vec must hold exactly one \ + (P0, Draw), not one per card (count-style turn ledger measures draw events)" + ); + // The set counterpart also records the drawer exactly once. + assert!( + state + .player_actions_this_way + .contains(&(PlayerId(0), PlayerActionKind::Draw)), + "the set ledger records the drawing player once" + ); +} diff --git a/crates/engine/tests/integration/kwain_drew_this_way_gains_life.rs b/crates/engine/tests/integration/kwain_drew_this_way_gains_life.rs new file mode 100644 index 0000000000..c08226be57 --- /dev/null +++ b/crates/engine/tests/integration/kwain_drew_this_way_gains_life.rs @@ -0,0 +1,209 @@ +//! Integration tests for Kwain, Itinerant Meddler's "each player who drew a card +//! this way gains 1 life" (CR 121.1 + CR 608.2c + CR 109.5). +//! +//! Oracle text (verbatim, Scryfall): "{T}: Each player may draw a card, then each +//! player who drew a card this way gains 1 life." +//! +//! The misparse this fixes: the player-scope SUBJECT "each player who drew a card +//! this way" dropped its "who drew a card this way" restriction, so the GainLife +//! clause parsed with `player_scope: All` — every player gained 1 life even if +//! they declined the optional "may draw" or had an empty library. The corrected +//! parse scopes the life gain to `PerformedActionThisWay { All, Draw }`, resolved +//! from the same `player_actions_this_way` ledger the quantity sibling (Cut a +//! Deal's "for each opponent who drew a card this way") reads. +//! +//! This is the subject-scope twin of `cut_a_deal_draw_this_way_count.rs`: the +//! shared `parse_who_action_this_way` this-way verb table now feeds both the +//! quantity path and the player-scope subject path. + +use engine::game::ability_utils::build_resolved_from_def; +use engine::game::effects::resolve_ability_chain; +use engine::game::scenario::{GameScenario, P0, P1}; +use engine::parser::oracle::parse_oracle_text; +use engine::types::ability::{ + AbilityDefinition, Effect, PlayerFilter, PlayerRelation, QuantityExpr, +}; +use engine::types::events::PlayerActionKind; +use engine::types::format::FormatConfig; +use engine::types::game_state::GameState; +use engine::types::identifiers::ObjectId; +use engine::types::phase::Phase; +use engine::types::player::PlayerId; + +const KWAIN_ORACLE: &str = + "{T}: Each player may draw a card, then each player who drew a card this way gains 1 life."; + +/// Walk an ability's `sub_ability` chain and return the first clause whose effect +/// is `GainLife` (Kwain's second clause — the "then … gains 1 life" tail). +fn gain_life_clause(def: &AbilityDefinition) -> Option<&AbilityDefinition> { + let mut cur = def; + loop { + if matches!(cur.effect.as_ref(), Effect::GainLife { .. }) { + return Some(cur); + } + cur = cur.sub_ability.as_deref()?; + } +} + +fn parse_kwain() -> engine::parser::oracle::ParsedAbilities { + parse_oracle_text( + KWAIN_ORACLE, + "Kwain, Itinerant Meddler", + &[], + &["Creature".to_string()], + &["Human".to_string(), "Rogue".to_string()], + ) +} + +fn life(state: &GameState, player: PlayerId) -> i32 { + state + .players + .iter() + .find(|p| p.id == player) + .expect("player exists") + .life +} + +/// CR 121.1 + CR 608.2c + CR 109.5: The parsed AST must scope the life gain to +/// the drawers. The "each player who drew a card this way" subject lowers to a +/// `GainLife` clause carrying `player_scope: PerformedActionThisWay { All, Draw }` +/// — NOT the `player_scope: All` the dropped-restriction misparse produced. +/// +/// Revert-failing: reverting `strip_performed_action_this_way_clause` leaves the +/// "who drew a card this way" clause unconsumed, so the scope falls back to `All` +/// and this `assert_eq!` flips. +#[test] +fn kwain_scopes_life_gain_to_players_who_drew_this_way() { + let parsed = parse_kwain(); + assert!( + !parsed.abilities.is_empty(), + "Kwain must produce an activated ability, got {:?}", + parsed.abilities + ); + + let gain = parsed + .abilities + .iter() + .find_map(gain_life_clause) + .expect("Kwain must produce a GainLife clause for the 'gains 1 life' tail"); + + assert!( + matches!( + gain.effect.as_ref(), + Effect::GainLife { + amount: QuantityExpr::Fixed { value: 1 }, + .. + } + ), + "the second clause gains exactly 1 life, got {:?}", + gain.effect + ); + assert_eq!( + gain.player_scope, + Some(PlayerFilter::PerformedActionThisWay { + relation: PlayerRelation::All, + action: PlayerActionKind::Draw, + }), + "'each player who drew a card this way' must scope the life gain to the \ + drawers (the misparse dropped the restriction and produced player_scope \ + All)" + ); +} + +/// CR 121.1 + CR 608.2c + CR 109.5: Mixed runtime discriminator through the real +/// resolution pipeline. P0 and P1 drew a card this way (recorded in +/// `player_actions_this_way`); P2 did NOT (declined the optional draw / empty +/// library). Resolving Kwain's parsed GainLife clause must gain 1 life for P0 and +/// P1 only, leaving P2 untouched. +/// +/// Positive reach-guard (same test): P0 and P1 each go 20 → 21, proving the +/// scoped `GainLife` instruction is reached and applied to the drawers. Negative +/// discriminator (same test): P2 stays at 20. Reverting the parser fix restores +/// `player_scope: All`, which gains life for every player including the +/// non-drawer P2, flipping the P2 assertion. +/// +/// Resolved at depth=1 (like `tempt_with_discovery.rs`) so the pre-populated +/// this-way ledger survives — a depth=0 top-level chain entry clears it. +#[test] +fn kwain_gain_life_reaches_only_the_players_who_drew() { + let parsed = parse_kwain(); + let gain = parsed + .abilities + .iter() + .find_map(gain_life_clause) + .expect("Kwain must produce a GainLife clause"); + + let mut state = GameState::new(FormatConfig::standard(), 3, 42); + // P0 (controller) and P1 drew a card this way; P2 did not. + state + .player_actions_this_way + .insert((PlayerId(0), PlayerActionKind::Draw)); + state + .player_actions_this_way + .insert((PlayerId(1), PlayerActionKind::Draw)); + + let ability = build_resolved_from_def(gain, ObjectId(9000), PlayerId(0)); + let mut events = Vec::new(); + resolve_ability_chain(&mut state, &ability, &mut events, 1).unwrap(); + + assert_eq!( + life(&state, PlayerId(0)), + 21, + "the controller drew this way and must gain 1 life" + ); + assert_eq!( + life(&state, PlayerId(1)), + 21, + "P1 drew this way and must gain 1 life" + ); + assert_eq!( + life(&state, PlayerId(2)), + 20, + "P2 did NOT draw this way and must NOT gain life — a change to 21 means \ + the restriction was dropped and the life gain over-applied to every \ + player (player_scope All)" + ); +} + +/// CR 121.1 + CR 608.2c + CR 109.5: End-to-end reach-guard through the production +/// activation pipeline. Three players, all with libraries, all accept Kwain's +/// optional "may draw"; each draws, records itself in the this-way ledger, and +/// then gains 1 life. Proves the parsed scope resolves correctly across the real +/// activate → per-player optional draw → "then" GainLife path (not just the +/// isolated clause). +#[test] +fn kwain_all_drawers_gain_life_through_activation() { + let mut scenario = GameScenario::new_n_player(3, 42); + scenario.at_phase(Phase::PreCombatMain); + + for _ in 0..2 { + scenario.add_card_to_library_top(P0, "Island"); + scenario.add_card_to_library_top(P1, "Plains"); + scenario.add_card_to_library_top(PlayerId(2), "Forest"); + } + + let kwain = scenario + .add_creature(P0, "Kwain, Itinerant Meddler", 1, 3) + .from_oracle_text(KWAIN_ORACLE) + .id(); + + let mut runner = scenario.build(); + runner.activate(kwain, 0).accept_optional().resolve(); + + let state = runner.state(); + for pid in [P0, P1, PlayerId(2)] { + let player = state + .players + .iter() + .find(|p| p.id == pid) + .expect("player exists"); + assert_eq!( + player.life, 21, + "{pid:?} accepted the optional draw and must gain 1 life this way" + ); + assert!( + !player.hand.is_empty(), + "{pid:?} must have drawn a card via Kwain's optional draw" + ); + } +} diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index c84929bede..1182e4dc08 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -148,6 +148,7 @@ mod curse_of_the_restless_dead_land_enters_trigger; mod curse_spell_cast_triggers; mod curse_static_effects; mod curse_upkeep_triggers; +mod cut_a_deal_draw_this_way_count; mod cybership_combat_damage_manifest; mod dalkovan_encampment_attack_trigger; mod daretti_emblem_simultaneous_death; @@ -725,6 +726,7 @@ mod krark_clan_ironworks_castability; mod krark_thumb_coin_flip; mod kroxa_titan_nonland_discard_life_loss; mod kutzils_flanker_mode_one_counter; +mod kwain_drew_this_way_gains_life; mod l02_bb1_activation_conditions; mod l02_bb2_cast_origin; mod l02_bb3_cast_permission_finality;