Skip to content

feat(engine): zone pipeline tranche 4 — route draws through move_object (CR 121.1) - #2996

Merged
matthewevans merged 1 commit into
mainfrom
ship/zone-t4
Jun 11, 2026
Merged

feat(engine): zone pipeline tranche 4 — route draws through move_object (CR 121.1)#2996
matthewevans merged 1 commit into
mainfrom
ship/zone-t4

Conversation

@matthewevans

Copy link
Copy Markdown
Member

Migrates the per-card draw delivery from the raw zones::move_to_zone bypass
to the unified pipeline (zone_pipeline::move_object via ZoneMoveRequest::draw)
with the inner Moved consult ENABLED + a CR 614.5 dedup guard that seeds the
outer ReplacementEvent::Draw pass's applied set into the inner consult so no
def fires at both levels. New non-exempt sourceless ZoneChangeCause::Draw.

Audit (PLAN Risk #5): zero production Moved defs match a Library->Hand draw
today — every destination-unconstrained Moved def is valid_card: SelfRef-bound
to a battlefield host; the only valid_card: None class (RIP/Leyline) is
destination-gated to Graveyard. So the migration is behavior-preserving and
the guard is defensive/future-proofing, per the maintainer's OQ#2 decision.

Incorporates Fable review (round 2):

  • MED-1: add draw_consult_runs_for_unseeded_moved_redirect — the positive
    discriminator that fails under the old raw bypass / an exempt-Draw
    regression (asserts an always-match redirect sends the drawn card to the
    graveyard). The prior two tests passed under the bypass and did not pin the
    migration; module doc corrected.
  • MED-2: replace the wrong-condition NeedsChoice debug_assert with a
    post-condition tripwire (card must land in Hand) that fires on any future
    redirect OR stranded pause; comment flags the open CardDrawn/counter
    semantics question and points a future migrator at the shared
    BatchCompletion machinery (OQ#1).
  • MED-3: move seed_applied from the shared ZoneMoveRequest struct onto the
    ZoneChangeCause::Draw variant payload (Draw is the only producer) — deletes
    7 empty initializers and the sba.rs edit; type-enforces the invariant.
  • LOW-1: dedup guard cites CR 614.5 (one-opportunity rule), not 614.6.
  • LOW-2: de-wildcard ZoneChangeCause::source() — explicit arms, no silent
    inheritance (matches the is_exempt mandate).
  • LOW-3: reword the dedup test's provenance comment (synthetic/forward-looking,
    not production-reachable).
  • Drop CR 120.1 (Damage) for CR 121.1 (Drawing a Card) throughout.

…ct (CR 121.1)

Migrates the per-card draw delivery from the raw zones::move_to_zone bypass
to the unified pipeline (zone_pipeline::move_object via ZoneMoveRequest::draw)
with the inner Moved consult ENABLED + a CR 614.5 dedup guard that seeds the
outer ReplacementEvent::Draw pass's applied set into the inner consult so no
def fires at both levels. New non-exempt sourceless ZoneChangeCause::Draw.

Audit (PLAN Risk #5): zero production Moved defs match a Library->Hand draw
today — every destination-unconstrained Moved def is valid_card: SelfRef-bound
to a battlefield host; the only valid_card: None class (RIP/Leyline) is
destination-gated to Graveyard. So the migration is behavior-preserving and
the guard is defensive/future-proofing, per the maintainer's OQ#2 decision.

Incorporates Fable review (round 2):
- MED-1: add draw_consult_runs_for_unseeded_moved_redirect — the positive
  discriminator that fails under the old raw bypass / an exempt-Draw
  regression (asserts an always-match redirect sends the drawn card to the
  graveyard). The prior two tests passed under the bypass and did not pin the
  migration; module doc corrected.
- MED-2: replace the wrong-condition NeedsChoice debug_assert with a
  post-condition tripwire (card must land in Hand) that fires on any future
  redirect OR stranded pause; comment flags the open CardDrawn/counter
  semantics question and points a future migrator at the shared
  BatchCompletion machinery (OQ#1).
- MED-3: move seed_applied from the shared ZoneMoveRequest struct onto the
  ZoneChangeCause::Draw variant payload (Draw is the only producer) — deletes
  7 empty initializers and the sba.rs edit; type-enforces the invariant.
- LOW-1: dedup guard cites CR 614.5 (one-opportunity rule), not 614.6.
- LOW-2: de-wildcard ZoneChangeCause::source() — explicit arms, no silent
  inheritance (matches the is_exempt mandate).
- LOW-3: reword the dedup test's provenance comment (synthetic/forward-looking,
  not production-reachable).
- Drop CR 120.1 (Damage) for CR 121.1 (Drawing a Card) throughout.
@matthewevans
matthewevans enabled auto-merge June 11, 2026 11:12

@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 migrates the card-drawing mechanism to the unified zone-change pipeline (move_object via ZoneMoveRequest::draw) to ensure drawing correctly consults Moved replacement effects (CR 121.1). It introduces a deduplication guard (CR 614.5) by seeding the outer ReplacementEvent::Draw applied set into the inner Moved consult to prevent replacement effects from firing twice, and adds extensive unit tests to verify this behavior. No review comments were provided, so there is no feedback to evaluate.

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.

@matthewevans
matthewevans added this pull request to the merge queue Jun 11, 2026
Merged via the queue into main with commit 4ec5ee3 Jun 11, 2026
10 checks passed
@matthewevans
matthewevans deleted the ship/zone-t4 branch June 11, 2026 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant