Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
3 changes: 2 additions & 1 deletion crates/engine/src/parser/oracle_effect/imperative.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8241,7 +8241,8 @@ pub(super) fn parse_counter_ast(text: &str, lower: &str) -> Option<ZoneCounterIm
// `parse_target("... spell with mana value X or less")` already scopes
// the spell phrase to the stack through the shared target parser. Keep this
// path on that building block and only apply the trailing X definition.
let target = super::apply_where_x_to_filter(target, where_x_expression.as_deref());
// CR 107.3c: fail honestly instead of fabricating a raw-text placeholder.
let target = super::apply_where_x_to_filter(target, where_x_expression.as_deref())?;
// CR 118.12: Parse "unless its controller pays {X}" for conditional counters
let unless_pay = parse_counter_unless_pay(rest)?;
Some(ZoneCounterImperativeAst::Counter {
Expand Down
293 changes: 206 additions & 87 deletions crates/engine/src/parser/oracle_effect/lower.rs

Large diffs are not rendered by default.

31 changes: 15 additions & 16 deletions crates/engine/src/parser/oracle_effect/mana.rs
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ pub(super) fn try_parse_add_mana_effect_with_context(
// `parse_mana_production_clause` so the where-X count is resolved here,
// co-located with `apply_where_x_count_expression`.
if let Some((count, color_options)) = parse_repeated_count_color_choice(clause) {
let (count, target) = apply_where_x_count_expression(count, where_x_expression.as_deref());
let (count, target) = apply_where_x_count_expression(count, where_x_expression.as_deref())?;
return Some(Effect::Mana {
produced: ManaProduction::AnyOneColor {
count,
Expand Down Expand Up @@ -297,7 +297,7 @@ pub(super) fn try_parse_add_mana_effect_with_context(

if let Some((count, rest)) = parse_mana_count_prefix(clause) {
let (count, where_x_target) =
apply_where_x_count_expression(count, where_x_expression.as_deref());
apply_where_x_count_expression(count, where_x_expression.as_deref())?;
let rest = rest.trim().trim_end_matches(['.', '"']).trim();
let rest_lower = rest.to_lowercase();

Expand Down Expand Up @@ -645,7 +645,7 @@ pub(super) fn try_parse_add_mana_effect_with_context(
.map(|(count, _)| count)
.unwrap_or(QuantityExpr::Fixed { value: 1 });
let (fallback_count, fallback_target) =
apply_where_x_count_expression(fallback_count, where_x_expression.as_deref());
apply_where_x_count_expression(fallback_count, where_x_expression.as_deref())?;

// Scan for mana production type at word boundaries using nom combinators.
let produced = scan_mana_production_type(&clause_lower, fallback_count.clone(), contribution)?;
Expand Down Expand Up @@ -1048,30 +1048,29 @@ pub(super) fn parse_mana_count_prefix(text: &str) -> Option<(QuantityExpr, &str)
))
}

/// CR 107.3c: Bind a "where X is …" mana count, or FAIL (`None`) when the
/// definition has no typed home. Never fabricates a raw-text placeholder — see
/// `apply_where_x_quantity_expression` for why such a node is dead at runtime.
pub(super) fn apply_where_x_count_expression(
count: QuantityExpr,
where_x_expression: Option<&str>,
) -> (QuantityExpr, Option<TargetFilter>) {
) -> Option<(QuantityExpr, Option<TargetFilter>)> {
match (&count, where_x_expression) {
(
QuantityExpr::Ref {
qty: QuantityRef::Variable { ref name },
},
Some(expression),
) if name.eq_ignore_ascii_case("X") => {
if let Some(count) = super::parse_where_x_quantity_expression(expression) {
return (count, where_x_expression_target_filter(expression));
}
(
QuantityExpr::Ref {
qty: QuantityRef::Variable {
name: expression.to_string(),
},
},
None,
)
// CR 107.3c: the clause DEFINES X. An unrepresentable definition is a
// PARSE FAILURE (`None`), never a raw-text placeholder: the fabricated
// `QuantityRef::Variable { name: "<oracle text>" }` is dead at runtime
// (game/quantity.rs resolves a non-`X` variable name to 0), so the mana
// clause produced ZERO mana while still reading as supported.
let count = super::parse_where_x_quantity_expression(expression)?;
Some((count, where_x_expression_target_filter(expression)))
}
_ => (count, None),
_ => Some((count, None)),
}
}

Expand Down
19 changes: 16 additions & 3 deletions crates/engine/src/parser/oracle_effect/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7960,7 +7960,18 @@ fn parse_effect_clause_inner(text: &str, ctx: &mut ParseContext) -> ParsedEffect
let (discover_tp, discover_where_x) = strip_trailing_where_x(tp);
if let Some((discover_player, limit, rest_orig)) = parse_discover_with_player(discover_tp) {
if rest_orig.trim().is_empty() {
let limit = apply_where_x_quantity_expression(limit, discover_where_x.as_deref());
// CR 107.3c: "discover X, where X is <expr>" — the clause DEFINES X.
// If the definition has no typed home, report the gap instead of
// fabricating a raw-text placeholder that resolves to 0 (a discover
// for mana value 0) while still reading as supported.
let Some(limit) = apply_where_x_quantity_expression(limit, discover_where_x.as_deref())
else {
let expression = discover_where_x.unwrap_or_default();
return parsed_clause(Effect::unimplemented(
"where_x_binding",
format!("where X is {expression}"),
));
};
return parsed_clause(Effect::Discover {
mana_value_limit: limit,
player: discover_player,
Expand Down Expand Up @@ -10719,7 +10730,8 @@ fn try_parse_reveal_until(tp: TextPair, player: TargetFilter) -> Option<ParsedEf
let (after_count_lower, raw_count) = parse_reveal_until_count(rest_lower).ok()?;
let (_, filter_text) = parse_reveal_until_active_filter_text(after_count_lower).ok()?;
let filter = build_reveal_until_filter(filter_text);
let count = apply_where_x_quantity_expression(raw_count, where_x_expression.as_deref());
// CR 107.3c: fail honestly instead of fabricating a raw-text placeholder.
let count = apply_where_x_quantity_expression(raw_count, where_x_expression.as_deref())?;
return Some(parsed_clause(Effect::RevealUntil {
player,
filter,
Expand Down Expand Up @@ -27794,7 +27806,8 @@ fn try_parse_put_zone_change_parts(
// unbounded. The expression is parsed by the shared
// `parse_where_x_quantity_expression` building block.
let where_x_expression = strip_trailing_where_x(after_put_tp).1;
let target = apply_where_x_to_filter(target, where_x_expression.as_deref());
// 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())?;
// 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`
Expand Down
6 changes: 4 additions & 2 deletions crates/engine/src/parser/oracle_effect/sequence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5057,7 +5057,8 @@ pub(super) fn parse_dig_from_among(lower: &str, original: &str) -> Option<Contin
let (parsed_filter, _) = parse_target(filter_text);
parsed_filter
};
let filter = apply_where_x_to_filter(filter, where_x_expression.as_deref());
// 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())?;

// CR 110.2a: "... under your control" routes the kept cards to the
// ability controller. Scan the FULL clause — the controller phrase
Expand Down Expand Up @@ -5158,7 +5159,8 @@ pub(super) fn parse_dig_from_among(lower: &str, original: &str) -> Option<Contin
};
// CR 202.3 + CR 107.3i: Bind the literal `X` in the filter's `Cmc` bound
// with the stripped "where X is <expression>" defining clause.
let filter = apply_where_x_to_filter(filter, where_x_expression.as_deref());
// 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())?;

// CR 110.2a + CR 708.2a/708.3: detect "under your control" / "face down" on
// the full clause for the from-among put-step.
Expand Down
162 changes: 161 additions & 1 deletion crates/engine/src/parser/oracle_effect/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ use crate::parser::parse_oracle_text;
use crate::types::ability::CardPlayMode::{Cast, Play};
use crate::types::ability::CastFromZoneDriver::{DuringResolution, LingeringPermission};
use crate::types::ability::{
AttachmentKind, ExcessRecipient, ForEachCategoryAction, PerpetualModification,
AttachmentKind, CastManaObjectScope, CastManaSpentMetric, ExcessRecipient,
ForEachCategoryAction, PerpetualModification,
};
use crate::types::card_type::CoreType;
use crate::types::mana::{ManaCost, ManaCostShard};
Expand Down Expand Up @@ -29143,6 +29144,165 @@ fn where_x_power_of_the_exiled_card_binds_exiled_card_power() {
}
}

/// CR 107.3c: the QUANTITY channel of the same "where X is …" defect #5706 fixed
/// for P/T. When the where-clause defined X with an expression the parser could
/// not type, `apply_where_x_quantity_expression` fabricated
/// `QuantityRef::Variable { name: "<raw oracle text>" }`. That node is well-typed
/// and renders as a supported dynamic quantity in the coverage report, but
/// `game/quantity.rs` (non-`"X"` `Variable` arm) resolves it through
/// `state.last_named_choice` and `.unwrap_or(0)` — so the effect read 0 (or an
/// unrelated stale number left by some earlier "choose a number").
///
/// These four expression classes each have a typed home AND a live resolver arm
/// that both already existed; the where-X interpreter simply never delegated to
/// them. Each assertion below is a regression pin: the raw-text fallback must
/// never come back.
#[test]
fn where_x_intensity_binds_source_intensity() {
// Arek, False Goldwarden: "Target opponent loses X life and you gain X life,
// where X is Arek's intensity." Self-reference is normalized to `~` upstream.
let def = parse_effect_chain(
"target opponent loses X life, where X is ~'s intensity",
AbilityKind::Spell,
);
let expected = QuantityExpr::Ref {
qty: QuantityRef::Intensity {
scope: ObjectScope::Source,
},
};
let Effect::LoseLife { amount, .. } = &*def.effect else {
panic!("expected LoseLife, got {:?}", def.effect);
};
assert_eq!(
amount, &expected,
"X must bind to Intensity{{Source}}; a raw-text Variable resolves to 0"
);
}

/// Mycelic Ballad spells the same quantity as "this spell's intensity" — the
/// possessive is the only axis that differs, so it must reach the same binding.
#[test]
fn where_x_this_spells_intensity_binds_source_intensity() {
let def = parse_effect_chain(
"you gain X life, where X is this spell's intensity",
AbilityKind::Spell,
);
let Effect::GainLife { amount, .. } = &*def.effect else {
panic!("expected GainLife, got {:?}", def.effect);
};
assert_eq!(
amount,
&QuantityExpr::Ref {
qty: QuantityRef::Intensity {
scope: ObjectScope::Source,
},
},
"\"this spell's intensity\" must reach the same Intensity binding as \"~'s intensity\""
);
}

/// Liquid Fire / Fluros of Myra's Marvels: "where X is the chosen number".
/// `QuantityRef::ChosenNumber` reads `ChosenAttribute::Number` off the source
/// object (game/quantity.rs) — the value the player actually chose.
#[test]
fn where_x_the_chosen_number_binds_chosen_number() {
let def = parse_effect_chain(
"~ deals X damage to target creature, where X is the chosen number",
AbilityKind::Spell,
);
let Effect::DealDamage { amount, .. } = &*def.effect else {
panic!("expected DealDamage, got {:?}", def.effect);
};
assert_eq!(
amount,
&QuantityExpr::Ref {
qty: QuantityRef::ChosenNumber
},
"X must bind to ChosenNumber, not a raw-text Variable"
);
}

/// Toph, Greatest Earthbender: "where X is the amount of mana spent to cast her".
/// CR 107.3c + the existing `ManaSpentToCast{SelfObject, Total}` typed home.
#[test]
fn where_x_mana_spent_to_cast_binds_mana_spent_to_cast() {
for text in [
"you gain X life, where X is the amount of mana spent to cast her",
"you gain X life, where X is the amount of mana spent to cast it",
"you gain X life, where X is the amount of mana spent to cast this spell",
] {
let def = parse_effect_chain(text, AbilityKind::Spell);
let Effect::GainLife { amount, .. } = &*def.effect else {
panic!("expected GainLife for {text:?}, got {:?}", def.effect);
};
assert_eq!(
amount,
&QuantityExpr::Ref {
qty: QuantityRef::ManaSpentToCast {
scope: CastManaObjectScope::SelfObject,
metric: CastManaSpentMetric::Total,
},
},
"X must bind to ManaSpentToCast{{Total}} for {text:?}"
);
}
}

/// CR 107.4h: "{S} … can also be used to refer to mana of any type produced by a
/// snow source spent to pay a cost." Graven Lore / Blessing of Frost / Blood on
/// the Snow: "where X is the amount of {S} spent to cast this spell".
/// `CastManaSpentMetric::FromSource` counts the mana whose PRODUCING source
/// matches the filter, so a Snow-supertype source filter is the exact model.
#[test]
fn where_x_snow_mana_spent_binds_mana_spent_from_snow_source() {
let def = parse_effect_chain(
"scry X, where X is the amount of {S} spent to cast this spell",
AbilityKind::Spell,
);
let Effect::Scry { count, .. } = &*def.effect else {
panic!("expected Scry, got {:?}", def.effect);
};
let QuantityExpr::Ref {
qty:
QuantityRef::ManaSpentToCast {
scope: CastManaObjectScope::SelfObject,
metric: CastManaSpentMetric::FromSource { source_filter },
},
} = count
else {
panic!("X must bind to ManaSpentToCast{{FromSource}}, got {count:?}");
};
assert!(
format!("{source_filter:?}").contains("Snow"),
"the source filter must select snow sources (CR 107.4h), got {source_filter:?}"
);
}

/// The convert half of the same defect: a where-X definition with NO typed home
/// must FAIL HONESTLY, not fabricate. Porcuparrot's "{T}: This creature deals X
/// damage to any target, where X is the number of times this creature has
/// mutated" has no QuantityRef and no resolver — mutation count is not modeled.
///
/// Pre-fix this lowered to `QuantityRef::Variable { name: "the number of times ~
/// has mutated" }`, which `game/quantity.rs` resolves through `last_named_choice`
/// to 0: Porcuparrot dealt ZERO damage while the coverage report called it
/// supported. The ability must lower to an Unimplemented gap so the report shows
/// red instead.
#[test]
fn where_x_unrepresentable_quantity_fails_honestly_instead_of_fabricating() {
let def = parse_effect_chain(
"~ deals X damage to any target, where X is the number of times ~ has mutated",
AbilityKind::Activated,
);
assert!(
matches!(&*def.effect, Effect::Unimplemented { .. }),
"an unrepresentable where-X definition must lower to Unimplemented (honest red), \
never to a raw-text Variable that resolves to 0 while reading as supported; \
got {:?}",
def.effect
);
}

#[test]
fn duration_preserved_with_for_each_suffix() {
// Goblin Piledriver pattern: "gets +2/+0 until end of turn for each other attacking Goblin"
Expand Down
31 changes: 18 additions & 13 deletions crates/engine/src/parser/oracle_effect/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,15 +106,18 @@ pub(crate) fn try_parse_token(_lower: &str, text: &str, ctx: &mut ParseContext)
if matches!(&count, QuantityExpr::Ref { qty: QuantityRef::Variable { ref name } } if name == "X")
{
if let Some(where_expression) = extract_token_where_x_expression(&text) {
count = super::parse_where_x_quantity_expression(&where_expression)
.or_else(|| {
// CR 107.3c: the clause DEFINES X. If the definition is not
// representable, this copy-token clause does not lower — fail the
// parse instead of fabricating a raw-text placeholder. The
// fabricated `QuantityRef::Variable { name: "<oracle text>" }` is
// well-typed but DEAD (game/quantity.rs resolves a non-`X` variable
// name to 0), so the effect copied ZERO tokens while the raw text
// 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(|| {
crate::parser::oracle_quantity::parse_cda_quantity(&where_expression)
})
.unwrap_or(QuantityExpr::Ref {
qty: QuantityRef::Variable {
name: where_expression,
},
});
})?;
}
}
return Some(Effect::CopyTokenOf {
Expand Down Expand Up @@ -703,15 +706,17 @@ fn parse_token_description_with_context(
// `parse_event_context_quantity` only fires when `parse_cda_quantity`
// returns None and itself returns None for unrecognized phrases, so
// it strictly widens coverage without disturbing existing matches.
// CR 122.1 + CR 608.2c: bind the deferred "a number of" count to the
// quantity its "equal to <expr>" clause names. An unrepresentable
// expression FAILS the token clause — the raw-text placeholder it used
// to fabricate is dead at runtime (game/quantity.rs resolves a non-`X`
// variable name to 0), so the card created ZERO tokens while still
// reading as supported.
count = crate::parser::oracle_quantity::parse_cda_quantity(&count_expression)
.or_else(|| {
crate::parser::oracle_quantity::parse_event_context_quantity(&count_expression)
})
.unwrap_or(QuantityExpr::Ref {
qty: QuantityRef::Variable {
name: count_expression,
},
});
.or_else(|| super::parse_where_x_quantity_expression(&count_expression))?;
}
}

Expand Down
Loading
Loading