feat(parser): played-by-opponents enter tapped replacement

[glorysr1209-png]

Summary

Closes 3390. High-score replacement parser gap: creatures played by opponents enter tapped (Uphill Battle class). Token score ~2.

Changes

• Extend crates/engine/src/parser/oracle_replacement.rs with focused parser arm + unit test
• Minimal diff scoped to one high-leverage replacement pattern family

Test plan

• New #[test] in oracle_replacement.rs covers representative Oracle text
• CI cargo test -p engine (authoritative gate — not run locally)

[gemini-code-assist]

Code Review

This pull request introduces parsing support for external ETB replacement effects that include the phrase 'played by your opponents' (such as Uphill Battle) by adding the parse_external_entry_subject helper and an accompanying test. Feedback highlights that the string-splitting approach (strip_suffix) violates the repository's architectural rule requiring nom-based combinators for parsing, and suggests refactoring it using modular parsers. Additionally, the new test was defined outside the mod tests block, which would cause it to compile in production builds, and should be moved inside.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

Avoid verbatim string equality or ad-hoc string splitting (like strip_suffix or hardcoded take_until with verbatim phrases) for parsing Oracle phrases. This bypasses the robust nom-based parser and creates fragile matches. Instead, decompose compound phrases into modular, reusable parsers for constituent parts (e.g., subjects, conjunctions) and compose them using idiomatic combinator aggregates (like nested alt and tag sequences) to prevent combinatorial explosion and improve maintainability.

fn parse_external_entry_subject(subject: &str) -> Option<TargetFilter> {
    let subject = subject.trim();
    if let Ok((rest, filter)) = parse_type_phrase(subject) {
        if let Ok((remaining, _)) = parse_played_by_opponents_clause(rest) {
            if remaining.trim().is_empty() {
                return Some(inject_controller(filter, ControllerRef::Opponent));
            }
        }
    }
    let (filter, rest) = parse_type_phrase(subject);
    if filter == TargetFilter::Any || !rest.trim().is_empty() {
        return None;
    }
    Some(filter)
}

References

1. 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 (e.g., subjects, conjunctions) and compose them using idiomatic combinator aggregates (like nested alt and tag sequences) to prevent combinatorial explosion and improve maintainability.

[gemini-code-assist]

Test Defined Outside #[cfg(test)] Module

The new test parses_played_by_opponents_enters_tapped is defined outside the mod tests block (which is gated with #[cfg(test)]). This causes the test function to be compiled in non-test/production builds, leading to unnecessary binary bloat and potential compilation issues in non-test environments.

Moving the test inside the mod tests block ensures it is only compiled and run during test execution.

    }

    #[test]
    fn parses_played_by_opponents_enters_tapped() {
        let def = parse_replacement_line(
            "Creatures played by your opponents enter tapped.",
            "Uphill Battle",
        ).expect("played by opponents ETB");
        assert_eq!(def.event, ReplacementEvent::ChangeZone);
    }
}

[matthewevans]

Maintainer review — changes requested

The seam is right: factoring external-entry subject parsing into parse_external_entry_subject and calling it from build_external_entry_replacement is the correct place to add the "played by your opponents" controller scope, and reusing parse_type_phrase + inject_controller(ControllerRef::Opponent) is the right building-block composition. Two issues before merge:

1. strip_suffix for parsing dispatch is the prohibited idiom here

if let Some(type_part) = subject.strip_suffix(" played by your opponents") {

CLAUDE.md is explicit that strip_prefix/strip_suffix must not be used for parsing dispatch — use a nom tag() instead. The sibling suffix helpers in this exact file already model the house idiom: extract_cast_from_zone_suffix, extract_you_attacked_this_turn_suffix, and extract_cast_using_web_slinging_suffix all locate their trailing phrase with take_until(...) + tag(...). Match that pattern — a small take_until(" played by your opponents") + tag(...) combinator that returns the leading type phrase — rather than strip_suffix on a verbatim string. Gemini raised the same point.

2. Merge state is DIRTY

This branch textually conflicts with origin/main (overlapping oracle_replacement.rs edits, including the mod tests region shared with sibling PR #3387). It must be rebased clean before it can enter the merge queue.

Note: the test parses_played_by_opponents_enters_tapped is in fact already inside the #[cfg(test)] mod tests block (lines 6574–12359), so Gemini's "test outside cfg(test)" finding is a false positive — no action needed there. The assertion (def.event == ChangeZone) is thin but discriminating enough for the parse path; consider also asserting the resulting filter carries ControllerRef::Opponent so a future regression in inject_controller is caught.

Once the dispatch uses tag() and the branch is rebased, this is mergeable.

[matthewevans]

Superseded by #3520 (kiannidev), which implements the played-by-opponents enter-tapped replacement through the shared typed parse_external_entry_suffix/build_external_entry_replacement seam with a discriminating runtime test (cast vs token vs put-without-cast). #3520 is queued to merge; recommend closing this in its favor once it lands. Thanks for the original work here — it pointed at the right mechanic.

[matthewevans]

Closing as superseded — #3520 (kiannidev) merged to main, implementing this card class through the shared parse_external_entry_suffix/build_external_entry_replacement seam with discriminating runtime coverage. Thanks for the contribution.