track game runtime by process and not just executable name#1225
Open
utkarshdalal wants to merge 1 commit into
Open
track game runtime by process and not just executable name#1225utkarshdalal wants to merge 1 commit into
utkarshdalal wants to merge 1 commit into
Conversation
Contributor
📝 WalkthroughWalkthroughAdded state management to prevent duplicate running process notifications and introduced a fallback detection mechanism for running applications when launch configuration is unavailable. Exposed the Changes
Sequence DiagramsequenceDiagram
participant Window as Window Event
participant VM as MainViewModel
participant Helper as isLaunchExeRunning
participant XServer as xServer Handler
participant Steam as SteamService
Window->>VM: onWindowMapped()
VM->>VM: Check launchConfig & hasSentPlayingNotification
alt launchConfig is null
VM->>VM: Derive normalized window base name
VM->>VM: Filter CORE_WINE_PROCESSES
VM->>VM: Collect launch executable names
VM->>Helper: isLaunchExeRunning(exeNames, timeout=2s)
Helper->>XServer: listProcesses()
XServer-->>Helper: Process list
Helper->>Helper: Check if any launch exe running
Helper-->>VM: Boolean result
alt Launch exe detected & not real Steam
VM->>Steam: notifyRunningProcesses(GameProcessInfo)
VM->>VM: Set hasSentPlayingNotification = true
end
else launchConfig exists & not already notified
VM->>Steam: notifyRunningProcesses(Steam process)
VM->>VM: Set hasSentPlayingNotification = true
end
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 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📝 Generate docstrings
🧪 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: 3
🧹 Nitpick comments (1)
app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt (1)
203-212: Move this shared process-name set out of the screen package.Making
CORE_WINE_PROCESSESpublic soMainViewModelcan import it creates aui.model -> ui.screendependency. A small util/core file would keep the shared matcher reusable without coupling the ViewModel toXServerScreen.🤖 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 203 - 212, The CORE_WINE_PROCESSES set currently declared inside XServerScreen should be moved to a small shared util (e.g., create a new file like ProcessMatchers.kt or CoreProcesses.kt) so it can be imported without creating a ui.model -> ui.screen dependency; extract the val CORE_WINE_PROCESSES to that new file as a public top-level val, update XServerScreen to import CORE_WINE_PROCESSES from the new util, and update MainViewModel to import the same symbol instead of referencing XServerScreen, ensuring no other screen-specific code is moved.
🤖 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/model/MainViewModel.kt`:
- Around line 729-737: The fallback notification is using Process.myPid()
instead of the matched guest PID, causing all fallbacks to report the host PID;
update MainViewModel where GameProcessInfo and AppProcessInfo are constructed
(currently using Process.myPid()) to use the guest process id returned by
isLaunchExeRunning() (make isLaunchExeRunning() return the matched ProcessInfo
rather than a Boolean), then pass that matched ProcessInfo.pid into
AppProcessInfo so SteamService.notifyRunningProcesses() receives the correct
guest/process identity.
- Around line 712-739: The fallback path can send duplicate notifications
because hasSentPlayingNotification is only set after the suspend call
isLaunchExeRunning; modify MainViewModel to atomically reserve/send guard before
any suspension by either (a) introducing a mutex or a volatile/in-flight flag
checked-and-set before calling isLaunchExeRunning, or (b) set
hasSentPlayingNotification (or a new inFlightNotification flag) immediately
after confirming windowBase not in CORE_WINE_PROCESSES and before awaiting
isLaunchExeRunning; keep the subsequent logic (ContainerUtils.getContainer,
isLaunchRealSteam, SteamService.notifyRunningProcesses) unchanged but ensure the
flag is cleared or remains set appropriately so concurrent coroutines cannot
both call SteamService.notifyRunningProcesses for the same gameId.
- Around line 69-112: In isLaunchExeRunning, normalize each ProcessInfo.name the
same way other WinHandler checks do before comparing to launchExeNames: for
example, strip surrounding quotes, remove directory path to get the basename,
trim a trailing “.exe” (case-insensitive), then lowercase the result before
testing membership in targets; update the snapshot.any check to normalize it
(e.g., normalizeProcessName(processInfo.name) ) and compare against targets so
path/quote/.exe variants match the basename-only launchExeNames.
---
Nitpick comments:
In `@app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt`:
- Around line 203-212: The CORE_WINE_PROCESSES set currently declared inside
XServerScreen should be moved to a small shared util (e.g., create a new file
like ProcessMatchers.kt or CoreProcesses.kt) so it can be imported without
creating a ui.model -> ui.screen dependency; extract the val CORE_WINE_PROCESSES
to that new file as a public top-level val, update XServerScreen to import
CORE_WINE_PROCESSES from the new util, and update MainViewModel to import the
same symbol instead of referencing XServerScreen, ensuring no other
screen-specific code is moved.
🪄 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: e34baafc-5e71-4db8-8251-ef5b4f0020dc
📒 Files selected for processing (2)
app/src/main/java/app/gamenative/ui/model/MainViewModel.ktapp/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt
Comment on lines
+69
to
+112
| private suspend fun isLaunchExeRunning(launchExeNames: List<String>): Boolean { | ||
| if (launchExeNames.isEmpty()) return false | ||
| val winHandler = PluviaApp.xServerView?.getxServer()?.winHandler ?: return false | ||
| val targets = launchExeNames.toSet() | ||
|
|
||
| return withContext(Dispatchers.IO) { | ||
| val previousListener = winHandler.getOnGetProcessInfoListener() | ||
| val snapshotDeferred = CompletableDeferred<List<ProcessInfo>>() | ||
| val lock = Any() | ||
| val currentList = mutableListOf<ProcessInfo>() | ||
| var expectedCount = -1 | ||
|
|
||
| val listener = OnGetProcessInfoListener { index, count, processInfo -> | ||
| previousListener?.onGetProcessInfo(index, count, processInfo) | ||
| synchronized(lock) { | ||
| if (count == 0 && processInfo == null) { | ||
| if (!snapshotDeferred.isCompleted) snapshotDeferred.complete(emptyList()) | ||
| return@synchronized | ||
| } | ||
| if (index == 0) { | ||
| currentList.clear() | ||
| expectedCount = count | ||
| } | ||
| if (processInfo != null) { | ||
| currentList.add(processInfo) | ||
| } | ||
| if (expectedCount >= 0 && currentList.size >= expectedCount && !snapshotDeferred.isCompleted) { | ||
| snapshotDeferred.complete(currentList.toList()) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| winHandler.setOnGetProcessInfoListener(listener) | ||
| try { | ||
| winHandler.listProcesses() | ||
| val snapshot = withTimeoutOrNull(PROCESS_LIST_TIMEOUT_MS) { | ||
| snapshotDeferred.await() | ||
| } ?: return@withContext false | ||
|
|
||
| snapshot.any { it.name.lowercase() in targets } | ||
| } finally { | ||
| winHandler.setOnGetProcessInfoListener(previousListener) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Normalize ProcessInfo.name before matching it.
Line 108 compares raw process-list names against basename-only launch exes. In this codebase the same WinHandler process names are normalized elsewhere before comparison because they may include a path, quotes, or a .exe suffix. As written, this fallback can miss the launched process and never send the playing notification.
♻️ Suggested normalization
+ private fun normalizeProcessName(name: String): String {
+ val base = name.trim().trim('"').substringAfterLast('/').substringAfterLast('\\')
+ return base.lowercase().removeSuffix(".exe")
+ }
+
private suspend fun isLaunchExeRunning(launchExeNames: List<String>): Boolean {
if (launchExeNames.isEmpty()) return false
val winHandler = PluviaApp.xServerView?.getxServer()?.winHandler ?: return false
- val targets = launchExeNames.toSet()
+ val targets = launchExeNames.map(::normalizeProcessName).toSet()
...
- snapshot.any { it.name.lowercase() in targets }
+ snapshot.any { normalizeProcessName(it.name) in targets }
} finally {🤖 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/model/MainViewModel.kt` around lines 69 -
112, In isLaunchExeRunning, normalize each ProcessInfo.name the same way other
WinHandler checks do before comparing to launchExeNames: for example, strip
surrounding quotes, remove directory path to get the basename, trim a trailing
“.exe” (case-insensitive), then lowercase the result before testing membership
in targets; update the snapshot.any check to normalize it (e.g.,
normalizeProcessName(processInfo.name) ) and compare against targets so
path/quote/.exe variants match the basename-only launchExeNames.
Comment on lines
+712
to
+739
| } else if (!hasSentPlayingNotification) { | ||
| val windowBase = window.className.substringAfterLast('\\') | ||
| .substringAfterLast('/').lowercase().removeSuffix(".exe") | ||
| if (windowBase in CORE_WINE_PROCESSES) return@let | ||
|
|
||
| val launchExeNames = SteamService.getWindowsLaunchInfos(gameId) | ||
| .map { Paths.get(it.executable.replace('\\', '/')).name.lowercase() } | ||
| if (isLaunchExeRunning(launchExeNames)) { | ||
| val shouldLaunchRealSteam = try { | ||
| val container = ContainerUtils.getContainer(context, appId) | ||
| container.isLaunchRealSteam() | ||
| } catch (e: Exception) { | ||
| false | ||
| } | ||
| if (!shouldLaunchRealSteam) { | ||
| hasSentPlayingNotification = true | ||
| val installedBranch = SteamService.getInstalledApp(gameId)?.branch ?: "public" | ||
| val processInfo = GameProcessInfo( | ||
| appId = gameId, | ||
| branch = installedBranch, | ||
| processes = listOf(AppProcessInfo(Process.myPid(), Process.myPid(), true)), | ||
| ) | ||
| Timber.tag("MainViewModel").i( | ||
| "Sending playing notification via process-list fallback for appId=%d", gameId, | ||
| ) | ||
| SteamService.notifyRunningProcesses(processInfo) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
The fallback branch can still send duplicate playing notifications.
hasSentPlayingNotification is only flipped after the suspend call to isLaunchExeRunning(). If two windows map close together, both coroutines can enter this branch with false, both await the snapshot, and both call notifyRunningProcesses(). Guard the branch with a mutex/in-flight flag, or reserve the notification slot before awaiting.
🧰 Tools
🪛 detekt (1.23.8)
[warning] 723-723: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
🤖 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/model/MainViewModel.kt` around lines 712
- 739, The fallback path can send duplicate notifications because
hasSentPlayingNotification is only set after the suspend call
isLaunchExeRunning; modify MainViewModel to atomically reserve/send guard before
any suspension by either (a) introducing a mutex or a volatile/in-flight flag
checked-and-set before calling isLaunchExeRunning, or (b) set
hasSentPlayingNotification (or a new inFlightNotification flag) immediately
after confirming windowBase not in CORE_WINE_PROCESSES and before awaiting
isLaunchExeRunning; keep the subsequent logic (ContainerUtils.getContainer,
isLaunchRealSteam, SteamService.notifyRunningProcesses) unchanged but ensure the
flag is cleared or remains set appropriately so concurrent coroutines cannot
both call SteamService.notifyRunningProcesses for the same gameId.
Comment on lines
+729
to
+737
| val processInfo = GameProcessInfo( | ||
| appId = gameId, | ||
| branch = installedBranch, | ||
| processes = listOf(AppProcessInfo(Process.myPid(), Process.myPid(), true)), | ||
| ) | ||
| Timber.tag("MainViewModel").i( | ||
| "Sending playing notification via process-list fallback for appId=%d", gameId, | ||
| ) | ||
| SteamService.notifyRunningProcesses(processInfo) |
Contributor
There was a problem hiding this comment.
Use the matched guest PID here, not Process.myPid().
SteamService.notifyRunningProcesses() uses this processId as the running-game identity. Reporting the Android host PID means every fallback notification in the same app process has the same PID, so titles that only hit this path still are not tracked per launched process. isLaunchExeRunning() needs to return the matched ProcessInfo, not just a Boolean.
🤖 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/model/MainViewModel.kt` around lines 729
- 737, The fallback notification is using Process.myPid() instead of the matched
guest PID, causing all fallbacks to report the host PID; update MainViewModel
where GameProcessInfo and AppProcessInfo are constructed (currently using
Process.myPid()) to use the guest process id returned by isLaunchExeRunning()
(make isLaunchExeRunning() return the matched ProcessInfo rather than a
Boolean), then pass that matched ProcessInfo.pid into AppProcessInfo so
SteamService.notifyRunningProcesses() receives the correct guest/process
identity.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
2 issues found across 2 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/java/app/gamenative/ui/model/MainViewModel.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/model/MainViewModel.kt:712">
P2: Race condition: `hasSentPlayingNotification` is checked here but only set to `true` after the suspend call to `isLaunchExeRunning()`. If two windows map concurrently, both coroutines can observe `false`, both await the process snapshot, and both send duplicate playing notifications. Set the flag (or use a mutex) before the suspend point to prevent this.</violation>
<violation number="2" location="app/src/main/java/app/gamenative/ui/model/MainViewModel.kt:732">
P1: Fallback runtime notification reports the Android PID instead of the game/Wine process PID, causing incorrect process tracking.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| val processInfo = GameProcessInfo( | ||
| appId = gameId, | ||
| branch = installedBranch, | ||
| processes = listOf(AppProcessInfo(Process.myPid(), Process.myPid(), true)), |
Contributor
There was a problem hiding this comment.
P1: Fallback runtime notification reports the Android PID instead of the game/Wine process PID, causing incorrect process tracking.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/app/gamenative/ui/model/MainViewModel.kt, line 732:
<comment>Fallback runtime notification reports the Android PID instead of the game/Wine process PID, causing incorrect process tracking.</comment>
<file context>
@@ -652,6 +709,34 @@ class MainViewModel @Inject constructor(
+ val processInfo = GameProcessInfo(
+ appId = gameId,
+ branch = installedBranch,
+ processes = listOf(AppProcessInfo(Process.myPid(), Process.myPid(), true)),
+ )
+ Timber.tag("MainViewModel").i(
</file context>
| Timber.tag("MainViewModel").i("Skipping Steam process notification - real Steam will handle this") | ||
| } | ||
| } | ||
| } else if (!hasSentPlayingNotification) { |
Contributor
There was a problem hiding this comment.
P2: Race condition: hasSentPlayingNotification is checked here but only set to true after the suspend call to isLaunchExeRunning(). If two windows map concurrently, both coroutines can observe false, both await the process snapshot, and both send duplicate playing notifications. Set the flag (or use a mutex) before the suspend point to prevent this.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/app/gamenative/ui/model/MainViewModel.kt, line 712:
<comment>Race condition: `hasSentPlayingNotification` is checked here but only set to `true` after the suspend call to `isLaunchExeRunning()`. If two windows map concurrently, both coroutines can observe `false`, both await the process snapshot, and both send duplicate playing notifications. Set the flag (or use a mutex) before the suspend point to prevent this.</comment>
<file context>
@@ -652,6 +709,34 @@ class MainViewModel @Inject constructor(
Timber.tag("MainViewModel").i("Skipping Steam process notification - real Steam will handle this")
}
}
+ } else if (!hasSentPlayingNotification) {
+ val windowBase = window.className.substringAfterLast('\\')
+ .substringAfterLast('/').lowercase().removeSuffix(".exe")
</file context>
phobos665
reviewed
Apr 23, 2026
Contributor
There was a problem hiding this comment.
There's also the "parentProcess" which is worth considering if this is worth checking for too. I believe it comes from RunningAppInfo from javasteam
Owner
Author
There was a problem hiding this comment.
Yeah, looked into parentProcess, it's null unfortunately. I don't know if this one is worth merging, but we should probably find a better way to track playtime rather than just checking the exe name.
Contributor
There was a problem hiding this comment.
Agreed, happy to also look at this. I've created a thread here: https://discord.com/channels/1378308569287622737/1499071031795519548
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.
Description
Recording
Type of Change
Checklist
#code-changes, I have discussed this change there and it has been green-lighted. If I do not have access, I have still provided clear context in this PR. If I skip both, I accept that this change may face delays in review, may not be reviewed at all, or may be closed.CONTRIBUTING.md.Summary by cubic
Fixes playtime tracking by checking running Windows processes against the game’s launch executables, not just the window/exe name. Makes “playing” notifications reliable and avoids false positives from core Wine processes.
winHandler.listProcesses()(2s timeout) to launch exe names.CORE_WINE_PROCESSES.Written for commit a48b9f4. Summary will update on new commits.
Summary by CodeRabbit