Skip to content
Open
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
109 changes: 109 additions & 0 deletions crates/engine/src/game/coverage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4381,6 +4381,35 @@ fn static_details(stat: &StaticDefinition) -> Vec<(String, String)> {
if let Some(zone) = &stat.affected_zone {
d.push(("zone".into(), fmt_zone(zone)));
}
// CR 601.2f (#6375, MED-2): a `ModifyCost` static's payload is otherwise
// invisible to the parse-diff — `StaticMode`'s Display collapses `ModifyCost`
// to a bare "ReduceCost"/"RaiseCost"/"MinimumCost" (the `ParsedItem::label`),
// dropping `amount`, `spell_filter`, and `dynamic_count`, and `static_details`
// projected none of them. So a change to WHICH spells the reduction targets
// (Goreclaw's `spell_filter` flipping from `None` to a power-gated creature
// filter) produced a byte-identical signature and a false "No card-parse
// changes detected". Project every semantically-relevant ModifyCost field so
// the diff surfaces the change. Deterministic: `fmt_target` and `Debug` are
// stable, ordering-free encodings.
if let StaticMode::ModifyCost {
mode,
amount,
spell_filter,
dynamic_count,
} = &stat.mode
{
d.push(("cost mode".into(), format!("{mode:?}")));
d.push(("cost amount".into(), format!("{amount:?}")));
d.push((
"cost filter".into(),
spell_filter
.as_ref()
.map_or_else(|| "any".into(), fmt_target),
));
if let Some(dynamic) = dynamic_count {
d.push(("cost per".into(), format!("{dynamic:?}")));
}
}
d
}

Expand Down Expand Up @@ -10777,6 +10806,86 @@ mod tests {
use crate::types::statics::{BlockExceptionKind, ProhibitionScope};
use crate::types::zones::{EtbTapState, Zone};

/// CR 601.2f (#6375, MED-2): a `ModifyCost` static's `spell_filter` must be
/// visible in the parse-diff signature. Goreclaw's fix flipped `spell_filter`
/// from `None` (reduces ALL spells) to a power-gated creature filter, but the
/// `ParsedItem` label (Display) collapses `ModifyCost` to a bare "ReduceCost"
/// and `static_details` projected no ModifyCost payload — so both variants
/// hashed identically and CI reported "No card-parse changes detected".
///
/// Red/green: the two static_details projections differ ONLY when the
/// `spell_filter` row is emitted. Revert-probe: dropping the `static_details`
/// ModifyCost projection makes both vectors byte-identical and this fails.
#[test]
fn modify_cost_signature_exposes_spell_filter() {
use crate::types::ability::{
Comparator, FilterProp, PtStat, PtValueScope, QuantityExpr, TargetFilter, TypeFilter,
TypedFilter,
};
use crate::types::mana::ManaCost;
use crate::types::statics::{CostModifyMode, StaticMode};

let modify_cost_static = |spell_filter: Option<TargetFilter>| -> StaticDefinition {
StaticDefinition {
mode: StaticMode::ModifyCost {
mode: CostModifyMode::Reduce,
amount: ManaCost::generic(2),
spell_filter,
dynamic_count: None,
},
affected: Some(TargetFilter::Controller),
modifications: vec![],
condition: None,
per_player_condition: None,
affected_zone: None,
effect_zone: None,
active_zones: vec![],
characteristic_defining: false,
description: Some("cost reduction".to_string()),
attack_defended: None,
source_controller: None,
source_object: None,
bypass_beneficiary: None,
protection_does_not_remove: None,
}
};

// Goreclaw's real filter: creature spells with power 4 or greater.
let goreclaw_filter = TargetFilter::Typed(TypedFilter {
type_filters: vec![TypeFilter::Creature],
controller: None,
properties: vec![FilterProp::PtComparison {
stat: PtStat::Power,
scope: PtValueScope::Current,
comparator: Comparator::GE,
value: QuantityExpr::Fixed { value: 4 },
}],
});

let unfiltered = static_details(&modify_cost_static(None));
let filtered = static_details(&modify_cost_static(Some(goreclaw_filter)));

// The `spell_filter` change must surface in the projected signature.
assert_ne!(
unfiltered, filtered,
"a ModifyCost spell_filter change (None → power-gated creature filter) \
must produce a different coverage signature (issue #6375 MED-2)"
);
// Specifically, the "cost filter" row must reflect each value.
let filter_row = |d: &[(String, String)]| -> String {
d.iter()
.find(|(k, _)| k == "cost filter")
.map(|(_, v)| v.clone())
.expect("ModifyCost projection must emit a 'cost filter' row")
};
assert_eq!(filter_row(&unfiltered), "any", "None filter → \"any\"");
assert_ne!(
filter_row(&filtered),
"any",
"a power-gated creature filter must NOT project as \"any\""
);
}

#[test]
fn change_zone_signature_exposes_enters_attacking() {
// #5495: a parser change flipping `enters_attacking` (e.g. teaching
Expand Down
185 changes: 185 additions & 0 deletions crates/engine/src/game/filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3484,6 +3484,52 @@ fn spell_object_matches_property(
.get(&spell_id)
.is_some_and(|obj| !obj.kickers_paid.is_empty())
}),
// CR 208 + CR 601.2f: Power/toughness-gated cost modifiers (Goreclaw's
// "creature spells you cast with power 4 or greater cost {2} less") must read
// the LIVE spell object's P/T — the SpellCastRecord snapshot carries none.
// Resolve the spell object via the filter context and evaluate against
// object_pt_value (the same authority the live matcher uses). CR 208.3: a
// noncreature spell has power None → object_pt_value → 0, so a GE gate fails
// closed for it. Snapshot-only callers (no context / no spell_object_id) fall
// through to the fail-closed record arm, unchanged.
FilterProp::PtComparison {
stat,
scope,
comparator,
value,
} => {
let Some(context) = context else {
return false;
};
let Some(spell_id) = context.spell_object_id else {
return false;
};
let Some(spell_obj) = context.state.objects.get(&spell_id) else {
return false;
};
// A dynamic (non-`Fixed`) threshold resolves against the modifier's
// SOURCE, not the candidate spell. No in-class card needs that today
// (Goreclaw is `Fixed(4)`, and the cost-mod parser cannot yet emit a
// P/T-relative threshold), so this is the correct scope for the class;
// a future spell-relative threshold would resolve with a spell scope.
let threshold = match value {
QuantityExpr::Fixed { value } => *value,
_ => resolve_quantity(
context.state,
value,
context.source_controller,
context.source_id,
),
};
// CR 208.2a: resolve the candidate spell's off-battlefield power
// through its CDA (if any) before comparing — a dynamic-P/T creature
// spell (Tarmogoyf-class) must gate on its CDA-defined power, not the
// stored 0. A fixed-P/T spell reads its printed value unchanged.
comparator.evaluate(
spell_object_effective_pt_value(context.state, spell_obj, *stat, *scope),
threshold,
)
}
_ => spell_record_matches_property(record, prop),
}
}
Expand Down Expand Up @@ -4073,6 +4119,145 @@ fn object_pt_value(obj: &GameObject, stat: PtStat, scope: PtValueScope) -> i32 {
}
}

/// CR 208.2a + CR 613.4b: a power/toughness characteristic-defining ability
/// (CDA) functions in EVERY zone — hand, stack, graveyard — not just the
/// battlefield. A candidate spell object loaded from card data stores a printed
/// `*` P/T as `Some(0)` (`printed_cards::parse_pt` maps `PtValue::Variable`/
/// `Quantity` to 0), and the battlefield layer pass (`evaluate_layers`) only
/// recomputes P/T for battlefield objects — a hand/stack object keeps that
/// stored 0. So a power-gated cost modifier (Goreclaw: "creature spells you cast
/// with power 4 or greater cost {2} less") that read `object_pt_value` directly
/// would treat an eligible dynamic-P/T creature spell (Tarmogoyf-class) as power
/// 0 and never reduce it.
///
/// Resolve the candidate's off-battlefield P/T through the SAME authority the
/// layer pass uses, rather than re-deriving CDA evaluation here: gather the
/// continuous effects the object's own statics source
/// (`active_continuous_effects_from_static_source`), keep only the
/// characteristic-defining self-referential P/T modifications whose condition
/// currently holds — both fixed-constant *set* CDAs (`SetPower`/`SetToughness`,
/// Angry Mob's off-turn "are each 2" clause) and dynamic *set*/*add* CDAs
/// (Tarmogoyf) — resolving each dynamic magnitude with the layer pass's quantity
/// resolver (`resolve_quantity`, mirroring `apply_continuous_effect` at
/// layers.rs). Apply the layer-7a/7b *set* CDAs before the layer-7c *add* CDAs
/// (CR 613.4a: P/T characteristic-defining abilities apply first, in layer 7a).
/// A conditional CDA (Angry Mob's turn-window clauses) is gated exactly as the
/// layer pass gates it — see `self_cda` below. A fixed-P/T object (Fire
/// Elemental) sources no such effect, so this returns its stored
/// `object_pt_value` unchanged.
fn spell_object_effective_pt_value(
state: &GameState,
obj: &GameObject,
stat: PtStat,
scope: PtValueScope,
) -> i32 {
use crate::game::layers::{
active_continuous_effects_from_static_source, evaluate_condition_with_recipient,
};
use crate::game::quantity::{
continuous_modification_dynamic_quantity, quantity_expr_uses_recipient,
resolve_quantity_with_recipient,
};
use crate::types::ability::ContinuousModification;

// Stored printed value — 0 for an unevaluated `*` CDA card, the true fixed
// value for a fixed-P/T card.
let (mut power, mut toughness) = match scope {
PtValueScope::Current => (obj.power, obj.toughness),
PtValueScope::Base => (obj.base_power, obj.base_toughness),
};

let effects = active_continuous_effects_from_static_source(state, obj);
// Only the object's OWN CDA (SelfRef, characteristic-defining) functions
// off the battlefield per CR 208.2a; a non-CDA "as long as ..." dynamic P/T
// static is battlefield-only and is correctly excluded here.
// CR 611.3a + CR 613.4a: a conditional self-CDA (Angry Mob's turn-window
// clauses) functions only while its condition currently holds. Mirror the
// layer pass's per-recipient gate exactly (`apply_continuous_effect_filtered`,
// layers.rs: an effect applies to a recipient iff
// `effect.condition.is_none_or(evaluate_condition_with_recipient(.., recipient))`).
// For a self-CDA the recipient IS the source object, so `obj.id` is the
// recipient. `active_continuous_effects_from_static_source` already drops any
// def whose non-recipient-context condition fails at the source level (Angry
// Mob's `DuringYourTurn` / `Not{DuringYourTurn}` are pre-filtered there); this
// re-check additionally honors a RETAINED recipient-context condition so a
// conditional constant- or dynamic-P/T CDA is never applied outside its
// window. `None` condition (Tarmogoyf, Fire Elemental) passes unchanged.
let self_cda = |e: &&crate::types::layers::ActiveContinuousEffect| {
e.characteristic_defining
&& matches!(e.affected_filter, TargetFilter::SelfRef)
&& e.condition.as_ref().is_none_or(|condition| {
evaluate_condition_with_recipient(
state,
condition,
e.controller,
e.source_id,
obj.id,
)
})
};
// Mirror `apply_continuous_effect`'s recipient dispatch: a self-CDA's
// recipient IS its source, so both resolvers see the same object.
let resolve = |m: &ContinuousModification| -> Option<i32> {
let expr = continuous_modification_dynamic_quantity(m)?;
Some(if quantity_expr_uses_recipient(expr) {
resolve_quantity_with_recipient(state, expr, obj.controller, obj.id, obj.id)
} else {
resolve_quantity(state, expr, obj.controller, obj.id)
})
};

// CR 613.4a (layer 7a) + CR 613.4b (layer 7b): *set* CDAs establish the base
// P/T first. Both the fixed-constant setters (Angry Mob's off-turn "power and
// toughness are each 2" clause parses to `SetPower { value }` /
// `SetToughness { value }`) and the dynamic setters (Tarmogoyf) are applied in
// this single set stage, before the layer-7c *add* loop below — mirroring the
// layer authority, which lists all six P/T-set variants together (layers.rs
// ~5187-5192) and applies them in one combined stage in timestamp/def/mod
// order (the order this iteration over `effects` preserves for same-source
// self-CDAs).
for effect in effects.iter().filter(self_cda) {
match &effect.modification {
// CR 613.4a: fixed-constant set CDA — defines P/T directly, no
// quantity resolution.
ContinuousModification::SetPower { value } => power = Some(*value),
ContinuousModification::SetToughness { value } => toughness = Some(*value),
ContinuousModification::SetDynamicPower { .. }
| ContinuousModification::SetPowerDynamic { .. } => {
if let Some(v) = resolve(&effect.modification) {
power = Some(v);
}
}
ContinuousModification::SetDynamicToughness { .. }
| ContinuousModification::SetToughnessDynamic { .. } => {
if let Some(v) = resolve(&effect.modification) {
toughness = Some(v);
}
}
_ => {}
}
}
// CR 613.4c (layer 7c): dynamic *add* CDAs (e.g. a life-total CDA) stack on
// top of the set base.
for effect in effects.iter().filter(self_cda) {
match &effect.modification {
ContinuousModification::AddDynamicPower { .. } => {
if let Some(v) = resolve(&effect.modification) {
power = Some(power.unwrap_or(0) + v);
}
}
ContinuousModification::AddDynamicToughness { .. } => {
if let Some(v) = resolve(&effect.modification) {
toughness = Some(toughness.unwrap_or(0) + v);
}
}
_ => {}
}
}

pt_value_from_pair(stat, power, toughness)
}

fn zone_change_pt_value(record: &ZoneChangeRecord, stat: PtStat, scope: PtValueScope) -> i32 {
match scope {
PtValueScope::Current => pt_value_from_pair(stat, record.power, record.toughness),
Expand Down
47 changes: 39 additions & 8 deletions crates/engine/src/parser/oracle_static/static_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,13 +132,35 @@ fn strip_cost_mod_mana_value_qualifier(prefix: &str) -> (&str, Option<FilterProp
}
}

/// Compose an optional mana-value `FilterProp::Cmc` (from
/// `strip_cost_mod_mana_value_qualifier`) into the cost-modifier spell filter.
/// CR 208.1 + CR 601.2f: Peel a trailing "with power N or greater/less" spell-selection
/// qualifier off a cost-modifier subject (Goreclaw). Mirrors the mana-value qualifier;
/// reuses the canonical P/T-comparison combinator so GE/LE/EQ are all covered. Covers the
/// whole power-gated cost-modifier class, not one card.
fn parse_cost_mod_power_qualifier(prefix: &str) -> OracleResult<'_, (&str, FilterProp)> {
let (rest, before) = take_until(" with power ").parse(prefix)?;
let (rest, prop) = nom_filter::parse_pt_comparison(rest.trim_start())?;
Ok((rest, (before, prop)))
}
fn strip_cost_mod_power_qualifier(prefix: &str) -> Option<(&str, Option<FilterProp>)> {
if take_until::<_, _, OracleError<'_>>(" with power ")
.parse(prefix)
.is_err()
{
return Some((prefix, None));
}

let (rest, (before, prop)) = parse_cost_mod_power_qualifier(prefix).ok()?;
rest.trim().is_empty().then_some((before, Some(prop)))
}

/// Compose an optional qualifier `FilterProp` — a mana-value `Cmc` (from
/// `strip_cost_mod_mana_value_qualifier`) or a `PtComparison` power gate (from
/// `strip_cost_mod_power_qualifier`) — into the cost-modifier spell filter.
/// A typed filter absorbs the prop directly; an `Or` (or any non-`Typed`)
/// filter is `And`-wrapped with a card+prop leaf; a bare mana-value gate with no
/// type restriction ("spells you cast with mana value 4 or greater") becomes a
/// card filter carrying the prop.
fn compose_cost_mod_mana_value(
/// filter is `And`-wrapped with a card+prop leaf; a bare gate with no type
/// restriction ("spells you cast with mana value 4 or greater") becomes a card
/// filter carrying the prop. `None` is identity (no qualifier present).
fn compose_cost_mod_qualifier_prop(
filter: Option<TargetFilter>,
prop: Option<FilterProp>,
) -> Option<TargetFilter> {
Expand Down Expand Up @@ -682,6 +704,12 @@ pub(crate) fn try_parse_cost_modification(
// the type words and the whole type+MV restriction is dropped (The Scarlet
// Witch reduced EVERY spell, not just instants/sorceries — #5606).
let (without_chosen, mana_value_prop) = strip_cost_mod_mana_value_qualifier(without_chosen);
// CR 208.1 + CR 601.2f: Peel a trailing "with power N or greater/less" gate
// (Goreclaw: "Creature spells you cast with power 4 or greater cost {2} less")
// the same way. Without peeling, the gate sits after the "spells you cast"
// infix and blocks the type trims, dropping the whole type+power restriction
// (spell_filter came out null and EVERY spell was reduced — #6375).
let (without_chosen, power_prop) = strip_cost_mod_power_qualifier(without_chosen)?;
let type_desc = without_chosen
.trim_end_matches(" you cast") // allow-noncombinator: moved legacy static parser code; refactor-only split preserves behavior.
.trim_end_matches(" your opponents cast") // allow-noncombinator: moved legacy static parser code; refactor-only split preserves behavior.
Expand Down Expand Up @@ -732,8 +760,11 @@ pub(crate) fn try_parse_cost_modification(
(None, true) => Some(TargetFilter::HasChosenName),
(tf, false) => tf,
};
// CR 202.3: fold the peeled mana-value gate back into the spell filter.
compose_cost_mod_mana_value(base_filter, mana_value_prop)
// CR 202.3 + CR 208.1: fold the peeled mana-value and power gates back into
// the spell filter. MV and power are mutually exclusive on real cards, and
// composing with `None` is identity, so ordering is harmless.
let base = compose_cost_mod_qualifier_prop(base_filter, mana_value_prop);
compose_cost_mod_qualifier_prop(base, power_prop)
} else {
None
};
Expand Down
Loading
Loading