fix: show actionable dialog when Steam session is taken by another device#1306
Merged
utkarshdalal merged 8 commits intoutkarshdalal:masterfrom Apr 28, 2026
Merged
Conversation
…vice When a PC starts a Steam game while a game is already running in Wine on the device, Steam sends LoggedInElsewhere instead of PlayingSessionState. Handle this in onLoggedOff: if xEnvironment is running, set isPlayingBlocked and emit PlayingBlocked instead of closing. XServerScreen catches the event and shows an "App Running" dialog with Play Anyway / Cancel options.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Steam "playing conflict" handling: new Changes
Sequence Diagram(s)sequenceDiagram
participant SteamService as SteamService
participant EventBus as Event System
participant XServerUI as XServerScreen UI
participant User as User
SteamService->>SteamService: Detect LoggedInElsewhere or playing-blocked state
SteamService->>SteamService: set _isHandlingConflict = true\nset _isPlayingBlocked = true
SteamService->>EventBus: Emit SteamEvent.PlayingBlocked
EventBus->>XServerUI: Deliver PlayingBlocked
XServerUI->>XServerUI: Show AlertDialog (Kick / Cancel)
User->>XServerUI: Tap "Kick"
XServerUI->>SteamService: clearPlayingConflict()
XServerUI->>SteamService: kickPlayingSession(onlyGame = true)
alt User taps "Cancel"
User->>XServerUI: Tap "Cancel"
XServerUI->>XServerUI: Dismiss dialog and exit flow
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![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
🧹 Nitpick comments (2)
app/src/main/java/app/gamenative/PluviaApp.kt (1)
236-238: Consider wrappingclearPlayingConflict()inrunCatchingfor consistency.The earlier comment in this method explicitly states "per-step catch so one failing teardown doesn't prevent the rest from running", and every teardown call above is wrapped accordingly. While
clearPlayingConflict()is presumably a trivial flag reset (and unlikely to throw), wrapping it preserves the defensive pattern and protectsclearActiveSuspendState()from any future implementation changes.♻️ Suggested change
SteamService.keepAlive = false - SteamService.clearPlayingConflict() + runCatching { SteamService.clearPlayingConflict() } + .onFailure { Timber.e(it, "shutdownEnvironment: clearPlayingConflict") } clearActiveSuspendState()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/PluviaApp.kt` around lines 236 - 238, Wrap the call to SteamService.clearPlayingConflict() in a runCatching block to follow the existing per-step teardown pattern (so a failure in clearPlayingConflict() won't prevent clearActiveSuspendState() from running); locate the section where SteamService.keepAlive is set to false and replace the direct call to clearPlayingConflict() with a runCatching { SteamService.clearPlayingConflict() } invocation that logs or ignores the throwable consistently with the other teardown steps.app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt (1)
2260-2261: Dialog always renders "unknown game" — actual conflicting title never plumbed through.
main_app_running_messageis invoked withmain_app_running_unknown_gameas the format argument unconditionally, so users always see the fallback wording regardless of whether Steam knows what's running on the other device. The PR description framesmain_app_running_unknown_gameas a fallback "when the conflicting game name is unavailable," butSteamEvent.PlayingBlockedis a parameterless object, so there is no path to deliver the real name fromonLoggedOff/onPlayingSessionStateto this composable.Consider extending
SteamEvent.PlayingBlockedto carry the conflicting app id / display name (or expose it via aStateFlowonSteamService) and only fall back tomain_app_running_unknown_gamewhen it is genuinely unknown. Can be deferred to a follow-up if not in scope here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt` around lines 2260 - 2261, The dialog always shows the fallback string because SteamEvent.PlayingBlocked is parameterless and main_app_running_message is always called with main_app_running_unknown_game; fix by plumbing the actual conflicting game info into the UI: either extend SteamEvent.PlayingBlocked to carry the conflicting appId/displayName and propagate that from onPlayingSessionState/onLoggedOff into the state consumed by the XServerScreen composable, or expose a new StateFlow/String on SteamService with the current conflicting game's name and use that in the composable to call main_app_running_message with the real name (only using main_app_running_unknown_game when the name is null/empty). Ensure references: SteamEvent.PlayingBlocked, XServerScreen.kt composable where main_app_running_message is used, onPlayingSessionState/onLoggedOff handlers, and SteamService StateFlow.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/app/gamenative/service/SteamService.kt`:
- Around line 3559-3562: The PlayingBlocked emission path emits
SteamEvent.PlayingBlocked directly (in the PlayingSessionStateCallback handling)
without flipping the conflict sentinel _isHandlingConflict, so it can bypass the
reconnect()/onDisconnected() disconnect-suppression and re-emit repeatedly;
update the block that checks callback.isPlayingBlocked to first atomically set
the conflict flag (use _isHandlingConflict.compareAndSet(false, true) or
equivalent getAndSet logic) and only call
PluviaApp.events.emit(SteamEvent.PlayingBlocked) when that compareAndSet
succeeds, ensuring this path enters the same conflict-handling state machine and
emits at most once.
- Around line 421-424: The _isHandlingConflict flag remains true after a user
chooses "Play Anyway", breaking reconnect and subsequent conflict handling;
update kickPlayingSession's success branch to clear that flag (e.g., call
instance?._isHandlingConflict?.set(false) or getAndSet(false)) so that after
resolving a LoggedInElsewhere you also reset _isHandlingConflict (the flag is
set in onLoggedOff and currently only cleared in clearPlayingConflict), ensuring
reconnect()/onDisconnected() and future LoggedInElsewhere events behave
correctly.
In `@app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt`:
- Around line 2262-2271: The confirm button currently fires-and-forgets
SteamService.kickPlayingSession inside scope.launch and clears
showPlayingBlockedDialog immediately, which swallows failures and leaves
_isHandlingConflict true; change the handler in confirmButton to call
kickPlayingSession and await its Boolean result (catching exceptions/timeout),
only set showPlayingBlockedDialog = false and clear _isHandlingConflict when the
call succeeds, and on failure restore/show the dialog or display an error
message (and set _isHandlingConflict = false so a subsequent PlayingBlocked
event can re-show the dialog); update the lambda around
SteamService.kickPlayingSession, showPlayingBlockedDialog, and
_isHandlingConflict accordingly and add error handling/timeout handling for
exceptions.
In `@app/src/main/res/values/strings.xml`:
- Line 1068: The fallback string main_app_running_unknown_game is grammatically
awkward when substituted into main_app_running_message; replace its value
"another device" with a clearer fallback such as "another game" or "an unknown
game" so the message reads naturally (e.g., "...already playing another game");
update the string resource main_app_running_unknown_game accordingly.
---
Nitpick comments:
In `@app/src/main/java/app/gamenative/PluviaApp.kt`:
- Around line 236-238: Wrap the call to SteamService.clearPlayingConflict() in a
runCatching block to follow the existing per-step teardown pattern (so a failure
in clearPlayingConflict() won't prevent clearActiveSuspendState() from running);
locate the section where SteamService.keepAlive is set to false and replace the
direct call to clearPlayingConflict() with a runCatching {
SteamService.clearPlayingConflict() } invocation that logs or ignores the
throwable consistently with the other teardown steps.
In `@app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt`:
- Around line 2260-2261: The dialog always shows the fallback string because
SteamEvent.PlayingBlocked is parameterless and main_app_running_message is
always called with main_app_running_unknown_game; fix by plumbing the actual
conflicting game info into the UI: either extend SteamEvent.PlayingBlocked to
carry the conflicting appId/displayName and propagate that from
onPlayingSessionState/onLoggedOff into the state consumed by the XServerScreen
composable, or expose a new StateFlow/String on SteamService with the current
conflicting game's name and use that in the composable to call
main_app_running_message with the real name (only using
main_app_running_unknown_game when the name is null/empty). Ensure references:
SteamEvent.PlayingBlocked, XServerScreen.kt composable where
main_app_running_message is used, onPlayingSessionState/onLoggedOff handlers,
and SteamService StateFlow.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: c93ce219-621c-41cc-aa40-4a9f81c56532
📒 Files selected for processing (5)
app/src/main/java/app/gamenative/PluviaApp.ktapp/src/main/java/app/gamenative/events/SteamEvent.ktapp/src/main/java/app/gamenative/service/SteamService.ktapp/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.ktapp/src/main/res/values/strings.xml
ben-pearson
force-pushed
the
feature/session-conflict
branch
from
6a933f2 to
644f73b
Compare
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
5 issues found across 5 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/main/res/values/strings.xml">
<violation number="1" location="app/src/main/res/values/strings.xml:1068">
P2: Fallback value for unknown game title uses a device phrase, causing incorrect conflict dialog text ("already playing another device").</violation>
</file>
<file name="app/src/main/java/app/gamenative/service/SteamService.kt">
<violation number="1" location="app/src/main/java/app/gamenative/service/SteamService.kt:421">
P1: `_isHandlingConflict` is never reset after a successful kick. When the user taps "Play Anyway" and `kickPlayingSession` succeeds, the flag stays `true` for the rest of the game session. This silently suppresses all subsequent `Disconnected`/`RemotelyDisconnected` events (the user won't see disconnect banners for real outages) and prevents the conflict dialog from reappearing if another `LoggedInElsewhere` arrives. Reset `_isHandlingConflict` to `false` inside `kickPlayingSession` on the success branch.</violation>
<violation number="2" location="app/src/main/java/app/gamenative/service/SteamService.kt:3540">
P1: Conflict-handling flag is set but not cleared on unblock, causing later real disconnect events to stay suppressed for the session.</violation>
<violation number="3" location="app/src/main/java/app/gamenative/service/SteamService.kt:3559">
P2: This safety-net path emits `PlayingBlocked` without setting `_isHandlingConflict`, unlike the `onLoggedOff` path. This means disconnect suppression in `reconnect()`/`onDisconnected()` won't activate, so the dialog can appear behind a disconnect banner. Additionally, repeated `PlayingSessionStateCallback` calls with `isPlayingBlocked=true` will re-emit the event with no deduplication. Gate on `_isHandlingConflict.compareAndSet(false, true)` to match the invariants established in `onLoggedOff`.</violation>
</file>
<file name="app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt:2264">
P2: "Play Anyway" dismisses the dialog before awaiting the kick result. `kickPlayingSession` returns a `Boolean` indicating success, but it's launched fire-and-forget and the result is discarded. If the kick times out or fails, the dialog is already gone and the user is stuck in a blocked session with no feedback or way to retry. Await the result and re-show the dialog (or display an error) on failure.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt (1)
2263-2267:⚠️ Potential issue | 🟠 MajorHandle “Play Anyway” failures before dismissing the dialog.
At Line 2264, the dialog is closed before the result of
kickPlayingSessionis known. If the kick fails/times out, the user can be left blocked with no in-flow recovery prompt.♻️ Suggested fix
confirmButton = { TextButton(onClick = { - showPlayingBlockedDialog = false scope.launch { - SteamService.kickPlayingSession(onlyGame = true) + val kicked = SteamService.kickPlayingSession(onlyGame = true) + if (kicked) { + showPlayingBlockedDialog = false + } } }) { Text(text = stringResource(R.string.main_play_anyway)) } },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt` around lines 2263 - 2267, The dialog is being dismissed before the result of SteamService.kickPlayingSession is known; change the onClick handler on the button (where showPlayingBlockedDialog is toggled and scope.launch calls SteamService.kickPlayingSession) so it launches the coroutine, awaits the kickPlayingSession result, and only set showPlayingBlockedDialog = false on success; on failure/timeouts catch the error and keep the dialog open while showing an inline error state or retry option (or a Snackbar) so the user can recover; use the existing scope.launch, wrap the call to SteamService.kickPlayingSession in try/catch (and optionally with a timeout) and update UI state based on the outcome instead of immediately closing the dialog.app/src/main/java/app/gamenative/service/SteamService.kt (1)
3556-3562:⚠️ Potential issue | 🟠 MajorDrive
_isHandlingConflictfromPlayingSessionStateCallback, not justonLoggedOff.This callback currently emits
PlayingBlockedon everytrue, but it never enters the conflict state machine when this is the first signal, and it never exits it when Steam later reportsfalse. That leaves two bugs: duplicate dialog emissions on repeated blocked callbacks, and_isHandlingConflictstayingtrueafter the conflict is resolved, which suppresses later real disconnect banners and blocks future conflict dialogs for the rest of the session.MainViewModelonly learns about connection loss from those suppressed disconnect events.Suggested fix
private fun onPlayingSessionState(callback: PlayingSessionStateCallback) { Timber.d("onPlayingSessionState called with isPlayingBlocked = " + callback.isPlayingBlocked) _isPlayingBlocked.value = callback.isPlayingBlocked - if (callback.isPlayingBlocked) { - val event = SteamEvent.PlayingBlocked - PluviaApp.events.emit(event) - } + if (callback.isPlayingBlocked) { + if (_isHandlingConflict.compareAndSet(false, true)) { + PluviaApp.events.emit(SteamEvent.PlayingBlocked) + } + } else { + _isHandlingConflict.set(false) + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/service/SteamService.kt` around lines 3556 - 3562, The onPlayingSessionState handler must drive the conflict state from PlayingSessionStateCallback instead of only toggling in onLoggedOff: update the _isHandlingConflict state to match callback.isPlayingBlocked (use PlayingSessionStateCallback.isPlayingBlocked) and only emit PluviaApp.events.emit(SteamEvent.PlayingBlocked) when transitioning from not-blocked to blocked to avoid duplicate dialogs; when callback.isPlayingBlocked becomes false, clear _isHandlingConflict so the conflict machine exits and future disconnect events (e.g., onLoggedOff) are not suppressed. Ensure you reference onPlayingSessionState, PlayingSessionStateCallback, _isPlayingBlocked, _isHandlingConflict, and PluviaApp.events.emit(SteamEvent.PlayingBlocked) when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@app/src/main/java/app/gamenative/service/SteamService.kt`:
- Around line 3556-3562: The onPlayingSessionState handler must drive the
conflict state from PlayingSessionStateCallback instead of only toggling in
onLoggedOff: update the _isHandlingConflict state to match
callback.isPlayingBlocked (use PlayingSessionStateCallback.isPlayingBlocked) and
only emit PluviaApp.events.emit(SteamEvent.PlayingBlocked) when transitioning
from not-blocked to blocked to avoid duplicate dialogs; when
callback.isPlayingBlocked becomes false, clear _isHandlingConflict so the
conflict machine exits and future disconnect events (e.g., onLoggedOff) are not
suppressed. Ensure you reference onPlayingSessionState,
PlayingSessionStateCallback, _isPlayingBlocked, _isHandlingConflict, and
PluviaApp.events.emit(SteamEvent.PlayingBlocked) when making the change.
In `@app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt`:
- Around line 2263-2267: The dialog is being dismissed before the result of
SteamService.kickPlayingSession is known; change the onClick handler on the
button (where showPlayingBlockedDialog is toggled and scope.launch calls
SteamService.kickPlayingSession) so it launches the coroutine, awaits the
kickPlayingSession result, and only set showPlayingBlockedDialog = false on
success; on failure/timeouts catch the error and keep the dialog open while
showing an inline error state or retry option (or a Snackbar) so the user can
recover; use the existing scope.launch, wrap the call to
SteamService.kickPlayingSession in try/catch (and optionally with a timeout) and
update UI state based on the outcome instead of immediately closing the dialog.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0bf693dc-c69c-4180-9f40-874393eaf3ee
📒 Files selected for processing (5)
app/src/main/java/app/gamenative/PluviaApp.ktapp/src/main/java/app/gamenative/events/SteamEvent.ktapp/src/main/java/app/gamenative/service/SteamService.ktapp/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.ktapp/src/main/res/values/strings.xml
✅ Files skipped from review due to trivial changes (1)
- app/src/main/res/values/strings.xml
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/app/gamenative/events/SteamEvent.kt
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt (1)
2263-2268:⚠️ Potential issue | 🟠 MajorOnly clear the conflict after
kickPlayingSession()succeeds.The dialog is dismissed before the kick completes, and the
Booleanresult is ignored. On timeout/exception, the user can stay blocked while the prompt disappears and the conflict state gets cleared anyway.♻️ Proposed fix
confirmButton = { TextButton(onClick = { - showPlayingBlockedDialog = false scope.launch { - SteamService.kickPlayingSession(onlyGame = true) - SteamService.clearPlayingConflict() + val kicked = SteamService.kickPlayingSession(onlyGame = true) + if (kicked) { + showPlayingBlockedDialog = false + SteamService.clearPlayingConflict() + } } }) { Text(text = stringResource(R.string.main_play_anyway)) } },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt` around lines 2263 - 2268, The dialog is being dismissed before kickPlayingSession completes and its Boolean result is ignored; change the onClick handler so it waits for scope.launch to complete and only clears the conflict if SteamService.kickPlayingSession(onlyGame = true) returns true, leaving showPlayingBlockedDialog true on failure or exception; catch exceptions/timeouts around the call to SteamService.kickPlayingSession in the launched coroutine and only call SteamService.clearPlayingConflict() on successful completion, and update showPlayingBlockedDialog based on the success result.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt`:
- Line 416: Replace the transient Compose state for the Steam conflict dialog so
it survives Activity recreation: change the declaration of
showPlayingBlockedDialog (in XServerScreen.kt) from using remember {
mutableStateOf(false) } to a saveable state (e.g., rememberSaveable {
mutableStateOf(false) }) so the dialog remains visible across configuration
changes/process recreation while SteamService emits PlayingBlocked; also ensure
any code that handles clearPlayingConflict() still sets showPlayingBlockedDialog
to false when the conflict is cleared.
---
Duplicate comments:
In `@app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt`:
- Around line 2263-2268: The dialog is being dismissed before kickPlayingSession
completes and its Boolean result is ignored; change the onClick handler so it
waits for scope.launch to complete and only clears the conflict if
SteamService.kickPlayingSession(onlyGame = true) returns true, leaving
showPlayingBlockedDialog true on failure or exception; catch exceptions/timeouts
around the call to SteamService.kickPlayingSession in the launched coroutine and
only call SteamService.clearPlayingConflict() on successful completion, and
update showPlayingBlockedDialog based on the success result.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: cb3297ac-2a87-4901-b934-3d01641185e8
📒 Files selected for processing (3)
app/src/main/java/app/gamenative/service/SteamService.ktapp/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.ktapp/src/main/res/values/strings.xml
✅ Files skipped from review due to trivial changes (1)
- app/src/main/res/values/strings.xml
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/app/gamenative/service/SteamService.kt
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt`:
- Around line 2274-2277: The Cancel handler currently force-unwraps xServerView
(xServerView!!) which can be null after Activity recreation; change the handler
to avoid the force-unwrap by safely resolving xServerView and its winHandler
before calling exit: capture a safe local reference (e.g. val server =
xServerView?.getxServer()?.winHandler) or guard with if (xServerView != null) {
exit(...) } and if null, perform a safe no-op or fallback (e.g. just
navigateBack/onExit) so exit(...) is only invoked when winHandler is non-null;
update the TextButton onClick to use this null-safe logic and still set
showPlayingBlockedDialog = false.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: e4db0a64-687f-431b-ba69-594ddd8e46c8
📒 Files selected for processing (1)
app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt
Contributor
There was a problem hiding this comment.
2 issues found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt:416">
P1: Changing `showPlayingBlockedDialog` to `rememberSaveable` means this dialog state now survives Activity recreation (e.g., screen rotation), but `xServerView` is stored with plain `remember` and will be null after recreation. If the user taps Cancel before `xServerView` is rebound, the `xServerView!!` non-null assertion in the dismiss button handler will throw an NPE. Use safe-call (`xServerView?.getxServer()?.winHandler`) instead, or revert to `remember` if the dialog doesn't need to survive configuration changes.</violation>
<violation number="2" location="app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt:2265">
P2: Conflict-handling state is cleared too early, re-enabling suppressed Steam conflict/disconnect events before session kick finishes.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Comment on lines
+2257
to
+2282
| if (showPlayingBlockedDialog) { | ||
| androidx.compose.material3.AlertDialog( | ||
| onDismissRequest = {}, | ||
| title = { Text(text = stringResource(R.string.main_app_running_title)) }, | ||
| text = { Text(text = stringResource(R.string.main_app_running_message, context.getString(R.string.main_app_running_unknown_game))) }, | ||
| confirmButton = { | ||
| TextButton(onClick = { | ||
| showPlayingBlockedDialog = false | ||
| SteamService.clearPlayingConflict() | ||
| scope.launch { | ||
| SteamService.kickPlayingSession(onlyGame = true) | ||
| } | ||
| }) { | ||
| Text(text = stringResource(R.string.main_play_anyway)) | ||
| } | ||
| }, | ||
| dismissButton = { | ||
| TextButton(onClick = { | ||
| showPlayingBlockedDialog = false | ||
| exit(xServerView?.getxServer()?.winHandler, frameRating, currentAppInfo, container, appId, onExit, navigateBack) | ||
| }) { | ||
| Text(text = stringResource(R.string.cancel)) | ||
| } | ||
| }, | ||
| ) | ||
| } |
Owner
There was a problem hiding this comment.
Curious why you decided to put the dialog here instead of PluviaMain with the others? Is it required to be here?
Contributor
Author
There was a problem hiding this comment.
the Cancel handler needs to call exit() which handles Wine process teardown via winHandler - that context only exists in XServerScreen, is there a better way to do this? I was trying to change as little as possible (reducing risk)
| <string name="main_app_running_message">You are logged in on another device already playing %s. \nYou can still play this game, but that will disconnect the other session from Steam.</string> | ||
| <string name="main_app_running_other_device">You are logged in on another device (%1$s) already playing %2$s (%3$s), and that save is not yet in the cloud. \nYou can still play this game, but that will disconnect the other session from Steam and may create a save conflict when that session progress is synced</string> | ||
| <string name="main_play_anyway">Play anyway</string> | ||
| <string name="main_app_running_unknown_game">a game</string> |
Contributor
Author
There was a problem hiding this comment.
added (AI generated translations) hope they are correct :-D 🏴
Owner
|
couple of comments but otherwise looks good |
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
2 participants
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.
Problem
When a PC starts a Steam game while a game is already running in Wine on the device, Steam sends
EResult.LoggedInElsewhereinsideLoggedOffCallback. The previous code treated this the same as any other logoff — it emittedForceCloseAppand silently killed the game with no feedback to the user.trim.EC957912-3A84-489C-87BA-EA4674055363.MOV
Root cause confirmed via logcat:
PlayingSessionStateCallbackwas assumed to fire in this scenario, but it never does.LoggedInElsewhereinsideLoggedOffCallbackis the only signal Steam sends.Changes
trim.61380BA6-1726-4AF5-9224-E388A88977F2.MOV
SteamEvent.kt— addsPlayingBlockedevent to signal the conflict to the UI layer.SteamService.ktonLoggedOff: whenLoggedInElsewherearrives and a game is running (xEnvironment != null), show the conflict dialog instead of force-closing. When no game is running, the existingForceCloseAppbehaviour is preserved._isHandlingConflict(AtomicBoolean): prevents the dialog being shown multiple times ifLoggedInElsewherefires repeatedly during reconnect attempts.clearPlayingConflict(): resets conflict state on game exit.reconnect()/onDisconnected(): suppressDisconnected/RemotelyDisconnectedevents while a conflict is being handled, so the reconnect cycle doesn't flash a "Disconnected from Steam" banner behind the dialog.onPlayingSessionState: safety net — also emitsPlayingBlockedifPlayingSessionStateCallbackfires withisPlayingBlocked=truewhile in-game.PluviaApp.kt— callsSteamService.clearPlayingConflict()inshutdownEnvironment()to reset state when the game exits.XServerScreen.kt— registers a listener forSteamEvent.PlayingBlockedand shows anAlertDialogwith:kickPlayingSession(onlyGame=true)to stop the PC's game sessiononDismissRequestis intentionally empty — the user must make an explicit choice.strings.xml— addsmain_app_running_unknown_game("another device"), used as the fallback game name in the dialog message when the conflicting game title cannot be determined.Note
Behaviour varies slightly depending on the conflicting device (PC, Steam Deck, Mac). This has been tested across all three and is improved in each case, but given the variance in how Steam signals session conflicts across platforms, further edge cases may exist.
Test plan
Summary by cubic
Show an actionable “App Running” dialog when Steam logs the user off because another device started a game, instead of silently closing the local game. The dialog persists across Activity recreation and hides the disconnect banner; users can kick the remote session or exit.
EResult.LoggedInElsewhereinSteamService.onLoggedOff: if a game is running, emitPlayingBlocked(notForceCloseApp); otherwise keep existing behavior.AlertDialoginXServerScreenonPlayingBlockedwith “Play anyway” (callSteamService.clearPlayingConflict()thenSteamService.kickPlayingSession(onlyGame=true)) and “Cancel” (clean exit); persist withrememberSaveableand safe-callxServerView._isHandlingConflictto prevent duplicate dialogs; suppressDisconnected/RemotelyDisconnectedwhile handling; reset viaSteamService.clearPlayingConflict()on shutdown and before kicking.PlayingBlockedwhenPlayingSessionStateCallback.isPlayingBlockedis true; addmain_app_running_unknown_gamefallback and translations across locales.Written for commit b9083cf. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
New Features
Bug Fixes