Skip to content

Fix Winter Orb / Static Orb#5394

Merged
matthewevans merged 4 commits into
phase-rs:mainfrom
rykerwilliams:card/winter-orb-static-orb-untap-cap
Jul 8, 2026
Merged

Fix Winter Orb / Static Orb#5394
matthewevans merged 4 commits into
phase-rs:mainfrom
rykerwilliams:card/winter-orb-static-orb-untap-cap

Conversation

@rykerwilliams

Copy link
Copy Markdown
Contributor

Summary

Fixes the "Granted/quoted ability or continuous modification dropped" misparse for Winter Orb, and the same underlying bug (independently confirmed, tracked in the backlog under a different, less-precise root-cause label) for Static Orb. Both currently parse to a no-op StaticMode::Continuous with modifications: [] — the leading "As long as this artifact is untapped, ..." condition breaks MaxUntapPerType's dispatch branch, so the untap-cap effect is silently dropped and both cards currently do nothing in-game.

Root cause: MaxUntapPerType's branch required its suffix parser to hit end-of-string. The general inverted-"as long as" rewrite machinery recurses into it with trailing text after the match point, so the branch never fires and a generic fallback swallows the effect. Fix gives MaxUntapPerType the same trailing-condition tolerance parse_continuous_gets_has already has, reusing existing building blocks — zero runtime changes needed, the condition-gating machinery already worked generically once the parser emits the right mode+condition.

Backlog entries for both cards removed per §3.1 list hygiene (second commit).

Files changed

  • crates/engine/src/parser/oracle_static/dispatch.rs
  • crates/engine/src/parser/oracle_static/anthem.rs
  • crates/engine/src/parser/oracle_static/tests.rs
  • crates/engine/src/game/turns.rs
  • docs/parser-misparse-backlog.md

CR references

  • CR 502.3 — the untap-cap restriction itself
  • CR 604.1 — static abilities are simply true whenever their condition holds
  • CR 611.3a — a continuous effect from a static ability isn't "locked in"; it applies whenever its text currently indicates (authorizing rule for gating the effect on live tapped-state)

Track

Developer

LLM

Model: claude-sonnet-5
Thinking: high

Verification

  • cargo fmt --all -- --check — clean
  • ./scripts/check-parser-combinators.sh — clean (no output, exit 0)
  • cargo clippy --all-targets -- -D warnings — clean, full workspace
  • cargo test -p engine --lib — 15758 passed, 0 failed, 6 ignored
  • New parser tests (static_max_untap_winter_orb_conditional, static_max_untap_static_orb_conditional) assert both cards' verbatim Oracle text parse to MaxUntapPerType with the correct filter/max and a populated Not(SourceIsTapped) condition
  • New runtime tests (winter_orb_max_untap_cap_gated_by_own_tapped_state, two_winter_orbs_gate_independently_on_own_tapped_state) drive the fix through the real active_static_definitions/evaluate_condition/max_untap_restrictions pipeline, including a multi-authority fixture proving two Winter Orbs gate independently on their own tapped state
  • cargo coverage — not run locally (MTGJSON/card-data.json not present in this environment); the primary coverage gate's "supported" boolean for these cards is unaffected by this fix either way (see judgment note below); cargo semantic-audit likewise not run locally for the same reason

Gate A

$ ./scripts/check-parser-combinators.sh
(no output — clean, exit 0)

Anchored on

  • crates/engine/src/parser/oracle_static/anthem.rs:782parse_continuous_gets_has's own " as long as " split-recurse-reattach shape, the established precedent this fix's new parse_max_untap_per_type_static (dispatch.rs:170) mirrors
  • crates/engine/src/parser/oracle_static/anthem.rs:729rewrite_self_pronoun_subject, reused (visibility bumped to pub(crate)) rather than re-implemented, for self-pronoun condition normalization

Tier: Standard

Scope Expansion

None.

Validation Failures

None.

CI Failures

None.

rykerwilliams and others added 2 commits July 8, 2026 17:29
…02.3)

MaxUntapPerType's dispatch branch required an EOF-anchored suffix match,
so when the general inverted-"as long as" rewrite recursed into it with
a trailing " as long as <condition>" clause, the branch never matched
and the effect silently fell through to a no-op Continuous static.

Winter Orb and Static Orb both print this leading-condition form
("As long as this artifact is untapped, players can't untap more than
...") and currently do nothing in-game as a result. Fixes both by
giving MaxUntapPerType the same trailing-condition tolerance
parse_continuous_gets_has already has, reusing parse_static_condition
and rewrite_self_pronoun_subject rather than a one-off check. The
unconditional family (Smoke, Damping Field, Imi Statue, Stoic Angel)
is unaffected and covered by regression assertions.

Verification: cargo fmt, check-parser-combinators.sh, cargo clippy
--all-targets -D warnings, and cargo test -p engine --lib (15758
passed) all clean. New parser tests assert both cards' verbatim
Oracle text parse to MaxUntapPerType with the condition populated;
new turns.rs tests drive the fix through the real
active_static_definitions/evaluate_condition/max_untap_restrictions
pipeline, including a multi-authority fixture proving two Winter Orbs
gate independently on their own tapped state.

Found via AI-CONTRIBUTOR.md's Old-School (93/94) misparse-backlog
cross-reference. Plan review and implementation review both passed
clean on the first round.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DMa6DrxXyFHBdGxz3uLgvM
Both cards now parse correctly per the preceding fix (verified by the
new discriminating parser and runtime tests) — list hygiene per
AI-CONTRIBUTOR.md §3.1: remove fixed cards from their root-cause
lists in the same PR. Updates the two affected section counts and the
top summary totals; neither list emptied so no section was deleted.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DMa6DrxXyFHBdGxz3uLgvM

@gemini-code-assist gemini-code-assist Bot left a comment

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.

Code Review

This pull request implements parsing and engine support for conditional max-untap static abilities (such as Winter Orb and Static Orb) gated by their own tapped state, aligning with CR 502.3, CR 611.3a, and CR 604.1. It introduces the parse_max_untap_per_type_static helper to handle trailing conditions and updates the parser backlog documentation. The feedback identifies a minor improvement opportunity in dispatch.rs to remove redundant double references when calling parse_max_untap_per_type_static.

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.

// because the restriction applies to whoever is the active player, not
// the source's controller. See `parse_max_untap_per_type_static` for the
// trailing-condition tolerance shared with `parse_continuous_gets_has`.
if let Some(def) = parse_max_untap_per_type_static(&tp, &text) {

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] Redundant double references in function call.

Why it matters: tp is already of type &TextPair<'_> and text is already of type &str. Passing &tp and &text creates unnecessary double references (&&TextPair<'_> and &&str) which are then coerced by the compiler.

Suggested fix: Pass tp and text directly without the redundant reference operators.

Suggested change
if let Some(def) = parse_max_untap_per_type_static(&tp, &text) {
if let Some(def) = parse_max_untap_per_type_static(tp, text) {

@matthewevans

Copy link
Copy Markdown
Member

Holding current head 51a59db5f6a92647bd924f288925498d6e142ec9 until the parser evidence lands. This PR changes engine/parser code, but the coverage-parse-diff sticky is absent and the Rust/card-data checks are still pending for this head. I’ll re-check once the current-head card-level diff is available.

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown

Parse changes introduced by this PR · 2 card(s), 3 signature(s) (baseline: main 3a89097a06ba)

2 card(s) · static/Continuous · removed: Continuous (affects=self, conditional=not (source is tapped))

Examples: Static Orb, Winter Orb

1 card(s) · static/MaxUntapPerType(1,Typed(TypedFilter { type_filters: [Land], controller: None, p… · added: MaxUntapPerType(1,Typed(TypedFilter { type_filters: [Land], controller: None, properties: [] })) (conditional=not (source is tapped))

Examples: Winter Orb

1 card(s) · static/MaxUntapPerType(2,Typed(TypedFilter { type_filters: [Permanent], controller: No… · added: MaxUntapPerType(2,Typed(TypedFilter { type_filters: [Permanent], controller: None, properties: [] })) (conditional=not (source is tapped))

Examples: Static Orb

2 card(s) had Oracle-text changes (errata/reprint) — excluded as non-parser.

@matthewevans matthewevans self-assigned this Jul 8, 2026
@matthewevans matthewevans added the bug Bug fix label Jul 8, 2026

@matthewevans matthewevans left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maintainer review: current head parses Winter Orb / Static Orb into conditional MaxUntapPerType, verifies own-source tapped-state gating at runtime, and the parse diff is limited to those two cards.

@matthewevans matthewevans added this pull request to the merge queue Jul 8, 2026
@matthewevans matthewevans removed their assignment Jul 8, 2026
Merged via the queue into phase-rs:main with commit 5a33775 Jul 8, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants