fix: resend steam presence after reconnect#1278
Conversation
📝 WalkthroughWalkthroughAdds a single-entry, thread-safe ActiveGameRegistry and integrates it into shutdown (PluviaApp), MainViewModel lifecycle events (window mapped / app exit), and SteamService reconnect/persona logging to re-notify running processes when appropriate. ChangesActive game lifecycle coordination
Sequence Diagram(s)sequenceDiagram
participant MainVM as MainViewModel
participant Registry as ActiveGameRegistry
participant Steam as SteamService
participant Backend as Steam Backend
participant App as PluviaApp
MainVM->>Registry: set(GameProcessInfo) / clearIfMatches(appId)
MainVM->>Steam: notifyRunningProcesses(GameProcessInfo) [when shouldLaunchRealSteam = false]
App->>Registry: clear() %% shutdown calls clear()
note over Steam,Registry: on Steam logon
Steam->>Registry: get()
alt registry has entry
Steam->>Backend: notifyRunningProcesses(active entry)
else no entry
Steam-->>Steam: debug log (no active games)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 3475-3480: The code captures ActiveGameRegistry.all() before
launching a coroutine, risking replaying a stale snapshot; update the coroutine
to re-read the registry immediately inside the launched block (call
ActiveGameRegistry.all() just before notifyRunningProcesses) or otherwise
serialize presence updates so exitSteamApp wins; specifically change the
scope.launch block that currently uses the outer activeGames variable to fetch a
fresh list and pass that to notifyRunningProcesses (or implement a
serialized/locked update path around notifyRunningProcesses/exitSteamApp to
ensure the latest lifecycle state is used).
In `@app/src/main/java/app/gamenative/ui/model/MainViewModel.kt`:
- Around line 490-493: Guard the registry removal so it only runs for Steam
containers: before calling ContainerUtils.extractGameIdFromContainerId(appId) /
ActiveGameRegistry.remove(gameId) check that the appId represents a Steam
container (e.g., add a ContainerUtils.isSteamContainer(appId) or equivalent
check), only extract the numeric gameId and call
ActiveGameRegistry.remove(gameId) when that Steam-check passes, and always call
SteamService.notifyRunningProcesses() after (or when appropriate) to keep
process state in sync.
🪄 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: 4886213f-b561-4df1-a9ae-18d484b49be0
📒 Files selected for processing (4)
app/src/main/java/app/gamenative/PluviaApp.ktapp/src/main/java/app/gamenative/service/ActiveGameRegistry.ktapp/src/main/java/app/gamenative/service/SteamService.ktapp/src/main/java/app/gamenative/ui/model/MainViewModel.kt
| val activeGames = ActiveGameRegistry.all() | ||
| if (activeGames.isNotEmpty()) { | ||
| Timber.i("Re-sending %d active game session(s) after Steam reconnect", activeGames.size) | ||
| scope.launch { | ||
| notifyRunningProcesses(*activeGames.toTypedArray()) | ||
| } |
There was a problem hiding this comment.
Avoid replaying a stale active-game snapshot.
activeGames is captured before the coroutine runs. If the game exits right after Line 3475, exitSteamApp() can remove it and send an empty notifyRunningProcesses(), but this queued coroutine may later send the old GameProcessInfo and resurrect Steam presence/playtime for a closed game.
Re-read the registry inside the launched work immediately before notifying, or serialize reconnect/exit presence updates so the latest lifecycle state wins.
🐛 Minimal stale-snapshot reduction
- val activeGames = ActiveGameRegistry.all()
- if (activeGames.isNotEmpty()) {
- Timber.i("Re-sending %d active game session(s) after Steam reconnect", activeGames.size)
- scope.launch {
- notifyRunningProcesses(*activeGames.toTypedArray())
- }
- } else {
- Timber.d("No active game sessions to re-send after Steam reconnect")
- }
+ scope.launch {
+ val activeGames = ActiveGameRegistry.all()
+ if (activeGames.isNotEmpty()) {
+ Timber.i("Re-sending %d active game session(s) after Steam reconnect", activeGames.size)
+ notifyRunningProcesses(*activeGames.toTypedArray())
+ } else {
+ Timber.d("No active game sessions to re-send after Steam reconnect")
+ }
+ }🤖 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 3475 -
3480, The code captures ActiveGameRegistry.all() before launching a coroutine,
risking replaying a stale snapshot; update the coroutine to re-read the registry
immediately inside the launched block (call ActiveGameRegistry.all() just before
notifyRunningProcesses) or otherwise serialize presence updates so exitSteamApp
wins; specifically change the scope.launch block that currently uses the outer
activeGames variable to fetch a fresh list and pass that to
notifyRunningProcesses (or implement a serialized/locked update path around
notifyRunningProcesses/exitSteamApp to ensure the latest lifecycle state is
used).
| val gameId = ContainerUtils.extractGameIdFromContainerId(appId) | ||
| Timber.tag("Exit").i("Got game id: $gameId") | ||
| ActiveGameRegistry.remove(gameId) | ||
| SteamService.notifyRunningProcesses() |
There was a problem hiding this comment.
Guard registry removal to Steam apps.
exitSteamApp() handles non-Steam sources too, but ActiveGameRegistry is Steam-specific and keyed only by numeric gameId. A GOG/Epic/custom game with the same numeric ID could remove an unrelated Steam presence entry.
🛡️ Proposed guard
val gameId = ContainerUtils.extractGameIdFromContainerId(appId)
Timber.tag("Exit").i("Got game id: $gameId")
- ActiveGameRegistry.remove(gameId)
- SteamService.notifyRunningProcesses()
+ if (ContainerUtils.extractGameSourceFromContainerId(appId) == GameSource.STEAM) {
+ ActiveGameRegistry.remove(gameId)
+ SteamService.notifyRunningProcesses()
+ }
handleExitCloudSync(context, appId, gameId)📝 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.
| val gameId = ContainerUtils.extractGameIdFromContainerId(appId) | |
| Timber.tag("Exit").i("Got game id: $gameId") | |
| ActiveGameRegistry.remove(gameId) | |
| SteamService.notifyRunningProcesses() | |
| val gameId = ContainerUtils.extractGameIdFromContainerId(appId) | |
| Timber.tag("Exit").i("Got game id: $gameId") | |
| if (ContainerUtils.extractGameSourceFromContainerId(appId) == GameSource.STEAM) { | |
| ActiveGameRegistry.remove(gameId) | |
| SteamService.notifyRunningProcesses() | |
| } | |
| handleExitCloudSync(context, appId, gameId) |
🤖 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 490
- 493, Guard the registry removal so it only runs for Steam containers: before
calling ContainerUtils.extractGameIdFromContainerId(appId) /
ActiveGameRegistry.remove(gameId) check that the appId represents a Steam
container (e.g., add a ContainerUtils.isSteamContainer(appId) or equivalent
check), only extract the numeric gameId and call
ActiveGameRegistry.remove(gameId) when that Steam-check passes, and always call
SteamService.notifyRunningProcesses() after (or when appropriate) to keep
process state in sync.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
|
So just to understand the original issue:
Now what happens with this PR:
Does that sum it up accurately? |
|
This is for tracking playtime? Have you tested it? Because it's not really making sense to me. |
|
@utkarshdalal this is for cases where you sleep fo long periods of time like 30 min or more, often with a network disconnect in between, and playtime count is dropped in the middle. In my tests with this after 30 minutes an network loss and regain it was able to keep playtime |
|
But playtime doesn't stop being tracked unless |
|
@utkarshdalal
logs from the test run: |
|
And you saw that your playtime isn't being added when your connection drops but it is after your change? Are you sure about this? Both the problem and solution are not adding up for me. |
|
Yeah, so say you start a game and play for 10 minutes then get disconnected from steam. Server commits that 10 minutes and adds it to your total. Then you reconnect later and you're on a new CM server, current code you'd not get any more playtime but with this PR you're now getting a new re-notify and new server starts a count again. The only thing you will lose is the gap while disconnected which was being lost regardless (and in cases of sleeping the device, is good and expected). Benefit is that now you will get subsequent playtime counts on reconnect. |
|
See this thread for issues with current behavior https://discord.com/channels/1378308569287622737/1489393232935915571 |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
app/src/main/java/app/gamenative/service/SteamService.kt (1)
3453-3458:⚠️ Potential issue | 🟠 MajorRe-read active sessions inside the coroutine to avoid replaying stale presence.
At Line 3453,
activeGamesis snapshotted beforescope.launch. If a game exits before the coroutine runs, this can re-send a staleGameProcessInfoand briefly resurrect presence/playtime for a closed game.🐛 Minimal fix
- val activeGames = ActiveGameRegistry.all() - if (activeGames.isNotEmpty()) { - Timber.i("Re-sending %d active game session(s) after Steam reconnect", activeGames.size) - scope.launch { - notifyRunningProcesses(*activeGames.toTypedArray()) - } - } else { - Timber.d("No active game sessions to re-send after Steam reconnect") - } + scope.launch { + val activeGames = ActiveGameRegistry.all() + if (activeGames.isNotEmpty()) { + Timber.i("Re-sending %d active game session(s) after Steam reconnect", activeGames.size) + notifyRunningProcesses(*activeGames.toTypedArray()) + } else { + Timber.d("No active game sessions to re-send after Steam reconnect") + } + }🤖 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 3453 - 3458, activeGames is captured before launching the coroutine, causing stale GameProcessInfo to be re-sent if sessions end before the coroutine runs; fix by moving the snapshot inside the coroutine: inside the scope.launch block call ActiveGameRegistry.all() again (and check isNotEmpty there) and pass its result into notifyRunningProcesses, referencing the existing ActiveGameRegistry.all(), scope.launch, notifyRunningProcesses(...) and the activeGames usage so you re-read the live registry at send-time instead of using the pre-captured list.
🤖 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 3453-3458: activeGames is captured before launching the coroutine,
causing stale GameProcessInfo to be re-sent if sessions end before the coroutine
runs; fix by moving the snapshot inside the coroutine: inside the scope.launch
block call ActiveGameRegistry.all() again (and check isNotEmpty there) and pass
its result into notifyRunningProcesses, referencing the existing
ActiveGameRegistry.all(), scope.launch, notifyRunningProcesses(...) and the
activeGames usage so you re-read the live registry at send-time instead of using
the pre-captured list.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e6292d4e-8cd3-4894-8c1b-a91ed06af4ef
📒 Files selected for processing (1)
app/src/main/java/app/gamenative/service/SteamService.kt
@utkarshdalal I think this is worth us discussing as a group a bit more. Seen quite a few people get tripped up by this behavoiur it seems. |
|
@phobos665 - sounds good, i'm checking with the people in the thread. For at least two of them, the games they're playing aren't detected as running at all, has nothing to do with this change of it not detecting when device is put to sleep. The fix there was to check the process name and not the name of the exe, I had a PR for that but it's quite brittle. |
|
@xXJSONDeruloXx - looks like your assessment was right here. Happy to merge but we need to store a single active game and not a list as this could cause weird bugs. Here's an explanation AI wrote for me:
TL;DR - if there's ever more than one game in the registry, the second game will keep adding playtime every onLoggedOn indefinitely |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/main/java/app/gamenative/service/ActiveGameRegistry.kt (1)
17-20: ⚡ Quick winConsider adding a defensive check or warning log when overwriting an active game.
While the single-slot design correctly reflects GameNative's one-game-at-a-time constraint, silently overwriting an existing entry could mask lifecycle bugs (e.g.,
set()called twice without an interveningclear()). A warning log would aid debugging without adding significant complexity.🛡️ Proposed defensive logging
`@Synchronized` fun set(gameProcessInfo: GameProcessInfo) { + if (activeGame != null) { + // Log warning: overwriting an existing active game entry + println("WARNING: ActiveGameRegistry.set() called while activeGame is already set (appId=${activeGame?.appId}). This may indicate a lifecycle bug.") + } activeGame = gameProcessInfo }🤖 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/service/ActiveGameRegistry.kt` around lines 17 - 20, The set(gameProcessInfo: GameProcessInfo) method in ActiveGameRegistry currently overwrites activeGame silently; change it to check the existing activeGame (the field activeGame) inside the `@Synchronized` set and, if non-null, emit a warning-level log that includes identifying details of the current activeGame and the incoming gameProcessInfo (so you can trace unexpected double-sets), then proceed to replace the value; use the class logger (or add one if missing) and keep synchronization and behavior unchanged other than the warning.
🤖 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.
Nitpick comments:
In `@app/src/main/java/app/gamenative/service/ActiveGameRegistry.kt`:
- Around line 17-20: The set(gameProcessInfo: GameProcessInfo) method in
ActiveGameRegistry currently overwrites activeGame silently; change it to check
the existing activeGame (the field activeGame) inside the `@Synchronized` set and,
if non-null, emit a warning-level log that includes identifying details of the
current activeGame and the incoming gameProcessInfo (so you can trace unexpected
double-sets), then proceed to replace the value; use the class logger (or add
one if missing) and keep synchronization and behavior unchanged other than the
warning.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 76785523-3f99-461c-b326-6f2719184011
📒 Files selected for processing (3)
app/src/main/java/app/gamenative/service/ActiveGameRegistry.ktapp/src/main/java/app/gamenative/service/SteamService.ktapp/src/main/java/app/gamenative/ui/model/MainViewModel.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- app/src/main/java/app/gamenative/ui/model/MainViewModel.kt
- app/src/main/java/app/gamenative/service/SteamService.kt
|
@utkarshdalal done |
|
thanks merging |
3 participants
Description
Simple fix where if steam connection is lost while game is running, the playtime would be dropped. this lets long running sessions keep properly reporting play time when steam connection is dropped and regained.
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
Resends Steam presence after reconnect so playtime and resume state recover correctly. Tracks a single active game session and re-notifies Steam after connection drops.
ActiveGameRegistrysingle-slot store to keep the currentGameProcessInfoacross reconnects and avoid stale re-announces.SteamServicerestores persona state and re-sendsnotifyRunningProcessesfor the active game.MainViewModelrecords/clears the session on launch/exit; skips and clears when real Steam handles presence; app teardown clears the registry.Written for commit 53f0bb2. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
New Features
Improvements
Bug Fixes / Stability