fix(parser): support 'your opponents control no [type]' static condition#4656
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new parser arm parse_your_opponents_control_no to handle the condition 'your opponents control no [type]', along with corresponding unit tests. Feedback suggests refactoring this new parser to share common logic with parse_you_control_no to avoid code duplication, and strengthening the test assertions for the 'permanents' variant to ensure correct type filtering.
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.
| fn parse_your_opponents_control_no(input: &str) -> OracleResult<'_, StaticCondition> { | ||
| let (rest, _) = tag("your opponents control no ").parse(input)?; | ||
| let (filter, remainder) = parse_type_phrase(rest); | ||
| if matches!(filter, TargetFilter::Any) { | ||
| return Err(nom::Err::Error(nom::error::Error::new( | ||
| input, | ||
| nom::error::ErrorKind::Fail, | ||
| ))); | ||
| } | ||
| // CR 109.4: battlefield-scoped presence controlled by an opponent. | ||
| let filter = inject_controller(filter, ControllerRef::Opponent); | ||
| let consumed = input.len() - remainder.len(); | ||
| Ok(( | ||
| &input[consumed..], | ||
| StaticCondition::Not { | ||
| condition: Box::new(StaticCondition::IsPresent { | ||
| filter: Some(filter), | ||
| }), | ||
| }, | ||
| )) | ||
| } |
There was a problem hiding this comment.
[MEDIUM] Code duplication with parse_you_control_no. Evidence: crates/engine/src/parser/oracle_nom/condition.rs:3260.
Why it matters: This function is nearly identical to parse_you_control_no, which violates the "Parameterize, don't proliferate" principle (R3) and increases maintenance overhead.
Suggested fix: Refactor the common logic into a helper function called by both parse_you_control_no and this new function.
References
- Decompose compound phrases into modular, reusable parsers for constituent parts and compose them using idiomatic combinator aggregates to prevent combinatorial explosion and improve maintainability.
| assert!( | ||
| matches!(c, StaticCondition::Not { .. }), | ||
| "expected Not(IsPresent), got {c:?}" | ||
| ); |
There was a problem hiding this comment.
[MEDIUM] Test assertion is weaker than it could be. Evidence: crates/engine/src/parser/oracle_nom/condition.rs:8049.
Why it matters: A more specific assertion ensures the parser correctly handles the "permanents" variant, not just that it produces a Not condition.
Suggested fix: Assert the TypedFilter contents for the "permanents" case, similar to the "creatures" case.
let tf = typed_presence_under_not(&c);
assert_eq!(tf.controller, Some(ControllerRef::Opponent));
assert!(tf.type_filters.contains(&TypeFilter::Permanent));
Parse changes introduced by this PR · 3 card(s), 3 signature(s) (baseline: main
|
matthewevans
left a comment
matthewevans
left a comment
There was a problem hiding this comment.
Reviewed current head fa4abe4. The new condition arm is at the existing control-condition parser seam and composes the existing type-phrase plus controller-injection helpers. It covers the collective-opponents "your opponents control no " class surfaced by Erebos's Titan, Kezzerdrix, and Chevill while preserving the existing article-guarded "no opponent controls a " path.
2 participants
Anchored on
Erebos's Titan — "As long as your opponents control no creatures, this creature has indestructible." The static condition "your opponents control no [type]" had no parser support.
Problem
"your opponents control no creatures" parsed to
StaticCondition::Unrecognized, so Erebos's Titan's conditional indestructible never applied.Root cause
The control-condition dispatcher already had "you control no [type]" (
parse_you_control_no) and "no opponent controls a/an [type]" (parse_no_opponent_controls_a, #4629), but no arm for the collective-opponents bare-plural form "your opponents control no [type]".Fix
Added
parse_your_opponents_control_no, mirroringparse_you_control_nowith the opponent controller axis: "your opponents control no [type]" →Not(IsPresent { inject_controller(filter, Opponent) }). It reusesparse_type_phrase(creatures / permanents / color-disjunctions) and is identical in meaning to "no opponent controls a creature" (#4629). No new types.CR 109.4 (battlefield-scoped presence controlled by an opponent), matching the sibling arm.
Tests
parse_inner_condition("your opponents control no creatures")→Not(IsPresent)with controllerOpponent+Creaturetype; the "no permanents" variant (Chevill) parses through the same arm. Whole-card parse of Erebos's Titan confirmed: the indestructible static's condition is nowNot(IsPresent), noUnrecognized.Scope
This covers the static "as long as your opponents control no [type]" form (Erebos's Titan). The
if/when your opponents control no [type]trigger-intervening forms (Chevill, Kezzerdrix, Penregon Besieged) route through a separate condition path and are not addressed here.Verification
cargo fmtclean; parser combinator gate exit 0; engine clippy-D warningsclean; the new test and the condition suite pass.Model: claude-opus-4-8
Tier: Frontier