Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 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 + CR 608.2k: 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 Down
76 changes: 76 additions & 0 deletions crates/engine/src/parser/oracle_effect/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28752,6 +28752,82 @@ fn resolve_it_pronoun_any_subject() {
assert_eq!(resolve_it_pronoun(&mut ctx), TargetFilter::SelfRef);
}

/// Issue #6507 (CR 122.1 + CR 608.2k): `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 },
}
));
// 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),
],
}));

// 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 },
}
));
}

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

#[test]
Expand Down
155 changes: 155 additions & 0 deletions crates/engine/src/parser/oracle_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22545,6 +22545,161 @@ fn azors_gateway_transform_condition_parses_with_zero_swallowed_clauses() {
assert!(transform.sub_ability.is_none());
}

/// Issue #6507 SHAPE: Gemstone Mine's sacrifice rider — "If there are no
/// mining counters on this land, sacrifice it." — must bind the bare "it" to
/// `SelfRef` (CR 608.2k: the rider's condition names the source, so the
/// pronoun anaphors to the source permanent). Pre-fix the sub-ability carried
/// `ParentTarget`, which a mana ability can never resolve (CR 605.1a — a mana
/// ability requires no target), so the rider silently no-oped at runtime.
#[test]
fn depletion_land_rider_sacrifice_binds_self_ref() {
let text = "This land enters with three mining counters on it.\n{T}, Remove a mining \
counter from this land: Add one mana of any color. If there are no mining \
counters on this land, sacrifice it.";
let parsed = parse(text, "Gemstone Mine", &[], &["Land"], &[]);

// Reach-guards: the parse succeeded with zero swallowed clauses and zero
// Unimplemented markers, so the shape assertions below are not vacuous.
assert!(
parsed.parse_warnings.is_empty(),
"expected zero parse warnings, got {:#?}",
parsed.parse_warnings
);
fn has_unimpl(def: &AbilityDefinition) -> bool {
matches!(def.effect.as_ref(), Effect::Unimplemented { .. })
|| def.sub_ability.as_deref().is_some_and(has_unimpl)
}
assert!(
!parsed.abilities.iter().any(has_unimpl),
"no Unimplemented anywhere in the parse: {:#?}",
parsed.abilities
);
assert_eq!(parsed.abilities.len(), 1);

let mana = &parsed.abilities[0];
assert!(
matches!(mana.effect.as_ref(), Effect::Mana { .. }),
"head effect must be the mana production, got {:?}",
mana.effect
);

let rider = mana.sub_ability.as_deref().expect("sacrifice sub-ability");
// CR 122.1: the gate reads the mining-counter total on the source.
assert_eq!(
rider.condition,
Some(AbilityCondition::QuantityCheck {
lhs: QuantityExpr::Ref {
qty: QuantityRef::CountersOn {
scope: ObjectScope::Source,
counter_type: Some(crate::types::counter::parse_counter_type("mining")),
},
},
comparator: Comparator::EQ,
rhs: QuantityExpr::Fixed { value: 0 },
})
);
// CR 608.2k: "sacrifice it" = sacrifice the source land itself.
assert!(
matches!(
rider.effect.as_ref(),
Effect::Sacrifice {
target: TargetFilter::SelfRef,
count: QuantityExpr::Fixed { value: 1 },
..
}
),
"the rider must sacrifice SelfRef, got {:?}",
rider.effect
);
}

/// Issue #6507 SHAPE (typed-subject trigger sibling): Last Light of Durin's
/// Day's rider — "If it has six or more quest counters on it, sacrifice it."
/// — must bind to `SelfRef`, not `TriggeringSource` (pre-fix the anaphor
/// rewriter bound the gated sacrifice to the triggering Mountain). The head
/// clause's explicit "on this enchantment" binding must be untouched (guards
/// against over-rewrite).
#[test]
fn typed_trigger_source_counter_rider_binds_self_ref_not_triggering_source() {
let text = "Whenever a Mountain you control enters, put a quest counter on this \
enchantment. If it has six or more quest counters on it, sacrifice it. If you \
do, search your hand and/or library for a Dragon card and put it onto the \
battlefield. If you search your library this way, shuffle.";
let parsed = parse(
text,
"Last Light of Durin's Day",
&[],
&["Enchantment"],
&[],
);

// Reach-guards: full trigger parse, zero warnings, zero Unimplemented.
assert!(
parsed.parse_warnings.is_empty(),
"expected zero parse warnings, got {:#?}",
parsed.parse_warnings
);
fn has_unimpl(def: &AbilityDefinition) -> bool {
matches!(def.effect.as_ref(), Effect::Unimplemented { .. })
|| def.sub_ability.as_deref().is_some_and(has_unimpl)
}
assert_eq!(parsed.triggers.len(), 1);
let execute = parsed.triggers[0]
.execute
.as_deref()
.expect("trigger must carry an execute chain");
assert!(
!has_unimpl(execute),
"no Unimplemented anywhere in the trigger chain: {execute:#?}"
);

// Head clause: the explicit "on this enchantment" subject stays SelfRef.
assert!(
matches!(
execute.effect.as_ref(),
Effect::PutCounter {
target: TargetFilter::SelfRef,
..
}
),
"the head counter placement must stay on the source, got {:?}",
execute.effect
);

let rider = execute
.sub_ability
.as_deref()
.expect("sacrifice sub-ability");
// CR 122.1: source-scoped quest-counter threshold gate.
assert_eq!(
rider.condition,
Some(AbilityCondition::QuantityCheck {
lhs: QuantityExpr::Ref {
qty: QuantityRef::CountersOn {
scope: ObjectScope::Source,
counter_type: Some(crate::types::counter::parse_counter_type("quest")),
},
},
comparator: Comparator::GE,
rhs: QuantityExpr::Fixed { value: 6 },
})
);
// CR 608.2k: the gated "sacrifice it" anaphors to the source enchantment,
// NOT the triggering Mountain.
assert!(
matches!(
rider.effect.as_ref(),
Effect::Sacrifice {
target: TargetFilter::SelfRef,
count: QuantityExpr::Fixed { value: 1 },
..
}
),
"the rider must sacrifice SelfRef (the source), got {:?}",
rider.effect
);
}

/// CR 104.2b + CR 104.3e + CR 114.1 + CR 611.3a + CR 205.3j: Gideon of the
/// Trials (verbatim MTGJSON Oracle text) — the third loyalty ability creates
/// an emblem carrying BOTH game-outcome locks, each gated on an `IsPresent`
Expand Down
Loading
Loading