diff --git a/crates/engine/src/game/casting.rs b/crates/engine/src/game/casting.rs index 02ca47fd4e..0e112b4a4f 100644 --- a/crates/engine/src/game/casting.rs +++ b/crates/engine/src/game/casting.rs @@ -17321,6 +17321,16 @@ pub(crate) fn resolve_non_self_discard_requirement( player: PlayerId, source_id: ObjectId, cost: &AbilityCost, +) -> Result)>, EngineError> { + resolve_non_self_discard_requirement_with_ability(state, player, source_id, cost, None) +} + +pub(crate) fn resolve_non_self_discard_requirement_with_ability( + state: &GameState, + player: PlayerId, + source_id: ObjectId, + cost: &AbilityCost, + ability: Option<&ResolvedAbility>, ) -> Result)>, EngineError> { // The activation/casting path handles ANY `FromHand` discard selection mode; the // mana-ability path (see `mana_abilities::discard_cost_choice`) is the only caller @@ -17334,7 +17344,12 @@ pub(crate) fn resolve_non_self_discard_requirement( if count == 0 { return Ok(None); } - let eligible = find_eligible_discard_targets(state, player, source_id, filter); + let eligible = ability.map_or_else( + || find_eligible_discard_targets(state, player, source_id, filter), + |ability| { + find_eligible_discard_targets_for_ability(state, player, source_id, filter, ability) + }, + ); if eligible.len() < count { return Err(EngineError::ActionNotAllowed( "Not enough cards in hand to discard".into(), @@ -17628,7 +17643,7 @@ fn find_eligible_hand_cost_targets( source: ObjectId, filter: Option<&TargetFilter>, ) -> Vec { - let effective_filter = super::cost_payability::exile_cost_effective_filter(filter); + let effective_filter = super::cost_payability::cost_filter_before_x_announcement(filter); let filter_ref = effective_filter.as_ref(); let ctx = super::filter::FilterContext::from_source(state, source); state @@ -17659,6 +17674,36 @@ pub(crate) fn find_eligible_discard_targets( find_eligible_hand_cost_targets(state, player, source, filter) } +/// CR 118.3 + CR 602.2b: Select the hand cards that can pay an activated +/// ability's discard cost by excluding the source and applying its optional +/// filter against the announced ability context. +pub(crate) fn find_eligible_discard_targets_for_ability( + state: &GameState, + player: PlayerId, + source: ObjectId, + filter: Option<&TargetFilter>, + ability: &ResolvedAbility, +) -> Vec { + let ctx = super::filter::FilterContext::from_ability(ability); + state + .players + .get(player.0 as usize) + .map(|player_state| { + player_state + .hand + .iter() + .copied() + .filter(|&id| { + id != source + && filter.is_none_or(|filter| { + super::filter::matches_target_filter(state, id, filter, &ctx) + }) + }) + .collect() + }) + .unwrap_or_default() +} + /// CR 701.20a + CR 601.2b: Eligible cards for an `AbilityCost::Reveal` payment /// whose `filter` is `Some` (a non-self reveal). The source spell is never a /// legal choice for its own additional cost, mirroring discard/exile. @@ -17683,7 +17728,7 @@ pub(crate) fn find_eligible_exile_for_cost_targets( zone: ExileCostSourceZone, filter: Option<&TargetFilter>, ) -> Vec { - let effective_filter = super::cost_payability::exile_cost_effective_filter(filter); + let effective_filter = super::cost_payability::cost_filter_before_x_announcement(filter); let filter_ref = effective_filter.as_ref(); match zone { ExileCostSourceZone::Hand => { @@ -19130,9 +19175,13 @@ pub fn handle_activate_ability( // Courier's "Discard your hand" on an empty hand) is paid by doing nothing — the // helper returns `Ok(None)` so we FALL THROUGH to the following cost detection // rather than surfacing a dead `PayCost { count: 0 }`. - if let Some((count, eligible)) = - resolve_non_self_discard_requirement(state, player, source_id, cost)? - { + if let Some((count, eligible)) = resolve_non_self_discard_requirement_with_ability( + state, + player, + source_id, + cost, + Some(&resolved), + )? { let mut pending_discard = PendingCast::new(source_id, CardId(0), resolved, ManaCost::NoCost); pending_discard.activation_cost = Some(cost.clone()); diff --git a/crates/engine/src/game/casting_costs.rs b/crates/engine/src/game/casting_costs.rs index 82ca0ed971..be3da65273 100644 --- a/crates/engine/src/game/casting_costs.rs +++ b/crates/engine/src/game/casting_costs.rs @@ -4431,7 +4431,13 @@ pub(crate) fn surface_next_unpaid_interactive_activation_cost( // helper returns `Ok(None)` so we FALL THROUGH to the next unpaid leg (the sacrifice arm // below) rather than surfacing a dead `PayCost { count: 0 }`. if let Some((count, eligible)) = - super::casting::resolve_non_self_discard_requirement(state, player, source_id, cost)? + super::casting::resolve_non_self_discard_requirement_with_ability( + state, + player, + source_id, + cost, + Some(&pending.ability), + )? { return Ok(Some(WaitingFor::PayCost { player, @@ -4607,7 +4613,7 @@ pub(crate) fn surface_next_unpaid_interactive_activation_cost( if let Some((count, exile_filter)) = super::casting::find_battlefield_exile_cost(cost) { let effective_filter = - super::cost_payability::exile_cost_effective_filter(Some(exile_filter)); + super::cost_payability::cost_filter_before_x_announcement(Some(exile_filter)); let eligible = super::cost_payability::eligible_exile_cost_objects( state, player, @@ -7425,7 +7431,7 @@ fn pay_additional_cost_with_source( == Zone::Battlefield => { let effective_filter = - super::cost_payability::exile_cost_effective_filter(filter.as_ref()); + super::cost_payability::cost_filter_before_x_announcement(filter.as_ref()); let eligible = super::cost_payability::eligible_exile_cost_objects( state, player, @@ -7817,6 +7823,17 @@ fn additional_cost_x_max( AbilityCost::PayEnergy { amount } if amount.contains_x() => { Some(state.players[player.0 as usize].energy) } + AbilityCost::Discard { + filter: Some(filter), + .. + } if super::cost_payability::target_filter_has_x_mana_value_constraint(filter) => Some( + super::casting::find_eligible_discard_targets(state, player, source_id, Some(filter)) + .into_iter() + .filter_map(|object_id| state.objects.get(&object_id)) + .map(|object| object.effective_mana_value()) + .max() + .unwrap_or(0), + ), AbilityCost::Sacrifice(cost) if cost.requirement == SacrificeRequirement::Count { count: u32::MAX } => { @@ -7931,11 +7948,58 @@ fn cost_needs_activation_x_announcement(cost: &AbilityCost) -> bool { match cost { AbilityCost::RemoveCounter { count, .. } => is_chosen_remove_counter_cost_count(*count), AbilityCost::PayEnergy { amount } => amount.contains_x(), + AbilityCost::Discard { + filter: Some(filter), + .. + } => super::cost_payability::target_filter_has_x_mana_value_constraint(filter), AbilityCost::Composite { costs } => costs.iter().any(cost_needs_activation_x_announcement), _ => false, } } +/// CR 107.3a + CR 601.2b: Once X is announced, a discard cost whose card +/// filter references X must have enough matching cards before target selection +/// can proceed. This preserves the all-or-nothing cast proposal when the +/// chosen value is within the numeric maximum but absent from the hand. +pub(crate) fn activation_cost_is_payable_after_x_choice( + state: &GameState, + player: PlayerId, + source_id: ObjectId, + cost: &AbilityCost, + ability: &ResolvedAbility, +) -> bool { + match cost { + AbilityCost::Discard { + count, + filter, + self_scope, + .. + } if !self_scope.is_source_card() => { + let count = super::quantity::resolve_quantity_with_targets(state, count, ability).max(0) + as usize; + super::casting::find_eligible_discard_targets_for_ability( + state, + player, + source_id, + filter.as_ref(), + ability, + ) + .len() + >= count + } + AbilityCost::Composite { costs } => costs.iter().all(|cost| { + activation_cost_is_payable_after_x_choice(state, player, source_id, cost, ability) + }), + AbilityCost::OneOf { costs } => costs.iter().any(|cost| { + activation_cost_is_payable_after_x_choice(state, player, source_id, cost, ability) + }), + AbilityCost::PerCounter { base, .. } => { + activation_cost_is_payable_after_x_choice(state, player, source_id, base, ability) + } + _ => true, + } +} + fn cost_has_targeted_symbolic_counter_removal(cost: &AbilityCost) -> bool { match cost { AbilityCost::RemoveCounter { count, target, .. } => { diff --git a/crates/engine/src/game/cost_payability.rs b/crates/engine/src/game/cost_payability.rs index 6a99d48f59..884c75ef89 100644 --- a/crates/engine/src/game/cost_payability.rs +++ b/crates/engine/src/game/cost_payability.rs @@ -32,7 +32,7 @@ use crate::types::GameState; use super::filter::{matches_target_filter, matches_target_filter_in_owner_zone, FilterContext}; -fn is_pitch_bound_cmc_eq_x_prop(prop: &FilterProp) -> bool { +fn is_x_mana_value_constraint(prop: &FilterProp) -> bool { matches!( prop, FilterProp::Cmc { @@ -44,16 +44,17 @@ fn is_pitch_bound_cmc_eq_x_prop(prop: &FilterProp) -> bool { ) } -/// True when a cost filter uses the Shoal pattern: "with mana value X" where X -/// is defined by the card chosen to pay the cost, not by a prior announcement. -pub(crate) fn target_filter_has_pitch_bound_x(filter: &TargetFilter) -> bool { +/// True when a cost filter contains a variable mana-value equality. +pub(crate) fn target_filter_has_x_mana_value_constraint(filter: &TargetFilter) -> bool { match filter { - TargetFilter::Typed(tf) => tf.properties.iter().any(is_pitch_bound_cmc_eq_x_prop), + TargetFilter::Typed(tf) => tf.properties.iter().any(is_x_mana_value_constraint), TargetFilter::Or { filters } | TargetFilter::And { filters } => { - filters.iter().any(target_filter_has_pitch_bound_x) + filters + .iter() + .any(target_filter_has_x_mana_value_constraint) } TargetFilter::Not { filter } | TargetFilter::TrackedSetFiltered { filter, .. } => { - target_filter_has_pitch_bound_x(filter) + target_filter_has_x_mana_value_constraint(filter) } TargetFilter::ExiledCardByIndex { .. } | TargetFilter::None @@ -108,26 +109,26 @@ pub(crate) fn target_filter_has_pitch_bound_x(filter: &TargetFilter) -> bool { } } -pub(crate) fn relax_pitch_bound_x_filter(filter: &TargetFilter) -> TargetFilter { +pub(crate) fn relax_x_mana_value_constraint(filter: &TargetFilter) -> TargetFilter { match filter { TargetFilter::Typed(tf) => TargetFilter::Typed(TypedFilter { properties: tf .properties .iter() - .filter(|p| !is_pitch_bound_cmc_eq_x_prop(p)) + .filter(|p| !is_x_mana_value_constraint(p)) .cloned() .collect(), ..tf.clone() }), TargetFilter::ExiledCardByIndex { .. } => filter.clone(), TargetFilter::Or { filters } => TargetFilter::Or { - filters: filters.iter().map(relax_pitch_bound_x_filter).collect(), + filters: filters.iter().map(relax_x_mana_value_constraint).collect(), }, TargetFilter::And { filters } => TargetFilter::And { - filters: filters.iter().map(relax_pitch_bound_x_filter).collect(), + filters: filters.iter().map(relax_x_mana_value_constraint).collect(), }, TargetFilter::Not { filter } => TargetFilter::Not { - filter: Box::new(relax_pitch_bound_x_filter(filter)), + filter: Box::new(relax_x_mana_value_constraint(filter)), }, TargetFilter::TrackedSetFiltered { id, @@ -135,7 +136,7 @@ pub(crate) fn relax_pitch_bound_x_filter(filter: &TargetFilter) -> TargetFilter caused_by, } => TargetFilter::TrackedSetFiltered { id: *id, - filter: Box::new(relax_pitch_bound_x_filter(filter)), + filter: Box::new(relax_x_mana_value_constraint(filter)), caused_by: *caused_by, }, TargetFilter::None @@ -190,12 +191,14 @@ pub(crate) fn relax_pitch_bound_x_filter(filter: &TargetFilter) -> TargetFilter } } -/// CR 107.3a + CR 118.9: Until the player chooses the pitched card, relax the -/// CMC=X constraint for 601.2b eligibility on Shoal-style exile costs. -pub(crate) fn exile_cost_effective_filter(filter: Option<&TargetFilter>) -> Option { +/// CR 107.3a + CR 601.2b: Before X is announced, relax its equality constraint +/// when checking which cards can pay a cost. +pub(crate) fn cost_filter_before_x_announcement( + filter: Option<&TargetFilter>, +) -> Option { filter.map(|f| { - if target_filter_has_pitch_bound_x(f) { - relax_pitch_bound_x_filter(f) + if target_filter_has_x_mana_value_constraint(f) { + relax_x_mana_value_constraint(f) } else { f.clone() } @@ -381,12 +384,13 @@ impl AbilityCost { } let resolved = super::quantity::resolve_quantity(state, count, player, source).max(0) as usize; + let effective_filter = cost_filter_before_x_announcement(filter.as_ref()); let ctx = FilterContext::from_source(state, source); p.hand .iter() .filter(|&&id| { id != source - && filter + && effective_filter .as_ref() .is_none_or(|f| matches_target_filter(state, id, f, &ctx)) }) @@ -427,7 +431,7 @@ impl AbilityCost { }; } let zone = exile_cost_effective_zone(*zone, filter.as_ref()); - let effective_filter = exile_cost_effective_filter(filter.as_ref()); + let effective_filter = cost_filter_before_x_announcement(filter.as_ref()); eligible_exile_cost_objects( state, player, @@ -841,7 +845,7 @@ pub(super) fn eligible_exile_cost_objects( .collect(); } }; - let effective_filter = exile_cost_effective_filter(filter); + let effective_filter = cost_filter_before_x_announcement(filter); let filter_ref = effective_filter.as_ref(); let ctx = FilterContext::from_source(state, source); ids.filter(|&id| { diff --git a/crates/engine/src/game/engine.rs b/crates/engine/src/game/engine.rs index da6e9eb192..e5f8450fed 100644 --- a/crates/engine/src/game/engine.rs +++ b/crates/engine/src/game/engine.rs @@ -8958,14 +8958,32 @@ fn apply_action( let player = *player; let convoke_mode = *convoke_mode; if let Some(pending) = state.pending_cast.as_ref() { + // CR 602.2b + CR 601.2b/h: An activation's announced X must + // make its full cost payable before the announcement commits, + // whether or not the ability has deferred targets. + let mut trial = pending.as_ref().clone(); + trial.ability.set_chosen_x_recursive(value); + trial.cost.concretize_x(value); + if trial.activation_ability_index.is_some() + && trial.activation_cost.as_ref().is_some_and(|cost| { + !casting_costs::activation_cost_is_payable_after_x_choice( + state, + player, + trial.object_id, + cost, + &trial.ability, + ) + }) + { + return Err(EngineError::InvalidAction(format!( + "X={value} cannot pay the activation cost" + ))); + } if pending.deferred_target_selection { // CR 601.2c: A chosen X that determines target count must // have a legal target assignment before it is locked into // the pending cast. // CR 601.2f: The same X value then determines the total cost. - let mut trial = pending.as_ref().clone(); - trial.ability.set_chosen_x_recursive(value); - trial.cost.concretize_x(value); let mut target_slots = build_target_slots(state, &trial.ability)?; // CR 601.2c + CR 601.2d: clamp a divided spell's slots to the // (now-known) pool so the legal-assignment probe matches what @@ -16410,7 +16428,7 @@ mod stage2_injector_tests { // // SET PRESERVATION: unchanged. Upstream adds no line matching the needle to this file and // neither does this branch — total still 37, partition still 5/7/25. - "game/engine.rs:11988".to_string(), + "game/engine.rs:12006".to_string(), ], "the five production producers, NAMED: the CR 603.5 gate in `resolve_chain_body` \ plus the two repeated-optional-payment drivers, the per-player acceptance cursor \ diff --git a/crates/engine/src/parser/oracle_cost.rs b/crates/engine/src/parser/oracle_cost.rs index 16e27b1948..9a23ee7b84 100644 --- a/crates/engine/src/parser/oracle_cost.rs +++ b/crates/engine/src/parser/oracle_cost.rs @@ -840,7 +840,10 @@ pub fn parse_single_cost(text: &str) -> AbilityCost { self_scope: crate::types::ability::DiscardSelfScope::SourceCard, }; } - if nom_on_lower(rest, &rest_lower, |i| value((), tag("a card")).parse(i)).is_some() { + if all_consuming(tag::<_, _, nom::error::Error<&str>>("a card")) + .parse(rest_lower.as_str()) + .is_ok() + { return AbilityCost::Discard { count: QuantityExpr::Fixed { value: 1 }, filter: None, @@ -2863,6 +2866,29 @@ mod tests { } } + /// CR 107.3a + CR 701.9a: the shared X in an activated discard cost is + /// retained as a typed mana-value filter rather than swallowed as "a card". + #[test] + fn cost_discard_card_with_mana_value_x() { + use crate::types::ability::{Comparator, FilterProp, QuantityExpr, QuantityRef}; + + match parse_oracle_cost("Discard a card with mana value X") { + AbilityCost::Discard { + filter: Some(TargetFilter::Typed(typed)), + .. + } => assert!(typed.properties.iter().any(|property| matches!( + property, + FilterProp::Cmc { + comparator: Comparator::EQ, + value: QuantityExpr::Ref { + qty: QuantityRef::Variable { name } + } + } if name == "X" + ))), + other => panic!("expected discard with CmcEQ(X), got {other:?}"), + } + } + #[test] fn cost_exile_colored_card_from_hand() { match parse_oracle_cost("Exile a blue card from your hand") { diff --git a/crates/engine/src/parser/oracle_effect/imperative.rs b/crates/engine/src/parser/oracle_effect/imperative.rs index 1442ed651f..7278021a5d 100644 --- a/crates/engine/src/parser/oracle_effect/imperative.rs +++ b/crates/engine/src/parser/oracle_effect/imperative.rs @@ -1311,12 +1311,26 @@ fn parse_discard_unless_filter<'a>( /// been consumed by `parse_count_expr`. So for "discard two creature cards" /// the count parser eats "two " and hands "creature cards" here. For "a card" /// (count = 1, no type qualifier) the count parser eats "a " and hands -/// "card" here, which has no leading type word and returns `None`. +/// "card" here. A bare `TypeFilter::Card` is intentionally discarded because +/// every object in a hand is a card. /// /// Mirrors `AbilityCost::Discard.filter` so the trigger-effect discard on /// Dokuchi Silencer ("you may discard a creature card") preserves the same /// filter data as cost-form discards like "Discard a creature card:". pub(crate) fn parse_discard_card_filter(tail: &str) -> Option { + let (filter, remainder) = parse_type_phrase(tail); + let is_bare_card = matches!( + &filter, + TargetFilter::Typed(TypedFilter { + type_filters, + controller: None, + properties, + }) if type_filters == &[TypeFilter::Card] && properties.is_empty() + ); + if remainder.trim().is_empty() && !matches!(filter, TargetFilter::Any) && !is_bare_card { + return Some(filter); + } + // Find the " card" / " cards" suffix — the type phrase lies before it. // No suffix or empty before-suffix → no type qualifier. let type_phrase = tail diff --git a/crates/engine/tests/integration/issue_6908_kozilek_discard_mana_value.rs b/crates/engine/tests/integration/issue_6908_kozilek_discard_mana_value.rs new file mode 100644 index 0000000000..95e4dd0aac --- /dev/null +++ b/crates/engine/tests/integration/issue_6908_kozilek_discard_mana_value.rs @@ -0,0 +1,99 @@ +//! Kozilek, the Great Distortion — a discard cost's X must be announced before +//! target selection and bind both the target spell and the discarded card. + +use engine::game::scenario::{GameScenario, P0, P1}; +use engine::game::zones::move_to_zone; +use engine::types::actions::GameAction; +use engine::types::game_state::{CastingVariant, StackEntry, StackEntryKind}; +use engine::types::mana::ManaCost; +use engine::types::zones::Zone; + +const KOZILEK_COUNTER_ABILITY: &str = + "Discard a card with mana value X: Counter target spell with mana value X."; + +/// CR 107.3a + CR 601.2b/c + CR 602.2b: X in an activation cost is announced +/// before selecting targets, then the same value restricts both the target and +/// the discarded card. +#[test] +fn kozilek_discards_a_card_matching_announced_x_to_counter_a_spell() { + let mut scenario = GameScenario::new(); + let kozilek = scenario + .add_creature_from_oracle( + P0, + "Kozilek, the Great Distortion", + 12, + 12, + KOZILEK_COUNTER_ABILITY, + ) + .id(); + let discard = scenario + .add_spell_to_hand(P0, "Mana Value Three Discard", false) + .with_mana_cost(ManaCost::generic(3)) + .id(); + let target = scenario + .add_spell_to_hand(P1, "Mana Value Three Target", false) + .with_mana_cost(ManaCost::generic(3)) + .id(); + let mut runner = scenario.build(); + + { + let state = runner.state_mut(); + let card_id = state.objects[&target].card_id; + let mut events = Vec::new(); + move_to_zone(state, target, Zone::Stack, &mut events); + state.stack.push_back(StackEntry { + id: target, + source_id: target, + controller: P1, + kind: StackEntryKind::Spell { + card_id, + ability: None, + casting_variant: CastingVariant::Normal, + actual_mana_spent: 0, + }, + }); + } + + let outcome = runner + .activate(kozilek, 0) + .x(3) + .target_object(target) + .pay_with(&[discard]) + .resolve(); + + outcome.assert_zone(&[discard, target], Zone::Graveyard); +} + +/// CR 107.3a + CR 602.2b: every activated X choice, including a no-target +/// ability, must bind the discard filter before the activation can proceed. +#[test] +fn kozilek_rejects_an_announced_x_without_a_matching_discard() { + let mut scenario = GameScenario::new(); + let kozilek = scenario + .add_creature_from_oracle( + P0, + "Kozilek, the Great Distortion", + 12, + 12, + "Discard a card with mana value X: Draw a card.", + ) + .id(); + scenario + .add_spell_to_hand(P0, "Mana Value Three Discard", false) + .with_mana_cost(ManaCost::generic(3)); + scenario + .add_spell_to_hand(P0, "Mana Value Five Discard", false) + .with_mana_cost(ManaCost::generic(5)); + let mut runner = scenario.build(); + + runner + .act(GameAction::ActivateAbility { + source_id: kozilek, + ability_index: 0, + }) + .expect("activation must reach X announcement"); + assert!( + runner.act(GameAction::ChooseX { value: 4 }).is_err(), + "X=4 must not combine cards with mana values 3 and 5 into one legal discard cost" + ); +} diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index 51063cb3bb..63b17548e0 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -687,6 +687,7 @@ mod issue_680_shalai_upkeep_move; mod issue_6858_draw_that_many_discard; mod issue_688_mind_into_matter; mod issue_689_resonating_lute_hand_size; +mod issue_6908_kozilek_discard_mana_value; mod issue_691_sheoldred_saga_lore; mod issue_6943_faerie_slumber_party; mod issue_6979_land_mana_amplification;