Skip to content

Mulligan bottom-cards selection rejected unless card order matches internal hand order #6342

Description

@PBernaerts

Bug

SelectCards submitted for the mulligan bottom-cards step is rejected as an
illegal action unless the submitted card order happens to match an internal,
arbitrary hand order. In practice this makes bottoming 2+ cards (any mulligan
taken more than once) fail almost every time a human clicks cards in a
natural order, since nothing in the UI or docs tells the player which order
is "correct."

Server-side error observed:

Illegal action: SelectCards { cards: [ObjectId(32), ObjectId(6), ObjectId(17)] }

Root cause

CR 103.5 lets a player choose any order when putting mulligan-owed cards on
the bottom of their library, and validate_bottom_selection
(crates/engine/src/game/mulligan.rs:535-560) correctly implements this: it
only checks the count and hand membership of the submitted cards, not their
order.

However, the server's legality pre-check in
crates/server-core/src/session.rs (around line 1287-1318) only bypasses the
enumerated-candidate legality gate for a fixed allowlist of "freeform"
WaitingFor states (accepts_freeform_card_selection, defined in
crates/engine/src/types/game_state.rs:9769-9777):

pub fn accepts_freeform_card_selection(&self) -> bool {
    matches!(
        self,
        WaitingFor::ScryChoice { .. }
            | WaitingFor::ArrangePlanarDeckTopChoice { .. }
            | WaitingFor::SurveilChoice { .. }
            | WaitingFor::DigChoice { .. }
    )
}

WaitingFor::MulliganDecision (its BottomCards sub-phase) and
WaitingFor::OpeningHandBottomCards are not in this list, even though they
are the same class of "count-constrained, any-order" selection as
Scry/Surveil/Dig.

Because the bypass is missing, SelectCards submissions during mulligan
bottoming fall through to the enumerated-candidate check
(legal_actions.contains(...)), which requires an exact match against
GameAction's derived PartialEq, which is order-sensitive for the
cards: Vec<ObjectId> field. The candidates themselves are generated by
bottom_card_actions/combinations in
crates/engine/src/ai_support/candidates.rs:4483-4515, which enumerates each
subset of the hand exactly once, in the hand's internal storage order, not
sorted, not in UI display order, and unrelated to click order.

On the client, selectedCardIds (client/src/stores/uiStore.ts) is a plain
array built by click/toggle order ([...state.selectedCardIds, cardId]), and
MulliganBottomCardsPrompt / handleBottomCards
(client/src/pages/GamePage.tsx:1140-1146, 2414-2453) sends that array
verbatim. Nothing reconciles the two orders, so unless a player's click order
coincidentally matches the hand's internal order, the submission is rejected.

Scope: server-hosted "Full" mode only, not solo/AI or P2P

This only affects games played through a server-authoritative session (a
phase-server instance in Full mode, over /ws), whether self-hosted or
otherwise. Confirmed by reading crates/engine-wasm/src/lib.rs: its
submit_action (used for solo-vs-AI and P2P play) calls the engine's apply()
directly with no enumerated-candidate legality pre-check at all, no
legal_actions.contains(...) gate, and no "Illegal action: {:?}" wrapper.
It relies solely on the engine's own internal validation
(validate_bottom_selection), which is already correct and order-insensitive.
The ordering gate described above exists only in
crates/server-core/src/session.rs. So solo/AI and P2P games cannot hit this;
any server-hosted multiplayer game can.

Suggested fix

Add WaitingFor::MulliganDecision { .. } (specifically while a pending
entry's phase is BottomCards) and WaitingFor::OpeningHandBottomCards { .. }
to accepts_freeform_card_selection, mirroring the existing Scry/Surveil/Dig
exception. validate_bottom_selection is already the correct, order-insensitive
authority and needs no changes.

Repro

  1. Start or join a game, take 2 or more mulligans (owes 2+ bottom cards).
  2. Keep the hand.
  3. Click any 2+ hand cards to bottom, in whatever order they visually appear.
  4. Confirm. Server rejects with Illegal action: SelectCards { cards: [...] }
    unless the click order happens to match the hand's internal order.

Workaround: repeatedly reselect the same cards in a different order until one
permutation is accepted (up to count! attempts).


Found via LLM-assisted source review (Claude, model: claude-sonnet-5) while
debugging a real in-game mulligan failure. All file/line references above
were read directly from the v0.34.0 source; not verified by the reporter with
an independent Rust build.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions