test(engine): Uphill Battle WasPlayed filter runtime discrimination

[kiannidev]

Summary

• Add runtime test that played-by-opponents enter-tapped replacements match cast creatures but not tokens put directly onto the battlefield

Fixes #3518

Note: Stacked on #3520 (parser + FilterProp::WasPlayed). Rebase to a single commit after #3520 merges.

Test plan

• cargo test -p engine --lib uphill_battle_was_played

Made with Cursor

[gemini-code-assist]

Code Review

This pull request introduces the FilterProp::WasPlayed property to identify objects entering the battlefield from being played or cast, along with parser support for replacement effects like 'played by your opponents enter tapped' (e.g., Uphill Battle). Feedback highlights a critical bug where any nontoken card entering the battlefield from another zone is incorrectly matched as WasPlayed. Additionally, the parser implementation violates the repository's style guide by using verbatim string matching instead of modular nom combinators, and the test suite should be expanded to cover nontoken creatures put onto the battlefield without being cast.

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]

[HIGH] Incorrectly matches any nontoken card entering the battlefield from another zone as WasPlayed.

Why it matters: Under MTG rules (CR 305.1, CR 601.2a), "played" or "cast" is a specific action. Simply entering the battlefield from another zone (e.g., via reanimation or Elvish Piper) is not playing or casting, but since record.from_zone is Some(...) in those cases, record.from_zone.is_some() will incorrectly evaluate to true.

Suggested fix: Ensure ZoneChangeRecord explicitly tracks whether the transition was a cast or play (e.g., via played_from_zone or cast_from_zone fields) and check those instead of from_zone.is_some().

[gemini-code-assist]

[MEDIUM] Use modular nom combinators for parsing dispatch instead of verbatim string matches or monolithic tags.

Why it matters: Avoid verbatim string equality or monolithic tags 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 to prevent combinatorial explosion and improve maintainability.

    let (filter, rest) = parse_type_phrase(norm_lower.trim());
    let rest = rest.trim();
    let parse_subject = nom::bytes::complete::tag("played by your opponents");
    let parse_action = nom::sequence::tuple((
        nom::bytes::complete::tag("enter"),
        nom::combinator::opt(nom::bytes::complete::tag(" the battlefield")),
    ));
    let parse_state = nom::bytes::complete::tag("tapped");
    let parser = nom::sequence::tuple((
        parse_subject,
        nom::character::complete::space1,
        parse_action,
        nom::character::complete::space1,
        parse_state,
        nom::combinator::opt(nom::bytes::complete::tag(".")),
    ));
    if nom::combinator::all_consuming(parser)(rest).is_err() {
        return None;
    }

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 and compose them using idiomatic combinator aggregates.

[gemini-code-assist]

[MEDIUM] Add a test case for nontoken creatures put onto the battlefield without being cast/played.

Why it matters: The current test suite only covers cast creatures and tokens, failing to verify that nontoken creatures put onto the battlefield from hand/graveyard/library (e.g., via Elvish Piper or reanimation) are correctly excluded from the WasPlayed filter.

Suggested fix: Add a third test case to uphill_battle_was_played_filter_matches_cast_creature_not_token that puts a nontoken creature onto the battlefield without setting cast_from_zone or played_from_zone.

        let token_event = ProposedEvent::CreateToken {
            owner: PlayerId(1),
            count: 1,
            spec: Box::new(test_token_spec(PlayerId(1), CoreType::Creature)),
            copy: None,
            enter_tapped: EtbTapState::Unspecified,
            applied: HashSet::new(),
        };
        let token_matches = find_applicable_replacements(&state, &token_event, &registry);
        assert!(
            !token_matches
                .iter() 
                .any(|rid| rid.source == uphill_id),
            "tokens put directly onto the battlefield must not match WasPlayed filter"
        );

        let put_creature = ObjectId(30);
        let mut put_obj = GameObject::new(
            put_creature,
            CardId(3),
            PlayerId(1),
            "Runeclaw Bear".to_string(),
            Zone::Hand,
        );
        put_obj.card_types.core_types.push(CoreType::Creature);
        state.objects.insert(put_creature, put_obj);

        let put_event = ProposedEvent::ZoneChange {
            object_id: put_creature,
            from: Zone::Hand,
            to: Zone::Battlefield,
            cause: None,
            attach_to: None,
            enter_tapped: EtbTapState::Unspecified,
            enter_with_counters: Vec::new(),
            controller_override: None,
            enter_transformed: false,
            face_down_profile: None,
            applied: HashSet::new(),
        };
        let put_matches = find_applicable_replacements(&state, &put_event, &registry);
        assert!(
            !put_matches
                .iter()
                .any(|rid| rid.source == uphill_id),
            "nontoken creatures put onto the battlefield without being cast must not match WasPlayed filter"
        );

[matthewevans]

Thanks for this — the WasPlayed filter design and the runtime test are solid. The runtime test uphill_battle_was_played_filter_matches_cast_creature_not_token drives find_applicable_replacements and discriminates correctly (cast creature with cast_from_zone=Some matches; a token put directly onto the battlefield does not). Parser tests cover the creature/land class. CR 305.1 + 601.2a annotations check out.

One blocker before this can merge: cargo fmt --all -- --check is failing (the "Rust lint (fmt, clippy, parser gate)" job). The diffs are in the test code you added:

• crates/engine/src/game/replacement.rs lines ~10482, ~10514, ~10530
• crates/engine/src/parser/oracle_replacement.rs line ~9837

Please run cargo fmt --all and push. Note the lint job stops at the fmt step, so clippy and the parser gate haven't actually run yet on this branch — once fmt is fixed, please confirm those pass too (the allow-noncombinator suffix-peel in parse_played_by_opponents_entry is gated behind scan_contains, which should be fine, but the gate needs a clean run to confirm). Re-request review once green.

[kiannidev]

Reopened and updated per feedback:

• cargo fmt --all applied
• Runtime test expanded with nontoken put-without-cast case (Elvish Piper / reanimate class)
• WasPlayed zone-change records now check cast_from_zone/played_from_zone instead of from_zone.is_some()
• Merged latest main

PR kept open alongside #3520. Ready for re-review.

[matthewevans]

Closing as superseded — the WasPlayed runtime discrimination test (uphill_battle_was_played_filter_matches_cast_creature_not_token) and all the WasPlayed matcher wiring from this PR are consolidated into #3520, per the plan to land the feature + its discriminating test together. Track the work there.