feat(engine): auto-pass priority-window overhaul (G1/G2/G3/G5/G6)#5346
Merged
Conversation
…5.1b/603.3) Stage 0+1 of the auto-pass correctness overhaul. `auto_pass_recommended` now holds priority (rung 5, own empty-stack main phases) when tapping one of the player's own currently-activatable mana sources would queue a NON-mana tap trigger whose effect benefits the tapper — opponent-scoped damage/life-loss or controller-scoped life gain (canonical: Zhur-Taa Druid, tap the dork to deal 1 to each opponent). Self-punisher and symmetric tap triggers (Manabarbs, Burning Earth, Price of Glory, Forbidden Orchard) correctly keep auto-passing. Value-gated by beneficiary sign, computed structurally from the parsed AST via `effect_benefits_trigger_controller` / `trigger_chain_benefits_controller` (mana_sources.rs) — the sign-inverted twin of `effect_controller_harm_amount`; no AI value layer. Firing reuses the trigger resolver's own authority (`taps_for_mana_card_matches` / `valid_player_matches`, bumped to pub(crate)) rather than a global rescan, and iterates both `ActivateAbility` and `TapLandForMana` action shapes so land-scoped triggers are covered. Stage 0: thread a single lazily-built `PriorityCastProbe` through `has_feasibly_castable_spell` (behavioral no-op) and share one mana-action sweep between rung 5 and rung 9 — avoids per-window `GameState` clone-storms and the rung double-evaluation hazard. G1 stage-1 is a clone-free battlefield AST walk gating the sweep. CR 605.1b/603.3 (a non-mana tap trigger fails the mana-ability test, so it uses the stack and a priority window), CR 605.4a (contrast: mana triggers resolve inline), CR 106.12a (tapped for mana), CR 119.3/120.3 (life/damage benefit sign), CR 603.2 (trigger execution).
…st path Stages 2+3 of the auto-pass overhaul, both fixing priority-window bugs in `auto_pass_recommended` (single authority for priority auto-pass). G3 (own-top hoist): moves the "own object on top of the stack -> auto-pass" rung above the castability rungs, so holding a castable instant no longer nags you to respond to your own spell/ability (false-HOLD). Accepted MTGA parity -- own triggers lose their implicit stop; explicit yields (CR 117.3d) still win, and the rung stays disjoint from the G1 empty-stack beneficial-tap hold (own-top requires a non-empty stack). G2 (gated upkeep/draw fast path): the bare `auto_passes_initial_priority_by_default` fast path fast-passed ANY empty-stack upkeep/draw window -- including an opponent's upkeep where you hold priority, and your own upkeep where you have a meaningful activated ability (false-PASS). Now gated on `active_player == player` plus a new `flat_actions_have_meaningful_noncast_priority` predicate that holds for meaningful non-cast flat actions while letting merely-castable instants keep auto-passing (MTGA parity, preserving the existing upkeep-instant regression). Kept separate from `flat_actions_have_meaningful_priority` so the CR 732.5 loop-detection firewall primitive stays byte-identical. 8 new tests drive the real `auto_pass_recommended` entry point with reach-guarded negatives; the existing upkeep-instant regression is untouched.
Stage 4 of the auto-pass overhaul. Parameterizes `YieldTarget` so a standing priority yield (CR 117.3d) is keyed per-trigger and tolerant of triggers that never latched an object incarnation, instead of collapsing every trigger from a source into one coarse yield. G5 (per-trigger precision): `ThisObject` and `AllCopies` gain `trigger_description: Option<String>`, latched from the stack entry's trigger description at yield time. `is_priority_yielded` matches on it, so yielding one of a source's triggers no longer silently yields its other triggers. A stored `None` description is a wildcard (source-level) match, preserving legacy persisted yields and coarse yields. G6 (synthetic-trigger latch, CR 400.7): `ThisObject.incarnation` becomes `Option<u64>`. A trigger with no latched incarnation (synthetic/delayed game-rule triggers) previously made a `ThisObject` yield a silent no-op; it now stores and matches a `None`-incarnation yield. Serialized surface (GameState -> saves / multiplayer / WASM): migration is serde-default-driven -- legacy bare-u64 incarnations deserialize as Some, absent trigger_description defaults to None. No migration code. FE contract (adapter/types.ts) and the StackEntry/PriorityYieldList display mirror the None-description wildcard so legacy/coarse yields still render the Revoke pill. engine_wasm.d.ts unchanged (YieldTarget serializes as opaque JSON).
![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 introduces fine-grained priority yields scoped to specific triggers using a trigger_description discriminator, and adds support for yielding synthetic/delayed triggers by allowing None incarnations. Additionally, it refactors the auto-pass recommendation engine to include a hold for beneficial non-mana tap triggers (CR 605.1b) and gates the upkeep/draw fast-pass to exclude merely-castable instants for MTGA parity. No review comments were provided, so I have 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.
Parse changes introduced by this PR✓ No card-parse changes detected. |
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.
Auto-pass priority-window overhaul
Fixes 5 of the 6 gaps from the auto-pass audit in
auto_pass_recommended(the single authority for priority auto-pass). Each is decomposed into reusable primitives, not card special-cases.G1 — beneficial mana-tap trigger hold (
83cc21d): holds priority when tapping your own mana source would fire a beneficial non-mana tap trigger (Zhur-Taa Druid pings each opponent). Self-punishers/symmetric triggers (Manabarbs, Price of Glory, Forbidden Orchard) correctly keep auto-passing. Value-gated by beneficiary sign computed structurally from the AST. CR 605.1b/603.3.G2 — gated upkeep/draw fast path (
6dadfe4): the bare fast path fast-passed any empty-stack upkeep/draw window — including an opponent's upkeep where you hold priority, and your own upkeep with a meaningful activated ability. Now gated onactive_player == player+ aflat_actions_have_meaningful_noncast_prioritypredicate that holds for meaningful non-cast actions while letting merely-castable instants keep auto-passing (MTGA parity). The loop-firewall primitive stays byte-identical.G3 — hoist own-top-of-stack (
6dadfe4): own object on top now outranks the castability rungs, so holding a castable instant no longer nags you to respond to your own spell/ability. Explicit CR 117.3d yields still win.G5 — per-trigger yield precision (
679be20):YieldTargetgainstrigger_description, so yielding one of a source's triggers no longer silently yields its siblings.None= source-level wildcard (preserves legacy yields).G6 — synthetic-trigger yield latch (
679be20):ThisObject.incarnationbecomesOption<u64>; synthetic/delayed triggers (previously a silent no-op) can now be yielded. CR 400.7.Verification
/review-implon each cycle — all clean.Serialized surface
YieldTargetis inGameState(saves/multiplayer/WASM). Migration is serde-default-driven — legacy bare-u64incarnations load asSome, absenttrigger_descriptiondefaultsNone. No migration code.engine_wasm.d.tsunchanged (opaque JSON).Deferred
G4 (feasible manual-float activated-ability hold) is intentionally not in this PR — it's the separable, deepest change (threads
ManaAffordabilitythroughcasting.rsmana internals) and needs its own discriminating fixture. Follow-up.