fix(client): reveal own face-down permanents - #7342
Merged
Merged
Conversation
Contributor
|
Warning Review limit reached
Next review available in: 15 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
matthewevans
enabled auto-merge
August 13, 2026 14:49
This was referenced Aug 18, 2026
matthewevans
added a commit
to traemyn/phase
that referenced
this pull request
Aug 20, 2026
…hase-rs#4381) (phase-rs#7542) * feat(engine): offer the turn-face-up special action (phase-rs#6732, phase-rs#4381) The engine accepted `GameAction::TurnFaceUp` and its Priority preflight counted it as progress, but `ai_support::candidates::priority_actions_ with_probe` — the list the client renders — never emitted it. Nothing can send an action the engine never advertises, so the whole morph / megamorph / disguise / manifest / cloak class was unturnable in play. Reported from a real game state: the controller had priority, a face-down Coral Trickster (morph {U}) and thirty untapped Islands, and `legalActions` held six casts, four land plays and a pass. phase-rs#7342 wired the client's dispatch and closed both reports; its test supplies the action to itself (`legalActions: [turnFaceUpAction]`), so it proves the client's half and cannot observe the engine's. That is how the gap survived a green suite. ## One admission authority `morph::turn_face_up_offer` answers "may this player take the action on this permanent right now, and in which shape". Both the Priority preflight and the offer list read it, so the engine's progress gate and the list it renders cannot disagree — which is the disagreement that produced this defect. `turn_face_up_prepare` stays the legality and cost authority underneath; the offer adds the special-action cost reduction and the affordability probe the reducer applies. ## The payment can now finish phase-rs#4538 was asked for this before it went stale: the affordability probe deliberately reports a mana source whose own cost pauses (CR 605.3b + CR 616.1) as payable, and the compatibility wrapper `pay_special_action_mana_cost` converts that `Paused` into an error. Offering the action without a resume would advertise a flip that cannot complete. The action now owns a typed, cost-snapshotted continuation like the two shipped precedents (`companion.rs`, `end_continuous_effect.rs`): `ManaAbilityResume::TurnFaceUp { player, object_id, cost, announced_x }`. `cost` is locked after the reduction and after CR 107.3d's {X} was concretized, so resumption cannot re-derive it against a board that changed while the choice was pending; `announced_x` travels with it because CR 702.37f / CR 702.168e publish that value to the permanent's own turn-face-up trigger, which fires after payment. `morph::handle_turn_face_up` is the single authority for the whole action — legality, the CR 106.6 spend-restricted payment, the X announcement and the flip — shared with the resume. The reducer arm delegates to it, which is what moved 80 lines out of `engine.rs` and re-pins the CR 603.5 prompt census by the same offset. ## Counter-probe | disabled | failing rows | |---|---| | the offer | `a_face_down_morph_permanent_is_offered_and_flips`, `a_paused_mana_source_resumes_the_locked_turn_face_up` | | the typed resume (old wrapper) | `a_paused_mana_source_resumes_the_locked_turn_face_up`, on the pause being reported as an error | The unpayable and opponent-controlled rows stay green under both, which is what keeps the positive row from passing for the wrong reason. ## Not covered * A morph/disguise cost with {X} (Warbreak Trumpeter, Bane of the Living, Aurelia's Vindicator). CR 107.3d says the player chooses X immediately before paying, so a flat action list has no value to offer and the engine must not choose one. Stated in the enumeration rather than silently dropped; it needs an X announcement for special actions on the client. * `GameAction::PlayFaceDown` is absent from the same list. Separate action, separate change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(PR-7542): preserve paused turn-up X=0 * fix(PR-7542): remove duplicate turn-up migration * fix(PR-7542): preserve the waiting state a turn-up replacement raises The turn-face-up completion returned `WaitingFor::Priority` unconditionally, overwriting any interactive choice the CR 614.1e "As ~ is turned face up" replacement pipeline installed (CR 616.1 ordering prompts included) and stranding the live `pending_replacement` record. It now seeds the settled outcome before the flip and hands back whatever the pipeline left, on both the fresh route and the paused-payment resume (where the seed also clears the just-answered mana-source prompt instead of resurrecting it). Preserving the pause exposed a second loss: the action's settled epilogue no longer runs, so the `TurnedFaceUp` observer triggers were dropped (measured: the "when turned face up" draw never reached the stack). The completion now parks them through `park_observer_triggers_if_paused`, the established authority for exactly this shape; a no-op on an undisturbed flip. Both halves are load-bearing: with the pre-fix return both new rows fail at the live-choice assertion; with the park removed both fail at the trigger assertion. The four existing rows stay green either way. Known remainder: `turn_face_up` still discards a `NeedsChoice` from TWO simultaneously applicable "as turned up" replacements on one permanent — unreachable from parsed cards today (the parser emits at most one self-anchored definition per card). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(engine): prove the chosen replacement order determines the result (phase-rs#6732, PR 7542) Review round 3 on PR 7542: both replacement-order regressions selected index 0 and accepted either 11 or 12, so a stale or extra candidate, a wrong candidate order, or a pipeline ignoring the selection could still pass. Each row now runs once per selectable order through the production GameAction reducer and asserts: - the prompt holds EXACTLY the two live counter modifiers (length 2 plus both names — no stale entries; on the resumed-payment route this also pins that the settled exile prompt does not resurface); - the selection is made by NAME, and the chosen order determines the exact count (CR 616.1): Plus One Modifier first yields (5+1)*2 = 12, Times Two Modifier first yields 5*2+1 = 11 — on the fresh route AND across both pauses of the resumed-payment route, where the X=0 binding and the mana source's graveyard arrival are asserted in both runs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: matthewevans <matthewevans@users.noreply.github.com> Co-authored-by: cuinhellcat <cuinhellcat@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.
Fixes #6732.
Also fixes #4381.