Skip to content
Merged
130 changes: 125 additions & 5 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,64 @@ fn condition_refs_source_object(condition: &AbilityCondition) -> bool {
}
}

fn generic_effect_has_source_counter_quantity_condition(effect: &Effect) -> bool {
matches!(
effect,
Effect::GenericEffect {
static_abilities,
..
} if static_abilities.iter().any(|definition| {
matches!(
&definition.condition,
Some(StaticCondition::QuantityComparison {
lhs: QuantityExpr::Ref {
qty: QuantityRef::CountersOn {
scope: ObjectScope::Source,
..
},
},
..
})
)
})
)
}

/// CR 122.1 + CR 608.2c: The bare recipient pronoun in a leading
/// `if it has … counter on it,` gate follows the prior chosen target. This is
/// the sole authority for that lexical form after its static definitions have
/// been finalized; explicit source subjects never reach this helper.
fn rewrite_generic_effect_source_counter_quantity_condition_to_recipient(effect: &mut Effect) {
let Effect::GenericEffect {
static_abilities, ..
} = effect
else {
return;
};

for definition in static_abilities {
let is_source_counter_quantity = matches!(
&definition.condition,
Some(StaticCondition::QuantityComparison {
lhs: QuantityExpr::Ref {
qty: QuantityRef::CountersOn {
scope: ObjectScope::Source,
..
},
},
..
})
);
if !is_source_counter_quantity {
continue;
}
definition.condition = definition
.condition
.take()
.map(crate::parser::oracle_static::rebind_source_object_quantities_to_recipient);
}
}

fn condition_refs_cost_paid_object(condition: &AbilityCondition) -> bool {
match condition {
AbilityCondition::CostPaidObjectMatchesFilter { .. } => true,
Expand Down Expand Up @@ -30137,6 +30236,7 @@ pub(crate) fn parse_effect_chain_ir(
(None, Some(unless_cond)) => Some(unless_cond),
(existing, None) => existing,
};
let prior_typed_referent = chain_has_prior_typed_referent(builder.clauses(), false);
// 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 +30574,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;
// GenericEffect static counter gates are rebound after their static
// definitions are finalized below.
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 +30594,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 @@ -31047,6 +31153,16 @@ pub(crate) fn parse_effect_chain_ir(
// carries the caster default (Controller). Per D-04, this is parse-time
// pronoun resolution that belongs in IR production.
let mut clause = clause;
if prior_typed_referent
&& generic_effect_has_source_counter_quantity_condition(&clause.effect)
&& crate::parser::oracle_nom::condition::is_leading_if_bare_recipient_counter_condition(
normalized_text,
)
{
rewrite_generic_effect_source_counter_quantity_condition_to_recipient(
&mut clause.effect,
);
}
// CR 608.2c: Bind a bare anaphoric "the difference" count placeholder in
// this clause against the two operands its own leading `QuantityCheck`
// condition established ("If the discovered card's mana value is less than
Expand Down Expand Up @@ -31266,7 +31382,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
4 changes: 3 additions & 1 deletion crates/engine/src/parser/oracle_static/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,9 @@ mod shared;
mod static_helpers;
mod type_change;

pub(crate) use shared::parse_commander_subject_filter_prefix;
pub(crate) use shared::{
parse_commander_subject_filter_prefix, rebind_source_object_quantities_to_recipient,
};

pub(crate) use dispatch::is_speed_unlock_sentence;
pub(crate) use dispatch::parse_may_look_at_face_down_filter;
Expand Down
Loading
Loading