diff --git a/crates/engine/src/parser/oracle_effect/assembly.rs b/crates/engine/src/parser/oracle_effect/assembly.rs index 3f9a8d473c..506df49f26 100644 --- a/crates/engine/src/parser/oracle_effect/assembly.rs +++ b/crates/engine/src/parser/oracle_effect/assembly.rs @@ -1229,7 +1229,11 @@ pub(crate) fn assemble_effect_chain(ir: &EffectChainIr) -> AbilityDefinition { if let Some(continuation) = clause_ir.disposition.followup() { apply_clause_continuation(&mut defs, continuation.clone(), kind, &env); env.observe(&defs, None, NodeRole::ContinuationProduct); - apply_where_x_to_latest_def(&mut defs, clause_ir.where_x_expression.as_deref()); + apply_where_x_to_latest_def( + &mut defs, + clause_ir.where_x_expression.as_deref(), + clause_ir.where_x_scope.as_ref(), + ); } true } else if let ClauseDisposition::Absorb { rider, kind } = &clause_ir.disposition { @@ -1567,6 +1571,7 @@ pub(crate) fn assemble_effect_chain(ir: &EffectChainIr) -> AbilityDefinition { apply_where_x_ability_expression( &mut new_def, clause_ir.where_x_expression.as_deref(), + clause_ir.where_x_scope.as_ref(), ); new_def.else_ability = Some(Box::new(last_def)); defs.push(new_def); @@ -1835,7 +1840,11 @@ pub(crate) fn assemble_effect_chain(ir: &EffectChainIr) -> AbilityDefinition { if let Some(continuation) = clause_ir.disposition.followup() { apply_clause_continuation(&mut defs, continuation.clone(), kind, &env); env.observe(&defs, None, NodeRole::ContinuationProduct); - apply_where_x_to_latest_def(&mut defs, clause_ir.where_x_expression.as_deref()); + apply_where_x_to_latest_def( + &mut defs, + clause_ir.where_x_expression.as_deref(), + clause_ir.where_x_scope.as_ref(), + ); } // ── Build AbilityDefinition from ClauseIr ── @@ -2187,7 +2196,11 @@ pub(crate) fn assemble_effect_chain(ir: &EffectChainIr) -> AbilityDefinition { current_defs.push(*sub.clone()); } for current in &mut current_defs { - apply_where_x_ability_expression(current, clause_ir.where_x_expression.as_deref()); + apply_where_x_ability_expression( + current, + clause_ir.where_x_expression.as_deref(), + clause_ir.where_x_scope.as_ref(), + ); } // CR 615.5 + CR 609.7: In a "damage is prevented this way" rider, the @@ -2413,7 +2426,11 @@ pub(crate) fn assemble_effect_chain(ir: &EffectChainIr) -> AbilityDefinition { if let Some(continuation) = clause_ir.disposition.intrinsic() { apply_clause_continuation(&mut defs, continuation.clone(), kind, &env); env.observe(&defs, None, NodeRole::ContinuationProduct); - apply_where_x_to_latest_def(&mut defs, clause_ir.where_x_expression.as_deref()); + apply_where_x_to_latest_def( + &mut defs, + clause_ir.where_x_expression.as_deref(), + clause_ir.where_x_scope.as_ref(), + ); } // CR 608.2c: Advance the separating boundary for the next normal-path diff --git a/crates/engine/src/parser/oracle_effect/imperative.rs b/crates/engine/src/parser/oracle_effect/imperative.rs index b35b1b84ca..b34c1dada0 100644 --- a/crates/engine/src/parser/oracle_effect/imperative.rs +++ b/crates/engine/src/parser/oracle_effect/imperative.rs @@ -13,7 +13,8 @@ use super::counter::{ }; use super::lower::{ parse_for_each_multiplier_prefix, parse_multi_target_count_expr, - parse_where_x_quantity_expression, strip_leading_quantifier, strip_trailing_where_x, + parse_where_x_quantity_expression, parse_where_x_quantity_expression_with_context, + strip_leading_quantifier, strip_trailing_where_x, }; use super::mana::{try_parse_activate_only_condition, try_parse_add_mana_effect_with_context}; use super::token::try_parse_token; @@ -8999,7 +9000,11 @@ pub(super) fn parse_counter_ast(text: &str, lower: &str) -> Option &str { /// resolved as a +0/+0 no-op — while the raw text still rendered as a supported /// dynamic quantity in the coverage report. The node was well-typed and /// completely dead. Honest failure is the only correct answer here. -fn apply_where_x_expression(value: PtValue, where_x_expression: Option<&str>) -> Option { +fn apply_where_x_expression( + value: PtValue, + where_x_expression: Option<&str>, + ctx: &ParseContext, +) -> Option { match (value, where_x_expression) { (PtValue::Variable(alias), Some(expression)) if alias.eq_ignore_ascii_case("X") => { - parse_where_x_quantity_expression(expression).map(PtValue::Quantity) + parse_where_x_quantity_expression_with_context(expression, ctx).map(PtValue::Quantity) } (PtValue::Variable(alias), Some(expression)) if alias.eq_ignore_ascii_case("-X") => { - parse_where_x_quantity_expression(expression).map(|inner| { + parse_where_x_quantity_expression_with_context(expression, ctx).map(|inner| { PtValue::Quantity(QuantityExpr::Multiply { factor: -1, inner: Box::new(inner), @@ -8278,7 +8282,8 @@ fn apply_where_x_expression(value: PtValue, where_x_expression: Option<&str>) -> // depth it sits; `apply_where_x_quantity_expression` is a no-op on a slot that // holds no X, so a concrete P/T is left untouched. (PtValue::Quantity(quantity), Some(_)) => { - apply_where_x_quantity_expression(quantity, where_x_expression).map(PtValue::Quantity) + apply_where_x_quantity_expression(quantity, where_x_expression, ctx) + .map(PtValue::Quantity) } (value, _) => Some(value), } @@ -8411,7 +8416,27 @@ fn parse_amount_of_mana_paid_this_way(input: &str) -> OracleResult<'_, ()> { Ok((input, ())) } +/// Context-free entry point: bind a "where X is …" count with no player-anaphor +/// scope in effect. Genuinely context-free probes (shape/`.is_some()` checks) and +/// callers with no `ParseContext` in scope route here; the `None`-scope CDA arm +/// takes the exact pre-#6508 `parse_cda_quantity` path, so their behavior is +/// byte-identical to before the anaphor-context work. pub(crate) fn parse_where_x_quantity_expression(where_x_expression: &str) -> Option { + parse_where_x_quantity_expression_with_context(where_x_expression, &ParseContext::default()) +} + +/// CR 109.5 + CR 608.2c + CR 603.2b: Context-aware "where X is …" count binding. +/// A third-person player anaphor inside the definition ("they control", "that +/// player controls") binds to `ctx.relative_player_scope`: `ScopedPlayer` for an +/// each-player phase trigger (Citadel of Pain) and `TargetPlayer` for a spell +/// whose clause targets a player (Jovial Evil, Pact of the Serpent, Inscription of +/// Abundance). When no scope is stamped (`None`), the CDA arm falls to the exact +/// pre-#6508 `parse_cda_quantity` path so caster-relative reads and probe callers +/// are unchanged. +pub(crate) fn parse_where_x_quantity_expression_with_context( + where_x_expression: &str, + ctx: &ParseContext, +) -> Option { let expression = where_x_expression.trim().trim_end_matches('.'); let expression_lower = expression.to_ascii_lowercase(); // CR 702.51c + CR 603.3: Knight-Errant of Eos reads the number of @@ -8486,7 +8511,9 @@ pub(crate) fn parse_where_x_quantity_expression(where_x_expression: &str) -> Opt .parse(expression_lower.as_str()) { let consumed = expression_lower.len() - rest_lower.len(); - if let Some(inner) = parse_where_x_quantity_expression(&expression[consumed..]) { + if let Some(inner) = + parse_where_x_quantity_expression_with_context(&expression[consumed..], ctx) + { let inner = if sign < 0 { QuantityExpr::Multiply { factor: -1, @@ -8562,7 +8589,34 @@ pub(crate) fn parse_where_x_quantity_expression(where_x_expression: &str) -> Opt // CDA-quantity classification takes precedence: it is the more specific // where-X interpreter (object counts, "that spell's mana value", // "the number of age counters on this enchantment", etc.). - if let Some(expr) = parse_cda_quantity(where_x_expression) { + // + // CR 109.5 + CR 608.2c + CR 603.2b/CR 102.1: third-person player anaphors + // inside a where-X definition ("they control", "that player controls") bind + // to the player scope threaded from the caller's context, exactly as the + // sibling for-each interpreter does (parse_for_each_clause_with_context). + // When the caller's `relative_player_scope` is stamped (`ScopedPlayer` for an + // each-player phase trigger — Citadel of Pain — via the trigger's scope; a + // per-opponent fanout iterand's `TargetPlayer`; etc.), install it as the + // anaphor context so "that player"/"they" reads the correct player. With no + // scope stamped (a plain targeted spell), take the exact pre-#6508 + // `parse_cda_quantity` path — the anaphor keeps its legacy caster-relative + // (`You`) binding, byte-identical to before the anaphor work, and never a + // hardcoded `ScopedPlayer`. Binding a spell's cross-clause / shared-sibling + // "that player" (Curious Herd, Pact of the Serpent) to its chosen target needs + // target carry-forward and is a separate change. "you control" is + // ctx-independent and unaffected in either arm. + let cda = match ctx.relative_player_scope { + Some(ref they) => { + let mut anaphor_ctx = + crate::parser::oracle_quantity::for_each_anaphor_context(ctx, they); + crate::parser::oracle_quantity::parse_cda_quantity_with_context( + where_x_expression, + &mut anaphor_ctx, + ) + } + None => crate::parser::oracle_quantity::parse_cda_quantity(where_x_expression), + }; + if let Some(expr) = cda { return Some(expr); } // CR 107.3i: Keep the compositional nom quantity grammar available to @@ -8795,6 +8849,7 @@ fn parse_where_x_scry_look_count(where_x_expression: &str) -> Option, + ctx: &ParseContext, ) -> Option { Some(match value { // CR 107.3i: Generic "X is N or more" condition parsing defaults to @@ -8805,26 +8860,29 @@ pub(super) fn apply_where_x_quantity_expression( qty: QuantityRef::CostXPaid, } if where_x_expression.is_some() => { let expression = where_x_expression.expect("checked is_some above"); - parse_where_x_quantity_expression(expression)? + parse_where_x_quantity_expression_with_context(expression, ctx)? } QuantityExpr::Ref { qty: QuantityRef::Variable { name }, } if where_x_expression.is_some() && name.eq_ignore_ascii_case("X") => { let expression = where_x_expression.expect("checked is_some above"); - parse_where_x_quantity_expression(expression)? + parse_where_x_quantity_expression_with_context(expression, ctx)? } // CR 107.3i: "search ... for up to X ..., where X is …" wraps the X // count in `UpTo`. Recurse into `max` so the defining clause rewrites // the inner `Variable("X")` (Oreskos Explorer's "up to X Plains cards" // must bind X to the where-clause population, not stay at 0). `up_to` // re-asserts the non-nesting invariant. - QuantityExpr::UpTo { max } => { - QuantityExpr::up_to(apply_where_x_quantity_expression(*max, where_x_expression)?) - } + QuantityExpr::UpTo { max } => QuantityExpr::up_to(apply_where_x_quantity_expression( + *max, + where_x_expression, + ctx, + )?), QuantityExpr::Offset { inner, offset } => QuantityExpr::Offset { inner: Box::new(apply_where_x_quantity_expression( *inner, where_x_expression, + ctx, )?), offset, }, @@ -8832,6 +8890,7 @@ pub(super) fn apply_where_x_quantity_expression( inner: Box::new(apply_where_x_quantity_expression( *inner, where_x_expression, + ctx, )?), minimum, }, @@ -8840,6 +8899,7 @@ pub(super) fn apply_where_x_quantity_expression( inner: Box::new(apply_where_x_quantity_expression( *inner, where_x_expression, + ctx, )?), }, QuantityExpr::DivideRounded { @@ -8850,6 +8910,7 @@ pub(super) fn apply_where_x_quantity_expression( inner: Box::new(apply_where_x_quantity_expression( *inner, where_x_expression, + ctx, )?), divisor, rounding, @@ -8857,23 +8918,25 @@ pub(super) fn apply_where_x_quantity_expression( QuantityExpr::Sum { exprs } => QuantityExpr::Sum { exprs: exprs .into_iter() - .map(|expr| apply_where_x_quantity_expression(expr, where_x_expression)) + .map(|expr| apply_where_x_quantity_expression(expr, where_x_expression, ctx)) .collect::>>()?, }, QuantityExpr::Max { exprs } => QuantityExpr::Max { exprs: exprs .into_iter() - .map(|expr| apply_where_x_quantity_expression(expr, where_x_expression)) + .map(|expr| apply_where_x_quantity_expression(expr, where_x_expression, ctx)) .collect::>>()?, }, QuantityExpr::Difference { left, right } => QuantityExpr::Difference { left: Box::new(apply_where_x_quantity_expression( *left, where_x_expression, + ctx, )?), right: Box::new(apply_where_x_quantity_expression( *right, where_x_expression, + ctx, )?), }, QuantityExpr::Power { base, exponent } => QuantityExpr::Power { @@ -8881,6 +8944,7 @@ pub(super) fn apply_where_x_quantity_expression( exponent: Box::new(apply_where_x_quantity_expression( *exponent, where_x_expression, + ctx, )?), }, other => other, @@ -8899,8 +8963,9 @@ fn bind_where_x_quantity( slot: &mut QuantityExpr, where_x_expression: Option<&str>, unbound: &mut Option, + ctx: &ParseContext, ) { - match apply_where_x_quantity_expression(slot.clone(), where_x_expression) { + match apply_where_x_quantity_expression(slot.clone(), where_x_expression, ctx) { Some(bound) => *slot = bound, None => *unbound = where_x_expression.map(str::to_string), } @@ -8912,9 +8977,10 @@ fn bind_where_x_optional_quantity( slot: Option<&mut QuantityExpr>, where_x_expression: Option<&str>, unbound: &mut Option, + ctx: &ParseContext, ) { if let Some(slot) = slot { - bind_where_x_quantity(slot, where_x_expression, unbound); + bind_where_x_quantity(slot, where_x_expression, unbound, ctx); } } @@ -8928,9 +8994,10 @@ fn bind_where_x_enter_with_counters( entries: &mut [(CounterType, QuantityExpr)], where_x_expression: Option<&str>, unbound: &mut Option, + ctx: &ParseContext, ) { for (_, count) in entries.iter_mut() { - bind_where_x_quantity(count, where_x_expression, unbound); + bind_where_x_quantity(count, where_x_expression, unbound, ctx); } } @@ -8941,11 +9008,12 @@ fn bind_where_x_optional_pt( slot: &mut Option, where_x_expression: Option<&str>, unbound: &mut Option, + ctx: &ParseContext, ) { let Some(value) = slot.as_ref() else { return; }; - match apply_where_x_expression(value.clone(), where_x_expression) { + match apply_where_x_expression(value.clone(), where_x_expression, ctx) { Some(bound) => *slot = Some(bound), None => *unbound = where_x_expression.map(str::to_string), } @@ -8954,6 +9022,7 @@ fn bind_where_x_optional_pt( pub(super) fn apply_where_x_effect_expression( effect: &mut Effect, where_x_expression: Option<&str>, + ctx: &ParseContext, ) { // CR 107.3c: set when the clause DEFINES X but the definition is not // representable. Recorded here and converted to a gap node after the match @@ -9017,7 +9086,7 @@ pub(super) fn apply_where_x_effect_expression( | Effect::SkipNextStep { count: amount, .. } | Effect::SkipNextTurn { count: amount, .. } | Effect::Surveil { count: amount, .. } => { - bind_where_x_quantity(amount, where_x_expression, &mut unbound_where_x); + bind_where_x_quantity(amount, where_x_expression, &mut unbound_where_x, ctx); } // Multi-slot carriers: a where-X clause defines ONE X, and every slot that // references it must bind to the same expression (CR 107.3i: X has a single value @@ -9028,25 +9097,26 @@ pub(super) fn apply_where_x_effect_expression( life_payment, .. } => { - bind_where_x_quantity(count, where_x_expression, &mut unbound_where_x); - bind_where_x_quantity(life_payment, where_x_expression, &mut unbound_where_x); + bind_where_x_quantity(count, where_x_expression, &mut unbound_where_x, ctx); + bind_where_x_quantity(life_payment, where_x_expression, &mut unbound_where_x, ctx); } Effect::CreateTokenCopyFromPool { mv_bound, count, .. } => { - bind_where_x_quantity(mv_bound, where_x_expression, &mut unbound_where_x); - bind_where_x_quantity(count, where_x_expression, &mut unbound_where_x); + bind_where_x_quantity(mv_bound, where_x_expression, &mut unbound_where_x, ctx); + bind_where_x_quantity(count, where_x_expression, &mut unbound_where_x, ctx); } Effect::PutSticker { count, max_ticket_cost, .. } => { - bind_where_x_quantity(count, where_x_expression, &mut unbound_where_x); + bind_where_x_quantity(count, where_x_expression, &mut unbound_where_x, ctx); bind_where_x_optional_quantity( max_ticket_cost.as_mut(), where_x_expression, &mut unbound_where_x, + ctx, ); } // The optional-count carriers: same single where-X slot, but absent by default @@ -9059,6 +9129,7 @@ pub(super) fn apply_where_x_effect_expression( count.as_mut(), where_x_expression, &mut unbound_where_x, + ctx, ); } Effect::ChooseAndSacrificeRest { @@ -9068,6 +9139,7 @@ pub(super) fn apply_where_x_effect_expression( total_power_cap.as_mut(), where_x_expression, &mut unbound_where_x, + ctx, ); } // CR 122.1: the mass-move counterpart of `ChangeZone`'s enters-with rider. @@ -9076,11 +9148,12 @@ pub(super) fn apply_where_x_effect_expression( enter_with_counters, .. } => { - bind_where_x_filter(target, where_x_expression, &mut unbound_where_x); + bind_where_x_filter(target, where_x_expression, &mut unbound_where_x, ctx); bind_where_x_enter_with_counters( enter_with_counters, where_x_expression, &mut unbound_where_x, + ctx, ); } Effect::Token { @@ -9090,7 +9163,7 @@ pub(super) fn apply_where_x_effect_expression( enter_with_counters, .. } => { - bind_where_x_quantity(count, where_x_expression, &mut unbound_where_x); + bind_where_x_quantity(count, where_x_expression, &mut unbound_where_x, ctx); // CR 122.1: the enters-with rider is a THIRD X site on a token, beside the // token count and its P/T. G'raha Tia, Scion Reborn creates a fixed 1/1 Hero // and puts X +1/+1 counters on it, so `count`/`power`/`toughness` are all @@ -9100,10 +9173,11 @@ pub(super) fn apply_where_x_effect_expression( enter_with_counters, where_x_expression, &mut unbound_where_x, + ctx, ); match ( - apply_where_x_expression(power.clone(), where_x_expression), - apply_where_x_expression(toughness.clone(), where_x_expression), + apply_where_x_expression(power.clone(), where_x_expression, ctx), + apply_where_x_expression(toughness.clone(), where_x_expression, ctx), ) { (Some(bound_power), Some(bound_toughness)) => { *power = bound_power; @@ -9118,15 +9192,15 @@ pub(super) fn apply_where_x_effect_expression( Effect::Animate { power, toughness, .. } => { - bind_where_x_optional_pt(power, where_x_expression, &mut unbound_where_x); - bind_where_x_optional_pt(toughness, where_x_expression, &mut unbound_where_x); + bind_where_x_optional_pt(power, where_x_expression, &mut unbound_where_x, ctx); + bind_where_x_optional_pt(toughness, where_x_expression, &mut unbound_where_x, ctx); } // CR 107.3i + CR 109.4 + CR 109.5: "search/seek for up to X …, where X // is …" binds the search count (Oreskos Explorer). Eldritch Evolution // binds the filter's `Cmc` bound when X appears in the card filter. Effect::SearchLibrary { filter, count, .. } | Effect::Seek { filter, count, .. } => { - bind_where_x_filter(filter, where_x_expression, &mut unbound_where_x); - bind_where_x_quantity(count, where_x_expression, &mut unbound_where_x); + bind_where_x_filter(filter, where_x_expression, &mut unbound_where_x, ctx); + bind_where_x_quantity(count, where_x_expression, &mut unbound_where_x, ctx); } // CR 107.3i + CR 400.7: "return/put up to one target creature card with // mana value X or less ..., where X is " binds the @@ -9142,29 +9216,31 @@ pub(super) fn apply_where_x_effect_expression( conditional_enter_with_counters, .. } => { - bind_where_x_filter(target, where_x_expression, &mut unbound_where_x); + bind_where_x_filter(target, where_x_expression, &mut unbound_where_x, ctx); // CR 122.1: same enters-with rider as `Token`/`ChangeZoneAll` — the moved // permanent's counter count is a where-X site of its own. bind_where_x_enter_with_counters( enter_with_counters, where_x_expression, &mut unbound_where_x, + ctx, ); for (_, _, count) in conditional_enter_with_counters.iter_mut() { - bind_where_x_quantity(count, where_x_expression, &mut unbound_where_x); + bind_where_x_quantity(count, where_x_expression, &mut unbound_where_x, ctx); } } Effect::Destroy { target, .. } | Effect::Bounce { target, .. } => { - bind_where_x_filter(target, where_x_expression, &mut unbound_where_x); + bind_where_x_filter(target, where_x_expression, &mut unbound_where_x, ctx); } // `BounceAll` carries an optional count ("return X target creatures …") beside its // filter; the filter-only arm left that count a bare placeholder. Effect::BounceAll { target, count, .. } => { - bind_where_x_filter(target, where_x_expression, &mut unbound_where_x); + bind_where_x_filter(target, where_x_expression, &mut unbound_where_x, ctx); bind_where_x_optional_quantity( count.as_mut(), where_x_expression, &mut unbound_where_x, + ctx, ); } // CR 601.2e: a cast permission may be BOUNDED by X ("you may cast a spell with mana @@ -9175,9 +9251,9 @@ pub(super) fn apply_where_x_effect_expression( Effect::CastFromZone { target, constraint, .. } => { - bind_where_x_filter(target, where_x_expression, &mut unbound_where_x); + bind_where_x_filter(target, where_x_expression, &mut unbound_where_x, ctx); if let Some(CastPermissionConstraint::ManaValue { value, .. }) = constraint.as_mut() { - bind_where_x_quantity(value, where_x_expression, &mut unbound_where_x); + bind_where_x_quantity(value, where_x_expression, &mut unbound_where_x, ctx); } } Effect::Dig { @@ -9187,19 +9263,20 @@ pub(super) fn apply_where_x_effect_expression( filter, .. } => { - bind_where_x_quantity(count, where_x_expression, &mut unbound_where_x); + bind_where_x_quantity(count, where_x_expression, &mut unbound_where_x, ctx); // "look at the top N …, keep X of them" — the KEPT count is a second, distinct // quantity slot that the original arm did not bind. bind_where_x_optional_quantity( keep_count_expr.as_mut(), where_x_expression, &mut unbound_where_x, + ctx, ); - bind_where_x_filter(player, where_x_expression, &mut unbound_where_x); - bind_where_x_filter(filter, where_x_expression, &mut unbound_where_x); + bind_where_x_filter(player, where_x_expression, &mut unbound_where_x, ctx); + bind_where_x_filter(filter, where_x_expression, &mut unbound_where_x, ctx); } Effect::Scry { count, .. } => { - bind_where_x_quantity(count, where_x_expression, &mut unbound_where_x); + bind_where_x_quantity(count, where_x_expression, &mut unbound_where_x, ctx); } Effect::Pump { power, toughness, .. @@ -9208,8 +9285,8 @@ pub(super) fn apply_where_x_effect_expression( power, toughness, .. } => { match ( - apply_where_x_expression(power.clone(), where_x_expression), - apply_where_x_expression(toughness.clone(), where_x_expression), + apply_where_x_expression(power.clone(), where_x_expression, ctx), + apply_where_x_expression(toughness.clone(), where_x_expression, ctx), ) { (Some(bound_power), Some(bound_toughness)) => { *power = bound_power; @@ -9232,7 +9309,7 @@ pub(super) fn apply_where_x_effect_expression( crate::types::ability::PreventionAmount::All | crate::types::ability::PreventionAmount::AllBut(_) ) { - *amount_dynamic = parse_where_x_quantity_expression(expr); + *amount_dynamic = parse_where_x_quantity_expression_with_context(expr, ctx); } } } @@ -9248,9 +9325,9 @@ pub(super) fn apply_where_x_effect_expression( // CR 118.1 + CR 118.5: per-object scaled mana (`scale`) tracks the // surrounding where-X binding before the cost amount itself. if let Some(times) = scale { - bind_where_x_quantity(times, where_x_expression, &mut unbound_where_x); + bind_where_x_quantity(times, where_x_expression, &mut unbound_where_x, ctx); } - apply_where_x_to_ability_cost(cost, where_x_expression, &mut unbound_where_x); + apply_where_x_to_ability_cost(cost, where_x_expression, &mut unbound_where_x, ctx); } Effect::GenericEffect { static_abilities, @@ -9278,6 +9355,7 @@ pub(super) fn apply_where_x_effect_expression( condition, where_x_expression, &mut unbound_where_x, + ctx, ); } // CR 107.3i + CR 611.2c: A continuous "gets +X/+X … where X is @@ -9297,6 +9375,7 @@ pub(super) fn apply_where_x_effect_expression( modification, where_x_expression, &mut unbound_where_x, + ctx, ); if rebind_target_anaphor { rebind_target_anaphor_continuous_modification(modification); @@ -9430,6 +9509,7 @@ fn apply_where_x_continuous_modification( modification: &mut ContinuousModification, where_x_expression: Option<&str>, unbound: &mut Option, + ctx: &ParseContext, ) { match modification { ContinuousModification::SetDynamicPower { value, .. } @@ -9438,10 +9518,10 @@ fn apply_where_x_continuous_modification( | ContinuousModification::SetToughnessDynamic { value, .. } | ContinuousModification::AddDynamicPower { value, .. } | ContinuousModification::AddDynamicToughness { value, .. } => { - bind_where_x_quantity(value, where_x_expression, unbound); + bind_where_x_quantity(value, where_x_expression, unbound, ctx); } ContinuousModification::AddDynamicKeyword { value, .. } => { - bind_where_x_quantity(value, where_x_expression, unbound); + bind_where_x_quantity(value, where_x_expression, unbound, ctx); // CR 613.4c + CR 702: a GRANTED keyword's "where X is its // power/toughness/mana value" refers to the keyword's RECIPIENT (the // creature that has the keyword), not the grant's source object. The @@ -9459,7 +9539,11 @@ fn apply_where_x_continuous_modification( | ContinuousModification::SetStartingLoyalty { .. } => {} ContinuousModification::GrantTrigger { trigger } => { if let Some(execute) = trigger.execute.as_mut() { - apply_where_x_ability_expression(execute, where_x_expression); + apply_where_x_ability_expression( + execute, + where_x_expression, + ctx.relative_player_scope.as_ref(), + ); } } // Non-dynamic modifications carry fixed integers, enum payloads, or @@ -9671,26 +9755,27 @@ fn apply_where_x_to_ability_cost( cost: &mut AbilityCost, where_x_expression: Option<&str>, unbound: &mut Option, + ctx: &ParseContext, ) { match cost { AbilityCost::PayLife { amount } | AbilityCost::PaySpeed { amount } | AbilityCost::PayEnergy { amount } | AbilityCost::ManaDynamic { quantity: amount } => { - bind_where_x_quantity(amount, where_x_expression, unbound); + bind_where_x_quantity(amount, where_x_expression, unbound, ctx); } // CR 701.9: "discard X cards, where X is …" — the discard count is a // `QuantityExpr` and must track the same where-X binding. AbilityCost::Discard { count, .. } => { - bind_where_x_quantity(count, where_x_expression, unbound); + bind_where_x_quantity(count, where_x_expression, unbound, ctx); } AbilityCost::Composite { costs } | AbilityCost::OneOf { costs } => { for sub in costs.iter_mut() { - apply_where_x_to_ability_cost(sub, where_x_expression, unbound); + apply_where_x_to_ability_cost(sub, where_x_expression, unbound, ctx); } } AbilityCost::PerCounter { base, .. } => { - apply_where_x_to_ability_cost(base, where_x_expression, unbound); + apply_where_x_to_ability_cost(base, where_x_expression, unbound, ctx); } // CR 107.3i + CR 118.1: An effect performed as a cost nests an `Effect` // (e.g. `PutCounter { count: QuantityExpr }`), whose own quantity can @@ -9699,7 +9784,7 @@ fn apply_where_x_to_ability_cost( // flows into the nested effect's count exactly as it does for the // sub-ability's effects — never re-implement the per-effect quantity walk. AbilityCost::EffectCost { effect } => { - apply_where_x_effect_expression(effect, where_x_expression); + apply_where_x_effect_expression(effect, where_x_expression, ctx); } // (the nested effect reports its own unrepresentable where-X binding by // rewriting itself to `Effect::unimplemented`, so no `unbound` plumbing @@ -9742,9 +9827,10 @@ fn apply_where_x_to_ability_cost( pub(super) fn apply_where_x_to_latest_def( defs: &mut [AbilityDefinition], where_x_expression: Option<&str>, + where_x_scope: Option<&ControllerRef>, ) { if let Some(def) = defs.last_mut() { - apply_where_x_ability_expression(def, where_x_expression); + apply_where_x_ability_expression(def, where_x_expression, where_x_scope); } } @@ -9755,8 +9841,9 @@ fn bind_where_x_filter( slot: &mut TargetFilter, where_x_expression: Option<&str>, unbound: &mut Option, + ctx: &ParseContext, ) { - match apply_where_x_to_filter(slot.clone(), where_x_expression) { + match apply_where_x_to_filter(slot.clone(), where_x_expression, ctx) { Some(bound) => *slot = bound, None => *unbound = where_x_expression.map(str::to_string), } @@ -9781,6 +9868,7 @@ fn bind_where_x_filter( pub(crate) fn apply_where_x_to_filter( filter: TargetFilter, where_x_expression: Option<&str>, + ctx: &ParseContext, ) -> Option { if where_x_expression.is_none() { return Some(filter); @@ -9790,24 +9878,24 @@ pub(crate) fn apply_where_x_to_filter( typed.properties = typed .properties .into_iter() - .map(|prop| apply_where_x_to_filter_prop(prop, where_x_expression)) + .map(|prop| apply_where_x_to_filter_prop(prop, where_x_expression, ctx)) .collect::>>()?; TargetFilter::Typed(typed) } TargetFilter::And { filters } => TargetFilter::And { filters: filters .into_iter() - .map(|filter| apply_where_x_to_filter(filter, where_x_expression)) + .map(|filter| apply_where_x_to_filter(filter, where_x_expression, ctx)) .collect::>>()?, }, TargetFilter::Or { filters } => TargetFilter::Or { filters: filters .into_iter() - .map(|filter| apply_where_x_to_filter(filter, where_x_expression)) + .map(|filter| apply_where_x_to_filter(filter, where_x_expression, ctx)) .collect::>>()?, }, TargetFilter::Not { filter } => TargetFilter::Not { - filter: Box::new(apply_where_x_to_filter(*filter, where_x_expression)?), + filter: Box::new(apply_where_x_to_filter(*filter, where_x_expression, ctx)?), }, TargetFilter::TrackedSetFiltered { id, @@ -9815,7 +9903,7 @@ pub(crate) fn apply_where_x_to_filter( caused_by, } => TargetFilter::TrackedSetFiltered { id, - filter: Box::new(apply_where_x_to_filter(*filter, where_x_expression)?), + filter: Box::new(apply_where_x_to_filter(*filter, where_x_expression, ctx)?), caused_by, }, other => other, @@ -9832,20 +9920,22 @@ fn apply_where_x_to_target_constraint( constraint: &mut TargetSelectionConstraint, where_x_expression: Option<&str>, unbound: &mut Option, + ctx: &ParseContext, ) { if let TargetSelectionConstraint::TotalManaValue { value, .. } = constraint { - bind_where_x_quantity(value, where_x_expression, unbound); + bind_where_x_quantity(value, where_x_expression, unbound, ctx); } } fn apply_where_x_to_filter_prop( prop: FilterProp, where_x_expression: Option<&str>, + ctx: &ParseContext, ) -> Option { Some(match prop { FilterProp::Cmc { comparator, value } => FilterProp::Cmc { comparator, - value: apply_where_x_quantity_expression(value, where_x_expression)?, + value: apply_where_x_quantity_expression(value, where_x_expression, ctx)?, }, FilterProp::Counters { counters, @@ -9854,7 +9944,7 @@ fn apply_where_x_to_filter_prop( } => FilterProp::Counters { counters, comparator, - count: apply_where_x_quantity_expression(count, where_x_expression)?, + count: apply_where_x_quantity_expression(count, where_x_expression, ctx)?, }, FilterProp::PtComparison { stat, @@ -9865,13 +9955,13 @@ fn apply_where_x_to_filter_prop( stat, scope, comparator, - value: apply_where_x_quantity_expression(value, where_x_expression)?, + value: apply_where_x_quantity_expression(value, where_x_expression, ctx)?, }, FilterProp::CanEnchant { target } => FilterProp::CanEnchant { - target: Box::new(apply_where_x_to_filter(*target, where_x_expression)?), + target: Box::new(apply_where_x_to_filter(*target, where_x_expression, ctx)?), }, FilterProp::DifferentNameFrom { filter } => FilterProp::DifferentNameFrom { - filter: Box::new(apply_where_x_to_filter(*filter, where_x_expression)?), + filter: Box::new(apply_where_x_to_filter(*filter, where_x_expression, ctx)?), }, FilterProp::SharesQuality { quality, @@ -9883,27 +9973,32 @@ fn apply_where_x_to_filter_prop( Some(filter) => Some(Box::new(apply_where_x_to_filter( *filter, where_x_expression, + ctx, )?)), None => None, }, relation, }, FilterProp::TargetsOnly { filter } => FilterProp::TargetsOnly { - filter: Box::new(apply_where_x_to_filter(*filter, where_x_expression)?), + filter: Box::new(apply_where_x_to_filter(*filter, where_x_expression, ctx)?), }, FilterProp::Targets { filter } => FilterProp::Targets { - filter: Box::new(apply_where_x_to_filter(*filter, where_x_expression)?), + filter: Box::new(apply_where_x_to_filter(*filter, where_x_expression, ctx)?), }, FilterProp::AnyOf { props } => FilterProp::AnyOf { props: props .into_iter() - .map(|p| apply_where_x_to_filter_prop(p, where_x_expression)) + .map(|p| apply_where_x_to_filter_prop(p, where_x_expression, ctx)) .collect::>>()?, }, // CR 608.2c: Descend into the negated inner prop so X-substitution // reaches it (mirrors the AnyOf transform). FilterProp::Not { prop } => FilterProp::Not { - prop: Box::new(apply_where_x_to_filter_prop(*prop, where_x_expression)?), + prop: Box::new(apply_where_x_to_filter_prop( + *prop, + where_x_expression, + ctx, + )?), }, other => other, }) @@ -10009,7 +10104,18 @@ fn strip_announce_lock(expression: &str) -> Option<&str> { pub(super) fn apply_where_x_ability_expression( def: &mut AbilityDefinition, where_x_expression: Option<&str>, + where_x_scope: Option<&ControllerRef>, ) { + // CR 109.5 + CR 608.2c: rebuild the parse-time player-anaphor scope captured on + // the clause into a `ParseContext` so every `parse_where_x_quantity_expression` + // reached from this def's rewrite walk reads "that player"/"they" against the + // correct player (`ScopedPlayer` phase player / `TargetPlayer` spell target). + // The context-free assembly walk lost the original `ParseContext`, so this is + // where it is reconstituted before threading down. + let wx_ctx = ParseContext { + relative_player_scope: where_x_scope.cloned(), + ..Default::default() + }; // CR 601.2b + CR 602.2b: an announce-time-locked "where X is …" clause defines X // as a count MEASURED AT ANNOUNCEMENT, overriding CR 107.3c's default that a // text-defined X "may change while that spell or ability is on the stack". Park @@ -10022,7 +10128,7 @@ pub(super) fn apply_where_x_ability_expression( // happens to be read (resolution, for a damage amount or a draw count), which is // precisely the behaviour the printed qualifier exists to forbid. if let Some(locked) = where_x_expression.and_then(strip_announce_lock) { - match parse_where_x_quantity_expression(locked) { + match parse_where_x_quantity_expression_with_context(locked, &wx_ctx) { Some(expr) => { def.announced_x = Some(expr); return; @@ -10054,10 +10160,10 @@ pub(super) fn apply_where_x_ability_expression( // rewrites below hold mutable borrows of `def`'s fields). let mut unbound_where_x: Option = None; if let Some(cond) = def.condition.as_mut() { - apply_where_x_ability_condition(cond, where_x_expression, &mut unbound_where_x); + apply_where_x_ability_condition(cond, where_x_expression, &mut unbound_where_x, &wx_ctx); } if let Some(repeat_for) = def.repeat_for.take() { - match apply_where_x_quantity_expression(repeat_for, where_x_expression) { + match apply_where_x_quantity_expression(repeat_for, where_x_expression, &wx_ctx) { Some(bound) => def.repeat_for = Some(bound), None => unbound_where_x = where_x_expression.map(str::to_string), } @@ -10068,7 +10174,7 @@ pub(super) fn apply_where_x_ability_expression( // rather than fabricating one. spec.map_quantities(|expr| { let mut slot = expr; - bind_where_x_quantity(&mut slot, where_x_expression, &mut unbound_where_x); + bind_where_x_quantity(&mut slot, where_x_expression, &mut unbound_where_x, &wx_ctx); slot }); } @@ -10078,9 +10184,14 @@ pub(super) fn apply_where_x_ability_expression( // inherits `Variable("X")` with no defining expression and the cap is // effectively unbounded. for constraint in def.target_constraints.iter_mut() { - apply_where_x_to_target_constraint(constraint, where_x_expression, &mut unbound_where_x); + apply_where_x_to_target_constraint( + constraint, + where_x_expression, + &mut unbound_where_x, + &wx_ctx, + ); } - apply_where_x_effect_expression(def.effect.as_mut(), where_x_expression); + apply_where_x_effect_expression(def.effect.as_mut(), where_x_expression, &wx_ctx); // CR 107.3c: the clause defines X, but we cannot represent that definition. // Report the gap instead of keeping a raw-text placeholder that resolves to // 0 while still reading as a supported dynamic quantity. @@ -10088,13 +10199,13 @@ pub(super) fn apply_where_x_ability_expression( *def.effect = Effect::unimplemented("where_x_binding", format!("where X is {expression}")); } if let Some(sub) = def.sub_ability.as_mut() { - apply_where_x_ability_expression(sub, where_x_expression); + apply_where_x_ability_expression(sub, where_x_expression, where_x_scope); } if let Some(else_ability) = def.else_ability.as_mut() { - apply_where_x_ability_expression(else_ability, where_x_expression); + apply_where_x_ability_expression(else_ability, where_x_expression, where_x_scope); } for mode_ability in &mut def.mode_abilities { - apply_where_x_ability_expression(mode_ability, where_x_expression); + apply_where_x_ability_expression(mode_ability, where_x_expression, where_x_scope); } } @@ -10107,22 +10218,23 @@ fn apply_where_x_ability_condition( cond: &mut AbilityCondition, where_x_expression: Option<&str>, unbound: &mut Option, + ctx: &ParseContext, ) { match cond { AbilityCondition::QuantityCheck { lhs, rhs, .. } => { - bind_where_x_quantity(lhs, where_x_expression, unbound); - bind_where_x_quantity(rhs, where_x_expression, unbound); + bind_where_x_quantity(lhs, where_x_expression, unbound, ctx); + bind_where_x_quantity(rhs, where_x_expression, unbound, ctx); } AbilityCondition::And { conditions } | AbilityCondition::Or { conditions } => { for c in conditions.iter_mut() { - apply_where_x_ability_condition(c, where_x_expression, unbound); + apply_where_x_ability_condition(c, where_x_expression, unbound, ctx); } } AbilityCondition::Not { condition } => { - apply_where_x_ability_condition(condition, where_x_expression, unbound); + apply_where_x_ability_condition(condition, where_x_expression, unbound, ctx); } AbilityCondition::ConditionInstead { inner } => { - apply_where_x_ability_condition(inner, where_x_expression, unbound); + apply_where_x_ability_condition(inner, where_x_expression, unbound, ctx); } _ => {} } @@ -10132,19 +10244,20 @@ fn apply_where_x_static_condition( condition: &mut StaticCondition, where_x_expression: Option<&str>, unbound: &mut Option, + ctx: &ParseContext, ) { match condition { StaticCondition::QuantityComparison { lhs, rhs, .. } => { - bind_where_x_quantity(lhs, where_x_expression, unbound); - bind_where_x_quantity(rhs, where_x_expression, unbound); + bind_where_x_quantity(lhs, where_x_expression, unbound, ctx); + bind_where_x_quantity(rhs, where_x_expression, unbound, ctx); } StaticCondition::And { conditions } | StaticCondition::Or { conditions } => { for condition in conditions { - apply_where_x_static_condition(condition, where_x_expression, unbound); + apply_where_x_static_condition(condition, where_x_expression, unbound, ctx); } } StaticCondition::Not { condition } => { - apply_where_x_static_condition(condition, where_x_expression, unbound); + apply_where_x_static_condition(condition, where_x_expression, unbound, ctx); } _ => {} } @@ -11451,7 +11564,10 @@ mod tests { } #[cfg(test)] mod where_x_tests { - use super::parse_where_x_quantity_expression; + use super::{ + parse_where_x_quantity_expression, parse_where_x_quantity_expression_with_context, + }; + use crate::parser::oracle_ir::context::ParseContext; use crate::types::ability::{ AbilityDefinition, AbilityKind, Comparator, ContinuousModification, ControllerRef, DigSource, Duration, Effect, FilterProp, ObjectScope, PlayerScope, PtValue, QuantityExpr, @@ -11781,6 +11897,105 @@ mod where_x_tests { ); } + /// Issue #6564 review: the ANAPHORIC "that player controls" where-X count must + /// bind to whatever player scope the caller threads through the context — never + /// a hardcoded `ScopedPlayer`. Given a `TargetPlayer`-scoped context (what a + /// caller supplies when "that player" is a chosen target), the count controller + /// is `TargetPlayer`; given the scope-free default wrapper it must NOT be forced + /// to `ScopedPlayer` (it keeps the legacy `You`). This is the exact case the + /// earlier hardcoded install regressed and the maintainer asked to cover. + /// CR 109.4 + CR 608.2c. + #[test] + fn where_x_that_player_controls_binds_caller_scope_not_scoped_player() { + // Context-aware entry point with a TargetPlayer scope → TargetPlayer. + let ctx = ParseContext { + relative_player_scope: Some(ControllerRef::TargetPlayer), + ..Default::default() + }; + let parsed = parse_where_x_quantity_expression_with_context( + "the number of artifacts that player controls", + &ctx, + ); + let Some(QuantityExpr::Ref { + qty: QuantityRef::ObjectCount { filter }, + }) = parsed + else { + panic!("expected an object count, got {parsed:?}"); + }; + let TargetFilter::Typed(typed) = filter else { + panic!("expected a typed object-count filter, got {filter:?}"); + }; + assert_eq!( + typed.controller, + Some(ControllerRef::TargetPlayer), + "the caller's TargetPlayer scope must thread into the anaphor" + ); + + // The scope-free default wrapper must NOT rebind the anaphor to + // ScopedPlayer (the regression). Legacy caster-relative binding is fine. + let bare = + parse_where_x_quantity_expression("the number of artifacts that player controls"); + if let Some(QuantityExpr::Ref { + qty: + QuantityRef::ObjectCount { + filter: TargetFilter::Typed(typed), + }, + }) = bare + { + assert_ne!( + typed.controller, + Some(ControllerRef::ScopedPlayer), + "the scope-free wrapper must not force ScopedPlayer" + ); + } + } + + /// Issue #6508: a where-X filter-controller anaphor ("they control") inside an + /// each-player phase trigger must bind to the scoped player, mirroring the + /// sibling for-each interpreter (CR 608.2c). The each-player phase context + /// (Citadel of Pain) carries `relative_player_scope = ScopedPlayer`, which the + /// assembly path threads into `parse_where_x_quantity_expression_with_context` + /// so "the number of untapped lands they control" counts the phase player's + /// untapped lands. (#6564 review: this scope is threaded from the caller's + /// context, NOT hardcoded — the scope-free default wrapper leaves the legacy + /// caster-relative binding untouched; see + /// `where_x_that_player_controls_binds_caller_scope_not_scoped_player`.) + #[test] + fn where_x_they_control_binds_scoped_player() { + let ctx = ParseContext { + relative_player_scope: Some(ControllerRef::ScopedPlayer), + ..Default::default() + }; + let parsed = parse_where_x_quantity_expression_with_context( + "the number of untapped lands they control", + &ctx, + ); + let Some(QuantityExpr::Ref { + qty: QuantityRef::ObjectCount { filter }, + }) = parsed + else { + panic!("expected an object count, got {parsed:?}"); + }; + let TargetFilter::Typed(typed) = filter else { + panic!("expected a typed object-count filter, got {filter:?}"); + }; + assert_eq!( + typed.controller, + Some(ControllerRef::ScopedPlayer), + "\"they control\" must bind to the scoped player under a ScopedPlayer context" + ); + assert!( + typed.type_filters.contains(&TypeFilter::Land), + "expected Land in the object-count filter, got {:?}", + typed.type_filters + ); + assert!( + typed.properties.contains(&FilterProp::Untapped), + "expected the Untapped qualifier, got {:?}", + typed.properties + ); + } + /// CR 107.3i + CR 202.3: the where-X traversal rebinds a `TotalManaValue` /// target constraint's `Variable("X")` cap to the die-result /// `EventContextAmount` (Ancient Brass Dragon's "where X is the result"). @@ -11848,6 +12063,7 @@ mod where_x_tests { &mut constraint, Some("the result"), &mut unbound, + &ParseContext::default(), ); assert_eq!( unbound, None, @@ -11903,6 +12119,7 @@ mod where_x_tests { &mut constraint, Some("the result"), &mut unbound, + &ParseContext::default(), ); assert_eq!( constraint, @@ -11936,8 +12153,12 @@ mod where_x_tests { ], }; - let rewritten = super::apply_where_x_quantity_expression(expression, Some("the result")) - .expect("\"the result\" is representable, so the bind must succeed"); + let rewritten = super::apply_where_x_quantity_expression( + expression, + Some("the result"), + &ParseContext::default(), + ) + .expect("\"the result\" is representable, so the bind must succeed"); let QuantityExpr::Sum { exprs } = rewritten else { panic!("expected Sum"); }; @@ -11997,7 +12218,11 @@ mod where_x_tests { enter_with_counters: vec![], }; - super::apply_where_x_effect_expression(&mut effect, Some("that spell's mana value")); + super::apply_where_x_effect_expression( + &mut effect, + Some("that spell's mana value"), + &ParseContext::default(), + ); let expected = QuantityExpr::Ref { qty: QuantityRef::ObjectManaValue { @@ -12069,7 +12294,11 @@ mod where_x_tests { end_cost: None, }; - super::apply_where_x_effect_expression(&mut effect, Some("its power")); + super::apply_where_x_effect_expression( + &mut effect, + Some("its power"), + &ParseContext::default(), + ); let Effect::GenericEffect { static_abilities, .. diff --git a/crates/engine/src/parser/oracle_effect/mod.rs b/crates/engine/src/parser/oracle_effect/mod.rs index 0bb81c1fb3..60b64f42aa 100644 --- a/crates/engine/src/parser/oracle_effect/mod.rs +++ b/crates/engine/src/parser/oracle_effect/mod.rs @@ -25,8 +25,8 @@ pub(super) use lower::{ apply_where_x_to_filter, extract_bounded_target_multi_target, extract_exact_target_multi_target, extract_optional_target_multi_target, parse_dynamic_counter_suffix_body, parse_multi_target_count_expr, - parse_where_x_quantity_expression, strip_exact_target_prefix, strip_optional_target_prefix, - try_parse_pump, + parse_where_x_quantity_expression, parse_where_x_quantity_expression_with_context, + strip_exact_target_prefix, strip_optional_target_prefix, try_parse_pump, }; // Test-only re-exports from lower module. #[cfg(test)] @@ -11692,7 +11692,11 @@ fn try_parse_reveal_until(tp: TextPair, player: TargetFilter) -> Option bool { ) } +/// CR 109.5 + CR 608.2c: Capture the player scope that a trailing where-X anaphor +/// ("they control" / "that player controls") must bind to for this clause. The +/// scope is exactly the `relative_player_scope` a trigger / for-each / fanout +/// setter stamped onto the parse context — `ScopedPlayer` for Citadel of Pain's +/// each-player phase count, `TargetPlayer` for a per-opponent fanout iterand, etc. +/// `None` leaves the legacy caster-relative (`You`) binding untouched, exactly as +/// on `main`; the context-aware entry point rebinds only when a scope is present. +/// +/// This deliberately does NOT auto-derive `TargetPlayer` from a spell clause's own +/// player target: a spell's "that player" anaphor is frequently CROSS-CLAUSE +/// ("Choose target opponent. You create X … artifacts that player controls" — +/// Curious Herd) or shared across sibling sub-effects ("target player draws X and +/// loses X …" — Pact of the Serpent), which the assembly-time single-clause view +/// cannot bind consistently (the shared-X siblings would diverge). Carrying a +/// chosen target forward to a later clause's anaphor is a separate change; here we +/// only ensure a stamped scope threads through and a hardcoded `ScopedPlayer` no +/// longer overrides it. +fn derive_where_x_scope(chunk_ctx: &ParseContext) -> Option { + chunk_ctx.relative_player_scope.clone() +} + /// CR 109.5: True when a `TargetFilter` denotes players (a controller-/player- /// scoped filter) rather than objects. Builds on `is_player_filter` (which covers /// `Player` and the controller-only `Typed` "each of your opponents" shape) and @@ -29151,6 +29176,7 @@ pub(crate) fn parse_effect_chain_ir( prev_clause.map(|c| AbilityDefinition::new(kind, c.parsed.effect.clone())); let inherited_where_x_expression = prev_clause.and_then(|c| c.where_x_expression.clone()); + let inherited_where_x_scope = prev_clause.and_then(|c| c.where_x_scope.clone()); if let Some(alt_ir) = try_parse_dig_instead_alternative(normalized_text, prev_temp.as_ref(), kind, ctx) { @@ -29170,6 +29196,7 @@ pub(crate) fn parse_effect_chain_ir( }, ) .where_x_expression(inherited_where_x_expression) + .where_x_scope(inherited_where_x_scope) .push(); continue; } @@ -30204,6 +30231,7 @@ pub(crate) fn parse_effect_chain_ir( if let Some(prefix_condition) = prefix_delayed { let (inner_text, inner_multi_target) = strip_any_number_quantifier(text_after_prefix); let inner_clause = parse_effect_clause(&inner_text, ctx); + let inner_where_x_scope = derive_where_x_scope(ctx); let mut inner_def = AbilityDefinition::new(kind, inner_clause.effect); if let Some(spec) = inner_multi_target.or(inner_clause.multi_target) { inner_def = inner_def.multi_target(spec); @@ -30225,7 +30253,11 @@ pub(crate) fn parse_effect_chain_ir( if let Some(up) = unless_pay.take() { inner_def.unless_pay = Some(up); } - apply_where_x_ability_expression(&mut inner_def, where_x_expression.as_deref()); + apply_where_x_ability_expression( + &mut inner_def, + where_x_expression.as_deref(), + inner_where_x_scope.as_ref(), + ); let delayed_effect = Effect::CreateDelayedTrigger { condition: prefix_condition.clone(), effect: Box::new(inner_def), @@ -30258,10 +30290,12 @@ pub(crate) fn parse_effect_chain_ir( ctx.push_diagnostic(d); } } + let delayed_clause = parsed_clause(delayed_effect); + let where_x_scope = derive_where_x_scope(&chunk_ctx); builder .clause( normalized_text, - parsed_clause(delayed_effect), + delayed_clause, chunk.boundary_after, ClauseDisposition::Emit { followup: None, @@ -30276,6 +30310,7 @@ pub(crate) fn parse_effect_chain_ir( .starting_with(starting_with.clone()) .prefix_delayed_condition(Some(prefix_condition)) .where_x_expression(where_x_expression.clone()) + .where_x_scope(where_x_scope) .target_selection_mode(chunk_ctx.target_selection_mode) .target_chooser(chunk_ctx.target_chooser.clone()) .push(); @@ -30292,6 +30327,7 @@ pub(crate) fn parse_effect_chain_ir( ctx.relative_player_scope = Some(chosen_scope.clone()); chain_chosen_player_count = ctx.chosen_player_count; chain_chosen_player_scope = Some(chosen_scope); + let where_x_scope = derive_where_x_scope(&chunk_ctx); builder .clause( normalized_text, @@ -30309,6 +30345,7 @@ pub(crate) fn parse_effect_chain_ir( .player_scope(player_scope) .starting_with(starting_with.clone()) .where_x_expression(where_x_expression.clone()) + .where_x_scope(where_x_scope) .push(); continue; } @@ -30995,6 +31032,7 @@ pub(crate) fn parse_effect_chain_ir( if let Some(ref cond) = condition { instead_def = instead_def.condition(cond.clone()); } + let where_x_scope = derive_where_x_scope(&chunk_ctx); builder .clause( normalized_text, @@ -31012,6 +31050,7 @@ pub(crate) fn parse_effect_chain_ir( .starting_with(starting_with.clone()) .multi_target(multi_target) .where_x_expression(where_x_expression) + .where_x_scope(where_x_scope) .push(); continue; } @@ -31055,6 +31094,7 @@ pub(crate) fn parse_effect_chain_ir( intrinsic_continuation_effect(&temp_def), full_text, ); + let where_x_scope = derive_where_x_scope(&chunk_ctx); builder .clause( normalized_text, @@ -31070,6 +31110,7 @@ pub(crate) fn parse_effect_chain_ir( .starting_with(starting_with.clone()) .multi_target(multi_target) .where_x_expression(where_x_expression) + .where_x_scope(where_x_scope) .push(); continue; } @@ -31386,6 +31427,7 @@ pub(crate) fn parse_effect_chain_ir( // Store the followup continuation — it applies to the previous clause. // We handle this by pushing an absorbed marker clause. if let Some(continuation) = followup_continuation { + let where_x_scope = derive_where_x_scope(&chunk_ctx); builder .clause( normalized_text, @@ -31403,6 +31445,7 @@ pub(crate) fn parse_effect_chain_ir( .starting_with(starting_with.clone()) .multi_target(multi_target) .where_x_expression(where_x_expression) + .where_x_scope(where_x_scope) .target_selection_mode(chunk_ctx.target_selection_mode) .target_chooser(chunk_ctx.target_chooser.clone()) .push(); @@ -31477,6 +31520,7 @@ pub(crate) fn parse_effect_chain_ir( // CR 115.1 + CR 701.9b: `target_selection_mode` snapshots the parser's // per-chunk selection mode. Set to `Random` by `parse_target_with_ctx` // when "random " was stripped from this chunk's target phrase. + let where_x_scope = derive_where_x_scope(&chunk_ctx); builder .clause( normalized_text, @@ -31496,6 +31540,7 @@ pub(crate) fn parse_effect_chain_ir( .delayed_condition(delayed_condition) .multi_target(multi_target) .where_x_expression(where_x_expression) + .where_x_scope(where_x_scope) .unless_pay(unless_pay) .target_selection_mode(chunk_ctx.target_selection_mode) .target_chooser(chunk_ctx.target_chooser.clone()) @@ -31852,7 +31897,13 @@ fn try_parse_put_zone_change_parts( // `parse_where_x_quantity_expression` building block. let where_x_expression = strip_trailing_where_x(after_put_tp).1; // CR 107.3c: fail honestly instead of fabricating a raw-text placeholder. - let target = apply_where_x_to_filter(target, where_x_expression.as_deref())?; + // Filter `Cmc`/counter bounds carry no player anaphor, so the default + // (scope-free) context is correct here. + let target = apply_where_x_to_filter( + target, + where_x_expression.as_deref(), + &ParseContext::default(), + )?; // CR 608.2c: Restrict the target to objects affected by the // preceding effect when a "this way" result phrase appears in the // target text. The relevant resolvers publish `state.tracked_object_sets` diff --git a/crates/engine/src/parser/oracle_effect/sequence.rs b/crates/engine/src/parser/oracle_effect/sequence.rs index 973b5df69f..e917d94cb9 100644 --- a/crates/engine/src/parser/oracle_effect/sequence.rs +++ b/crates/engine/src/parser/oracle_effect/sequence.rs @@ -5460,7 +5460,11 @@ pub(super) fn parse_dig_from_among( let filter = parse_dig_from_among_filter(filter_text, ctx); // CR 107.3c: fail honestly instead of fabricating a raw-text placeholder. - let filter = apply_where_x_to_filter(filter, where_x_expression.as_deref())?; + let filter = apply_where_x_to_filter( + filter, + where_x_expression.as_deref(), + &ParseContext::default(), + )?; // CR 110.2a: "... under your control" routes the kept cards to the // ability controller. Scan the FULL clause — the controller phrase @@ -5553,7 +5557,11 @@ pub(super) fn parse_dig_from_among( // CR 202.3 + CR 107.3i: Bind the literal `X` in the filter's `Cmc` bound // with the stripped "where X is " defining clause. // CR 107.3c: fail honestly instead of fabricating a raw-text placeholder. - let filter = apply_where_x_to_filter(filter, where_x_expression.as_deref())?; + let filter = apply_where_x_to_filter( + filter, + where_x_expression.as_deref(), + &ParseContext::default(), + )?; // CR 110.2a + CR 708.2a/708.3: detect "under your control" / "face down" on // the full clause for the from-among put-step. diff --git a/crates/engine/src/parser/oracle_effect/token.rs b/crates/engine/src/parser/oracle_effect/token.rs index fd9cf21dfb..e2a56c2cc2 100644 --- a/crates/engine/src/parser/oracle_effect/token.rs +++ b/crates/engine/src/parser/oracle_effect/token.rs @@ -115,7 +115,8 @@ pub(crate) fn try_parse_token(_lower: &str, text: &str, ctx: &mut ParseContext) // still rendered as a supported dynamic quantity. This mirrors the // sibling non-copy token path below. count = - super::parse_where_x_quantity_expression(&where_expression).or_else(|| { + super::parse_where_x_quantity_expression_with_context(&where_expression, ctx) + .or_else(|| { crate::parser::oracle_quantity::parse_cda_quantity(&where_expression) })?; } @@ -680,9 +681,10 @@ fn parse_token_description_with_context( // rendered as a supported dynamic quantity in the coverage report — // a fabricated green. Honest failure is the only correct answer. let bound = - super::parse_where_x_quantity_expression(&where_expression).or_else(|| { - crate::parser::oracle_quantity::parse_cda_quantity(&where_expression) - })?; + super::parse_where_x_quantity_expression_with_context(&where_expression, ctx) + .or_else(|| { + crate::parser::oracle_quantity::parse_cda_quantity(&where_expression) + })?; if matches!(&count, QuantityExpr::Ref { qty: QuantityRef::Variable { ref name } } if name == "X") { count = bound.clone(); @@ -724,7 +726,9 @@ fn parse_token_description_with_context( .or_else(|| { crate::parser::oracle_quantity::parse_event_context_quantity(&count_expression) }) - .or_else(|| super::parse_where_x_quantity_expression(&count_expression)) + .or_else(|| { + super::parse_where_x_quantity_expression_with_context(&count_expression, ctx) + }) .or_else(|| { // CR 608.2c: bare anaphoric "the difference" — the two operands // live on the enclosing ability's condition, not this clause diff --git a/crates/engine/src/parser/oracle_ir/effect_chain.rs b/crates/engine/src/parser/oracle_ir/effect_chain.rs index 32c8940a40..38c3fb628a 100644 --- a/crates/engine/src/parser/oracle_ir/effect_chain.rs +++ b/crates/engine/src/parser/oracle_ir/effect_chain.rs @@ -778,6 +778,15 @@ pub(crate) struct ClauseIr { pub(crate) multi_target: Option, /// CR 107.3i: "where X is " binding. pub(crate) where_x_expression: Option, + /// CR 109.5 + CR 608.2c: The player scope a third-person anaphor inside the + /// `where_x_expression` ("they control" / "that player controls") binds to. + /// Captured at parse time (the assembly walk that re-interprets the where-X + /// count is context-free), then rebuilt into a `ParseContext` and threaded to + /// `parse_where_x_quantity_expression_with_context` during lowering. + /// `ScopedPlayer` for an each-player phase trigger; `TargetPlayer` for a spell + /// whose clause targets a player; `None` = caster-relative legacy binding. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub(crate) where_x_scope: Option, /// CR 118.12: Resolution-time "unless [player] pays" modifier carried by /// this clause. pub(crate) unless_pay: Option, @@ -947,6 +956,7 @@ impl ClauseIrBuilder { prefix_delayed_condition: None, multi_target: None, where_x_expression: None, + where_x_scope: None, unless_pay: None, target_selection_mode: TargetSelectionMode::Chosen, target_chooser: None, @@ -998,6 +1008,7 @@ impl ClauseIrBuilder { .prefix_delayed_condition(c.prefix_delayed_condition) .multi_target(c.multi_target) .where_x_expression(c.where_x_expression) + .where_x_scope(c.where_x_scope) .unless_pay(c.unless_pay) .target_selection_mode(c.target_selection_mode) .target_chooser(c.target_chooser) @@ -1030,6 +1041,7 @@ pub(crate) struct ClauseDraft<'a> { prefix_delayed_condition: Option, multi_target: Option, where_x_expression: Option, + where_x_scope: Option, unless_pay: Option, target_selection_mode: TargetSelectionMode, target_chooser: Option, @@ -1079,6 +1091,10 @@ impl ClauseDraft<'_> { self.where_x_expression = v; self } + pub(crate) fn where_x_scope(mut self, v: Option) -> Self { + self.where_x_scope = v; + self + } pub(crate) fn unless_pay(mut self, v: Option) -> Self { self.unless_pay = v; self @@ -1123,6 +1139,7 @@ impl ClauseDraft<'_> { prefix_delayed_condition: self.prefix_delayed_condition, multi_target: self.multi_target, where_x_expression: self.where_x_expression, + where_x_scope: self.where_x_scope, unless_pay: self.unless_pay, target_selection_mode: self.target_selection_mode, target_chooser: self.target_chooser, diff --git a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__boseiju_who_endures_ir.snap b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__boseiju_who_endures_ir.snap index e849a994db..1c1bc4c59c 100644 --- a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__boseiju_who_endures_ir.snap +++ b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__boseiju_who_endures_ir.snap @@ -78,6 +78,7 @@ expression: "&ir" "prefix_delayed_condition": null, "multi_target": null, "where_x_expression": null, + "where_x_scope": "ParentTargetController", "unless_pay": null } ], diff --git a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__fevered_visions_ir.snap b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__fevered_visions_ir.snap index e7c1e3f164..af63b5987f 100644 --- a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__fevered_visions_ir.snap +++ b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__fevered_visions_ir.snap @@ -101,6 +101,7 @@ expression: "&ir" "prefix_delayed_condition": null, "multi_target": null, "where_x_expression": null, + "where_x_scope": "ScopedPlayer", "unless_pay": null }, { @@ -183,6 +184,7 @@ expression: "&ir" "prefix_delayed_condition": null, "multi_target": null, "where_x_expression": null, + "where_x_scope": "ScopedPlayer", "unless_pay": null } ], diff --git a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__kroxa_titan_ir.snap b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__kroxa_titan_ir.snap index 651f21a37f..d99ef41ecb 100644 --- a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__kroxa_titan_ir.snap +++ b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__kroxa_titan_ir.snap @@ -103,6 +103,7 @@ expression: "&ir" "prefix_delayed_condition": null, "multi_target": null, "where_x_expression": null, + "where_x_scope": "ScopedPlayer", "unless_pay": null } ], diff --git a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__liliana_of_the_veil_ir.snap b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__liliana_of_the_veil_ir.snap index d22437a372..49ad4424a6 100644 --- a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__liliana_of_the_veil_ir.snap +++ b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__liliana_of_the_veil_ir.snap @@ -81,6 +81,7 @@ expression: "&ir" "prefix_delayed_condition": null, "multi_target": null, "where_x_expression": null, + "where_x_scope": "ScopedPlayer", "unless_pay": null } ], diff --git a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__nashi_moon_sages_scion_ir.snap b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__nashi_moon_sages_scion_ir.snap index 50c8d1f03d..a31fab20cd 100644 --- a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__nashi_moon_sages_scion_ir.snap +++ b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__nashi_moon_sages_scion_ir.snap @@ -139,6 +139,7 @@ expression: "&ir" "prefix_delayed_condition": null, "multi_target": null, "where_x_expression": null, + "where_x_scope": "TargetPlayer", "unless_pay": null }, { @@ -193,6 +194,7 @@ expression: "&ir" "prefix_delayed_condition": null, "multi_target": null, "where_x_expression": null, + "where_x_scope": "TargetPlayer", "unless_pay": null }, { diff --git a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__questing_beast_ir.snap b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__questing_beast_ir.snap index aa0597ddaa..42bfb13645 100644 --- a/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__questing_beast_ir.snap +++ b/crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__questing_beast_ir.snap @@ -300,6 +300,7 @@ expression: "&ir" "prefix_delayed_condition": null, "multi_target": null, "where_x_expression": null, + "where_x_scope": "TargetPlayer", "unless_pay": null } ], diff --git a/crates/engine/src/parser/oracle_quantity.rs b/crates/engine/src/parser/oracle_quantity.rs index 956ee48e57..23d30f0844 100644 --- a/crates/engine/src/parser/oracle_quantity.rs +++ b/crates/engine/src/parser/oracle_quantity.rs @@ -3334,7 +3334,10 @@ fn parse_for_each_clause_with_they_controller( None } -fn for_each_anaphor_context(ctx: &ParseContext, they_controller: &ControllerRef) -> ParseContext { +pub(crate) fn for_each_anaphor_context( + ctx: &ParseContext, + they_controller: &ControllerRef, +) -> ParseContext { ParseContext { relative_player_scope: Some(they_controller.clone()), subject: ctx.subject.clone(), diff --git a/crates/engine/src/parser/oracle_trigger.rs b/crates/engine/src/parser/oracle_trigger.rs index 826db7e2fa..b517402c6c 100644 --- a/crates/engine/src/parser/oracle_trigger.rs +++ b/crates/engine/src/parser/oracle_trigger.rs @@ -1677,6 +1677,20 @@ pub(crate) fn lower_trigger_ir(ir: &TriggerIr) -> TriggerDefinition { crate::parser::oracle_effect::rewrite_player_quantity_refs_to_source_chosen(ability); } } + // CR 603.2b + CR 102.1: each-player/each-opponent PHASE triggers bind + // "their hand"/"their life" possessives to the phase's active player, which + // the runtime stamps onto `scoped_player` (build_triggered_ability, + // game/triggers.rs). Reuses the identical rewrite the TargetPlayer event + // triggers use; `PlayerScope::Controller` ("your hand") is deliberately NOT + // rewritten by that pass, so mixed-anaphor cards (Dark Suspicions) keep the + // controller side intact. Mutually exclusive with the SourceChosenPlayer + // branch above: `relative_player_scope_for_condition` checks the chosen-player + // phase before the scoped-phase player, so The Rack never enters here. + if modifiers.relative_player_scope == Some(ControllerRef::ScopedPlayer) { + if let Some(ability) = execute.as_deref_mut() { + crate::parser::oracle_effect::rewrite_event_player_quantity_refs_to_scoped(ability); + } + } if let Some(ability) = execute.as_deref_mut() { rewrite_each_other_player_scope_for_any_caster_spell_triggers( &def, diff --git a/crates/engine/src/parser/oracle_trigger_tests.rs b/crates/engine/src/parser/oracle_trigger_tests.rs index 16c5194245..4e203e92d2 100644 --- a/crates/engine/src/parser/oracle_trigger_tests.rs +++ b/crates/engine/src/parser/oracle_trigger_tests.rs @@ -17749,6 +17749,240 @@ fn phase_trigger_blinkmoth_urn_that_player_adds_mana_for_their_artifacts() { } } +/// Issue #6508 SHAPE — Citadel of Pain: "At the beginning of each player's end +/// step, this enchantment deals X damage to that player, where X is the number +/// of untapped lands they control." The where-X filter-controller anaphor +/// ("they control") must bind to the scoped (phase) player, not the source +/// controller. CR 608.2c: a per-player-scoped count reads the iterating player. +#[test] +fn citadel_of_pain_each_player_end_step_scoped_amount() { + let def = parse_trigger_line( + "At the beginning of each player's end step, this enchantment deals X damage to that player, where X is the number of untapped lands they control.", + "Citadel of Pain", + ); + assert_eq!(def.mode, TriggerMode::Phase); + assert_eq!(def.phase, Some(Phase::End)); + assert_eq!(def.constraint, None); + let exec = def + .execute + .as_ref() + .expect("Citadel of Pain must have execute"); + match exec.effect.as_ref() { + Effect::DealDamage { amount, target, .. } => { + assert_eq!( + *target, + TargetFilter::ScopedPlayer, + "damage recipient must be the phase player (ScopedPlayer)" + ); + let QuantityExpr::Ref { + qty: + QuantityRef::ObjectCount { + filter: TargetFilter::Typed(tf), + }, + } = amount + else { + panic!("expected ObjectCount amount, got {amount:?}"); + }; + assert!( + tf.type_filters.contains(&TypeFilter::Land), + "count must be lands, got {:?}", + tf.type_filters + ); + assert!( + tf.properties.contains(&FilterProp::Untapped), + "count must be UNTAPPED lands, got {:?}", + tf.properties + ); + assert_eq!( + tf.controller, + Some(ControllerRef::ScopedPlayer), + "\"they control\" must bind to the scoped player (CR 608.2c)" + ); + } + other => panic!("expected Effect::DealDamage, got {other:?}"), + } +} + +/// Issue #6508 SHAPE (Part B) — Iron Maiden: "At the beginning of each +/// opponent's upkeep, this artifact deals X damage to that player, where X is +/// the number of cards in their hand minus 4." The possessive hand-count +/// ("their hand") is a context-free `TargetZoneCardCount` at parse time; the +/// scoped-phase-trigger lowering rewrites it to `HandSize { ScopedPlayer }` +/// (CR 603.2b + CR 102.1). The `minus 4` offset is preserved. +#[test] +fn iron_maiden_each_opponent_upkeep_scoped_hand_size() { + let def = parse_trigger_line( + "At the beginning of each opponent's upkeep, this artifact deals X damage to that player, where X is the number of cards in their hand minus 4.", + "Iron Maiden", + ); + assert_eq!(def.mode, TriggerMode::Phase); + assert_eq!(def.phase, Some(Phase::Upkeep)); + let exec = def.execute.as_ref().expect("Iron Maiden must have execute"); + match exec.effect.as_ref() { + Effect::DealDamage { amount, target, .. } => { + assert_eq!(*target, TargetFilter::ScopedPlayer); + let QuantityExpr::Offset { inner, offset } = amount else { + panic!("expected Offset amount, got {amount:?}"); + }; + assert_eq!(*offset, -4, "the \"minus 4\" offset must be preserved"); + assert_eq!( + **inner, + QuantityExpr::Ref { + qty: QuantityRef::HandSize { + player: PlayerScope::ScopedPlayer, + }, + }, + "\"cards in their hand\" must bind to the scoped player" + ); + } + other => panic!("expected Effect::DealDamage, got {other:?}"), + } +} + +/// Issue #6508 SHAPE (multi-authority, Part B) — Dark Suspicions: "At the +/// beginning of each opponent's upkeep, that player loses X life, where X is the +/// number of cards in that player's hand minus the number of cards in your +/// hand." The scoped-player hand-count moves to `ScopedPlayer` while the +/// controller-side "your hand" MUST stay `Controller` (CR 109.5) — the rewrite +/// touches only the Target/possessive side, never `You`. +#[test] +fn dark_suspicions_scoped_hand_minus_controller_hand() { + let def = parse_trigger_line( + "At the beginning of each opponent's upkeep, that player loses X life, where X is the number of cards in that player's hand minus the number of cards in your hand.", + "Dark Suspicions", + ); + assert_eq!(def.mode, TriggerMode::Phase); + assert_eq!(def.phase, Some(Phase::Upkeep)); + let exec = def + .execute + .as_ref() + .expect("Dark Suspicions must have execute"); + match exec.effect.as_ref() { + Effect::LoseLife { amount, .. } => { + let QuantityExpr::Sum { exprs } = amount else { + panic!("expected Sum amount, got {amount:?}"); + }; + assert_eq!( + exprs.len(), + 2, + "sum of scoped-player hand and negated controller hand, got {exprs:?}" + ); + assert_eq!( + exprs[0], + QuantityExpr::Ref { + qty: QuantityRef::HandSize { + player: PlayerScope::ScopedPlayer, + }, + }, + "\"that player's hand\" must bind to the scoped player" + ); + assert_eq!( + exprs[1], + QuantityExpr::Multiply { + factor: -1, + inner: Box::new(QuantityExpr::Ref { + qty: QuantityRef::HandSize { + player: PlayerScope::Controller, + }, + }), + }, + "\"your hand\" must remain Controller (CR 109.5) — the rewrite must not move it" + ); + } + other => panic!("expected Effect::LoseLife, got {other:?}"), + } +} + +/// Issue #6508 SHAPE (REQUIRED — only exerciser of the `PlayerScope::Target → +/// ScopedPlayer` life-total arm) — Havoc Festival: "At the beginning of each +/// player's upkeep, that player loses half their life, rounded up." The +/// life-total possessive ("their life") must bind to the scoped player +/// (CR 603.2b + CR 102.1). Reach-guard: the loss is a parsed `DivideRounded` +/// (not `Unimplemented`). +#[test] +fn havoc_festival_life_total_binds_scoped_player() { + let def = parse_trigger_line( + "At the beginning of each player's upkeep, that player loses half their life, rounded up.", + "Havoc Festival", + ); + assert_eq!(def.mode, TriggerMode::Phase); + assert_eq!(def.phase, Some(Phase::Upkeep)); + let exec = def + .execute + .as_ref() + .expect("Havoc Festival must have execute"); + match exec.effect.as_ref() { + Effect::LoseLife { amount, .. } => { + let QuantityExpr::DivideRounded { + inner, + divisor, + rounding, + } = amount + else { + panic!( + "reach-guard failed: expected parsed DivideRounded life loss, got {amount:?}" + ); + }; + assert_eq!(*divisor, 2); + assert_eq!(*rounding, crate::types::ability::RoundingMode::Up); + assert_eq!( + **inner, + QuantityExpr::Ref { + qty: QuantityRef::LifeTotal { + player: PlayerScope::ScopedPlayer, + }, + }, + "\"their life\" must bind to the scoped player (PlayerScope::Target → ScopedPlayer)" + ); + } + other => panic!("expected Effect::LoseLife, got {other:?}"), + } +} + +/// Issue #6508 negative (with reach-guard) — "you control" inside a where-X of +/// an each-player phase trigger must STAY bound to the controller (You), +/// unaffected by the scoped-player anaphor fix (CR 109.5). Reach-guard: the +/// recipient is still `ScopedPlayer` and the amount is a real parsed +/// `ObjectCount` (not `Unimplemented`), proving the where-X actually parsed and +/// the assertion is not vacuously true. +#[test] +fn each_player_end_step_where_x_you_control_stays_you() { + let def = parse_trigger_line( + "At the beginning of each player's end step, this enchantment deals X damage to that player, where X is the number of creatures you control.", + "Test Card", + ); + let exec = def.execute.as_ref().expect("must have execute"); + match exec.effect.as_ref() { + Effect::DealDamage { amount, target, .. } => { + assert_eq!( + *target, + TargetFilter::ScopedPlayer, + "reach-guard: the recipient is still the scoped player" + ); + let QuantityExpr::Ref { + qty: + QuantityRef::ObjectCount { + filter: TargetFilter::Typed(tf), + }, + } = amount + else { + panic!("reach-guard failed: expected a parsed ObjectCount, got {amount:?}"); + }; + assert!( + tf.type_filters.contains(&TypeFilter::Creature), + "count must be creatures, got {:?}", + tf.type_filters + ); + assert_eq!( + tf.controller, + Some(ControllerRef::You), + "\"you control\" must remain You (CR 109.5)" + ); + } + other => panic!("expected Effect::DealDamage, got {other:?}"), + } +} + #[test] fn trigger_each_of_your_main_phases_uses_main_phase_constraint() { let def = parse_trigger_line( diff --git a/crates/engine/tests/integration/citadel_of_pain_each_player_end_step_6508.rs b/crates/engine/tests/integration/citadel_of_pain_each_player_end_step_6508.rs new file mode 100644 index 0000000000..05bd8be5b0 --- /dev/null +++ b/crates/engine/tests/integration/citadel_of_pain_each_player_end_step_6508.rs @@ -0,0 +1,219 @@ +//! Citadel of Pain (#6508) — each-player phase-trigger anaphor binding. +//! +//! Oracle (Citadel of Pain): +//! At the beginning of each player's end step, this enchantment deals X +//! damage to that player, where X is the number of untapped lands they +//! control. +//! +//! The bug: the `where X is … they control` count bound to the SOURCE's +//! controller (`ControllerRef::You`) instead of the phase's active player +//! (`ScopedPlayer`), so on an opponent's end step Citadel dealt the CONTROLLER's +//! untapped-land count to the opponent (frequently 0 when the controller tapped +//! out). The recipient ("to that player") was already `ScopedPlayer` and is +//! unchanged — these tests additionally pin that the recipient is correct. +//! +//! CR references: +//! - CR 513.1: the end step begins; "at the beginning of each player's end +//! step" triggers fire (CR 603.2b) with the phase's active player +//! (CR 102.1) stamped as the scoped player. +//! - CR 503.1a: upkeep triggers (Iron Maiden) go on the stack as the upkeep +//! step begins. +//! +//! Revert map (discriminating tests fail if Part A / Part B is reverted): +//! * `opponent_end_step_damages_phase_player_by_their_untapped_lands` (T1) — +//! asymmetric counts (P0=1, P1=3). Post-fix P1 takes 3; pre-fix P1 takes the +//! controller's count (1). REVERT-FAILING for Part A. +//! * `phase_player_with_no_untapped_lands_takes_zero` (T2) — P1's lands are all +//! tapped, so post-fix X=0 (exercises the `Untapped` count at resolution, +//! per the 2004-10-04 ruling). Pre-fix X = P0's untapped count (2). +//! REVERT-FAILING for Part A. +//! * `controller_end_step_takes_own_count` (T3) — companion, NON-discriminating +//! (on the controller's own end step scoped == controller, so pre- and +//! post-fix agree). Pins the caster-relative reading is preserved. +//! * `iron_maiden_upkeep_damage_equals_scoped_hand_minus_four` (T4) — Iron +//! Maiden's possessive hand-count. Post-fix deals hand−4 = 3; pre-fix the +//! `TargetZoneCardCount` resolves 0 with no player target, so it deals 0. +//! REVERT-FAILING for Part B. + +use engine::game::scenario::{GameRunner, GameScenario, P0, P1}; +use engine::types::game_state::WaitingFor; +use engine::types::mana::ManaColor; +use engine::types::phase::Phase; +use engine::types::player::PlayerId; + +const CITADEL_ORACLE: &str = "At the beginning of each player's end step, this enchantment deals X damage to that player, where X is the number of untapped lands they control."; + +const IRON_MAIDEN_ORACLE: &str = "At the beginning of each opponent's upkeep, this artifact deals X damage to that player, where X is the number of cards in their hand minus 4."; + +/// Build Citadel of Pain (as an enchantment) under P0's control, give P0 and P1 +/// the requested number of untapped basic lands, and set `active` as the active +/// player whose end step we will advance into. When `p1_lands_tapped` is set, +/// P1's lands are all tapped after build so they no longer count as untapped. +fn setup_citadel( + p0_untapped_lands: usize, + p1_lands: usize, + p1_lands_tapped: bool, + active: PlayerId, +) -> GameRunner { + let mut scenario = GameScenario::new(); + // Start after combat so advancing to the end step does not halt at + // DeclareAttackers (mirrors bre_of_clan_stoutarm_endstep). + scenario.at_phase(Phase::PostCombatMain); + + scenario + .add_creature_from_oracle(P0, "Citadel of Pain", 0, 0, CITADEL_ORACLE) + .as_enchantment(); + + for _ in 0..p0_untapped_lands { + scenario.add_basic_land(P0, ManaColor::Green); + } + let mut p1_land_ids = Vec::new(); + for _ in 0..p1_lands { + p1_land_ids.push(scenario.add_basic_land(P1, ManaColor::Green)); + } + + // Library padding so nothing decks during resolution. + for _ in 0..10 { + scenario.add_card_to_library_top(P0, "Plains"); + scenario.add_card_to_library_top(P1, "Plains"); + } + + let mut runner = scenario.build(); + // Make the whole priority triple consistent for `active`. `at_phase` stamped + // `waiting_for = Priority { P0 }` (the default active player at build time); + // overriding only `active_player`/`priority_player` would leave `waiting_for` + // stale on a priority phase (PostCombatMain), which stalls `advance_to_phase`. + runner.state_mut().active_player = active; + runner.state_mut().priority_player = active; + runner.state_mut().waiting_for = WaitingFor::Priority { player: active }; + + if p1_lands_tapped { + // CR 110.5: a tapped land no longer satisfies the `Untapped` count + // qualifier (tapped/untapped are status categories). + for id in &p1_land_ids { + runner.state_mut().objects.get_mut(id).unwrap().tapped = true; + } + } + + runner +} + +/// T1 (REVERT-FAILING, Part A): Citadel under P0; P0 has 1 untapped land, P1 has +/// 3. On P1's end step the damage must equal P1's untapped-land count (3), dealt +/// to P1 (the phase player). Pre-fix the amount counts P0's untapped lands (1). +#[test] +fn opponent_end_step_damages_phase_player_by_their_untapped_lands() { + let mut runner = setup_citadel(1, 3, false, P1); + + let p0_before = runner.state().players[P0.0 as usize].life; + let p1_before = runner.state().players[P1.0 as usize].life; + + runner.advance_to_end_step(); + runner.advance_until_stack_empty(); + + let p1_delta = runner.state().players[P1.0 as usize].life - p1_before; + let p0_delta = runner.state().players[P0.0 as usize].life - p0_before; + + assert_eq!( + p1_delta, -3, + "P1's end step: Citadel must deal P1's untapped-land count (3) to P1, \ + not the controller's count; got delta {p1_delta}" + ); + assert_eq!( + p0_delta, 0, + "the damage recipient is the phase player (P1), so P0 takes none" + ); +} + +/// T2 (REVERT-FAILING, Part A): P1's lands are all tapped, so at P1's end step +/// the untapped-land count is 0 and P1 takes no damage. Pre-fix the amount reads +/// P0's untapped count (2) and P1 wrongly takes 2. Exercises the `Untapped` +/// qualifier at resolution (2004-10-04 ruling). +#[test] +fn phase_player_with_no_untapped_lands_takes_zero() { + let mut runner = setup_citadel(2, 3, true, P1); + + let p1_before = runner.state().players[P1.0 as usize].life; + + runner.advance_to_end_step(); + runner.advance_until_stack_empty(); + + let p1_delta = runner.state().players[P1.0 as usize].life - p1_before; + assert_eq!( + p1_delta, 0, + "all of P1's lands are tapped, so the untapped-land count is 0 and P1 \ + takes no damage; got delta {p1_delta}" + ); +} + +/// T3 (companion, NON-discriminating): on the controller's OWN end step the +/// scoped player is the controller, so pre- and post-fix agree. Pins that the +/// caster-relative reading of "they control" is preserved. +#[test] +fn controller_end_step_takes_own_count() { + let mut runner = setup_citadel(2, 3, false, P0); + + let p0_before = runner.state().players[P0.0 as usize].life; + let p1_before = runner.state().players[P1.0 as usize].life; + + runner.advance_to_end_step(); + runner.advance_until_stack_empty(); + + let p0_delta = runner.state().players[P0.0 as usize].life - p0_before; + let p1_delta = runner.state().players[P1.0 as usize].life - p1_before; + + assert_eq!( + p0_delta, -2, + "P0's own end step: Citadel deals P0's untapped-land count (2) to P0" + ); + assert_eq!(p1_delta, 0, "P1 is not the phase player, takes none"); +} + +/// Build Iron Maiden (as an artifact) under P0's control and give P1 a hand of +/// `p1_hand` cards. The scenario starts on P0's post-combat main phase; +/// `advance_to_upkeep` then crosses the turn boundary into P1's turn, firing +/// Iron Maiden's each-opponent upkeep trigger from a consistently-transitioned +/// game state (active == P1, priority stamped by the engine) rather than a +/// hand-poked active player — CR 500.1 / CR 503.1a. P1's upkeep precedes its +/// draw step, so P1's hand is still `p1_hand` when the trigger resolves. +fn setup_iron_maiden(p1_hand: usize) -> GameRunner { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PostCombatMain); + + scenario + .add_creature_from_oracle(P0, "Iron Maiden", 0, 0, IRON_MAIDEN_ORACLE) + .as_artifact(); + + for _ in 0..p1_hand { + scenario.add_card_to_hand(P1, "Plains"); + } + + // Library padding so crossing the turn boundary (draw steps) doesn't deck + // either player during priority advancement. + for _ in 0..20 { + scenario.add_card_to_library_top(P0, "Plains"); + scenario.add_card_to_library_top(P1, "Plains"); + } + + scenario.build() +} + +/// T4 (REVERT-FAILING, Part B): Iron Maiden under P0; P1's hand is 7. On P1's +/// upkeep the damage is hand − 4 = 3, dealt to P1. Pre-fix the possessive +/// hand-count is a `TargetZoneCardCount` that resolves 0 with no player target, +/// so Iron Maiden deals max(0, 0 − 4) = 0. +#[test] +fn iron_maiden_upkeep_damage_equals_scoped_hand_minus_four() { + let mut runner = setup_iron_maiden(7); + + let p1_before = runner.state().players[P1.0 as usize].life; + + runner.advance_to_upkeep(); + runner.advance_until_stack_empty(); + + let p1_delta = runner.state().players[P1.0 as usize].life - p1_before; + assert_eq!( + p1_delta, -3, + "P1's hand is 7, so Iron Maiden deals 7 − 4 = 3 to P1; got delta {p1_delta}" + ); +} diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index 446cbd2097..e6718a617a 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -68,6 +68,7 @@ mod chain_of_smog_copy; mod chandra_revolution_doesnt_untap_slot; mod charging_cinderhorn_issue_2868; mod chatterstorm_storm; +mod citadel_of_pain_each_player_end_step_6508; mod claim_jumper_repeat; mod cleave_text_changing_cost; mod cloud_key_chosen_type_cost;