Use native engine for hosted P2P games - #6515
Merged
Merged
Conversation
matthewevans
enabled auto-merge
July 22, 2026 20:51
Contributor
📝 WalkthroughWalkthroughThis change adds native P2P pregame transport and optional native host authority, introduces revisioned state protocol messages and host abandonment, updates persistence and resume flows, and revises gameplay settings translations. ChangesNative P2P protocol and hosting
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Host
participant P2PHostAdapter
participant NativeP2PBridge
participant phase-server
Host->>P2PHostAdapter: Start P2P hosting
P2PHostAdapter->>NativeP2PBridge: Initialize native authority
NativeP2PBridge->>phase-server: Attach loopback seat sockets
phase-server-->>NativeP2PBridge: SessionAttached and slot updates
P2PHostAdapter->>NativeP2PBridge: Start game or submit action
NativeP2PBridge->>phase-server: Forward native command
phase-server-->>NativeP2PBridge: Revisioned state update
NativeP2PBridge-->>P2PHostAdapter: Fan out authoritative state
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8)client/src/i18n/locales/de/settings.jsonTraceback (most recent call last): client/src/i18n/locales/es/settings.jsonTraceback (most recent call last): client/src/i18n/locales/fr/settings.jsonTraceback (most recent call last):
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
client/src/adapter/ws-adapter.ts (1)
1329-1342: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winReject initialization on
Errormessages. This branch leavespregameRejectandgameStartedRejectpending on native pregame/reconnect flows; callrejectInitialization(...)here so all lifecycle waiters settle on a serverError.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/src/adapter/ws-adapter.ts` around lines 1329 - 1342, Update the "Error" message case to call rejectInitialization(...) with the server error details, ensuring pending pregame and game-start/reconnect lifecycle waiters settle on native server errors. Keep the existing mutation, abandon, event emission, and deck-rejection handling unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@client/src/i18n/locales/de/settings.json`:
- Around line 27-28: Translate both skipLowUsePriorityWindows and
skipLowUsePriorityWindowsDescription in client/src/i18n/locales/de/settings.json
lines 27-28 into German, client/src/i18n/locales/es/settings.json lines 27-28
into Spanish, client/src/i18n/locales/fr/settings.json lines 27-28 into French,
client/src/i18n/locales/it/settings.json lines 27-28 into Italian,
client/src/i18n/locales/pl/settings.json lines 27-28 into Polish, and
client/src/i18n/locales/pt/settings.json lines 27-28 into Portuguese, preserving
the existing keys and meaning.
In `@client/src/providers/GameProvider.tsx`:
- Around line 820-840: Gate the native engine setup around ensureNativeEngine
and nativeP2P assignment so it does not run for an isWasmResume, even when
shouldUseNativeP2P is true. Preserve native setup for native resumes and
non-resume sessions, allowing WASM resumes to continue with the persisted WASM
snapshot and non-native adapter.
In `@client/src/stores/multiplayerStore.ts`:
- Around line 1034-1050: Add an attempt-generation or abort mechanism to the
hosting flow around startP2PHostingSession, invalidating the current attempt in
cancelHosting and whenever a newer hosting attempt replaces it. After every
await, including ensureNativeEngine and room initialization, verify the attempt
is still current before creating rooms or publishing state such as
hostingStatus: "waiting"; stale attempts must exit without side effects.
In `@crates/lobby-broker/src/protocol.rs`:
- Around line 374-375: Update the `MIN_SUPPORTED_PROTOCOL` assertion in the
protocol version test to expect 20, matching
`PROTOCOL_VERSION.saturating_sub(1)` when `PROTOCOL_VERSION` is 21; leave the
`PROTOCOL_VERSION` assertion unchanged.
---
Outside diff comments:
In `@client/src/adapter/ws-adapter.ts`:
- Around line 1329-1342: Update the "Error" message case to call
rejectInitialization(...) with the server error details, ensuring pending
pregame and game-start/reconnect lifecycle waiters settle on native server
errors. Keep the existing mutation, abandon, event emission, and deck-rejection
handling unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 305a5079-4d5a-4aef-b694-8be89b6833c0
📒 Files selected for processing (24)
client/src/adapter/__tests__/ws-adapter.test.tsclient/src/adapter/p2p-adapter.tsclient/src/adapter/ws-adapter.tsclient/src/components/settings/__tests__/PreferencesModal.priorityPassing.test.tsxclient/src/i18n/locales/de/settings.jsonclient/src/i18n/locales/en/settings.jsonclient/src/i18n/locales/es/settings.jsonclient/src/i18n/locales/fr/settings.jsonclient/src/i18n/locales/it/settings.jsonclient/src/i18n/locales/pl/settings.jsonclient/src/i18n/locales/pt/settings.jsonclient/src/providers/GameProvider.tsxclient/src/services/gamePersistence.tsclient/src/stores/multiplayerStore.tsclient/src/stores/preferencesStore.tscrates/lobby-broker/src/protocol.rscrates/phase-server/src/main.rscrates/server-core/src/client_message_wire_guard.rscrates/server-core/src/persist.rscrates/server-core/src/protocol.rscrates/server-core/src/session.rsfixtures/adapter-contract/game_started.jsonfixtures/adapter-contract/state_update.jsonscripts/check-protocol-version.mjs
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@client/src/components/settings/PreferencesModal.tsx`:
- Line 1240: Update the range input’s styling near the existing h-2 class to
provide at least a 44pt hit area, using a minimum height such as min-h-11 while
preserving the thin, visually centered slider track through appropriate track
styling.
In `@client/src/stores/multiplayerStore.ts`:
- Around line 992-993: Increment p2pHostingAttempt at the beginning of
startHosting, before any resource cleanup or awaited server-hosting operations,
so pending P2P setup callbacks fail their isCurrentAttempt check and cannot
overwrite server-host state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d39c1a69-65b1-4059-a859-ca39ee236426
📒 Files selected for processing (14)
client/src/adapter/__tests__/ws-adapter.test.tsclient/src/adapter/ws-adapter.tsclient/src/components/menu/MenuActionTile.tsxclient/src/components/settings/PreferencesModal.tsxclient/src/i18n/locales/de/settings.jsonclient/src/i18n/locales/es/settings.jsonclient/src/i18n/locales/fr/settings.jsonclient/src/i18n/locales/it/settings.jsonclient/src/i18n/locales/pl/settings.jsonclient/src/i18n/locales/pt/settings.jsonclient/src/providers/GameProvider.tsxclient/src/stores/multiplayerStore.tscrates/lobby-broker/src/protocol.rscrates/server-core/src/lib.rs
🚧 Files skipped from review as they are similar to previous changes (9)
- client/src/i18n/locales/de/settings.json
- crates/lobby-broker/src/protocol.rs
- client/src/i18n/locales/es/settings.json
- client/src/i18n/locales/pl/settings.json
- client/src/adapter/tests/ws-adapter.test.ts
- client/src/i18n/locales/pt/settings.json
- client/src/i18n/locales/it/settings.json
- client/src/providers/GameProvider.tsx
- client/src/adapter/ws-adapter.ts
| onDoubleClick={() => onChange(defaultValue)} | ||
| aria-label={label} | ||
| className="flex-1 accent-cyan-500" | ||
| className="h-2 flex-1 cursor-pointer rounded-full bg-slate-700 accent-cyan-500 focus-visible:ring-2 focus-visible:ring-cyan-400/70 focus-visible:outline-none" |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve a 44pt touch target for the slider.
h-2 makes the range input itself roughly 8px tall, so the control falls well below the required 44pt touch target. Keep the visual track thin while giving the input a larger hit area, for example with min-h-11 and centered track styling.
Proposed fix
- className="h-2 flex-1 cursor-pointer rounded-full bg-slate-700 accent-cyan-500 focus-visible:ring-2 focus-visible:ring-cyan-400/70 focus-visible:outline-none"
+ className="min-h-11 flex-1 cursor-pointer rounded-full bg-slate-700 accent-cyan-500 focus-visible:ring-2 focus-visible:ring-cyan-400/70 focus-visible:outline-none"As per path instructions, touch targets must be at least 44pt.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| className="h-2 flex-1 cursor-pointer rounded-full bg-slate-700 accent-cyan-500 focus-visible:ring-2 focus-visible:ring-cyan-400/70 focus-visible:outline-none" | |
| className="min-h-11 flex-1 cursor-pointer rounded-full bg-slate-700 accent-cyan-500 focus-visible:ring-2 focus-visible:ring-cyan-400/70 focus-visible:outline-none" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@client/src/components/settings/PreferencesModal.tsx` at line 1240, Update the
range input’s styling near the existing h-2 class to provide at least a 44pt hit
area, using a minimum height such as min-h-11 while preserving the thin,
visually centered slider track through appropriate track styling.
Source: Path instructions
Comment on lines
+992
to
+993
| const attempt = ++p2pHostingAttempt; | ||
| const isCurrentAttempt = () => p2pHostingAttempt === attempt; |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Invalidate P2P setup when switching to server hosting.
Lines 992-993 only invalidate prior P2P attempts for cancellation or another P2P start. startHosting disposes current resources but does not increment p2pHostingAttempt; a pending P2P setup can therefore resume after ensureNativeEngine, hostRoom, or broker registration and overwrite the new server-host state with hostingStatus: "waiting". Increment the attempt counter before startHosting begins cleanup.
As per path instructions, async races must be checked in this hosting flow.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@client/src/stores/multiplayerStore.ts` around lines 992 - 993, Increment
p2pHostingAttempt at the beginning of startHosting, before any resource cleanup
or awaited server-hosting operations, so pending P2P setup callbacks fail their
isCurrentAttempt check and cannot overwrite server-host state.
Source: Path instructions
This was referenced Jul 22, 2026
jsdevninja
pushed a commit
to jsdevninja/phase
that referenced
this pull request
Jul 24, 2026
* Use native engine for hosted P2P games * Fix native P2P review findings * Show pacing slider tracks --------- Co-authored-by: matthewevans <matthewevans@users.noreply.github.com>
This was referenced Jul 29, 2026
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 by CodeRabbit