Fix gametime tracking - #1803
Conversation
📝 WalkthroughWalkthroughSteam window mapping now filters non-Steam games, skips already active games, and recognizes application windows that are not Wine system processes. ChangesSteam window mapping
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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}
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 `@app/src/main/java/app/gamenative/ui/model/MainViewModel.kt`:
- Around line 691-694: Update the parent traversal loop associated with
isGameWindow to assign parentWindow from currentWindow.parent on each iteration,
rather than window.parent. Preserve the existing traversal and termination
behavior so non-Explorer parent chains eventually exit and Steam notification
occurs.
In `@app/src/main/java/app/gamenative/utils/WineProcessSnapshotHelper.kt`:
- Around line 32-35: Update normalizeProcessName(), used by
isSystemProcessName(), to perform case conversion with Locale.ROOT instead of
Locale.getDefault(). Preserve the existing normalization behavior otherwise so
process identifiers such as MSIEXEC.EXE match the system process allowlists
regardless of the device locale.
🪄 Autofix
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 Plus
Run ID: a7029425-597c-4157-b9f2-1ca240d4dfe5
📒 Files selected for processing (2)
app/src/main/java/app/gamenative/ui/model/MainViewModel.ktapp/src/main/java/app/gamenative/utils/WineProcessSnapshotHelper.kt
| val isGameWindow = matchesLaunchConfig || | ||
| (window.isApplicationWindow() && !WineProcessSnapshotHelper.isSystemProcessName(window.className)) | ||
|
|
||
| if (launchConfig != null) { | ||
| if (isGameWindow) { |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Fix the parent traversal before broadening window matching.
The new fallback can enter the process loop for more windows. That loop assigns parentWindow from window.parent on every iteration at Line 699, instead of from currentWindow.parent. If the mapped window has a non-Explorer parent, parentWindow stays non-null and the loop never ends. The coroutine repeatedly appends process entries and never notifies Steam.
Read the parent from currentWindow on each iteration.
Proposed fix
- var parentWindow: Window? = window.parent
+ var parentWindow: Window? = currentWindow.parent🤖 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 `@app/src/main/java/app/gamenative/ui/model/MainViewModel.kt` around lines 691
- 694, Update the parent traversal loop associated with isGameWindow to assign
parentWindow from currentWindow.parent on each iteration, rather than
window.parent. Preserve the existing traversal and termination behavior so
non-Explorer parent chains eventually exit and Steam notification occurs.
| fun isSystemProcessName(name: String): Boolean { | ||
| val normalized = normalizeProcessName(name) | ||
| if (normalized.isEmpty()) return true | ||
| return normalized in buildEssentialProcessAllowlist() || normalized in systemWindowProcesses |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use locale-invariant process-name normalization.
normalizeProcessName() uses Locale.getDefault(). Under a Turkish locale, MSIEXEC.EXE normalizes to msıexec, so this method does not match "msiexec". The fallback then classifies that system window as a game window. Use Locale.ROOT for process identifiers.
Proposed fix
- val lower = base.lowercase(Locale.getDefault())
+ val lower = base.lowercase(Locale.ROOT)🤖 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 `@app/src/main/java/app/gamenative/utils/WineProcessSnapshotHelper.kt` around
lines 32 - 35, Update normalizeProcessName(), used by isSystemProcessName(), to
perform case conversion with Locale.ROOT instead of Locale.getDefault().
Preserve the existing normalization behavior otherwise so process identifiers
such as MSIEXEC.EXE match the system process allowlists regardless of the device
locale.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 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:682">
P2: The new dedup guard `if (ActiveGameRegistry.get()?.appId == gameId) return@launch` runs before the current window is confirmed to be a game window, but the fallback detection was widened so that ANY application window that isn't a system process counts as the game window. For games that spawn a launcher/splash window first, that launcher window will be the first to map and will be registered as the running game (and reported to Steam). The real game exe window that maps afterwards is then skipped by this same guard because the registry already matches the appId, so Steam ends up tracking the launcher process tree instead of the game - which is exactly the gametime-tracking failure this PR sets out to fix. Consider applying the dedup only after confirming the window is actually a game window (and ideally only suppressing the redundant re-notification rather than the real game window), or make the broadened fallback narrower so launcher/overlay windows aren't the ones that win the race.</violation>
</file>
<file name="app/src/main/java/app/gamenative/utils/WineProcessSnapshotHelper.kt">
<violation number="1" location="app/src/main/java/app/gamenative/utils/WineProcessSnapshotHelper.kt:33">
P2: System-window filtering becomes locale-dependent, so uppercase executable names can be mistaken for games on Turkish/Azeri devices; normalize process identifiers with `Locale.ROOT` rather than device locale.</violation>
<violation number="2" location="app/src/main/java/app/gamenative/utils/WineProcessSnapshotHelper.kt:35">
P3: `isSystemProcessName` rebuilds the essential-process allowlist (via `buildEssentialProcessAllowlist()`, which calls `WineUtils.getEssentialServiceNames()`) on every invocation, and this helper is now called on every window-mapped event. The allowlist and `systemWindowProcesses` are both constant for the process lifetime, so recomputing them per call is wasted work on what can be a hot path. Consider hoisting the combined allowlist to a cached (e.g. `by lazy`) value so it is built once and reused.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| SteamService.getAppInfoOf(gameId)?.let { appInfo -> | ||
| // TODO: this should not be a search, the app should have been launched with a specific launch config that we then use to compare | ||
| val launchConfig = SteamService.getWindowsLaunchInfos(gameId).firstOrNull { | ||
| if (ActiveGameRegistry.get()?.appId == gameId) { |
There was a problem hiding this comment.
P2: The new dedup guard if (ActiveGameRegistry.get()?.appId == gameId) return@launch runs before the current window is confirmed to be a game window, but the fallback detection was widened so that ANY application window that isn't a system process counts as the game window. For games that spawn a launcher/splash window first, that launcher window will be the first to map and will be registered as the running game (and reported to Steam). The real game exe window that maps afterwards is then skipped by this same guard because the registry already matches the appId, so Steam ends up tracking the launcher process tree instead of the game - which is exactly the gametime-tracking failure this PR sets out to fix. Consider applying the dedup only after confirming the window is actually a game window (and ideally only suppressing the redundant re-notification rather than the real game window), or make the broadened fallback narrower so launcher/overlay windows aren't the ones that win the race.
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 682:
<comment>The new dedup guard `if (ActiveGameRegistry.get()?.appId == gameId) return@launch` runs before the current window is confirmed to be a game window, but the fallback detection was widened so that ANY application window that isn't a system process counts as the game window. For games that spawn a launcher/splash window first, that launcher window will be the first to map and will be registered as the running game (and reported to Steam). The real game exe window that maps afterwards is then skipped by this same guard because the registry already matches the appId, so Steam ends up tracking the launcher process tree instead of the game - which is exactly the gametime-tracking failure this PR sets out to fix. Consider applying the dedup only after confirming the window is actually a game window (and ideally only suppressing the redundant re-notification rather than the real game window), or make the broadened fallback narrower so launcher/overlay windows aren't the ones that win the race.</comment>
<file context>
@@ -671,17 +672,26 @@ class MainViewModel @Inject constructor(
SteamService.getAppInfoOf(gameId)?.let { appInfo ->
- // TODO: this should not be a search, the app should have been launched with a specific launch config that we then use to compare
- val launchConfig = SteamService.getWindowsLaunchInfos(gameId).firstOrNull {
+ if (ActiveGameRegistry.get()?.appId == gameId) {
+ return@launch
+ }
</file context>
| ) | ||
|
|
||
| fun isSystemProcessName(name: String): Boolean { | ||
| val normalized = normalizeProcessName(name) |
There was a problem hiding this comment.
P2: System-window filtering becomes locale-dependent, so uppercase executable names can be mistaken for games on Turkish/Azeri devices; normalize process identifiers with Locale.ROOT rather than device locale.
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/utils/WineProcessSnapshotHelper.kt, line 33:
<comment>System-window filtering becomes locale-dependent, so uppercase executable names can be mistaken for games on Turkish/Azeri devices; normalize process identifiers with `Locale.ROOT` rather than device locale.</comment>
<file context>
@@ -17,6 +17,24 @@ object WineProcessSnapshotHelper {
+ )
+
+ fun isSystemProcessName(name: String): Boolean {
+ val normalized = normalizeProcessName(name)
+ if (normalized.isEmpty()) return true
+ return normalized in buildEssentialProcessAllowlist() || normalized in systemWindowProcesses
</file context>
| fun isSystemProcessName(name: String): Boolean { | ||
| val normalized = normalizeProcessName(name) | ||
| if (normalized.isEmpty()) return true | ||
| return normalized in buildEssentialProcessAllowlist() || normalized in systemWindowProcesses |
There was a problem hiding this comment.
P3: isSystemProcessName rebuilds the essential-process allowlist (via buildEssentialProcessAllowlist(), which calls WineUtils.getEssentialServiceNames()) on every invocation, and this helper is now called on every window-mapped event. The allowlist and systemWindowProcesses are both constant for the process lifetime, so recomputing them per call is wasted work on what can be a hot path. Consider hoisting the combined allowlist to a cached (e.g. by lazy) value so it is built once and reused.
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/utils/WineProcessSnapshotHelper.kt, line 35:
<comment>`isSystemProcessName` rebuilds the essential-process allowlist (via `buildEssentialProcessAllowlist()`, which calls `WineUtils.getEssentialServiceNames()`) on every invocation, and this helper is now called on every window-mapped event. The allowlist and `systemWindowProcesses` are both constant for the process lifetime, so recomputing them per call is wasted work on what can be a hot path. Consider hoisting the combined allowlist to a cached (e.g. `by lazy`) value so it is built once and reused.</comment>
<file context>
@@ -17,6 +17,24 @@ object WineProcessSnapshotHelper {
+ fun isSystemProcessName(name: String): Boolean {
+ val normalized = normalizeProcessName(name)
+ if (normalized.isEmpty()) return true
+ return normalized in buildEssentialProcessAllowlist() || normalized in systemWindowProcesses
+ }
+
</file context>
…lback Port of upstream 4c3269c (utkarshdalal#1803).
1 participant
Description
Some games launch a window with a different exe from what we boot.
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 game time tracking for Steam games that launch a different executable or window class under Wine. We now detect the real game window and ignore system/utility processes so playtime starts consistently.
systemWindowProcessesandisSystemProcessName()to filter non-game windows likeconhost,wineconsole,steam, andsteamwebhelper.Written for commit 5eee466. Summary will update on new commits.
Summary by CodeRabbit