feat(desktop): pinned native-engine WebSocket bridge and native-AI client routing - #6305
Conversation
📝 WalkthroughWalkthroughAdds native-engine AI session routing through a Tauri loopback WebSocket bridge, with frontend transport abstractions, engine state and preference updates, native-to-WASM fallback handling, lifecycle cleanup, and coverage for bridge, socket, adapter, provider, and UI behavior. ChangesNative AI engine integration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant GamePage
participant GameProvider
participant NativeEngine
participant NativeEngineSocket
participant NativeBridge
GamePage->>GameProvider: initialize AI game
GameProvider->>NativeEngine: ensure native engine
GameProvider->>NativeEngineSocket: create native transport
NativeEngineSocket->>NativeBridge: connect_native_engine
NativeBridge->>NativeEngine: register loopback bridge
NativeBridge-->>NativeEngineSocket: bridge events
NativeEngineSocket-->>GameProvider: WebSocket-style events
GameProvider-->>GamePage: game and connection events
GameProvider->>NativeEngineSocket: dispose or concede
NativeEngineSocket->>NativeBridge: close native bridge
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
client/src/providers/__tests__/GameProvider.nativeEngine.test.tsx (1)
280-300: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffConsider a regression test against the real
WebSocketAdapterfor the fallback path.This suite's mock
WebSocketAdapter.disposeis a no-opvi.fn(), so it can't catch issues in the real dispose/sendConcede/send/emitchain that only manifest when a nativeinitialize()failure races with listener teardown (see the critical finding onGameProvider.tsx's catch block). Once that's fixed, a test using the realws-adapter.tsclass (mocking only the socket factory, asws-adapter.test.tsalready does) would catch regressions here.🤖 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/providers/__tests__/GameProvider.nativeEngine.test.tsx` around lines 280 - 300, Add a regression test for the native-initialize failure fallback that uses the real WebSocketAdapter rather than the suite’s no-op dispose mock. Mock only the socket factory, following ws-adapter.test.ts, and exercise the GameProvider path where nativeAdapterInitialize rejects to cover dispose, sendConcede, send, and emit during listener teardown. Preserve the existing assertions for WASM fallback and verify the real adapter completes without throwing.
🤖 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-tauri/src/native_bridge.rs`:
- Around line 95-103: Wrap the connect_async(request).await call in
native_engine bridge connection setup with tokio::time::timeout using an
appropriate finite duration. Handle the timeout result by closing the native
engine bridge and returning NativeEngineBridgeError::Connect with timeout
details, while preserving the existing successful connection and
connection-error handling.
In `@client/src/providers/GameProvider.tsx`:
- Around line 1447-1573: Update the native setup catch block in setupNativeAi to
call wsUnsubscribe() and clear the subscription before disposing nativeAdapter,
mirroring the cleanup function’s unsubscribe-before-dispose ordering. Then
dispose and clear the adapter as currently intended, ensuring failed
initialization cannot emit an error through the still-attached event listener
before WASM fallback runs.
- Around line 1545-1549: Update the native game setup in the provider around
createGameLoopController so aiSeatIds is populated from
nativeAiSeatsFromDeckList before starting the controller. Keep the controller
mode as "online" to preserve server-authoritative native engine behavior, while
ensuring telemetry game_end can classify native-AI winners.
---
Nitpick comments:
In `@client/src/providers/__tests__/GameProvider.nativeEngine.test.tsx`:
- Around line 280-300: Add a regression test for the native-initialize failure
fallback that uses the real WebSocketAdapter rather than the suite’s no-op
dispose mock. Mock only the socket factory, following ws-adapter.test.ts, and
exercise the GameProvider path where nativeAdapterInitialize rejects to cover
dispose, sendConcede, send, and emit during listener teardown. Preserve the
existing assertions for WASM fallback and verify the real adapter completes
without throwing.
🪄 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: 6503a7b4-d967-4c66-b43c-de053639cd8e
⛔ Files ignored due to path filters (6)
client/src-tauri/Cargo.lockis excluded by!**/*.lockclient/src-tauri/gen/schemas/acl-manifests.jsonis excluded by!**/gen/**client/src-tauri/gen/schemas/capabilities.jsonis excluded by!**/gen/**client/src-tauri/gen/schemas/desktop-schema.jsonis excluded by!**/gen/**client/src-tauri/gen/schemas/macOS-schema.jsonis excluded by!**/gen/**client/src/vite-env.d.tsis excluded by!**/*.d.ts
📒 Files selected for processing (22)
client/src-tauri/Cargo.tomlclient/src-tauri/capabilities/default.jsonclient/src-tauri/permissions/legacy-storage.tomlclient/src-tauri/src/lib.rsclient/src-tauri/src/native_bridge.rsclient/src-tauri/src/native_engine.rsclient/src/adapter/__tests__/ws-adapter.test.tsclient/src/adapter/ws-adapter.tsclient/src/pages/GamePage.tsxclient/src/pages/GameSetupPage.tsxclient/src/pages/MultiplayerPage.tsxclient/src/pages/__tests__/GamePage.bracketViolation.test.tsxclient/src/providers/GameProvider.tsxclient/src/providers/__tests__/GameProvider.nativeEngine.test.tsxclient/src/services/__tests__/nativeEngineSocket.test.tsclient/src/services/__tests__/openPhaseSocket.test.tsclient/src/services/nativeEngine.tsclient/src/services/nativeEngineSocket.tsclient/src/services/openPhaseSocket.tsclient/src/stores/gameStore.tsclient/src/stores/preferencesStore.tsclient/vite.config.ts
| let socket = match connect_async(request).await { | ||
| Ok((socket, _)) => socket, | ||
| Err(error) => { | ||
| native_engine::close_native_engine_bridge(bridge_id); | ||
| return Err(NativeEngineBridgeError::Connect { | ||
| detail: error.to_string(), | ||
| }); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
tokio-tungstenite 0.29 connect_async connect timeout default
💡 Result:
The tokio-tungstenite library (including version 0.29) does not implement a default connection timeout for the connect_async function [1][2]. When you call connect_async or its variants like connect_async_with_config, the function relies on the underlying TcpStream::connect or the TLS handshake process [1][2]. Because there is no internal timeout configuration provided within the tokio-tungstenite connection functions themselves, the operation will wait indefinitely until the underlying operating system's default TCP connection timeout is reached [1][3][2]. To implement a connection timeout, you should use tokio::time::timeout to wrap the connect_async future [3]. Example: use tokio::time::{timeout, Duration}; use tokio_tungstenite::connect_async; let connect_future = connect_async("ws://example.com"); let timeout_duration = Duration::from_secs(5); match timeout(timeout_duration, connect_future).await { Ok(Ok((stream, response))) => { // Connection successful } Ok(Err(e)) => { // Connection failed (e.g., DNS, handshake) } Err(_) => { // Timed out } } [3]
Citations:
- 1: https://docs.rs/tokio-tungstenite/latest/src/tokio_tungstenite/connect.rs.html
- 2: https://github.com/snapview/tokio-tungstenite/blob/aafb2f9e036162f7bffa002cfea502376a690724/src/connect.rs
- 3: https://stackoverflow.com/questions/78475232/how-to-do-a-blocking-read-with-timeout-with-tokio-tungstenite
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the bridge file and related native engine cleanup helpers.
grep -nR "connect_native_engine\|close_native_engine_bridge\|abort_all_native_engine_bridges\|Abortable\|connect_async(request)" -n client/src-tauri/src || true
echo '--- native_bridge.rs (around the referenced lines) ---'
sed -n '1,220p' client/src-tauri/src/native_bridge.rs
echo '--- search for cleanup/abort helpers ---'
grep -nR "fn close_native_engine_bridge\|fn abort_all_native_engine_bridges\|struct NativeEngineBridge\|abort_native_engine_bridge" client/src-tauri/src native_engine* 2>/dev/null || trueRepository: phase-rs/phase
Length of output: 9960
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the native engine bridge cleanup functions and the JS-side socket flow.
fd -a "native_bridge.rs|native_engine|openPhaseSocket|closeNativeEngineSocket|bridgeId" client src . 2>/dev/null || true
echo '--- candidate files around bridge lifecycle ---'
for f in \
client/src-tauri/src/native_bridge.rs \
client/src-tauri/src/lib.rs \
client/src-tauri/src/main.rs \
client/src-tauri/src/native_engine.rs \
client/src-tauri/src/*bridge*.rs
do
if [ -f "$f" ]; then
echo "### $f"
wc -l "$f"
fi
doneRepository: phase-rs/phase
Length of output: 1071
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- client/src/services/openPhaseSocket.ts ---'
cat -n client/src/services/openPhaseSocket.ts | sed -n '1,260p'
echo '--- client/src/services/__tests__/openPhaseSocket.test.ts ---'
cat -n client/src/services/__tests__/openPhaseSocket.test.ts | sed -n '1,260p'
echo '--- client/src-tauri/src/native_engine.rs (bridge lifecycle section) ---'
sed -n '440,520p' client/src-tauri/src/native_engine.rs
echo '--- client/src-tauri/src/lib.rs (navigation handling) ---'
sed -n '1,100p' client/src-tauri/src/lib.rsRepository: phase-rs/phase
Length of output: 27313
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Grep for any timeout handling around the shell/bridge handshake.
rg -n "timeout|invoke\\(|openPhaseSocket|connect_native_engine|bridgeId" client/src -g '!**/dist/**' -g '!**/build/**'
# Show the specific bridge registration/abort logic.
python3 - <<'PY'
from pathlib import Path
p = Path("client/src-tauri/src/native_engine.rs")
lines = p.read_text().splitlines()
for start, end in [(456, 520), (1050, 1085), (1560, 1590)]:
print(f"\n--- native_engine.rs {start}-{end} ---")
for i in range(start-1, min(end, len(lines))):
print(f"{i+1}: {lines[i]}")
PYRepository: phase-rs/phase
Length of output: 24705
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the frontend socket helper for whether it bounds the bridge setup.
python3 - <<'PY'
from pathlib import Path
p = Path("client/src/services/openPhaseSocket.ts")
lines = p.read_text().splitlines()
for start, end in [(1, 260)]:
print(f"--- openPhaseSocket.ts {start}-{end} ---")
for i in range(start-1, min(end, len(lines))):
print(f"{i+1}: {lines[i]}")
PYRepository: phase-rs/phase
Length of output: 10124
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- client/src/services/nativeEngineSocket.ts ---'
cat -n client/src/services/nativeEngineSocket.ts | sed -n '1,220p'
echo '--- client/src/services/__tests__/nativeEngineSocket.test.ts ---'
cat -n client/src/services/__tests__/nativeEngineSocket.test.ts | sed -n '1,220p'Repository: phase-rs/phase
Length of output: 12365
Add a timeout around connect_async
connect_async(request).await runs before the Abortable wrapper, so abort_all_native_engine_bridges cannot cancel a stalled handshake. If the loopback connect hangs, connect_native_engine never resolves and JS cannot call native_engine_bridge_close because bridgeId is still null. Wrap this await in tokio::time::timeout (client/src-tauri/src/native_bridge.rs:95-103).
🤖 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-tauri/src/native_bridge.rs` around lines 95 - 103, Wrap the
connect_async(request).await call in native_engine bridge connection setup with
tokio::time::timeout using an appropriate finite duration. Handle the timeout
result by closing the native engine bridge and returning
NativeEngineBridgeError::Connect with timeout details, while preserving the
existing successful connection and connection-error handling.
| if (shouldUseNativeAi && nativeEngineKey) { | ||
| const setupNativeAi = async () => { | ||
| try { | ||
| // Native sessions deliberately do not resume a local snapshot. A | ||
| // pre-existing state belongs to the established WASM path instead. | ||
| if (await loadGame(gameId)) { | ||
| setEngineMode("wasm"); | ||
| await setupLocal(); | ||
| return; | ||
| } | ||
| if (cancelled) return; | ||
|
|
||
| const activeDeckName = localStorage.getItem(ACTIVE_DECK_KEY); | ||
| const randomPlayerDeck = isRandomDeckSelection(activeDeckName); | ||
| const parsedDeck = randomPlayerDeck ? null : loadActiveDeck(); | ||
| const suppliesDeck = formatConfig ? formatSuppliesDeck(formatConfig.format) : false; | ||
| if (!parsedDeck && !suppliesDeck && !randomPlayerDeck) { | ||
| onNoDeckRef.current?.(); | ||
| return; | ||
| } | ||
|
|
||
| const deckList = await buildLocalAiDeckList( | ||
| tRef.current, | ||
| randomPlayerDeck ? null : (parsedDeck ?? EMPTY_PARSED_DECK), | ||
| playerCount ?? 2, | ||
| formatConfig, | ||
| matchConfig?.match_type, | ||
| loadActiveDeckBracket(), | ||
| ); | ||
| if (cancelled) return; | ||
|
|
||
| // Native games are server-authoritative and have no client resume | ||
| // contract in v1, so remove the setup-page pointer before hosting. | ||
| clearActiveGame(); | ||
| await ensureNativeEngine(nativeEngineKey); | ||
| if (cancelled) return; | ||
|
|
||
| nativeAdapter = new WebSocketAdapter( | ||
| "native-engine", | ||
| "host", | ||
| deckList.player, | ||
| undefined, | ||
| undefined, | ||
| undefined, | ||
| "Player", | ||
| { | ||
| nativeAi: { | ||
| socketFactory: () => new NativeEngineSocket(), | ||
| aiSeats: nativeAiSeatsFromDeckList(deckList), | ||
| playerCount: playerCount ?? 2, | ||
| formatConfig, | ||
| matchConfig, | ||
| expectedServerVersion: | ||
| "release" in nativeEngineKey ? nativeEngineKey.release.version : undefined, | ||
| }, | ||
| }, | ||
| ); | ||
| wsUnsubscribe = nativeAdapter.onEvent((event) => { | ||
| if (event.type === "stateChanged") { | ||
| const adapter = nativeAdapter; | ||
| if (!useGameStore.getState().adapter && adapter) { | ||
| useGameStore.setState({ adapter }); | ||
| } | ||
| processRemoteUpdate(event.snapshot, event.events, event.logEntries); | ||
| } | ||
| if (event.type === "gameOver") { | ||
| useGameStore.setState({ | ||
| waitingFor: { type: "GameOver", data: { winner: event.winner } }, | ||
| }); | ||
| } | ||
| if (event.type === "reconnectFailed" || event.type === "error") { | ||
| const adapter = nativeAdapter; | ||
| nativeAdapter = null; | ||
| controller?.dispose(); | ||
| controller = null; | ||
| adapter?.dispose(); | ||
| if (useGameStore.getState().adapter === adapter) { | ||
| useGameStore.setState({ adapter: null }); | ||
| } | ||
| // GamePage's existing reconnect-failed/error surface is terminal | ||
| // and provides the Return-to-Menu action for this native session. | ||
| onWsEventRef.current?.(event); | ||
| } | ||
| }); | ||
|
|
||
| await initGame( | ||
| gameId, | ||
| nativeAdapter, | ||
| undefined, | ||
| formatConfig, | ||
| playerCount, | ||
| matchConfig, | ||
| ); | ||
| if (cancelled) { | ||
| nativeAdapter.dispose({ concede: true }); | ||
| return; | ||
| } | ||
|
|
||
| setGameMode("native-ai"); | ||
| setEngineMode("native"); | ||
| controller = createGameLoopController({ mode: "online" }); | ||
| controller.start(); | ||
| audioManager.setContext("battlefield"); | ||
| } catch (error) { | ||
| nativeAdapter?.dispose({ concede: true }); | ||
| nativeAdapter = null; | ||
| if (cancelled) return; | ||
|
|
||
| setGameMode("ai"); | ||
| setEngineMode("wasm", nativeFallbackReason(error)); | ||
| saveWasmAiResumePointer(gameId, difficulty, playerCount, formatConfig); | ||
| await setupLocal(); | ||
| } | ||
| }; | ||
|
|
||
| void setupNativeAi(); | ||
|
|
||
| return () => { | ||
| cancelled = true; | ||
| if (controller) controller.dispose(); | ||
| if (wsUnsubscribe) wsUnsubscribe(); | ||
| nativeAdapter?.dispose({ concede: true }); | ||
| audioManager.setContext("menu"); | ||
| clearPromptOverlayState(); | ||
| scheduleStoreReset(reset); | ||
| }; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | 🏗️ Heavy lift
Critical: native-init failure leaves the game screen permanently stuck on "Connection lost".
Trace: any initGame(...) failure here (version mismatch, handshake error, WS error — i.e. every failure before this.ws is assigned in ws-adapter.ts's attachSocket) hits the catch block and calls nativeAdapter?.dispose({ concede: true }) without first calling wsUnsubscribe() — unlike the unmount cleanup below, which unsubscribes before disposing.
Since this.ws was never assigned, dispose({concede:true}) → sendConcede() → send() synchronously emits a synthetic {type:"error", message:"Cannot send message..."} event through the still-attached listener registered at line 1504. That listener's own "error" branch re-enters dispose() and then calls onWsEventRef.current?.(event), forwarding this spurious event to GamePage's handleWsEvent, which (since isOnlineMode is false for mode="ai") sets reconnectState to "failed" — permanently, since nothing ever resets it back to "idle" after the WASM fallback that follows in this same catch block succeeds.
Net effect: every native-engine setup failure (not just version mismatch) that falls back to WASM leaves the board pointer-events-none with a stuck "Connection lost" banner, even though the WASM game underneath is actually running fine. None of the current tests catch this because GameProvider.nativeEngine.test.tsx fully mocks WebSocketAdapter.dispose, and GamePage.bracketViolation.test.tsx mocks GameProvider itself.
As per path instructions, "follow existing lifecycle patterns end-to-end ... rather than inventing parallel ad-hoc flows" — this block should mirror the cleanup function's unsubscribe-before-dispose ordering.
🐛 Proposed fix: unsubscribe before disposing in the failure path
} catch (error) {
+ if (wsUnsubscribe) wsUnsubscribe();
nativeAdapter?.dispose({ concede: true });
nativeAdapter = null;
if (cancelled) return;📝 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.
| if (shouldUseNativeAi && nativeEngineKey) { | |
| const setupNativeAi = async () => { | |
| try { | |
| // Native sessions deliberately do not resume a local snapshot. A | |
| // pre-existing state belongs to the established WASM path instead. | |
| if (await loadGame(gameId)) { | |
| setEngineMode("wasm"); | |
| await setupLocal(); | |
| return; | |
| } | |
| if (cancelled) return; | |
| const activeDeckName = localStorage.getItem(ACTIVE_DECK_KEY); | |
| const randomPlayerDeck = isRandomDeckSelection(activeDeckName); | |
| const parsedDeck = randomPlayerDeck ? null : loadActiveDeck(); | |
| const suppliesDeck = formatConfig ? formatSuppliesDeck(formatConfig.format) : false; | |
| if (!parsedDeck && !suppliesDeck && !randomPlayerDeck) { | |
| onNoDeckRef.current?.(); | |
| return; | |
| } | |
| const deckList = await buildLocalAiDeckList( | |
| tRef.current, | |
| randomPlayerDeck ? null : (parsedDeck ?? EMPTY_PARSED_DECK), | |
| playerCount ?? 2, | |
| formatConfig, | |
| matchConfig?.match_type, | |
| loadActiveDeckBracket(), | |
| ); | |
| if (cancelled) return; | |
| // Native games are server-authoritative and have no client resume | |
| // contract in v1, so remove the setup-page pointer before hosting. | |
| clearActiveGame(); | |
| await ensureNativeEngine(nativeEngineKey); | |
| if (cancelled) return; | |
| nativeAdapter = new WebSocketAdapter( | |
| "native-engine", | |
| "host", | |
| deckList.player, | |
| undefined, | |
| undefined, | |
| undefined, | |
| "Player", | |
| { | |
| nativeAi: { | |
| socketFactory: () => new NativeEngineSocket(), | |
| aiSeats: nativeAiSeatsFromDeckList(deckList), | |
| playerCount: playerCount ?? 2, | |
| formatConfig, | |
| matchConfig, | |
| expectedServerVersion: | |
| "release" in nativeEngineKey ? nativeEngineKey.release.version : undefined, | |
| }, | |
| }, | |
| ); | |
| wsUnsubscribe = nativeAdapter.onEvent((event) => { | |
| if (event.type === "stateChanged") { | |
| const adapter = nativeAdapter; | |
| if (!useGameStore.getState().adapter && adapter) { | |
| useGameStore.setState({ adapter }); | |
| } | |
| processRemoteUpdate(event.snapshot, event.events, event.logEntries); | |
| } | |
| if (event.type === "gameOver") { | |
| useGameStore.setState({ | |
| waitingFor: { type: "GameOver", data: { winner: event.winner } }, | |
| }); | |
| } | |
| if (event.type === "reconnectFailed" || event.type === "error") { | |
| const adapter = nativeAdapter; | |
| nativeAdapter = null; | |
| controller?.dispose(); | |
| controller = null; | |
| adapter?.dispose(); | |
| if (useGameStore.getState().adapter === adapter) { | |
| useGameStore.setState({ adapter: null }); | |
| } | |
| // GamePage's existing reconnect-failed/error surface is terminal | |
| // and provides the Return-to-Menu action for this native session. | |
| onWsEventRef.current?.(event); | |
| } | |
| }); | |
| await initGame( | |
| gameId, | |
| nativeAdapter, | |
| undefined, | |
| formatConfig, | |
| playerCount, | |
| matchConfig, | |
| ); | |
| if (cancelled) { | |
| nativeAdapter.dispose({ concede: true }); | |
| return; | |
| } | |
| setGameMode("native-ai"); | |
| setEngineMode("native"); | |
| controller = createGameLoopController({ mode: "online" }); | |
| controller.start(); | |
| audioManager.setContext("battlefield"); | |
| } catch (error) { | |
| nativeAdapter?.dispose({ concede: true }); | |
| nativeAdapter = null; | |
| if (cancelled) return; | |
| setGameMode("ai"); | |
| setEngineMode("wasm", nativeFallbackReason(error)); | |
| saveWasmAiResumePointer(gameId, difficulty, playerCount, formatConfig); | |
| await setupLocal(); | |
| } | |
| }; | |
| void setupNativeAi(); | |
| return () => { | |
| cancelled = true; | |
| if (controller) controller.dispose(); | |
| if (wsUnsubscribe) wsUnsubscribe(); | |
| nativeAdapter?.dispose({ concede: true }); | |
| audioManager.setContext("menu"); | |
| clearPromptOverlayState(); | |
| scheduleStoreReset(reset); | |
| }; | |
| } | |
| } catch (error) { | |
| if (wsUnsubscribe) wsUnsubscribe(); | |
| nativeAdapter?.dispose({ concede: true }); | |
| nativeAdapter = null; | |
| if (cancelled) return; | |
| setGameMode("ai"); | |
| setEngineMode("wasm", nativeFallbackReason(error)); | |
| saveWasmAiResumePointer(gameId, difficulty, playerCount, formatConfig); | |
| await setupLocal(); | |
| } |
🤖 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/providers/GameProvider.tsx` around lines 1447 - 1573, Update the
native setup catch block in setupNativeAi to call wsUnsubscribe() and clear the
subscription before disposing nativeAdapter, mirroring the cleanup function’s
unsubscribe-before-dispose ordering. Then dispose and clear the adapter as
currently intended, ensuring failed initialization cannot emit an error through
the still-attached event listener before WASM fallback runs.
Source: Path instructions
| setGameMode("native-ai"); | ||
| setEngineMode("native"); | ||
| controller = createGameLoopController({ mode: "online" }); | ||
| controller.start(); | ||
| audioManager.setContext("battlefield"); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
aiSeatIds never gets populated for native games, breaking game_end telemetry classification.
createGameLoopController({ mode: "online" }) is the right choice here to avoid double-running a client-side AI controller against a server-authoritative native engine, but aiSeatIds (per its doc comment, "Consumed by telemetry game_end to classify winner_kind") is only populated when config.mode === "ai". Unlike true online games — where AI seat info genuinely isn't known client-side — native games do know their AI seats (nativeAiSeatsFromDeckList), so this is an avoidable gap: every native-AI game's winner will be misclassified as "unknown" in telemetry.
♻️ Proposed fix: set aiSeatIds directly, independent of controller mode
setGameMode("native-ai");
setEngineMode("native");
+ useGameStore.setState({
+ aiSeatIds: nativeAiSeatsFromDeckList(deckList).map((seat) => seat.seatIndex),
+ });
controller = createGameLoopController({ mode: "online" });📝 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.
| setGameMode("native-ai"); | |
| setEngineMode("native"); | |
| controller = createGameLoopController({ mode: "online" }); | |
| controller.start(); | |
| audioManager.setContext("battlefield"); | |
| setGameMode("native-ai"); | |
| setEngineMode("native"); | |
| useGameStore.setState({ | |
| aiSeatIds: nativeAiSeatsFromDeckList(deckList).map((seat) => seat.seatIndex), | |
| }); | |
| controller = createGameLoopController({ mode: "online" }); | |
| controller.start(); | |
| audioManager.setContext("battlefield"); |
🤖 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/providers/GameProvider.tsx` around lines 1545 - 1549, Update the
native game setup in the provider around createGameLoopController so aiSeatIds
is populated from nativeAiSeatsFromDeckList before starting the controller. Keep
the controller mode as "online" to preserve server-authoritative native engine
behavior, while ensuring telemetry game_end can classify native-AI winners.
Summary by CodeRabbit
New Features
Bug Fixes