feat: show Steam cloud save status#1383
Open
kiequoo wants to merge 4 commits into
Open
Conversation
Contributor
📝 WalkthroughWalkthroughAdds end-to-end cloud-save support: CloudSaveStatus type and strings, CloudPathResolver and phase callbacks in SteamAutoCloud, SteamService per-app status tracking and public APIs, UI components/wiring (status/action/conflict), and small data/schema and selection-state updates. ChangesCloud Save Synchronization
Sequence DiagramsequenceDiagram
participant UI as LibraryAppScreen
participant Event as AndroidEventBus
participant Service as SteamService
participant Cloud as SteamAutoCloud
participant Storage as SteamCloudStorage
UI->>Service: onForceCloudSync(appId, SaveLocation)
Service->>Service: markCloudSyncStarted(appId, UPLOADING/DOWNLOADING)
Service->>Event: emit CloudStatusChanged(appId, status)
Event->>UI: Update cloud status UI
Service->>Cloud: syncUserFiles(appInfo, onPhaseStarted)
Cloud->>Cloud: CloudPathResolver resolves prefixes/paths
Cloud->>Storage: Upload/download file deltas
Storage-->>Cloud: Return remote changes / acks
Cloud->>Service: onPhaseStarted(isUploading=false/true)
Service->>Service: markCloudSyncStarted(appId, DOWNLOADING/UPLOADING)
Cloud-->>Service: Return SyncResult
Service->>Service: markCloudSyncFinished(appId, result)
Service->>Event: emit CloudStatusChanged(appId, UP_TO_DATE/CONFLICT/FAILED)
Service->>Service: releaseSync (remove cloudSyncStatuses entry)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
1 similar comment
Contributor
✅ Actions performedReview triggered.
|
Extract the Steam Auto Cloud path mapping, local save scanning, file diffing, and remote-change decision code out of syncUserFiles(). Keep syncUserFiles() on the same launch-time decision path from master: it still uses the cached local change list, the effective local change number, the cache rehydration shortcut for byte-identical local and remote files, and the same preferred-save conflict handling. This commit is intentionally behavior-preserving so the cloud save status indicator can call the same logic in the following commit instead of duplicating or approximating it.
Switches selectedAppId from remember to rememberSaveable so it survives navigation to XServerScreen. Adds currentSelectedLibraryItem to re-hydrate the detail pane from the app list on return, so the user lands back on the game info screen rather than the library list.
![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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/src/main/java/app/gamenative/service/SteamAutoCloud.kt (1)
183-218:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep embedded
%GameInstall%files in the%GameInstall%prefix bucket.
getFullFilePath()already compensates for Steam returningprefix=""plusfilename="%GameInstall%...", butgetFilePrefix()still returns""for that case.hasHashConflicts()keyslocalUserFilesbygetFilePrefix(), so these entries never match the%GameInstall%bucket and post-download validation can incorrectly report success.Suggested fix
fun getFilePrefix(file: AppFileInfo, fileList: AppFileChangeList): String { + val gameInstallPrefix = "%${PathType.GameInstall.name}%" + if (file.filename.startsWith(gameInstallPrefix)) { + return gameInstallPrefix + } + return if (file.pathPrefixIndex < fileList.pathPrefixes.size) { Paths.get(fileList.pathPrefixes[file.pathPrefixIndex]).pathString } else { "" } }🤖 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/SteamAutoCloud.kt` around lines 183 - 218, getFilePrefix currently returns "" when Steam put the embedded "%GameInstall%" in file.filename, causing hasHashConflicts (which keys by getFilePrefix) to miss those files; change getFilePrefix(file, fileList) to detect when file.filename startsWith the gameInstallPrefix ("%${PathType.GameInstall.name}%") and return that prefix in that case, otherwise keep the existing logic (checking file.pathPrefixIndex against fileList.pathPrefixes and returning that prefix or ""). This ensures entries keyed by getFilePrefix match the same "%GameInstall%" bucket that getFullFilePath handles.app/src/main/java/app/gamenative/service/SteamService.kt (1)
2587-2630:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEmit a terminal cloud status when close-time sync fails.
This path only calls
markCloudSyncFinished()whensyncUserFiles()returns a non-null result. If all retries fail or the sync exits early,releaseSync()clears the in-progress state without ever emittingFAILED, so the UI can drop fromCHECKING/UPLOADINGback to idle with no error.Suggested fix
+ var syncResult = SyncResult.UnknownFail val maxAttempts = 3 for (attempt in 1..maxAttempts) { try { PrefManager.clientId?.let { clientId -> instance?.let { steamInstance -> @@ - postSyncInfo?.let { info -> - markCloudSyncFinished(appId, info.syncResult) - } + postSyncInfo?.let { info -> + syncResult = info.syncResult + } steamCloud.signalAppExitSyncDone( appId = appId, clientId = clientId, uploadsCompleted = postSyncInfo?.uploadsCompleted == true, uploadsRequired = postSyncInfo?.uploadsRequired == false, @@ } finally { + markCloudSyncFinished(appId, syncResult) releaseSync(appId) }🤖 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/SteamService.kt` around lines 2587 - 2630, The close-time sync loop can exit without ever calling markCloudSyncFinished(appId, ...) if syncUserFiles() returns null or all retries throw AsyncJobFailedException, leaving releaseSync(appId) to clear progress without emitting a FAILED status; update the logic in the retry loop around SteamAutoCloud.syncUserFiles(...) so that any terminal failure path (null postSyncInfo after all attempts or catch where attempt == maxAttempts) calls markCloudSyncFinished(appId, SyncResult.FAILED) (or the equivalent FAILED enum/state your code uses) before breaking/propagating, and ensure the AsyncJobFailedException catch block and the post-loop path both set FAILED when appropriate so the UI receives the terminal status prior to releaseSync(appId).
🤖 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/service/SteamAutoCloud.kt`:
- Around line 221-239: In fileChangeListToUserFiles, resolve the root by using
the indexed prefix from appFileListChange.pathPrefixes (using
it.pathPrefixIndex) instead of indexing into the flattened pathTypePairs; call
PathType.from on the actual prefix string at
appFileListChange.pathPrefixes[it.pathPrefixIndex] and if the index is out of
range fall back to PathType.DEFAULT (not PathType.GameInstall); keep the path
value aligned by reading appFileListChange.pathPrefixes[it.pathPrefixIndex] when
in-range and using "" on fallback; references: function
fileChangeListToUserFiles, variable appFileListChange, property pathPrefixes,
index pathPrefixIndex, helper getPathTypePairs, and enum PathType.
In `@app/src/main/java/app/gamenative/service/SteamService.kt`:
- Around line 441-449: The helper markCloudSyncFinished is collapsing all
non-success/non-conflict SyncResult values into CloudSaveStatus.FAILED; update
its when expression to handle SyncResult.PendingOperations separately (the value
produced by beginLaunchApp()/signalAppLaunchIntent()) and map it to the
appropriate pending CloudSaveStatus (e.g., CloudSaveStatus.PENDING_OPERATIONS)
instead of FAILED so the UI shows a recoverable pending state; keep existing
mappings for Success/UpToDate and Conflict unchanged.
In
`@app/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.kt`:
- Around line 385-418: The LaunchedEffect watching gameId and
cloudConnectivityVersion should also include isInstalled so the cloud-status
resolution re-runs when install state changes; update the LaunchedEffect key
list (the one wrapping the block that reads hasCloudSaves, isInstalled,
SteamService.* and updates cloudSaveStatus, syncStateText, conflictUfsVersion,
conflictLocalTimestamp, conflictRemoteTimestamp) to include isInstalled (and
optionally hasCloudSaves if you want updates when that toggles) so the initial
cloud snapshot is fetched once the app becomes installed.
- Around line 364-370: The onCloudStatusChanged lambda only refreshes dialog
metadata when status == FAILED, so when status flips to CloudSaveStatus.CONFLICT
the conflict dialog can show stale/empty details; update the handler so that
when event.status == CloudSaveStatus.CONFLICT you also retrigger the
snapshot-resolution/metadata refresh (the same path used for FAILED) — e.g.,
call the same refresh routine or increment cloudConnectivityVersion (or invoke
the existing snapshot resolution method) inside the onCloudStatusChanged block
when status == CONFLICT so the dialog timestamps/UFS version are repopulated.
---
Outside diff comments:
In `@app/src/main/java/app/gamenative/service/SteamAutoCloud.kt`:
- Around line 183-218: getFilePrefix currently returns "" when Steam put the
embedded "%GameInstall%" in file.filename, causing hasHashConflicts (which keys
by getFilePrefix) to miss those files; change getFilePrefix(file, fileList) to
detect when file.filename startsWith the gameInstallPrefix
("%${PathType.GameInstall.name}%") and return that prefix in that case,
otherwise keep the existing logic (checking file.pathPrefixIndex against
fileList.pathPrefixes and returning that prefix or ""). This ensures entries
keyed by getFilePrefix match the same "%GameInstall%" bucket that
getFullFilePath handles.
In `@app/src/main/java/app/gamenative/service/SteamService.kt`:
- Around line 2587-2630: The close-time sync loop can exit without ever calling
markCloudSyncFinished(appId, ...) if syncUserFiles() returns null or all retries
throw AsyncJobFailedException, leaving releaseSync(appId) to clear progress
without emitting a FAILED status; update the logic in the retry loop around
SteamAutoCloud.syncUserFiles(...) so that any terminal failure path (null
postSyncInfo after all attempts or catch where attempt == maxAttempts) calls
markCloudSyncFinished(appId, SyncResult.FAILED) (or the equivalent FAILED
enum/state your code uses) before breaking/propagating, and ensure the
AsyncJobFailedException catch block and the post-loop path both set FAILED when
appropriate so the UI receives the terminal status prior to releaseSync(appId).
🪄 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: 94d74446-d3d9-4333-8a56-8c27bc492a6a
📒 Files selected for processing (11)
app/src/main/java/app/gamenative/data/SteamApp.ktapp/src/main/java/app/gamenative/events/AndroidEvent.ktapp/src/main/java/app/gamenative/service/SteamAutoCloud.ktapp/src/main/java/app/gamenative/service/SteamService.ktapp/src/main/java/app/gamenative/ui/data/CloudSaveStatus.ktapp/src/main/java/app/gamenative/ui/data/GameDisplayInfo.ktapp/src/main/java/app/gamenative/ui/screen/library/LibraryAppScreen.ktapp/src/main/java/app/gamenative/ui/screen/library/LibraryScreen.ktapp/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.ktapp/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.ktapp/src/main/res/values/strings.xml
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
5 issues found across 11 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/service/SteamAutoCloud.kt">
<violation number="1" location="app/src/main/java/app/gamenative/service/SteamAutoCloud.kt:226">
P1: Resolve root/path directly from `appFileListChange.pathPrefixes[pathPrefixIndex]` rather than indexing the deduplicated `pathTypePairs`; using `pathPrefixIndex` against a `distinct()` list can misalign prefixes and produce incorrect remote file metadata.</violation>
</file>
<file name="app/src/main/java/app/gamenative/service/SteamService.kt">
<violation number="1" location="app/src/main/java/app/gamenative/service/SteamService.kt:447">
P1: Do not fall through pending sync outcomes to `FAILED`; map `SyncResult.PendingOperations` to a pending cloud status so the UI does not show a false failure state.</violation>
</file>
<file name="app/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.kt:368">
P2: Trigger a cloud-status re-resolution when status becomes `CONFLICT` as well as `FAILED`; otherwise the conflict dialog can show stale or missing timestamps/UFS version.</violation>
<violation number="2" location="app/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.kt:385">
P2: Cloud save status `LaunchedEffect` is missing reactive dependencies `hasCloudSaves` and `isInstalled` in its key set. If these values change (e.g., the user installs the game while on the screen), the effect will not rerun and cloud save status resolution is skipped, leaving the UI stale until an unrelated key-changing event occurs.</violation>
<violation number="3" location="app/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.kt:412">
P2: Conflict timestamps are never cleared when resolution has no conflict data, so stale timestamps can persist in the UI model and conflict dialog details.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
kiequoo
force-pushed
the
feat/steam-cloud-save-status
branch
from
a4f2bab to
461d370
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/res/values/strings.xml`:
- Line 1248: The string resource named cloud_saves_pending_operations uses
singular wording ("Remote Operation Pending") but the key/state is plural;
update the value of string name="cloud_saves_pending_operations" to use plural
phrasing (e.g., "Remote Operations Pending" or "Remote operations pending") so
the text matches the plural key and UI state.
🪄 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: 0b508aff-750a-4acb-a456-b481c5eb574b
📒 Files selected for processing (24)
app/src/main/java/app/gamenative/data/SteamApp.ktapp/src/main/java/app/gamenative/events/AndroidEvent.ktapp/src/main/java/app/gamenative/service/SteamAutoCloud.ktapp/src/main/java/app/gamenative/service/SteamService.ktapp/src/main/java/app/gamenative/ui/data/CloudSaveStatus.ktapp/src/main/java/app/gamenative/ui/data/GameDisplayInfo.ktapp/src/main/java/app/gamenative/ui/screen/library/LibraryAppScreen.ktapp/src/main/java/app/gamenative/ui/screen/library/LibraryScreen.ktapp/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.ktapp/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.ktapp/src/main/res/values-da/strings.xmlapp/src/main/res/values-de/strings.xmlapp/src/main/res/values-es/strings.xmlapp/src/main/res/values-fr/strings.xmlapp/src/main/res/values-it/strings.xmlapp/src/main/res/values-ko/strings.xmlapp/src/main/res/values-pl/strings.xmlapp/src/main/res/values-pt-rBR/strings.xmlapp/src/main/res/values-ro/strings.xmlapp/src/main/res/values-ru/strings.xmlapp/src/main/res/values-uk/strings.xmlapp/src/main/res/values-zh-rCN/strings.xmlapp/src/main/res/values-zh-rTW/strings.xmlapp/src/main/res/values/strings.xml
✅ Files skipped from review due to trivial changes (9)
- app/src/main/res/values-pt-rBR/strings.xml
- app/src/main/res/values-fr/strings.xml
- app/src/main/res/values-it/strings.xml
- app/src/main/res/values-zh-rTW/strings.xml
- app/src/main/res/values-ru/strings.xml
- app/src/main/res/values-ro/strings.xml
- app/src/main/res/values-uk/strings.xml
- app/src/main/res/values-es/strings.xml
- app/src/main/res/values-ko/strings.xml
🚧 Files skipped from review as they are similar to previous changes (10)
- app/src/main/java/app/gamenative/ui/data/CloudSaveStatus.kt
- app/src/main/java/app/gamenative/events/AndroidEvent.kt
- app/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.kt
- app/src/main/java/app/gamenative/ui/screen/library/LibraryAppScreen.kt
- app/src/main/java/app/gamenative/ui/data/GameDisplayInfo.kt
- app/src/main/java/app/gamenative/ui/screen/library/LibraryScreen.kt
- app/src/main/java/app/gamenative/service/SteamAutoCloud.kt
- app/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.kt
- app/src/main/java/app/gamenative/data/SteamApp.kt
- app/src/main/java/app/gamenative/service/SteamService.kt
| <string name="cloud_saves_up_to_date">Up to Date</string> | ||
| <string name="cloud_saves_pending_download">Pending Download</string> | ||
| <string name="cloud_saves_pending_upload">Pending Upload</string> | ||
| <string name="cloud_saves_pending_operations">Remote Operation Pending</string> |
Contributor
There was a problem hiding this comment.
Use plural wording for pending operations status.
Line 1248 is singular (Remote Operation Pending) while the key/state is plural (cloud_saves_pending_operations). This can read as inconsistent in UI.
Suggested text tweak
- <string name="cloud_saves_pending_operations">Remote Operation Pending</string>
+ <string name="cloud_saves_pending_operations">Remote Operations Pending</string>📝 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.
Suggested change
| <string name="cloud_saves_pending_operations">Remote Operation Pending</string> | |
| <string name="cloud_saves_pending_operations">Remote Operations Pending</string> |
🤖 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/res/values/strings.xml` at line 1248, The string resource named
cloud_saves_pending_operations uses singular wording ("Remote Operation
Pending") but the key/state is plural; update the value of string
name="cloud_saves_pending_operations" to use plural phrasing (e.g., "Remote
Operations Pending" or "Remote operations pending") so the text matches the
plural key and UI state.
Comment on lines
-103
to
-116
| fun syncUserFiles( | ||
| appInfo: SteamApp, | ||
| clientId: Long, | ||
| steamInstance: SteamService, | ||
| steamCloud: SteamCloud, | ||
| preferredSave: SaveLocation = SaveLocation.None, | ||
| parentScope: CoroutineScope = CoroutineScope(Dispatchers.IO), | ||
| prefixToPath: (String) -> String, | ||
| overrideLocalChangeNumber: Long? = null, | ||
| onProgress: ((message: String, progress: Float) -> Unit)? = null, | ||
| ): Deferred<PostSyncInfo?> = parentScope.async { | ||
| val postSyncInfo: PostSyncInfo? | ||
|
|
||
| Timber.i("Retrieving save files of ${appInfo.name}") |
Owner
There was a problem hiding this comment.
Changing syncUserFiles is setting off alarm bells here. I'm not sure if you've just added new methods and this has just been moved, or if there are actual changes. Can you please update so that the diff only shows what is changed?
This is of course a very sensitive file and I'll need to check each change carefully.
Contributor
Author
There was a problem hiding this comment.
@utkarshdalal it's better to review each commit, then you can see more clearly that the sync logic wasn't actually changed.
It's only been refactored in such a way as to make the current sync status accessible outside the sync process
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
Adds the cloud save status UI and sync controls for Steam games.
This comes from the broader cloud saves status indicator work originally developed on
feat/cloud-saves-game-info. This PR intentionally narrows the scope to Steam only for now, so the Steam status UI and service behavior can be reviewed independently.What this PR includes:
CloudSaveStatusstate used by the library UI.SteamService.The commits are best reviewed separately: the first refactors Steam Auto-Cloud helpers, the second adds the Steam cloud save status UI/service wiring, and the third preserves returning to the app info screen after closing a game.
What this PR intentionally does not include:
Those can follow separately after this Steam-only version is reviewed.
Screenshots
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
Adds Steam-only cloud save status and controls to the game info screen with live updates, conflict handling, and a force-sync action. Refactors Auto-Cloud to expose a reusable sync snapshot and emit status events.
New Features
AndroidEvent.CloudStatusChanged; resolves status on load using service logic; conflict dialog shows local/remote timestamps and upgrade text when available.SteamService.launchForceSync().SteamApp.supportsCloudSavesand hidden for local-saves-only containers; keeps you on the game info screen after closing a game.Refactors
SteamAutoCloud.syncUserFiles()into helpers andfetchSyncSnapshot, reused bySteamService.resolveCloudSaveStatus().CloudSaveStatusmodel and display strings;SteamApp.supportsCloudSavesto gate UI; newAndroidEvent.CloudStatusChanged.SteamServicehelpers for status resolution, active phase markers (start/finish), in-progress status queries, and alaunchForceSync()entry point.Written for commit 461d370. Summary will update on new commits.
Summary by CodeRabbit