multiplayer: lobby UX overhaul + handshake + P2P Commander - #57
Merged
Conversation
Protocol extensions (additive, #[serde(default)] for back-compat):
- ServerHello/ClientHello handshake with protocol_version + build_commit gates
- ServerMode::{Full,LobbyOnly} advertised by the server
- LobbyGame gains current_players, max_players, format, host_version,
host_build_commit, and room_name
- LobbyGameUpdated broadcast so clients see slot counts tick live
Lobby UX:
- Room name distinct from player name — GameListItem shows room as primary
- Full games disabled in the list, proactive password modal, join-with-context
banner in the deck picker, richer empty state with Host/Refresh CTAs
- Lobby rows disabled on host-build-commit mismatch
- Server region picker (US/EU presets + self-hosted URL) + ServerOfflinePrompt
replacing the auto-flip-to-P2P with an explicit modal
Player identity UX:
- Inline PlayerIdentityBanner editor on the multiplayer page — no more
hunting in Preferences -> Multiplayer to change your name
- HostSetup relabels "Display Name" -> "Your Name" with write-through to
the store on change, plus a separate optional "Room Name" field
- WaitingScreen game code is now a copy-to-clipboard button
Reconnect resilience:
- Toast store migrated to Map<string, Toast> so concurrent player disconnects
in 3-4p games don't overwrite each other; every toast carries an absolute
wall-clock expiresAt so tab throttling cannot drift countdowns
- ConnectionToast stacks countdowns at top, plain toasts at bottom; dismissal
is a pure function of (expiresAt <= now) so Map-mutation re-renders
cannot reset 5s plain-toast timers
- Stale in-game overlay countdown removed; the toast owns the live timer
P2P 4-player Commander:
- HostSetup filter/caps/picker respect P2P_MAX_PEERS=4 (matches the WebRTC
mesh ceiling in p2p-adapter.ts)
- MultiplayerPage P2P-host URL threads format + players so GamePage
rehydrates them into P2PHostAdapter end-to-end
matthewevans
enabled auto-merge (rebase)
April 15, 2026 17:07
The PlayerIdentityBanner rendered above HostSetup in MultiplayerPage already
edits the player's name globally, so HostSetup's "Your Name" field was a
second input bound to the same store value — confusing ("Player Name" above
vs "Your Name" below) and redundant.
HostSetup now focuses on per-match settings only (Room Name, format, etc.)
and reads `displayName` from the store at submit time.
Two bugs + a dead-end UX: 1. Clicking "Change" on the active-deck banner cleared `pendingAction` but the deck-select view's confirm button was hardcoded to `pendingAction?.type === "host" ? "Host Game" : "Join Game"` — so null fell through to "Join Game", and `handleDeckConfirm` returned early on null (the no-op the user reported). 2. The back handler with `pendingAction = null` dumped the user into lobby regardless of where they came from; another back from lobby escaped multiplayer entirely. 3. The confirm-step was pointless: there's no deck preview on the select screen, so clicking a tile to highlight + clicking a second button to commit is two clicks for one decision. Now: - A new `deckSelectReturn` state remembers which view rendered the "Change" button (lobby or host-setup), and the back handler routes back to it. - `handleSelectDeck` runs the full flow in one click: persist the choice, then either execute the pending host/join action or `setView` back to the caller. The separate confirm button is removed.
The deck picker filtered by `pendingAction.settings.formatConfig.format` for submit-without-deck and by `pendingAction.format` for lobby joins, but the "Change Deck" path clears `pendingAction` — leaving `selectedFormat` undefined and the picker showing every deck regardless of format. HostSetup now mirrors its in-flight format to the store on every change, and MultiplayerPage falls back to that value when no pending action is present. Also seeds HostSetup's initial format from the store so the user's last choice survives navigating away and back.
matthewevans
disabled auto-merge
April 15, 2026 17:17
matthewevans
added a commit
to real-venus/phase
that referenced
this pull request
Jul 13, 2026
* fix(parser): bind the chain-set anaphor across its participle axis (CR 608.2c)
`QuantityRef::TrackedSetAggregate` and its live resolver both already existed,
but the parser's anaphor list recognized only the EXILE forms ("those exiled
cards", "the card exiled this way"). Every other participle fell through to an
honest red and the whole surrounding clause was dropped:
Ill-Timed Explosion "the greatest mana value among cards discarded this way"
Sword of the Ages "the total power of the creatures sacrificed this way"
Reign of the Pit (same expression)
Ruinous Intrusion "the mana value of the permanent exiled this way"
Astarion's Thirst "the power of the creature exiled this way"
Verdant Rejuvenation "the HIGHEST mana value among creatures you control"
Three axes were enumerated as phrase tables where they are independent choices,
so each new card needed a new full-string tag. They are now composed:
- extremum adjective (greatest | highest | largest) -> AggregateFunction::Max
- object property (power | toughness | mana value)
- chain-set anaphor ([the ] <noun> <participle> this way)
`parse_tracked_set_anaphor` is now the single authority for chain-set anaphora,
composed on noun x participle rather than listed as permutations. Verdant
Rejuvenation needed nothing but the word "highest" — the aggregate axis had been
a table of six full-phrase tags.
A SINGULAR "this way" referent ("the <property> of the <noun> <participle> this
way") carries no aggregate adjective, so it cannot ride the extremum/total
prefix. It gets its own gated arm, reading the same chain set — `Sum` over a
one-member set is that member's value, following the precedent already set for
the singular "the card exiled this way".
PARTICIPLE LIST IS DELIBERATELY RESTRICTED to the causes the engine actually
stamps (`ThisWayCause::{Exiled, Discarded, Sacrificed, Milled}`, via
`publish_tracked_set_with_causes`). "goaded" is EXCLUDED: `effects/goad.rs`
publishes no tracked set, so binding Havoc Eater's "the total power of creatures
goaded this way" would aggregate over an empty set and silently resolve to 0 —
a well-typed lie of exactly the class phase-rs#57 exists to kill. It stays an honest red
and is pinned by a negative-control test.
The bare PRE-nominal form ("the exiled card") stays reachable only behind an
aggregate prefix, where it is unambiguous. Offering it as a bare referent
regressed two other referents that claim the same words — the linked-exile pool
("the mana value of the exiled card") and the craft-material pool ("... used to
craft it"). Caught by the existing suite; the explicit "this way" shape is what
makes the bare-referent arm safe.
FULL-POOL LEDGER (35,396 faces, base 4e6e5aa; whole-face structural diff,
gains and reds separate, never netted):
8 faces changed
8 gained binding
0 gained honest red
0 re-shaped
Target faces: Verdant Rejuvenation, Ill-Timed Explosion, Ruinous Intrusion,
Astarion's Thirst, Reign of the Pit, Sword of the Ages (partial - its "those
creature cards" exile clause is a separate, still-open red).
COLLATERAL GAINS (2), both verified correct, neither in the unit's brief:
Fateful Tempest "the total mana value of cards milled this way"
Shadowgrange Archfiend "the greatest power among creatures sacrificed this way"
Tests: 4 parser witnesses + 4 runtime witnesses (3 positive + 1 negative
control). The runtime witnesses hand the parsed QuantityExpr to the live
resolver against a populated chain set, and were WATCHED FAILING pre-fix via a
toggle probe (participle arms disabled -> both aggregate witnesses read 0;
restored). The discarded-mana-value witness was VACUOUS in its first draft:
`add_creature` leaves the mana cost empty, so every member's mana value was 0
and the assertion would have passed for the wrong reason (0 == 0). It now
establishes the costs explicitly via `with_mana_cost`.
Co-authored-by: matthewevans <matthewevans@users.noreply.github.com>
* fix(engine): give the previous-effect quantity its EXCESS channel (CR 120.10)
`DamageChannel` existed. `GameState::last_effect_excess_amount` existed, was
stamped by the damage effects, and was cleared at depth-0. The CONDITION peer
`AbilityCondition::PreviousEffectAmount { channel }` already read both channels.
Only the QUANTITY side was missing its channel:
AbilityCondition::PreviousEffectAmount -> channel: DamageChannel -> reads both
QuantityRef::PreviousEffectAmount -> BARE, no channel -> total only
`ability.rs` even documented `QuantityRef::PreviousEffectAmount` as the
total-channel twin. The excess twin was simply never built, so there was no way
to TYPE "the amount of excess damage dealt to that creature this way" — and the
whole surrounding instruction was dropped:
Goblin Negotiation "Create ... 1/1 red Goblin creature tokens equal to the
amount of excess damage dealt to that creature this way."
Hell to Pay (same shape, tapped Treasures)
Lacerate Flesh (same shape, Blood tokens)
Adds `channel: DamageChannel` to the variant (serde-elided `Total`) plus the
resolver arm. This is a parameterization, not a new leaf: a sibling
`PreviousEffectExcessAmount` would be the textbook sibling-cluster smell, and
the axis lies wholly inside CR 120 (120.6 total / 120.10 excess), so it does not
cross a rule-section boundary. The mtgish FIELD gate does not apply — it is
scoped to the `Effect` enum, and mtgish-import never names this variant
(confirmed by a clean `cargo check --workspace`, not by grep).
The compiler was the census: 20 sites across 11 files, including one in
`phase-ai` that a `crates/engine` grep would have missed. No wildcard arms were
used to absorb the new field.
"THAT EXCESS DAMAGE" IS DELIBERATELY NOT BOUND, and that is a CORRECTNESS
constraint. The bare demonstrative's antecedent is fixed by its sibling clause,
and the two readings resolve from DIFFERENT state:
Contest of Claws "If excess damage was dealt THIS WAY, ... where X is that
excess damage." -> same resolution, field is live -> Excess is right.
Fall of Cair Andros "Whenever a creature ... is dealt excess noncombat damage,
amass Orcs X, where X is that excess damage."
-> the antecedent is the TRIGGERING EVENT. The triggered
ability resolves as its own top-level chain and the depth-0
prelude CLEARS last_effect_excess_amount, so the bind reads
None -> 0.
I shipped that bind into a local export and the whole-face ledger caught it: Fall
of Cair Andros moved from `Amass { count: Variable{name:"X"} }` (the phase-rs#57 residual
fabrication, resolving to 0) to `Amass { count: PreviousEffectAmount{Excess} }` —
the SAME wrong answer wearing a more convincing type. Strictly worse. A
channel-scoped ledger would have missed it: it shows up only as "red count
unchanged, tree changed".
A context-free leaf combinator cannot separate those two readings; the
disambiguator lives one layer up. So only the shape carrying an explicit "this
way" IN THE PHRASE is bound. The bare demonstrative stays an honest red, pinned
by a negative control, and the clause-layer rebind is filed as follow-up work
(along with the Fall of Cair Andros phase-rs#57 residual leak, which hides inside the
legitimate name:"X" alias population and is therefore invisible to t57's probe).
FULL-POOL LEDGER (35,396 faces, base 4e6e5aa; whole-face structural diff,
cumulative with the chain-set anaphor commit; gains and reds separate, never
netted):
11 faces changed
11 gained binding
0 gained honest red
0 re-shaped
This commit's share: Goblin Negotiation, Hell to Pay, Lacerate Flesh.
BYTE-IDENTITY: the diff is empty outside the bound-face census, so the `Total`
serde elision leaves every pre-existing PreviousEffectAmount face unchanged.
NOT CLAIMED, and filed instead of hidden:
- Archaic's Agony is a LYING GREEN — "Exile cards ... equal to the excess damage
dealt to that creature this way" parses to ChangeZone with count: null and ZERO
Unimplemented. This bind does not reach it: the top-of-library exile mover never
consults the quantity parser for its count. Filed as a mover-path defect.
- Mephit's Enthusiasm, Molten Impact (`note`), Brazen Boarding (`conjure`),
Overclocked Electromancer (energy `get`, and turn-scoped -> wants
DamageDealtThisTurn{Excess}, a third channel) are OUTPUT-INERT: their
surrounding effect is still unimplemented, so no gain is claimed for them.
Tests: 4 runtime witnesses (2 positive, 2 negative controls). WATCHED FAILING
pre-fix via a toggle probe: with the excess arm disabled, every positive witness
fails with `Unimplemented { name: "effect_structure" }` — proving the clause was
dropped whole, not merely mis-valued. The Total-channel control passes in both
states, so a resolver that always read the excess field would not survive it.
Co-authored-by: matthewevans <matthewevans@users.noreply.github.com>
* fix(engine): migrate 23 main-drift PreviousEffectAmount sites to the channel field (CR 120.6)
Rebase fallout: main gained 23 bare-form QuantityRef::PreviousEffectAmount
sites after this unit's base (mostly phase-rs#5721-era where-X test pins), all E0533
against the new struct variant. Every site is Total-channel semantics
(drain gains, reveal-draw counts, for-each counter tails, repeat_for), so
expression sites get channel: DamageChannel::Total explicitly and pattern
sites match { .. } per the unit's own idiom. Census: CI's full E0533 list
(21 engine sites) plus 2 phase-ai sites CI could not reach because the
engine crate failed first; grep census cross-checked against both.
---------
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.
Summary
Ships the combined multiplayer UX overhaul, server/client version handshake, and P2P 4-player Commander support as a single coherent unit. Intermediate splits would leave the tree in broken states (e.g. protocol field defined but unused), so everything lands together.
Protocol (additive,
#[serde(default)]for back-compat)ServerHello/ClientHellohandshake withprotocol_version+build_commitgates;ServerMode::{Full,LobbyOnly}advertised on acceptLobbyGamegainscurrent_players,max_players,format,host_version,host_build_commit, androom_nameLobbyGameUpdatedbroadcast so clients see slot counts tick live as guests joinLobby UX
GameListItemshows the room as the primary title with "by {host}" as secondary metadataServerPicker(US/EU presets + self-hosted URL) reachable via a pill next to the online-count badgeServerOfflinePromptreplaces the auto-flip-to-P2P with an explicit "Keep trying / Use direct code" modalPlayer identity UX
PlayerIdentityBanneron the multiplayer page — inline name editing, no more hunting in Preferences → MultiplayerHostSetuprelabels "Display Name" → "Your Name" with write-through to the store on change, plus a separate optional "Room Name" fieldWaitingScreengame code is now a copy-to-clipboard buttonReconnect resilience
Map<string, Toast>so concurrent player disconnects in 3-4p games don't overwrite each otherexpiresAtso tab throttling can't drift countdowns, and dismissal is a pure function of(expiresAt <= now)— immune to Map-mutation re-rendersConnectionToaststacks countdowns at top, plain toasts at bottomP2P 4-player Commander
HostSetupfilter/caps/picker respectP2P_MAX_PEERS = 4(matches the WebRTC mesh ceiling inp2p-adapter.ts)MultiplayerPageP2P-host URL now threadsformat+playerssoGamePagerehydrates them intoP2PHostAdapterend-to-end (previously silently defaulted to 2p Standard)Test plan
cargo fmt --all -- --checkcargo clippy --all-targets -- -D warningscargo test -p server-core -p phase-server(84+ passing)pnpm type-checkpnpm lintpnpm test -- --run(387/387 passing)PlayerIdentityBanner, confirm it persists and appears on next hostphase-serverwhile in lobby — confirmServerOfflinePromptappears with both options