Skip to content
Merged
131 changes: 125 additions & 6 deletions crates/engine/src/parser/oracle_effect/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,12 +238,53 @@ pub(crate) fn resolve_it_pronoun(ctx: &mut ParseContext) -> TargetFilter {
}
}

/// CR 122.1 + CR 608.2k: true when the expression reads a counter total on the
/// ability's own source object ("counters on ~" / "counters on this land").
/// Recognizes `CountersOn { scope: Source }` through the arithmetic wrappers so
/// a wrapped threshold still registers as source-referential. Mirrors the
/// exhaustive wrapper walk of `quantity_expr_uses_recipient`
/// (`game/quantity.rs`) so the compiler flags any future `QuantityExpr` variant.
fn quantity_expr_reads_source_counters(expr: &QuantityExpr) -> bool {
match expr {
QuantityExpr::Fixed { .. } => false,
QuantityExpr::Ref { qty } => matches!(
qty,
QuantityRef::CountersOn {
scope: ObjectScope::Source,
..
}
),
QuantityExpr::DivideRounded { inner, .. }
| QuantityExpr::Offset { inner, .. }
| QuantityExpr::ClampMin { inner, .. }
| QuantityExpr::Multiply { inner, .. } => quantity_expr_reads_source_counters(inner),
QuantityExpr::Sum { exprs } | QuantityExpr::Max { exprs } => {
exprs.iter().any(quantity_expr_reads_source_counters)
}
QuantityExpr::UpTo { max } => quantity_expr_reads_source_counters(max),
QuantityExpr::Power { exponent, .. } => quantity_expr_reads_source_counters(exponent),
QuantityExpr::Difference { left, right } => {
quantity_expr_reads_source_counters(left) || quantity_expr_reads_source_counters(right)
}
}
}

fn condition_refs_source_object(condition: &AbilityCondition) -> bool {
match condition {
AbilityCondition::SourceMatchesFilter { .. }
| AbilityCondition::SourceEnteredThisTurn
| AbilityCondition::SourceIsTapped
| AbilityCondition::SourceAttachedToCreature => true,
// CR 122.1: a counter-threshold gate scoped to the SOURCE
// ("if there are no mining counters on this land", "if it has six or
// more quest counters on it") refers to the ability's own untargeted
// source object, so a bare "it" in the gated body anaphors to the
// source permanent — Gemstone Mine / the Mercadian depletion lands
// (issue #6507), Tourach's Gate, Daredevil Dragster, Last Light of
// Durin's Day.
AbilityCondition::QuantityCheck { lhs, rhs, .. } => {
quantity_expr_reads_source_counters(lhs) || quantity_expr_reads_source_counters(rhs)
}
AbilityCondition::Not { condition }
| AbilityCondition::ConditionInstead { inner: condition } => {
condition_refs_source_object(condition)
Expand All @@ -255,6 +296,61 @@ fn condition_refs_source_object(condition: &AbilityCondition) -> bool {
}
}

/// CR 122.1 + CR 608.2c: Bind a source-defaulted counter condition to the
/// prior chosen target only for the leading bare-pronoun grammar. Conditions
/// belong to `ClauseIr` until lowering, so this is the single authority before
/// a continuous grant receives its final `StaticCondition`.
fn rebind_source_counter_condition_to_recipient(condition: &mut AbilityCondition) {
match condition {
AbilityCondition::QuantityCheck { lhs, rhs, .. } => {
rebind_source_counter_quantity_expr_to_recipient(lhs);
rebind_source_counter_quantity_expr_to_recipient(rhs);
}
AbilityCondition::Not { condition }
| AbilityCondition::ConditionInstead { inner: condition } => {
rebind_source_counter_condition_to_recipient(condition);
}
AbilityCondition::And { conditions } | AbilityCondition::Or { conditions } => {
for condition in conditions {
rebind_source_counter_condition_to_recipient(condition);
}
}
_ => {}
}
}

fn rebind_source_counter_quantity_expr_to_recipient(expr: &mut QuantityExpr) {
match expr {
QuantityExpr::Fixed { .. } => {}
QuantityExpr::Ref { qty } => {
if let QuantityRef::CountersOn { scope, .. } = qty {
if *scope == ObjectScope::Source {
*scope = ObjectScope::Recipient;
}
}
}
QuantityExpr::DivideRounded { inner, .. }
| QuantityExpr::Offset { inner, .. }
| QuantityExpr::ClampMin { inner, .. }
| QuantityExpr::Multiply { inner, .. } => {
rebind_source_counter_quantity_expr_to_recipient(inner);
}
QuantityExpr::Sum { exprs } | QuantityExpr::Max { exprs } => {
for expr in exprs {
rebind_source_counter_quantity_expr_to_recipient(expr);
}
}
QuantityExpr::UpTo { max } => rebind_source_counter_quantity_expr_to_recipient(max),
QuantityExpr::Power { exponent, .. } => {
rebind_source_counter_quantity_expr_to_recipient(exponent);
}
QuantityExpr::Difference { left, right } => {
rebind_source_counter_quantity_expr_to_recipient(left);
rebind_source_counter_quantity_expr_to_recipient(right);
}
}
}

fn condition_refs_cost_paid_object(condition: &AbilityCondition) -> bool {
match condition {
AbilityCondition::CostPaidObjectMatchesFilter { .. } => true,
Expand Down Expand Up @@ -28769,6 +28865,10 @@ pub(crate) fn parse_effect_chain_ir(
if normalized_text.is_empty() {
continue;
}
let has_bare_recipient_counter_gate =
crate::parser::oracle_nom::condition::is_leading_if_bare_recipient_counter_condition(
normalized_text,
);
if lower::recognize_zada_copy_distinct_target_rider(&normalized_text.to_ascii_lowercase()) {
continue;
}
Expand Down Expand Up @@ -30130,13 +30230,22 @@ pub(crate) fn parse_effect_chain_ir(
} else {
(None, text)
};
let condition = match (condition, unless_same_name_condition) {
let mut condition = match (condition, unless_same_name_condition) {
(Some(existing), Some(unless_cond)) => Some(AbilityCondition::And {
conditions: vec![existing, unless_cond],
}),
(None, Some(unless_cond)) => Some(unless_cond),
(existing, None) => existing,
};
let prior_typed_referent = chain_has_prior_typed_referent(builder.clauses(), false);
if prior_typed_referent
&& has_bare_recipient_counter_gate
&& condition.as_ref().is_some_and(condition_refs_source_object)
{
rebind_source_counter_condition_to_recipient(
condition.as_mut().expect("condition checked above"),
);
}
// CR 701.34a + CR 122.1: keep the whole "for each kind of counter on
// target permanent or player, give … another counter of that kind"
// clause intact so the targeted-proliferate recognizer in
Expand Down Expand Up @@ -30474,7 +30583,14 @@ pub(crate) fn parse_effect_chain_ir(
}
.or_else(|| ctx.actor.clone());
let if_you_do_anchor = if_you_do_object_anchor(builder.clauses(), &condition);
let chunk_subject = if condition.as_ref().is_some_and(condition_refs_source_object) {
// CR 608.2k: An `AbilityCondition` source-counter gate binds a bare
// body pronoun to the source only when no prior clause chose a typed
// target. This preserves the depletion-land / counter-rider class;
// the leading bare-recipient gate is instead rebound on `condition`
// before lowering above.
let binds_source_counter_pronoun =
condition.as_ref().is_some_and(condition_refs_source_object) && !prior_typed_referent;
let chunk_subject = if binds_source_counter_pronoun {
Some(TargetFilter::SelfRef)
} else {
if_you_do_anchor.clone().or_else(|| ctx.subject.clone())
Expand All @@ -30487,9 +30603,8 @@ pub(crate) fn parse_effect_chain_ir(
// this flag when the outer chain has such a referent (Brilliance Unleashed).
// It is false on every top-level and non-else nested parse, so this OR is a
// no-op for all pre-existing cards.
let parent_target_available = ctx.parent_target_available
|| if_you_do_anchor.is_some()
|| chain_has_prior_typed_referent(builder.clauses(), false);
let parent_target_available =
ctx.parent_target_available || if_you_do_anchor.is_some() || prior_typed_referent;
// CR 608.2c + CR 601.2a: a strict subset of `parent_target_available`
// restricted to chosen-target referents (Emry), excluding impulse
// publishers (Territorial Bruntar's `ExileFromTopUntil`). An "if you
Expand Down Expand Up @@ -31266,7 +31381,11 @@ pub(crate) fn parse_effect_chain_ir(
// which is scoped to exactly the reported bug class.
if condition.is_some()
&& !is_distributed_chunk
&& !condition.as_ref().is_some_and(condition_refs_source_object)
// CR 608.2k: only a GENUINE source-counter gate (no prior chosen
// target — see `binds_source_counter_pronoun`) keeps the SelfRef
// binding; a mis-scoped bare "it" over a prior typed target
// (Revelation of Power) still rewrites to the parent target here.
&& !binds_source_counter_pronoun
&& !builder.is_empty()
&& has_anaphoric_reference(&text_lower)
&& !matches!(if_you_do_anchor, Some(TargetFilter::SelfRef))
Expand Down
140 changes: 140 additions & 0 deletions crates/engine/src/parser/oracle_effect/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29361,6 +29361,146 @@ fn resolve_it_pronoun_any_subject() {
assert_eq!(resolve_it_pronoun(&mut ctx), TargetFilter::SelfRef);
}

/// Issue #6507 (CR 122.1): `condition_refs_source_object` must
/// recognize a source-scoped counter-threshold `QuantityCheck` ("if there are
/// no mining counters on this land") as source-referential — that gate is what
/// threads `SelfRef` as the chunk subject so the rider's bare "it" binds to
/// the source. Adjacent-variant hostiles: `Target`/`Recipient`-scoped counter
/// reads (the deliberately excluded "if that creature has … counters" class)
/// must stay non-source-referential.
#[test]
fn condition_refs_source_object_source_counter_quantity_check() {
fn counters_check(scope: ObjectScope) -> AbilityCondition {
AbilityCondition::QuantityCheck {
lhs: QuantityExpr::Ref {
qty: QuantityRef::CountersOn {
scope,
counter_type: Some(crate::types::counter::parse_counter_type("mining")),
},
},
comparator: Comparator::EQ,
rhs: QuantityExpr::Fixed { value: 0 },
}
}

// Positive: source-scoped counter threshold (the Gemstone Mine class).
assert!(condition_refs_source_object(&counters_check(
ObjectScope::Source
)));
// Wrapped-expression positive: the walker must see through arithmetic
// wrappers (`Offset { CountersOn { Source } }` still reads the source).
assert!(condition_refs_source_object(
&AbilityCondition::QuantityCheck {
lhs: QuantityExpr::Offset {
inner: Box::new(QuantityExpr::Ref {
qty: QuantityRef::CountersOn {
scope: ObjectScope::Source,
counter_type: None,
},
}),
offset: 1,
},
comparator: Comparator::GE,
rhs: QuantityExpr::Fixed { value: 2 },
}
));
Comment thread
matthewevans marked this conversation as resolved.
// Existing wrapper recursion still applies to the new arm.
assert!(condition_refs_source_object(&AbilityCondition::Not {
condition: Box::new(counters_check(ObjectScope::Source)),
}));
assert!(condition_refs_source_object(&AbilityCondition::And {
conditions: vec![
AbilityCondition::IsYourTurn,
counters_check(ObjectScope::Source),
],
}));
// `QuantityCheck` must inspect both sides of the comparison, not only the
// customary left-hand counter threshold.
assert!(condition_refs_source_object(
&AbilityCondition::QuantityCheck {
lhs: QuantityExpr::Fixed { value: 0 },
comparator: Comparator::EQ,
rhs: QuantityExpr::Ref {
qty: QuantityRef::CountersOn {
scope: ObjectScope::Source,
counter_type: None,
},
},
}
));

// Adjacent-variant hostiles: target-/recipient-scoped counter reads keep
// their current (non-source) binding.
assert!(!condition_refs_source_object(&counters_check(
ObjectScope::Target
)));
assert!(!condition_refs_source_object(&counters_check(
ObjectScope::Recipient
)));
// A QuantityCheck with no counter read at all stays non-source-referential.
assert!(!condition_refs_source_object(
&AbilityCondition::QuantityCheck {
lhs: QuantityExpr::Ref {
qty: QuantityRef::HandSize {
player: PlayerScope::Controller,
},
},
comparator: Comparator::GE,
rhs: QuantityExpr::Fixed { value: 7 },
}
));

// Two-operand wrapper arms: the walker must recurse into BOTH branches so a
// source-counter read hidden in either operand still registers, and a
// wrapper with neither operand reading the source stays negative. These
// arms were previously undriven by this test (only Ref/Offset were).
let source_read = QuantityExpr::Ref {
qty: QuantityRef::CountersOn {
scope: ObjectScope::Source,
counter_type: None,
},
};
// Sum { exprs } — recurse via `.any(..)`: source read in one operand → true.
assert!(condition_refs_source_object(
&AbilityCondition::QuantityCheck {
lhs: QuantityExpr::Sum {
exprs: vec![QuantityExpr::Fixed { value: 1 }, source_read.clone()],
},
comparator: Comparator::GE,
rhs: QuantityExpr::Fixed { value: 2 },
}
));
// Difference { left, right } — recurse via `left || right`: source read on
// the right operand alone still registers.
assert!(condition_refs_source_object(
&AbilityCondition::QuantityCheck {
lhs: QuantityExpr::Difference {
left: Box::new(QuantityExpr::Fixed { value: 3 }),
right: Box::new(source_read.clone()),
},
comparator: Comparator::EQ,
rhs: QuantityExpr::Fixed { value: 0 },
}
));
// Neither-operand-source Sum stays negative (no false positive).
assert!(!condition_refs_source_object(
&AbilityCondition::QuantityCheck {
lhs: QuantityExpr::Sum {
exprs: vec![
QuantityExpr::Fixed { value: 1 },
QuantityExpr::Ref {
qty: QuantityRef::HandSize {
player: PlayerScope::Controller,
},
},
],
},
comparator: Comparator::GE,
rhs: QuantityExpr::Fixed { value: 2 },
}
));
}

// --- Suffix condition extraction tests ---

#[test]
Expand Down
34 changes: 34 additions & 0 deletions crates/engine/src/parser/oracle_nom/condition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use nom::multi::many0;
use nom::sequence::{preceded, terminated};
use nom::Parser;

use super::bridge::nom_on_lower;
use super::error::{oracle_err, OracleError, OracleResult};
use super::primitives::{
parse_article, parse_color, parse_keyword_name, parse_mana_cost, parse_number,
Expand Down Expand Up @@ -2223,6 +2224,20 @@ fn parse_has_counters_axes(
Ok((rest, (subject, counters, minimum, maximum)))
}

/// CR 122.1 + CR 608.2c: identifies only a leading `if it has … counter on it,`
/// condition. The generic counter-condition parser resolves bare `it` to the
/// source by default; effect-chain parsing uses this narrow lexical fact to
/// rebind that condition when an earlier clause established a typed referent.
/// Explicit source subjects (`~`, `this creature`, `this land`) deliberately do
/// not match and retain their source scope.
pub(crate) fn is_leading_if_bare_recipient_counter_condition(input: &str) -> bool {
let lower = input.to_lowercase();
nom_on_lower(input, &lower, |i| {
terminated(preceded(tag("if "), parse_has_counters_axes), tag(",")).parse(i)
})
.is_some_and(|((subject, ..), _)| matches!(subject, CounterConditionSubject::RecipientPronoun))
}

/// Subject axis for counter-has conditions. Accepts the canonical
/// source-referential subjects, the bound pronoun `"it "`, and the
/// demonstrative anaphor `"that creature/land/permanent "` used in
Expand Down Expand Up @@ -19052,6 +19067,25 @@ mod tests {
);
}

/// CR 122.1 + CR 608.2c: effect-chain parsing may rebind only the bare
/// recipient-pronoun form after an earlier typed target. Explicit source
/// and demonstrative-recipient subjects must remain distinguishable.
#[test]
fn leading_if_bare_recipient_counter_condition_is_narrow() {
assert!(is_leading_if_bare_recipient_counter_condition(
"If it has a counter on it, it gains flying"
));
assert!(!is_leading_if_bare_recipient_counter_condition(
"If this creature has a counter on it, it gains flying"
));
assert!(!is_leading_if_bare_recipient_counter_condition(
"If that creature has a counter on it, it gains flying"
));
assert!(!is_leading_if_bare_recipient_counter_condition(
"If it is tapped, it gains flying"
));
}

/// CR 611.3a: the bound pronoun "it" in a self-referential combat-state gate
/// binds to the source permanent. Intrepid Ace's "it isn't attacking or
/// blocking" must parse to `Not(Or[SourceIsAttacking, SourceIsBlocking])`,
Expand Down
Loading
Loading