fix: skip achievements generation when Steam not logged in

[jeremybernstein]

Closes #834

Summary

• generateAchievementsFile NPEs on instance!!._steamUser!! when Steam is offline
• Guard with SteamService.isLoggedIn check — achievements can't be fetched from Steam API offline anyway; previous cached files in steam_settings/ persist from prior online launches

Test plan

• Launch Steam game with steamOfflineMode while Steam is offline — no crash
• Launch Steam game online — achievements still generated normally

---

Summary by cubic

Skip achievements generation when Steam isn’t logged in to prevent a null-pointer crash in generateAchievementsFile, and keep using cached files in steam_settings/. When online, achievements still generate; failures are now caught and logged instead of crashing.

^{Written for commit 37d2c1a. Summary will update on new commits.}

Summary by CodeRabbit

• Bug Fixes
  • Added Steam authentication validation to skip achievement generation when not logged in, with a warning log.
  • Improved error handling around achievement generation to log failures instead of allowing crashes.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 89f4d38b-c4e6-48c2-91dc-dc705b362cd3

📥 Commits

Reviewing files that changed from the base of the PR and between a253bcc and 37d2c1a.

📒 Files selected for processing (1)

• app/src/main/java/app/gamenative/utils/SteamUtils.kt

🚧 Files skipped from review as they are similar to previous changes (1)

• app/src/main/java/app/gamenative/utils/SteamUtils.kt

---

📝 Walkthrough

Walkthrough

Added a guard in generateAchievementsFile to return early when SteamService.isLoggedIn is false, and wrapped the SteamService.generateAchievements call in a try/catch that logs failures with Timber to avoid crashes when Steam is offline or generation throws.

Changes

Cohort / File(s)	Summary
Steam achievements guard & error handling app/src/main/java/app/gamenative/utils/SteamUtils.kt	Added an early-return guard checking SteamService.isLoggedIn in generateAchievementsFile; wrapped SteamService.generateAchievements in a try/catch and log errors with Timber to prevent crashes when Steam is offline or generation throws.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A cautious hop, a careful stride,
I check if Steam is by my side.
If offline, I skip the dance,
If errors bloom, I log the glance,
Safe gardens keep the code alive!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: adding a guard to skip achievements generation when Steam is not logged in, which directly addresses the offline crash issue.
Linked Issues check	✅ Passed	The code changes directly address issue #834 by adding a guard using SteamService.isLoggedIn to prevent NPE when Steam is offline, and wrapping generation in error handling to prevent crashes.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing the offline crash issue; no unrelated modifications to other functionality or systems are present.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

📝 Coding Plan

• Generate coding plan for human review comments

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

app/src/main/java/app/gamenative/utils/SteamUtils.kt (1)

1344-1357: ⚠️ Potential issue | 🟠 Major

Good guard, but a logout race can still crash achievements generation.

Line 1344 checks a snapshot (isLoggedIn), but SteamService.generateAchievements(...) at Line 1356 can still hit force-unwrapped nulls if Steam logs out between check and call. Please harden this call path so session flips do not crash.

Suggested mitigation at this call site

 fun generateAchievementsFile(dllPath: Path, appId: String) {
     if (!SteamService.isLoggedIn) {
         Timber.w("Skipping achievements generation for $appId — Steam not logged in")
         return
     }

     val steamAppId = ContainerUtils.extractGameIdFromContainerId(appId)
     val settingsDir = dllPath.parent.resolve("steam_settings")
     if (Files.notExists(settingsDir)) {
         Files.createDirectories(settingsDir)
     }

     runBlocking {
-        SteamService.generateAchievements(steamAppId, settingsDir.absolutePathString())
+        runCatching {
+            SteamService.generateAchievements(steamAppId, settingsDir.absolutePathString())
+        }.onFailure { e ->
+            Timber.w(e, "Skipping achievements generation for $appId due to Steam session change")
+        }
     }
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/main/java/app/gamenative/utils/SteamUtils.kt` around lines 1344 -
1357, The snapshot check of SteamService.isLoggedIn can race with logout before
SteamService.generateAchievements(...) runs; update the call site so you obtain
a stable auth/session or re-check inside the coroutine and guard the call: e.g.,
inside runBlocking re-evaluate SteamService.isLoggedIn or call a
SteamService.getActiveSession()/currentSession method (or otherwise capture a
non-null session) and only call SteamService.generateAchievements(steamAppId,
settingsDir.absolutePathString()) when that session is present, and wrap the
call in try/catch to log and return safely if the session becomes null or
throws; adjust uses of SteamService.isLoggedIn and
SteamService.generateAchievements accordingly.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@app/src/main/java/app/gamenative/utils/SteamUtils.kt`:
- Around line 1344-1357: The snapshot check of SteamService.isLoggedIn can race
with logout before SteamService.generateAchievements(...) runs; update the call
site so you obtain a stable auth/session or re-check inside the coroutine and
guard the call: e.g., inside runBlocking re-evaluate SteamService.isLoggedIn or
call a SteamService.getActiveSession()/currentSession method (or otherwise
capture a non-null session) and only call
SteamService.generateAchievements(steamAppId, settingsDir.absolutePathString())
when that session is present, and wrap the call in try/catch to log and return
safely if the session becomes null or throws; adjust uses of
SteamService.isLoggedIn and SteamService.generateAchievements accordingly.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4fab5695-f245-43bf-8f1c-e9bfe582877c

📥 Commits

Reviewing files that changed from the base of the PR and between 6fcda17 and a253bcc.

📒 Files selected for processing (1)

• app/src/main/java/app/gamenative/utils/SteamUtils.kt

[cubic-dev-ai]

No issues found across 1 file

[jeremybernstein]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@app/src/main/java/app/gamenative/utils/SteamUtils.kt`:
- Around line 1344-1357: The snapshot check of SteamService.isLoggedIn can race
with logout before SteamService.generateAchievements(...) runs; update the call
site so you obtain a stable auth/session or re-check inside the coroutine and
guard the call: e.g., inside runBlocking re-evaluate SteamService.isLoggedIn or
call a SteamService.getActiveSession()/currentSession method (or otherwise
capture a non-null session) and only call
SteamService.generateAchievements(steamAppId, settingsDir.absolutePathString())
when that session is present, and wrap the call in try/catch to log and return
safely if the session becomes null or throws; adjust uses of
SteamService.isLoggedIn and SteamService.generateAchievements accordingly.

ℹ️ Review info

theoretical... but fixing it is cheap. force-pushed. @coderabbitai

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Read the relevant section of SteamUtils.kt around lines 1330-1380
fd "SteamUtils.kt" --exec sed -n '1330,1380p' {}

Length of output: 1191

---

@jeremybernstein: The fix looks great. The two-layer approach is clean and solid:

1. Early guard — if (!SteamService.isLoggedIn) short-circuits immediately when offline.
2. try/catch around runBlocking { ... } — handles the narrow race window where logout could occur between the guard check and the actual generateAchievements call; instead of crashing, it logs and returns safely.

This covers the original NPE and the theoretical race in one go. ✅

---

🧠 Learnings used

Learnt from: jeremybernstein
Repo: utkarshdalal/GameNative PR: 801
File: app/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.kt:174-188
Timestamp: 2026-03-10T22:11:59.430Z
Learning: In `app/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.kt`, the `applyConfigForContainer` function calls `installMissingComponentsForConfig` before `BestConfigService.parseConfigToContainerData`. This install-before-parse ordering is pre-existing behavior, not introduced by any recent PR. Do not flag this ordering as an issue in future reviews of this file.

---

Learnt from: jeremybernstein
Repo: utkarshdalal/GameNative PR: 801
File: app/src/main/java/app/gamenative/utils/BestConfigService.kt:45-51
Timestamp: 2026-03-10T21:46:10.135Z
Learning: In `app/src/main/java/app/gamenative/utils/BestConfigService.kt`, the singleton mutable state pattern (`lastMissingComponents` / `consumeLastMissingComponents()`) — where `parseConfigToContainerData()` writes a global and callers read it via `consume*()` — is a pre-existing design. Refactoring it to return missing components as part of the parse result has a large blast radius and is intentionally deferred. Do not flag this pattern as an issue in future reviews of this file.

---

Learnt from: jeremybernstein
Repo: utkarshdalal/GameNative PR: 0
File: :0-0
Timestamp: 2026-03-10T11:01:21.903Z
Learning: In `app/src/main/java/app/gamenative/ui/model/LibraryViewModel.kt` (around lines 371-396), the `Loading` parental state in `SteamService.parentalControlState` intentionally maps to `PrefManager.parentalAllowedAppIds` (which can be null) rather than `emptyList()`. This fail-open behavior on the `Loading` state is by design: on first launch/login the account type (managed child vs. non-child) is unknown, so showing all apps is preferable to incorrectly blocking everything. Subsequent sessions use the PrefManager cache, so restrictions are enforced correctly after the first login. Do not flag this as a security/correctness issue in future reviews.

---

Learnt from: jeremybernstein
Repo: utkarshdalal/GameNative PR: 0
File: :0-0
Timestamp: 2026-03-10T22:13:54.429Z
Learning: In `app/src/main/java/app/gamenative/utils/BestConfigService.kt`, around lines 403-418, missing Box64/FEXCore preset failures (via `Box86_64PresetManager.getPreset("box64", ...)` and `FEXCorePresetManager.getPreset(...)`) are intentionally appended to the same `missingComponents` list as installable components, even though `resolveMissingManifestInstallRequests()` cannot install presets. This is a known limitation deferred for future work — the long-term direction is to serialize non-default presets into config files and auto-install them. Do not flag this mixing of installable vs. non-installable blockers in `missingComponents` as an issue in future reviews of this file.

---

Learnt from: utkarshdalal
Repo: utkarshdalal/GameNative PR: 159
File: app/src/main/java/app/gamenative/service/DownloadService.kt:6-6
Timestamp: 2025-09-19T17:07:27.941Z
Learning: In app/src/main/java/app/gamenative/service/DownloadService.kt, the getSizeFromStoreDisplay() function uses runBlocking to call SteamService.getDownloadableDepots() because the latter was converted from synchronous to suspend in this PR. The user confirmed the previous synchronous implementation worked fine, and if performance is acceptable, keeping runBlocking is a valid approach.

---

Learnt from: unbelievableflavour
Repo: utkarshdalal/GameNative PR: 811
File: app/src/main/java/app/gamenative/ui/util/WindowSize.kt:38-44
Timestamp: 2026-03-11T11:17:43.205Z
Learning: In `app/src/main/java/app/gamenative/ui/util/WindowSize.kt`, `shouldShowGamepadUI()` reads `PrefManager.showGamepadHints` as a plain (non-reactive) property. This is intentional: when the user toggles the setting in the Settings screen and navigates back, the composable recomposes and picks up the updated value. Real-time reactivity (e.g., via StateFlow/collectAsState) is not required here. Do not flag this as an issue in future reviews.

---

Learnt from: jeremybernstein
Repo: utkarshdalal/GameNative PR: 0
File: :0-0
Timestamp: 2026-03-10T21:56:45.006Z
Learning: In `app/src/main/java/app/gamenative/utils/BestConfigService.kt`, the `available*` lists (availableDxvk, availableVkd3d, availableBox64Bionic, availableBox64Glibc, availableWowBox64, availableFexcore, availableWineBionic, availableWineGlibc, availableDrivers) passed to `validateComponentVersions()` intentionally include manifest entries (base + installed + manifest). This allows configs referencing installable-but-not-yet-on-disk versions to pass validation. A separate subsequent step, `resolveMissingManifestInstallRequests()`, handles actually installing any manifest items not yet present on disk. Do not suggest removing manifest entries from these lists in future reviews.

---

Learnt from: utkarshdalal
Repo: utkarshdalal/GameNative PR: 344
File: app/src/main/java/app/gamenative/service/SteamAutoCloud.kt:415-419
Timestamp: 2025-12-17T05:14:05.133Z
Learning: In SteamAutoCloud.kt, when uploading files to Steam Cloud API (beginFileUpload and commitFileUpload), the filename parameter intentionally uses different formats: `file.path + file.filename` (relative path without placeholder) when `appInfo.ufs.saveFilePatterns.isEmpty()` is true (fallback case), and `file.prefixPath` (includes placeholder like %SteamUserData%) when patterns are configured. This difference is by design.

---

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!