diff --git a/crates/engine/src/parser/oracle_effect/lower.rs b/crates/engine/src/parser/oracle_effect/lower.rs index b7e2581a49..5abbd9072d 100644 --- a/crates/engine/src/parser/oracle_effect/lower.rs +++ b/crates/engine/src/parser/oracle_effect/lower.rs @@ -3588,15 +3588,39 @@ pub(crate) fn target_filter_is_single_object_target(filter: &TargetFilter) -> bo } } +/// #5994: whether the per-opponent fanout slot is optional (min 0) or +/// mandatory (min 1), for verbs that fall through to this detector because +/// they aren't in `MULTI_TARGET_VERBS` (e.g. "put", "gain control of") — a +/// `MULTI_TARGET_VERBS` verb like "exile" takes its min from +/// `stripped_multi_target` upstream and never reaches this function. Scans at +/// word boundaries for an "up to N target …" quantifier anywhere in the +/// clause, not just immediately after the verb, so one detector covers every +/// non-`MULTI_TARGET_VERBS` verb instead of each needing its own hardcoded +/// prefix (the prior version only recognized "gain control of "). This does +/// NOT recognize "any number of target …" — that arm lives in +/// `strip_leading_quantifier`, which this function doesn't call; no card in +/// the per-opponent-fanout class currently uses that form. Reusing +/// `strip_optional_target_prefix` (rather than the bare `strip_leading_quantifier` +/// used by `MULTI_TARGET_VERBS`) is the safety property this relies on: it only +/// accepts a quantifier immediately followed by "target "/"other target "/ +/// "another target ", so it can't misfire on a resource-count quantifier that +/// happens to precede the object noun (e.g. "put up to three +1/+1 counters on +/// target creature" — the quantity there modifies the counters, not the +/// target, and the "target " guard declines it). fn per_opponent_target_fanout_min(text: &str) -> usize { let lower = text.to_ascii_lowercase(); - let Some((_, rest)) = nom_on_lower(text, &lower, |input| { - value((), tag("gain control of ")).parse(input) - }) else { - return 1; - }; - let (_, spec) = strip_optional_target_prefix(rest); - if spec.is_some_and(|spec| spec.min_is_fixed_zero()) { + let found_optional_target_slot = + nom_primitives::scan_at_word_boundaries(lower.as_str(), |input| { + match strip_optional_target_prefix(input) { + (rest, Some(spec)) if spec.min_is_fixed_zero() => Ok((rest, ())), + _ => Err(nom::Err::Error(OracleError::new( + input, + nom::error::ErrorKind::Fail, + ))), + } + }) + .is_some(); + if found_optional_target_slot { 0 } else { 1 diff --git a/crates/engine/src/parser/oracle_effect/tests.rs b/crates/engine/src/parser/oracle_effect/tests.rs index ca5a304bbe..19b3967dbf 100644 --- a/crates/engine/src/parser/oracle_effect/tests.rs +++ b/crates/engine/src/parser/oracle_effect/tests.rs @@ -32074,6 +32074,62 @@ fn effect_for_each_opponent_gain_control_uses_per_opponent_target_fanout() { } } +/// #5994: Riptide Gearhulk's ETB — "for each opponent, put up to one target +/// nonland permanent that player controls into its owner's library third +/// from the top." The per-opponent fanout already binds the target's +/// controller to `TargetPlayer` correctly here (`Effect::PutAtLibraryPosition` +/// is wired into `Effect::target_filter()`, and the noun phrase is structurally +/// identical to the working `GainControl`/`ChangeZone` fanout precedents), so +/// the caster-vs-opponent aliasing half of the bug was already fixed upstream. +/// What survived was `MultiTargetSpec.min`: `per_opponent_target_fanout_min` +/// only recognized the min-0 ("up to") shape after a literal "gain control of " +/// prefix, so every other per-opponent-fanout verb ("put", "exile", …) fell +/// back to `min: 1` — forcing a target from every opponent's permanents even +/// though "up to one" should allow skipping — which is the "fizzles if +/// skipped" half of the report. +#[test] +fn effect_for_each_opponent_put_at_library_position_uses_optional_per_opponent_fanout() { + let def = parse_effect_chain( + "for each opponent, put up to one target nonland permanent that player controls into its owner's library third from the top.", + AbilityKind::Spell, + ); + + assert!(def.repeat_for.is_none()); + assert_eq!( + def.multi_target, + Some(MultiTargetSpec::bounded( + 0, + QuantityExpr::Ref { + qty: QuantityRef::PlayerCount { + filter: PlayerFilter::Opponent, + }, + }, + )), + "an \"up to one\" per-opponent slot must be optional (min 0), not mandatory" + ); + match &*def.effect { + Effect::PutAtLibraryPosition { + target: TargetFilter::Typed(tf), + position: LibraryPosition::NthFromTop { n }, + .. + } => { + assert_eq!( + tf.controller, + Some(ControllerRef::TargetPlayer), + "target must be scoped to the iterated opponent, not the caster" + ); + assert!(tf + .type_filters + .iter() + .any(|filter| matches!(filter, TypeFilter::Permanent))); + assert_eq!(*n, 3); + } + other => { + panic!("expected PutAtLibraryPosition TargetPlayer nonland permanent, got {other:?}") + } + } +} + #[test] fn choose_two_target_creatures_controlled_by_different_players_sets_target_constraints() { let def = parse_effect_chain(