Skip to content
33 changes: 26 additions & 7 deletions crates/engine/src/parser/oracle_effect/lower.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3588,15 +3588,34 @@ 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). Scans at word boundaries for an "up to N target …" /
/// "any number of target …" quantifier anywhere in the clause — not just
/// after "gain control of" — so every verb in the per-opponent-target-fanout
/// class ("gain control of up to one target …", "exile up to one target …",
/// "put up to one target … into its owner's library …") shares one min-0
/// detector instead of each verb needing its own hardcoded prefix. 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();
Comment on lines +3591 to +3622

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the detector, the shared helper, and nearby tests.
FILE="crates/engine/src/parser/oracle_effect/lower.rs"

# Show a compact outline first.
ast-grep outline "$FILE" --view expanded | sed -n '1,220p'

echo
echo "=== helper occurrences ==="
rg -n "strip_optional_target_prefix|scan_at_word_boundaries|any number of target|up to .*target" "$FILE"

echo
echo "=== nearby source around helper and detector ==="
sed -n '5660,5745p' "$FILE"
echo
sed -n '3580,3635p' "$FILE"

echo
echo "=== tests mentioning optional target / any number of target ==="
rg -n "any number of target|up to one target|optional target|per_opponent_target_fanout|min_is_fixed_zero|stripped_multi_target" crates/engine/src/parser -g '*.rs'

Repository: phase-rs/phase

Length of output: 50370


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Pull the relevant test cases if they exist.
FILE="crates/engine/src/parser/oracle_effect/lower.rs"

# Locate test module sections in this file.
rg -n "mod tests|#\[test\]|any number of target|up to one target|optional" "$FILE"

# Show the test region if present.
sed -n '6200,6900p' "$FILE"

Repository: phase-rs/phase

Length of output: 41276


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Try a narrower probe for the helper and the detector's contract.
FILE="crates/engine/src/parser/oracle_effect/lower.rs"

python3 - <<'PY'
from pathlib import Path
text = Path("crates/engine/src/parser/oracle_effect/lower.rs").read_text()

targets = [
    "any number of target",
    "strip_optional_target_prefix",
    "per_opponent_target_fanout_min",
]
for t in targets:
    print(f"== {t} ==")
    idx = text.find(t)
    if idx >= 0:
        start = max(0, idx - 500)
        end = min(len(text), idx + 1000)
        print(text[start:end])
    else:
        print("not found")
PY

Repository: phase-rs/phase

Length of output: 4754


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path
path = Path("crates/engine/src/parser/oracle_effect/lower.rs")
text = path.read_text().splitlines()

def show(anchor, before=20, after=60):
    for i, line in enumerate(text, 1):
        if anchor in line:
            start = max(1, i-before)
            end = min(len(text), i+after)
            print(f"=== {anchor} @ line {i} ===")
            for j in range(start, end+1):
                print(f"{j:5d}: {text[j-1]}")
            print()
            return
    print(f"=== {anchor} not found ===")

show("fn strip_optional_target_prefix")
show("fn per_opponent_target_fanout_min")
show("any number of target")
PY

Repository: phase-rs/phase

Length of output: 12762


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="crates/engine/src/parser/oracle_effect/lower.rs"

sed -n '3490,3565p' "$FILE"
echo
sed -n '5770,5805p' "$FILE"
echo
sed -n '32040,32120p' crates/engine/src/parser/oracle_effect/tests.rs

Repository: phase-rs/phase

Length of output: 7774


🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n "parse_for_each_opponent_target_fanout_clause\\(" crates/engine/src/parser/oracle_effect/lower.rs crates/engine/src/parser/oracle_effect -g '*.rs' -A 4 -B 4

echo
sed -n '3460,3555p' crates/engine/src/parser/oracle_effect/lower.rs

Repository: phase-rs/phase

Length of output: 6602


Cover any number of target … or remove the claim. strip_optional_target_prefix only accepts up to N target …, so this detector does not currently make any number of target … optional and those clauses fall back to min: 1. Add the missing any number of branch with the existing MultiTargetSpec plumbing, or narrow the comment to the supported grammar.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/engine/src/parser/oracle_effect/lower.rs` around lines 3591 - 3617,
The function per_opponent_target_fanout_min currently detects only up-to
quantifiers, despite claiming to support “any number of target” clauses. Extend
the detector and existing MultiTargetSpec/strip_optional_target_prefix plumbing
to recognize “any number of” target expressions as fixed-zero optional slots,
preserving the current behavior for up-to quantifiers and unrelated
resource-count quantifiers.

Source: Path instructions

if found_optional_target_slot {
0
} else {
1
Expand Down
56 changes: 56 additions & 0 deletions crates/engine/src/parser/oracle_effect/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Loading