Skip to content

feat(parser): explore replacement with scry prelude #3389

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
glorysr1209-png wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(parser): explore replacement with scry prelude #3389

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
48 changes: 48 additions & 0 deletions crates/engine/src/parser/oracle_replacement.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -540,6 +540,14 @@
return Some(def);
}

if nom_primitives::scan_contains(&lower, "would explore")
&& nom_primitives::scan_contains(&lower, "instead")
{
if let Some(def) = parse_explore_replacement(&norm_lower, &text) {
return Some(def);
}
}

// --- Life-floor damage replacement: "if you control a [filter], damage that would
// reduce your life total to less than N reduces it to N instead" ---
// CR 614.1a: Worship-class replacement effect.
Expand Down Expand Up @@ -6282,6 +6290,35 @@
///
/// Identical to [`parse_life_floor_damage_replacement`] but without the Worship-class
/// "if you control a [filter]," guard. Dispatched after the conditional arm.
/// CR 614.1a + CR 701.20: Explore replacement with scry prelude (Twists and Turns).
fn parse_explore_replacement(
norm_lower: &str,
original_text: &str,
) -> Option<ReplacementDefinition> {
let (rest, _) = tag::<_, _, OracleError<'_>>("if a creature you control would explore, ")
.parse(norm_lower)
.ok()?;
let (rest, _) = tag::<_, _, OracleError<'_>>("instead you scry 1, then that creature explores")
.parse(rest)
.ok()?;
let (_, _) = all_consuming(opt(tag::<_, _, OracleError<'_>>(".")))
.parse(rest)
.ok()?;
let scry = AbilityDefinition::new(
AbilityKind::Spell,
Effect::Scry {
count: QuantityExpr::Fixed { value: 1 },
target: TargetFilter::Controller,
},
);
let explore = AbilityDefinition::new(AbilityKind::Spell, Effect::Explore);
Some(
ReplacementDefinition::new(ReplacementEvent::Explore)
.execute(scry.sub_ability(explore))
.description(original_text.to_string()),
)
}
Comment on lines +6294 to +6320

@gemini-code-assist gemini-code-assist Bot Jun 15, 2026

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.

high

[HIGH] Decompose verbatim string matching and fix missing valid_card filter. Evidence: crates/engine/src/parser/oracle_replacement.rs:6182.
Why it matters: Verbatim string matching is fragile and fails to support other creature types or scry amounts, and omitting the valid_card filter incorrectly applies the replacement effect to opponents' creatures. Suggested fix: Use nom combinators to parse the creature filter and scry amount dynamically, and set valid_card on the definition.

fn parse_explore_replacement(
    norm_lower: &str,
    original_text: &str,
) -> Option<ReplacementDefinition> {
    let (rest, _) = tag::<_, _, OracleError<'_>>("if ").parse(norm_lower).ok()?;
    let (rest, filter_text) = take_until::<_, _, OracleError<'_>>(" would explore, ").parse(rest).ok()?;

    let filter_text = alt((tag("a "), tag("an "), tag("any ")))
        .parse(filter_text.trim())
        .map_or(filter_text.trim(), |(r, _)| r);
    let (filter, leftover) = parse_type_phrase(filter_text);
    if !leftover.trim().is_empty() {
        return None;
    }

    let (rest, _) = tag(" would explore, ").parse(rest).ok()?;
    let (rest, _) = tag("instead you scry ").parse(rest).ok()?;
    let (rest, scry_amount) = nom_primitives::parse_number(rest).ok()?;
    let (rest, _) = tag(", then that creature explores").parse(rest).ok()?;
    let (_, _) = all_consuming(opt(tag::<_, _, OracleError<'_>>("."))).parse(rest).ok()?;

    // CR 701.20a: "you scry N" effect definition
    let scry = AbilityDefinition::new(
        AbilityKind::Spell,
        Effect::Scry {
            count: QuantityExpr::Fixed { value: scry_amount as i32 },
            target: TargetFilter::Controller,
        },
    );
    // CR 701.44a: "that creature explores" effect definition
    let explore = AbilityDefinition::new(AbilityKind::Spell, Effect::Explore);

    // CR 614.1a: Construct the replacement definition with the parsed filter as valid_card
    Some(
        ReplacementDefinition::new(ReplacementEvent::Explore)
            .valid_card(filter)
            .execute(scry.sub_ability(explore))
            .description(original_text.to_string()),
    )
}
References
  1. Rule R1 requires using nom combinators and avoiding verbatim string matching for parsing dispatch. Rule R3 requires parameterizing parsers rather than proliferating leaf-level variants. (link)
  2. Avoid verbatim string equality for parsing Oracle phrases as it bypasses the robust nom-based parser and creates fragile matches. Instead, decompose compound phrases into modular, reusable parsers for constituent parts.


fn parse_unconditional_life_floor_damage_replacement(
norm_lower: &str,
) -> Option<ReplacementDefinition> {
Expand Down Expand Up @@ -12004,6 +12041,17 @@
);
assert_eq!(def.valid_player, Some(ReplacementPlayerScope::You));
}

#[test]
fn parses_explore_replacement_with_scry() {
let def = parse_replacement_line(
"If a creature you control would explore, instead you scry 1, then that creature explores.",
"Twists and Turns",
)
.expect("explore replacement");

Check warning on line 12051 in crates/engine/src/parser/oracle_replacement.rs

View workflow job for this annotation

GitHub Actions / Rust lint (fmt, clippy, parser gate) 

Diff in /home/runner/work/phase/phase/crates/engine/src/parser/oracle_replacement.rs
assert_eq!(def.event, ReplacementEvent::Explore);
}
Comment on lines +12045 to +12053

@gemini-code-assist gemini-code-assist Bot Jun 15, 2026

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.

medium

[MEDIUM] Assert valid_card filter in explore replacement test. Evidence: crates/engine/src/parser/oracle_replacement.rs:11855.
Why it matters: Asserting the valid_card filter ensures that the replacement effect is correctly scoped to the appropriate creature and prevents future regressions. Suggested fix: Add an assertion verifying that def.valid_card matches the expected creature filter.

    #[test]
    fn parses_explore_replacement_with_scry() {
        let def = parse_replacement_line(
            "If a creature you control would explore, instead you scry 1, then that creature explores.",
            "Twists and Turns",
        ).expect("explore replacement");
        assert_eq!(def.event, ReplacementEvent::Explore);
        assert_eq!(
            def.valid_card,
            Some(TargetFilter::Typed(TypedFilter {
                type_filters: vec![TypeFilter::Creature],
                controller: Some(ControllerRef::You),
                properties: vec![],
            }))
        );
    }


}

/// Snapshot tests locking current replacement parser output before/after the IR split.
Expand Down
Loading