exit container cleanly if game is exited #438
Conversation
📝 WalkthroughWalkthroughAdds an asynchronous exit watcher to XServerScreen that monitors live process snapshots for unmapped game windows and triggers exit when only essential processes remain. Also extracts Wine service defaults into Changes
Sequence DiagramsequenceDiagram
actor Game
participant Screen as XServerScreen
participant Watcher as ExitWatcher
participant ProcMon as ProcessMonitor
participant Exit as ExitHandler
Game->>Screen: onUnmapWindow(window)
Screen->>Watcher: startExitWatchForUnmappedGameWindow(window)
Watcher->>ProcMon: subscribe OnGetProcessInfoListener
loop polling snapshots
ProcMon->>Watcher: onGetProcessInfo(snapshot)
Watcher->>Watcher: normalize executables & filter essential list
alt only essential processes remain (stable / within timeout)
Watcher->>Exit: invoke exit() on UI thread
Exit->>Screen: shutdown sequence
Watcher->>ProcMon: unsubscribe listener
else non-essential processes present
Watcher->>Watcher: delay and continue polling
end
end
Screen->>Watcher: onDispose -> cancel Job and restore previous listener
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt`:
- Around line 178-183: The process-name normalization uses locale-dependent case
folding; update normalizeProcessName to use a stable locale by replacing
Locale.getDefault() with Locale.ROOT when calling lowercase so identifiers are
normalized consistently (e.g., change the call in normalizeProcessName that
computes lower to use Locale.ROOT).
🧹 Nitpick comments (1)
app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt (1)
323-390: Guard listener restoration to prevent clobbering future concurrent listeners.Currently, only the exit watcher installs listeners, so there's no immediate risk. However, defensively checking if the current listener is still the watcher's before restoring the previous one follows best practices for safe cleanup in potentially shared APIs. Since
listProcesses()is queued viaaddAction()and processed on a single-threaded executor, it's already thread-safe from the background thread context.🛡️ Defensive restore
- } finally { - winHandler.setOnGetProcessInfoListener(previousListener) + } finally { + if (winHandler.getOnGetProcessInfoListener() === listener) { + winHandler.setOnGetProcessInfoListener(previousListener) + } synchronized(lock) { pendingSnapshot = null } }
| private fun normalizeProcessName(name: String): String { | ||
| val trimmed = name.trim().trim('"') | ||
| val base = trimmed.substringAfterLast('/').substringAfterLast('\\') | ||
| val lower = base.lowercase(Locale.getDefault()) | ||
| return if (lower.endsWith(".exe")) lower.removeSuffix(".exe") else lower | ||
| } |
There was a problem hiding this comment.
Use Locale.ROOT for process-name normalization.
Locale-dependent case folding can cause mismatches (e.g., Turkish locale). Use a stable locale for identifier normalization.
♻️ Suggested tweak
- val lower = base.lowercase(Locale.getDefault())
+ val lower = base.lowercase(Locale.ROOT)🤖 Prompt for AI Agents
In `@app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt` around
lines 178 - 183, The process-name normalization uses locale-dependent case
folding; update normalizeProcessName to use a stable locale by replacing
Locale.getDefault() with Locale.ROOT when calling lowercase so identifiers are
normalized consistently (e.g., change the call in normalizeProcessName that
computes lower to use Locale.ROOT).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt`:
- Around line 361-382: The watcher currently ignores empty but valid snapshots
because of the snapshot.isNotEmpty() guard, causing stalls; remove the
isNotEmpty check so an empty snapshot is treated as "no non‑essential processes"
(since snapshot.any { ... } will be false for empty lists) and then call
exit(...) with the existing parameters (winHandler, PluviaApp.xEnvironment,
frameRating, currentAppInfo, container, onExit, navigateBack) when
!hasNonEssential; keep the withTimeoutOrNull(EXIT_PROCESS_RESPONSE_TIMEOUT_MS)
and deferred.await() logic unchanged.
♻️ Duplicate comments (1)
app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt (1)
178-183: Locale-dependent normalization remains.
This is the same locale-sensitivity issue already noted previously; consider switching toLocale.ROOTfor stable case folding.
Summary by cubic
Cleanly exits the container when the game closes by watching for the game window to unmap and exiting once only essential Wine processes remain. Prevents lingering processes and ensures a single, clean exit, while forcing fullscreen for the correct executable.
New Features
Refactors
Written for commit 1061152. Summary will update on new commits.
Summary by CodeRabbit
Improvements
New Features
✏️ Tip: You can customize this high-level summary in your review settings.