feat: show Steam cloud save status#1383
Open
kiequoo wants to merge 6 commits into
Open
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 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 data 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)
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.
|
![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
kiequoo
force-pushed
the
feat/steam-cloud-save-status
branch
3 times, most recently
from
11dd5af to
cbc25e4
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.kt (1)
552-553: ⚡ Quick winAdd documentation for the new hook.
Other hook functions in this class include KDoc comments explaining their purpose. Please add documentation following the existing pattern for consistency.
📝 Suggested documentation
+ /** + * Get the force cloud sync callback for this game source. + * Returns a callback that accepts a SaveLocation (Local/Remote) to resolve conflicts. + * Override this in subclasses that support cloud save sync (e.g., Steam). + */ protected open fun getForceCloudSync(context: Context, libraryItem: LibraryItem): ((SaveLocation) -> Unit)? = null🤖 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/screen/library/appscreen/BaseAppScreen.kt` around lines 552 - 553, Add a KDoc comment above the protected open function getForceCloudSync(context: Context, libraryItem: LibraryItem): ((SaveLocation) -> Unit)? that follows the existing pattern used for other hook methods in this class: describe the purpose of the hook (optional override to provide a callback to force cloud sync for a LibraryItem), explain the parameters (context and libraryItem) and the return value (a nullable function taking a SaveLocation and returning Unit, or null to indicate no forced sync), and include any remarks about default behavior and when subclasses should override it.
🤖 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/ui/screen/library/appscreen/BaseAppScreen.kt`:
- Around line 552-553: Add a KDoc comment above the protected open function
getForceCloudSync(context: Context, libraryItem: LibraryItem): ((SaveLocation)
-> Unit)? that follows the existing pattern used for other hook methods in this
class: describe the purpose of the hook (optional override to provide a callback
to force cloud sync for a LibraryItem), explain the parameters (context and
libraryItem) and the return value (a nullable function taking a SaveLocation and
returning Unit, or null to indicate no forced sync), and include any remarks
about default behavior and when subclasses should override it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2da842f0-8d9b-484b-a73c-6e77f738da1e
📒 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 (6)
- app/src/main/java/app/gamenative/events/AndroidEvent.kt
- app/src/main/res/values-es/strings.xml
- app/src/main/res/values-de/strings.xml
- app/src/main/res/values/strings.xml
- app/src/main/res/values-pt-rBR/strings.xml
- app/src/main/res/values-ro/strings.xml
🚧 Files skipped from review as they are similar to previous changes (16)
- app/src/main/res/values-pl/strings.xml
- app/src/main/res/values-ru/strings.xml
- app/src/main/res/values-ko/strings.xml
- app/src/main/res/values-da/strings.xml
- app/src/main/java/app/gamenative/ui/data/GameDisplayInfo.kt
- app/src/main/res/values-zh-rTW/strings.xml
- app/src/main/res/values-uk/strings.xml
- app/src/main/res/values-it/strings.xml
- app/src/main/java/app/gamenative/data/SteamApp.kt
- app/src/main/res/values-zh-rCN/strings.xml
- app/src/main/java/app/gamenative/ui/data/CloudSaveStatus.kt
- app/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.kt
- app/src/main/java/app/gamenative/ui/screen/library/LibraryScreen.kt
- app/src/main/java/app/gamenative/ui/screen/library/LibraryAppScreen.kt
- app/src/main/java/app/gamenative/service/SteamAutoCloud.kt
- app/src/main/java/app/gamenative/service/SteamService.kt
Owner
|
Made a PR to your branch so that I can review the actual changes, can you please check? Secondly, we'll need to move the message for cloud saves elsewhere - it's not going to appear correctly in portrait mode. Maybe a small icon top right or something like that. Let's discuss on Discord. |
Contributor
Author
|
Hi @utkarshdalal if your PR makes your job easier then fine, but I think it's important to remove the code duplication ASAP as soon as it would be merged, because if the actual sync code and the status check code start to diverge in any way then what the user sees is going to happen when they sync might not actually be what will really happen. But then I think that follow-up PR would be just as difficult to review. BTW just to check, did you see that I re-worked the commit history of this PR last week? There are now 4 commits which are purely refactoring (no logic changes) before any of the changes related to the status check. Those 4 commits are all less than 100 lines each and shouldn't be too hard to review in isolation |
Move the Steam Auto Cloud prefix and absolute-path resolution helpers out of syncUserFiles() so the same path mapping can be reused by the later status snapshot code. This is intended to be a mechanical extraction only: the resolver keeps the existing rootoverride, addPath, embedded %GameInstall%, and remote file-list conversion behavior, and syncUserFiles() continues to call the same logic through the new resolver instance.
Move the local-vs-cached UserFileInfo diff calculation out of syncUserFiles() so both sync and status resolution can use one comparison function. This is intended to be a mechanical extraction only: the created, deleted, modified, and changesExist calculations are unchanged, including the existing SHA comparison logging.
Move local save discovery out of syncUserFiles() so the later cloud status snapshot can inspect local files without duplicating scan behavior. This is intended to be a mechanical extraction only: Windows UFS patterns, SteamUserData fallback scanning, prefix keys, timestamps, hashes, cloudRoot, and cloudPath are preserved.
Move the remote-newer conflict decision out of syncUserFiles() so the later status snapshot can classify pending download, upload, and conflict states with the same rules. This is intended to be a mechanical extraction only: cached local-change detection remains timed in microsecAcPrepUserFiles, the cache rehydration shortcut still writes the same DB rows, and conflict timestamps/UFS version are preserved.
Expose a read-only snapshot path for the UI to resolve cloud status without starting a sync. Add an optional phase callback to syncUserFiles() so SteamService can emit DOWNLOADING and UPLOADING status changes while preserving the existing sync flow.
Wire the Steam cloud snapshot and sync phase events into the library and app detail UI. This keeps the SteamAutoCloud sync behavior in the earlier mechanical commits and limits this commit to status presentation, user actions, and service-level state tracking.
kiequoo
force-pushed
the
feat/steam-cloud-save-status
branch
from
cbc25e4 to
a7abdc2
Compare
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 (with timestamps), and a force-sync action. Extracts shared Auto-Cloud helpers and exposes a read-only sync snapshot so the UI can show status without starting a sync.
New Features
AndroidEvent.CloudStatusChanged; active phases set to DOWNLOADING/UPLOADING; status resolved withSteamService.resolveCloudSaveStatus().SteamService.launchForceSync(); quick action shown when pending or in conflict.SteamApp.supportsCloudSavesand hidden for local-saves-only containers; selection persists so you return to the game info screen after closing a game or process recreation.Refactors
CloudPathResolver) andSteamAutoCloud.fetchSyncSnapshot(); preserves change-number and cache rehydration behavior.syncUserFiles();SteamServicetracks in-progress state and emits status changes; adds helpers:resolveCloudSaveStatus(),isSyncInProgress,getActiveCloudSyncStatus, prefix-path builder, andlaunchForceSync().Written for commit a7abdc2. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
New Features
UI
Bug Fixes
Localization