fix(engine): offer draw-replacement independently per unit of a multi-card draw (CR 121.6b)

[rykerwilliams]

Summary

Multi-card draws (Effect::Draw{count: N > 1}) treated the whole count as one atomic replaceable event: accepting a substitute replacement (Dredge, Notion Thief, Hullbreacher, or any other ReplacementEvent::Draw producer) zeroed the entire proposed count, not just the unit it replaced. A count: 2 draw with one dredge-eligible card in the graveyard, if dredged, delivered zero cards instead of one dredged + one normal draw — reproducing a live user report: "drew no cards" when dredging a Stinkweed Imp during Bazaar of Baghdad's "draw two cards" (the unconditional "discard three cards" step still fired correctly afterward, which is what made the bug visible).

CR 121.6b: "If an effect replaces a draw within a sequence of card draws, the replacement effect is completed before resuming the sequence." Each unit of a multi-card draw is an independent event and must independently offer replacement.

Design

Adds resume_multi_draw (game/effects/draw.rs), looping one unit at a time through the existing draw_through_replacement/apply_draw_after_replacement primitives instead of proposing the whole count as one event. A unit that pauses on its own replacement choice stashes a new PendingMultiDraw{player, remaining, accumulated} continuation, drained by handle_replacement_choice's Draw arm using the same re-pause-composes-arbitrarily-many-times contract already proven by the zone-move batch-delivery machinery (library_placement_survives_two_sequential_parks).

CR 609.3: apply_draw_after_replacement now returns the per-unit actually-drawn count instead of writing state.last_effect_count itself — resume_multi_draw accumulates the true total across every unit of the original instruction and commits it exactly once, so a chained "discard that many" sees the real total, not just the last unit's count.

CR 800.4a: PendingMultiDraw is added to replacement::abandon_post_replacement_continuation (the single authority for tearing down a departing player's in-flight continuations) rather than hand-listed in elimination.rs directly — that function exists specifically because a prior bug (issue #4886) was caused by hand-listing fields at the call site instead of routing through one authority.

engine_debug.rs's DebugAction::DrawCards routes through the same primitive for parity with the real draw pipeline.

Explicitly out of scope: Connive's own multi-count draw path (Connive N, N > 1). connive.rs documents a previously-fixed collision between a shared generic mid-draw continuation and its own draw-then-connive ordering requirements — reusing pending_multi_draw at the same resume site risked reintroducing that exact failure mode. Filed as a separate follow-up rather than bundled in here.

Test plan

• multi_draw_dredges_one_of_two_units_other_draws_normally — drives the real GameAction::ChooseReplacement dispatch, asserts exactly 1 card drawn (not 0, not 2) and last_effect_count == 1 (the true total, not the naive per-unit count)
• multi_draw_decline_dredge_unit_one_still_draws_unit_two_normally — declining unit 1 still offers the same dredge for unit 2
• Elimination-cleanup: departing player's pending_multi_draw cleared; surviving player's preserved (extends existing elimination.rs tests)
• Full engine test suite re-run clean: 15698 passed, 0 failed, 0 regressions
• Clippy clean (-D warnings)
• nom-combinator gate passes (no parser files touched)

🤖 Generated with Claude Code

https://claude.ai/code/session_01DMa6DrxXyFHBdGxz3uLgvM

[gemini-code-assist]

Code Review

This pull request implements independent replacement choices for multi-card draws per CR 121.6b, introducing resume_multi_draw to process draws sequentially and track in-progress state via pending_multi_draw. A high-severity issue was identified where cards drawn by a paused unit after its replacement choice is resolved are omitted from the accumulated total, leading to an incorrect state.last_effect_count when the sequence resumes.

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] Drawn cards from paused multi-draw units are omitted from the accumulated total. Evidence: crates/engine/src/game/effects/draw.rs:315.
Why it matters: When a multi-card draw pauses for a replacement choice and is then resumed, any cards actually drawn by the paused unit are completely omitted from state.last_effect_count because apply_draw_after_replacement ignores the accumulated total and the resume path passes the stale pending.accumulated count.
Suggested fix: Update state.pending_multi_draw's accumulated count inside apply_draw_after_replacement so it is correctly preserved when resuming.

    let drawn_count = cards_to_draw.len() as u32;
    if let Some(pending) = &mut state.pending_multi_draw {
        pending.accumulated += drawn_count;
    }

[matthewevans]

[MED] The paused draw unit is omitted from the resumed multi-draw total. Evidence: crates/engine/src/game/engine_replacement.rs:357 calls apply_draw_after_replacement(state, draw, events) but ignores the returned drawn count, then crates/engine/src/game/engine_replacement.rs:665 resumes the tail with pending.accumulated unchanged. resume_multi_draw only adds counts for units it applies itself (crates/engine/src/game/effects/draw.rs:192-199), so a declined/replacement-resolved paused unit that actually draws cards is not included before state.last_effect_count is stamped at crates/engine/src/game/effects/draw.rs:210. Why it matters: chained effects that read EventContextAmount/last_effect_count after a multi-card draw can see a lower total whenever one unit paused on a replacement choice and then drew normally. Suggested fix: capture the u32 returned by the ProposedEvent::Draw resume arm and add it to pending.accumulated before calling resume_multi_draw; add a regression where unit 1 declines Dredge, unit 2 draws normally, and the final last_effect_count includes both drawn units.

This confirms Gemini's current-head concern. I did not approve or enqueue. Confidence: high. Contradicting evidence would be another path that adds the resumed unit's apply_draw_after_replacement return value into PendingMultiDraw.accumulated before the tail resumes; I did not find one on this head.

[github-actions]

Parse changes introduced by this PR

✓ No card-parse changes detected.

[rykerwilliams]

@matthewevans Confirmed, great catch — verified the exact mechanism before fixing: `handle_replacement_choice`'s `Draw` arm resolves the paused unit directly via `apply_draw_after_replacement` and was discarding its returned count, so it never reached `resume_multi_draw`'s accumulator when the tail resumed.

Fixed in 8c7d5e9: fold the just-resolved unit's drawn count into `PendingMultiDraw.accumulated` right where `apply_draw_after_replacement` returns it, before the drain below reads it. Extended the existing decline regression to also decline unit 2's offer and assert `last_effect_count == 2` (both units' real draws) — the exact fixture you suggested. Full engine suite re-verified clean (15698 passed).

[matthewevans]

Reviewed current head 4e49588: prior multi-draw accumulator finding is fixed at the replacement resume seam; CI is green; parse-diff shows no card-parse changes.