Skip to content

fix(engine): parse Land Equilibrium's chained control-gated sacrifice replacement#5602

Merged
matthewevans merged 1 commit into
phase-rs:mainfrom
rykerwilliams:fix/land-equilibrium-then-clause
Jul 11, 2026
Merged

fix(engine): parse Land Equilibrium's chained control-gated sacrifice replacement#5602
matthewevans merged 1 commit into
phase-rs:mainfrom
rykerwilliams:fix/land-equilibrium-then-clause

Conversation

@rykerwilliams

Copy link
Copy Markdown
Contributor

Summary

Land Equilibrium ({2}{U}{U}, Enchantment, Legends): "If an opponent who controls at least as many lands as you do would put a land onto the battlefield, that player instead puts that land onto the battlefield then sacrifices a land of their choice."

Previously parsed with zero coverage — the chained "then sacrifices" clause was swallowed (SwallowedClause/Replacement_Instead parse warning, replacements: []).

  • Adds a nom combinator for the "an opponent who controls at least as many <X> as you do" subject-relative clause (CR 614.1a), reusing the existing comparator-agnostic player_count_comparison_filters helper rather than the existential-aggregate shape used by "an opponent controls more lands than you."
  • Wires this into a new ReplacementDefinition that gates on the entering opponent's own land count — not an aggregate check across all opponents — via a new ScopedPlayer-aware QuantityContext threaded through OnlyIfQuantity/UnlessQuantity condition evaluation (game/replacement.rs). Verified this is additive: none of the 27 existing cards using these condition variants reference ScopedPlayer today.
  • Generalizes the sacrifice-execute and event-tag derivation from the same parsed type filter rather than hardcoding "land" in three places, so the whole replacement (condition + event-tag + execute) stays internally consistent if a sibling card with a different permanent noun is ever found; a mismatch fails safely (None, falls through to the next dispatch branch) rather than misparsing.
  • Fixes a latent continuation-source bug in stash_post_replacement_continuation: the stashed source used rid.source (the replacement's bearer) instead of the entering object. This is a no-op for every existing card reaching this code path (all 21 Devour-family cards have valid_card: SelfRef, so bearer == entering object by construction — verified via a whole-database query), but is required for Land Equilibrium's TokenEntry drain path (an opponent creating a land token under Land Equilibrium), since it's the first external (non-self-referential) replacement to reach this code.

Test plan

  • New parser unit test: Land Equilibrium's real Oracle text produces exactly one Moved replacement with OnlyIfQuantity{GE} and a Sacrifice execute (no SwallowedClause), asserting exact target-filter equality.
  • New integration test suite (land_equilibrium_forced_sacrifice.rs, 5 scenarios), driving the real GameRunner/PlayLand pipeline:
    • Opponent with more lands than caster is forced to sacrifice (land still enters first).
    • Opponent with fewer lands plays normally, no sacrifice.
    • Equal counts still trigger (GE boundary).
    • 3-player fixture proves the gate binds to the specific entering opponent, not an aggregate check (one gated opponent, one not, despite the caster's own count being fixed).
    • An existing Devour-family card's post-replacement continuation is unaffected by the stash-source change (regression guard).
  • cargo clippy -p engine --tests -- -D warnings clean
  • cargo fmt --all
  • Parser combinator gate (./scripts/check-parser-combinators.sh) clean
  • All CR citations (614.1a, 614.1c, 614.13, 608.2c, 109.5, 701.21a) verified against the Comprehensive Rules text

🤖 Generated with Claude Code

https://claude.ai/code/session_01XbgwGxbU9NHN9kou9isp8K

… replacement

Land Equilibrium ("If an opponent who controls at least as many lands as
you do would put a land onto the battlefield, that player instead puts
that land onto the battlefield then sacrifices a land of their choice")
previously parsed as a swallowed clause with zero replacement coverage.

Adds a nom combinator for the "an opponent who controls at least as many
<X> as you do" subject-relative clause (CR 614.1a), reusing the existing
comparator-agnostic player_count_comparison_filters helper, and wires it
into a new ReplacementDefinition builder that gates on the entering
opponent's own land count (not an aggregate/existential check across all
opponents) via a new ScopedPlayer-aware QuantityContext threaded through
OnlyIfQuantity/UnlessQuantity condition evaluation.

Also fixes a latent continuation-source bug in
stash_post_replacement_continuation (game/replacement.rs) that only
manifests on the TokenEntry drain path — the existing land-play and
general zone-change drains already rebind correctly, so this is a no-op
for all 21 existing Devour-family cards and closes the one remaining gap
for external (non-self-referential) replacements creating land tokens.

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

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

rykerwilliams added a commit to rykerwilliams/phase that referenced this pull request Jul 11, 2026
@github-actions

Copy link
Copy Markdown

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

1 card(s) · ability/ChangeZone · removed: ChangeZone (target=parent target, to=battlefield)

Examples: Land Equilibrium

1 card(s) · replacement/Moved · added: Moved

Examples: Land Equilibrium

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

@matthewevans matthewevans self-assigned this Jul 11, 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.

Approved — right seam, verified rules evidence, and a surgical one-card parse-diff. Held last sweep only for missing evidence; both gaps have now closed favorably.

✅ Clean

Right seam — the combinator rejects the shape that would have been subtly wrong. The new nom combinator reuses player_count_comparison_filters and explicitly declines the existential-aggregate PlayerFilter::ControlsCount shape. That matters: an aggregate filter would answer "does some player control more lands?" when the rule needs "does this entering opponent control more lands?" — it would pass casual tests and bind the wrong player in a multiplayer game. Rejecting it is the harder and correct choice.

Shared-seam change is correctly scoped. rid.sourcenew_event.affected_object_id().unwrap_or(rid.source) touches the Applied arm only. Leaving the Prevented arm alone is right — a prevented event has no entering object to bind to — and the unwrap_or fallback plus the Devour-family regression test guard the blast radius.

Evidence, verified rather than taken on trust:

  • Oracle text matches Scryfall exactly (checked against the card, not the PR description).
  • All 6 CR citations grep-verify against docs/MagicCompRules.txt: 614.1a, 614.1c, 614.13, 608.2c, 109.5, 701.21a.
  • Parse-diff: exactly 1 card, 2 signatures. Land Equilibrium loses a bogus ChangeZone (target=parent target, to=battlefield) and gains replacement/Moved. Zero collateral anywhere else in the pool — precisely the claimed scope, which is the signature you want from a replacement-effect fix.
  • Tests are registered in tests/integration/main.rs (not inert) and discriminating: three_player_gate_binds_to_specific_entering_player would fail against an aggregate implementation.

CI is genuinely green — both Rust shards ran real work (9m14s / 9m15s), lint 8m18s, card-data 10m21s. Not a hollow pass.

Recommendation: approve and enqueue. Nothing blocking.

@matthewevans matthewevans added the bug Bug fix label Jul 11, 2026
@matthewevans
matthewevans added this pull request to the merge queue Jul 11, 2026
@matthewevans matthewevans removed their assignment Jul 11, 2026
Merged via the queue into phase-rs:main with commit 5eb8a18 Jul 11, 2026
13 checks passed
rykerwilliams added a commit to rykerwilliams/phase that referenced this pull request Jul 12, 2026
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