feat: auto-pass control suite — priority yields, scoped phase stops, turn boundaries#5132
Merged
Conversation
…stable spells (CR 117.1d) The #4814 guard held priority on an opponent's turn whenever ANY object-level mana action was activatable — a bare Forest with an empty hand blocked auto-pass at every priority window, making smart-pass inert on opponents' turns. Hold only when the mana could enable an action: the opponent-turn rung now delegates to has_feasibly_castable_spell (covers auto-tap AND manual-float payment), placed above the upkeep/draw fast path; the own-turn castability rung is active_player-gated so the predicate runs at most once per call and never on your own upkeep/draw. Meaningful non-mana abilities and sacrifice-for-mana (#544) still hold via the untouched has_meaningful_priority_action rung; a has_feasibly_activatable_ability opponent-turn seam is documented as deferred. Follow-up to #4814 / #4388.
Players can pre-decline priority for a specific trigger source (yield this trigger, CR 400.7 incarnation-guarded) or every trigger sharing a card identity (yield all copies), MTGO-style. Yields short-circuit auto_pass_recommended ahead of the castability rungs and keep until-end-of-turn auto-pass sessions running through yielded opponent triggers. Identity is latched at trigger push on ResolvedAbility (existing source_incarnation plus new source_card_id) and re-latched at Add time from the on-stack entry, never live objects, so ceased tokens (CR 704.5d) stay yieldable. SetPriorityYield is an actor-routed preference action: legal in any state, exempt from auto-pass session clearing, redacted per viewer, persisted across turns. FE adds a long-press yield menu on triggered stack entries, an active yield list with revoke/clear-all, and i18n across all 7 locales.
Phase stops gain a turn-direction scope: PhaseStop { phase, scope } with
PhaseStopScope { AllTurns, OwnTurn, OpponentsTurns }, resolved against the
active player at every engine auto-pass site via GameState::phase_stop_hit.
Legacy bare-Phase stops (persisted saves, old action payloads) deserialize
as AllTurns through an untagged serde compat type. The FE empty-stack
auto-pass gate moves into engine auto_pass_recommended; session.rs
SetPhaseStops now delegates to apply(). Client: preferences v22→v23
migration, PhaseStopBar scope cycling, i18n scope labels across 7 locales.
AutoPassMode/AutoPassRequest UntilEndOfTurn becomes
UntilTurnBoundary { until: TurnBoundary } with
TurnBoundary { EndOfCurrentTurn (default), MyNextTurnStart }. Fixes the
docstring/behavior mismatch (the stored-mode doc claimed pass-until-my-
next-turn while the clear at turn start fired on every turn boundary):
docs now state EndOfCurrentTurn semantics, and MyNextTurnStart delivers
the documented behavior as an engine-reachable option — it survives
opponents' turn starts and clears only when the owner's own turn begins
(CR 102.1), which differs in multiplayer by N-1 turns. Legacy persisted
{"type":"UntilEndOfTurn"} payloads deserialize to EndOfCurrentTurn via
serde alias + defaulted field under internal tagging. UI still sends
EndOfCurrentTurn only.
The artificial auto-pass wait now composes the user's Animation Speed multiplier with the existing stack-pressure scaling, matching the duration-scaling convention of banner/dice pacing. At 0 (instant) the beat collapses to 0ms but the pass still dispatches — only the wait is skipped, never the action.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request implements MTG Comprehensive Rule 117.3d, introducing standing priority-yield preferences (allowing players to pre-commit to passing priority on specific triggered abilities) and turn-direction scopes for phase stops (AllTurns, OwnTurn, and OpponentsTurns). Key changes include migrating phase-stop gating from the frontend to the engine, updating auto-pass to support turn boundaries, adding a priority-yield context menu to stack entries, and rendering a list of active yields. Feedback on these changes highlights three main areas of improvement: falling back to the engine-stamped source_card_id in StackEntry.tsx to correctly resolve yields for ceased tokens, updating phase stops in-place in PhaseStopBar.tsx to preserve array order and prevent redundant sync dispatches, and increasing the touch target size of the priority-yield menu buttons to meet the style guide's mobile accessibility standards.
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.
- Match AllCopies priority yields via the engine-stamped source_card_id so a ceased token's trigger can still surface Revoke (CR 400.7 / CR 704.5d); the TS ResolvedAbility mirror was missing the field, forcing the display layer to re-derive identity from the now-absent source object. - Cycle phase stops in place (map) instead of appending, preserving array order so usePhaseStopsSync's positional dedup doesn't fire a redundant dispatch. - Give the yield-menu buttons min-h-[44px] touch targets (mobile a11y). Adds discriminating tests: ceased-token Revoke match and in-place cycle order.
The priority-yield commit added source_card_id to ResolvedAbility and updated the exhaustive destructures in ability_scan.rs and stack.rs, but missed the one in walk_ability (ability_rw.rs), so a clean build failed with E0027. It is a latched card-identity token with no read/write effect on the RwProfile, so bind it as `source_card_id: _` alongside the sibling source_incarnation latch.
Parse changes introduced by this PR · 2 card(s), 2 signature(s) (baseline: main
|
build_triggered_ability stamps the source's card identity (source_card_id) on every triggered ability for AllCopies priority-yield matching. That field rides in the derived ResolvedAbility ==, but normalize_ability_identity — which strips per-instance identity before the CR 603.3b indistinguishability comparison — only zeroed source_id, not source_card_id. Two outcome-identical triggers off distinct sources (distinct CardId) then compared as distinguishable and lost their auto-ordering, regressing order_triggers_identical_no_input_triggers_auto_order, issue_860 (two Luminarch Aspirants), multiple_suspended_cards_all_tick_on_upkeep, and issue_4269 (two Shamblers). Zero source_card_id alongside source_id: it is a source-identity latch with no bearing on a trigger's game outcome.
github-merge-queue Bot
pushed a commit
that referenced
this pull request
Jul 6, 2026
* feat(client): discoverable tap affordance for priority yields (CR 117.3d) The priority-yield auto-pass control shipped in #5132 was reachable only by long-pressing a triggered stack entry — an undiscoverable gesture that also collided with the mobile card-preview long-press (both fired on the same triggered entry, so trigger cards could no longer be previewed). Replace it with an always-visible tap button on triggered-ability entries that toggles the existing yield menu, and restore long-press to a single uniform meaning (inspect-and-pin) for every entry. The button sits on the center-right edge — the one card zone clear of the status badges, chip row, and ability overlay — and turns amber (bell-off icon) when a yield already stands for the trigger. Pure display/dispatch: the engine still owns all yield state via SetPriorityYield / priority_yields. * fix(engine): auto-pass past bare sacrifice-for-mana with no downstream use (CR 117.1d) auto_pass_recommended held frontend priority at every window whenever the player controlled a sacrifice-for-mana permanent (Eldrazi Spawn, Krark-Clan Ironworks, Phyrexian Altar), even when the mana could pay for nothing and the sacrifice triggered nothing. On an opponent's turn with an uncastable hand the player was asked for priority endlessly (repro: 8 Eldrazi Spawn, 3 sorceries in hand, opponent trigger on the stack, legal_actions == [PassPriority]). Narrow the issue-#544 sac-for-mana hold: it now blocks auto-pass only when the sacrifice or its mana enables a concrete follow-up — a feasibly castable spell (already handled by the #4388 castability rungs above), a mana-costed non-mana activated ability (case 2), or a leaves-battlefield/dies/sacrifice trigger (case 3, aristocrats/altars). A bare sac-for-mana with none of these auto-passes. The narrowing lives entirely in auto_pass_recommended. has_meaningful_priority_action is kept byte-identical (extracted into flat_actions_have_meaningful_priority + has_activatable_sacrifice_for_mana, then recomposed) because the CR 732.5 loop-shortcut firewall depends on it still counting a sacrifice as a board change. mana_sources.rs is untouched. Cases 2 and 3 are conservative presence checks that err toward holding, so no real play is ever silently auto-passed; a precise sac->activate affordability check is a documented follow-up. 'Counter unless pay {1}' is unaffected — that payment is a WaitingFor::UnlessPayment prompt at resolution, never a priority window (regression test V7). CR 605.3a + 603.6 + 603.6c + 117.1d + 118.12a. * fix(stack): make yield affordance discoverable and dismiss preview on open The always-visible yield control on triggered stack entries was a bare dark icon that read as decoration and was easy to miss. Replace it with a high-contrast labeled pill (indigo resting / amber when a yield stands) carrying a compact 'Auto-pass' label so its purpose is legible without hovering. Add menuButtonShort / menuButtonShortActive across all 7 locales. Also clear the hovered/sticky card preview when the yield menu opens and gate the hover handler while it's open, so the preview no longer lingers on top of the menu while the player reads the options (mouseenter never re-fires or clears as the pointer moves onto the menu's child elements). * fix(client): make priority-yield menu options work and collapse standing yields into a compact chip (CR 117.3d) - PopoverMenu: seal onPointerDown as well as onClick on the portal container so a menu interaction never leaks through the React tree to the card's useLongPress (which was firing the card preview and leaving the menu open — the "options do nothing" bug). Bubble-phase only, so the capture-phase outside-click dismissal is unaffected. - PriorityYieldList: collapse the unbounded vertical list into one fixed-footprint amber "Auto-passing N" chip that opens the revocable list inside the now-sealed PopoverMenu, keeping the action rail height constant regardless of yield count. - FullControlToggle: shorten the visible label to "Control" (aria-pressed + descriptive aria-label preserve toggle semantics; tooltip elaborates), placed on the same row as the chip. - Extract the shared YieldMuteIcon glyph; add PopoverMenu and PriorityYieldList regression tests (the PopoverMenu test fails without the pointer-event seal). --------- Co-authored-by: matthewevans <matthewevans@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Five commits landing the auto-pass control improvements:
All engine work ran the plan-review/impl-review pipeline to clean verdicts; serde compat covers persisted IndexedDB/SQLite states and old action payloads.