feat: allow disabling of cloud saves per game#596
Open
xXJSONDeruloXx wants to merge 11 commits intoutkarshdalal:masterfrom
Open
feat: allow disabling of cloud saves per game#596xXJSONDeruloXx wants to merge 11 commits intoutkarshdalal:masterfrom
xXJSONDeruloXx wants to merge 11 commits intoutkarshdalal:masterfrom
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 a local-saves-only flag threaded through container data, settings, menus, and launch/exit flows; introduces a Steam-client local-saves warning dialog (DialogType.STEAM_CLIENT_SYNC_WARNING) and an ignoreSteamClientLocalSavesWarning parameter to preLaunchApp; cloud-sync steps are conditionally skipped when local-saves-only is active. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
2 issues found across 10 files
Prompt for AI agents (all 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/screen/library/appscreen/SteamAppScreen.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.kt:809">
P2: isLocalSavesOnly is computed before the early return, and it calls getOrCreateContainer. This can create container data or do I/O even for games that are not installed or are downloading. Move the call below the guard to avoid side effects for those cases.</violation>
</file>
<file name="app/src/main/java/app/gamenative/utils/ContainerUtils.kt">
<violation number="1" location="app/src/main/java/app/gamenative/utils/ContainerUtils.kt:1030">
P2: isLocalSavesOnly creates containers as a side effect; this can trigger heavy container creation when UI code only needs to read a flag (e.g., in menu rendering), causing unexpected I/O and container creation for apps without containers.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
app/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.kt
Outdated
Show resolved
Hide resolved
![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: 1
🧹 Nitpick comments (2)
app/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.kt (1)
809-812: Same staleness/performance concern asEpicAppScreen; also runs before the early-return guard unnecessarily.
ContainerUtils.isLocalSavesOnlyis evaluated on every recomposition with noremember, and it executes even when the function is about to returnemptyList()(lines 811-813). Both issues can be addressed by moving the call after the early-return guard and memoising it.♻️ Suggested fix
val isDownloadInProgress = SteamService.getDownloadingAppInfoOf(gameId) != null - val isLocalSavesOnly = ContainerUtils.isLocalSavesOnly(context, appId) if (!isInstalled || isDownloadInProgress) { return emptyList() } val scope = rememberCoroutineScope() + val isLocalSavesOnly = remember(appId) { + ContainerUtils.isLocalSavesOnly(context, appId) + }🤖 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/screen/library/appscreen/SteamAppScreen.kt` around lines 809 - 812, Move the ContainerUtils.isLocalSavesOnly call so it runs after the early-return guard (the checks of isInstalled and isDownloadInProgress) and memoize it with Compose's remember; e.g. replace the immediate call to ContainerUtils.isLocalSavesOnly(context, appId) with a remembered value computed after the if (!isInstalled || isDownloadInProgress) return, using remember(context, appId) { ContainerUtils.isLocalSavesOnly(context, appId) } so the expensive call doesn't run on every recomposition or when the function returns early.app/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.kt (1)
563-573: WrapisLocalSavesOnlyinremember(or expose it as ComposeState) to avoid stale enabled state and redundant recomputation.
ContainerUtils.isLocalSavesOnly(context, libraryItem.appId)is called on the main thread on every recomposition ofgetSourceSpecificMenuOptionswith no memoisation. Two problems:
- Staleness – if
isLocalSavesOnlyreads from a non-Compose-observable source (e.g. SharedPreferences / a file), changes to the setting won't retrigger recomposition, so theForceCloudSyncbutton's enabled state stays stale until an unrelated recompose occurs.- Performance – if the read involves any I/O, it blocks the main thread on every frame that triggers this composable.
♻️ Suggested fix
- val isLocalSavesOnly = app.gamenative.utils.ContainerUtils.isLocalSavesOnly(context, libraryItem.appId) + val isLocalSavesOnly = remember(libraryItem.appId) { + app.gamenative.utils.ContainerUtils.isLocalSavesOnly(context, libraryItem.appId) + }For full reactivity, consider making
ContainerUtils.isLocalSavesOnlyreturn aStateFlow<Boolean>and collecting it withcollectAsState()here.🤖 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/screen/library/appscreen/EpicAppScreen.kt` around lines 563 - 573, The call to ContainerUtils.isLocalSavesOnly(context, libraryItem.appId) inside getSourceSpecificMenuOptions should be memoized/observed so the ForceCloudSync AppMenuOption's enabled state is not stale or recomputed every recomposition; replace the direct call with a Compose-aware value (e.g. obtain a State<Boolean> by either wrapping the current boolean in remember { mutableStateOf(...) } if the value is static for the composition, or better: change ContainerUtils.isLocalSavesOnly to expose a StateFlow<Boolean> and collect it here with collectAsState(), then use that state value when setting enabled for AppOptionMenuType.ForceCloudSync so updates and recompositions occur correctly and no blocking IO runs on every recomposition.
🤖 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/utils/ContainerUtils.kt`:
- Around line 1029-1032: The current isLocalSavesOnly calls getOrCreateContainer
which may create a container as a side effect; change it to perform a
non‑creating lookup (e.g., use an existing read-only getter like
getContainer/getContainerIfExists or add one) and return false when no container
or key exists; specifically replace the getOrCreateContainer(context, appId)
call inside isLocalSavesOnly with a non-creating lookup, read
EXTRA_LOCAL_SAVES_ONLY only if the container exists, and default to "false".
---
Nitpick comments:
In
`@app/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.kt`:
- Around line 563-573: The call to ContainerUtils.isLocalSavesOnly(context,
libraryItem.appId) inside getSourceSpecificMenuOptions should be
memoized/observed so the ForceCloudSync AppMenuOption's enabled state is not
stale or recomputed every recomposition; replace the direct call with a
Compose-aware value (e.g. obtain a State<Boolean> by either wrapping the current
boolean in remember { mutableStateOf(...) } if the value is static for the
composition, or better: change ContainerUtils.isLocalSavesOnly to expose a
StateFlow<Boolean> and collect it here with collectAsState(), then use that
state value when setting enabled for AppOptionMenuType.ForceCloudSync so updates
and recompositions occur correctly and no blocking IO runs on every
recomposition.
In
`@app/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.kt`:
- Around line 809-812: Move the ContainerUtils.isLocalSavesOnly call so it runs
after the early-return guard (the checks of isInstalled and
isDownloadInProgress) and memoize it with Compose's remember; e.g. replace the
immediate call to ContainerUtils.isLocalSavesOnly(context, appId) with a
remembered value computed after the if (!isInstalled || isDownloadInProgress)
return, using remember(context, appId) {
ContainerUtils.isLocalSavesOnly(context, appId) } so the expensive call doesn't
run on every recomposition or when the function returns early.
Contributor
There was a problem hiding this comment.
2 issues found across 3 files (changes from recent commits).
Prompt for AI agents (all 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/screen/library/appscreen/SteamAppScreen.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.kt:814">
P2: Memoizing isLocalSavesOnly with remember(appId) prevents the menu from updating when the setting toggles; ForceCloudSync can stay stale until screen recreation.</violation>
</file>
<file name="app/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.kt:565">
P2: `remember(libraryItem.appId)` caches `isLocalSavesOnly` so toggling local-only saves for the same game won’t update the menu enabled state until the composable is recreated.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
app/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.kt
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/ui/screen/library/appscreen/EpicAppScreen.kt`:
- Around line 565-567: The cached isLocalSavesOnly value uses
remember(libraryItem.appId) which only invalidates when appId changes and
therefore becomes stale after settings change; replace the remembered call so
that you call ContainerUtils.isLocalSavesOnly(context, libraryItem.appId)
directly (remove the remember wrapper around the isLocalSavesOnly assignment in
EpicAppScreen.kt) so the check runs on every recomposition and reflects current
settings.
app/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.kt
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/ui/screen/library/appscreen/SteamAppScreen.kt`:
- Around line 814-815: The call to ContainerUtils.isLocalSavesOnly(context,
appId) performs main-thread disk I/O on every recomposition; wrap this call in
Compose's remember keyed by appId (e.g., val isLocalSavesOnly = remember(appId)
{ ContainerUtils.isLocalSavesOnly(context, appId) }) so the filesystem scan via
ContainerManager/loadContainers only runs once per appId instead of on every
recomposition.
app/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.kt
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/src/main/res/values/strings.xml (1)
496-497: Add translations for the new local-saves toggle strings in other locales.
local_saves_onlyand its description are only defined in the base locale here; other locale files in this PR only add the warning strings. Consider adding localized versions (or mark as non‑translatable if intentional) so non‑English UIs don’t fall back to English for the toggle/description.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/res/values/strings.xml` around lines 496 - 497, The two new string resources local_saves_only and local_saves_only_description are only defined in the base locale so other locales fall back to English; update each translated strings.xml (the same locale files where you already added the warning strings) to include localized versions of local_saves_only and local_saves_only_description, or if these labels must remain identical across locales mark them as translatable="false" on the <string> entries; ensure you update the string names local_saves_only and local_saves_only_description consistently across all locale resource files.
🤖 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/res/values-it/strings.xml`:
- Around line 1014-1015: The string resource steam_client_sync_warning_message
contains an invalid escape (a backslash-based/unrecognized unicode escape)
causing the build to fail; open the string for steam_client_sync_warning_message
and remove the backslash escape(s) and replace any escaped apostrophe sequences
with a proper XML escape (e.g., use ' or ') or a plain apostrophe,
ensuring the string is valid XML so resources compile.
In `@app/src/main/res/values-pt-rBR/strings.xml`:
- Around line 885-886: Fix the Portuguese grammar in the string named
steam_client_sync_warning_message by correcting the subject-verb agreement:
update the message to use a singular setting label or plural verb form (e.g., "O
modo somente salvamentos locais está ativado, mas o cliente Steam está
configurado para iniciar..." or "Somente salvamentos locais estão ativados, mas
o cliente Steam está configurado para iniciar...") so the subject
("salvamentos") and verb agree; keep the rest of the sentence unchanged.
---
Nitpick comments:
In `@app/src/main/res/values/strings.xml`:
- Around line 496-497: The two new string resources local_saves_only and
local_saves_only_description are only defined in the base locale so other
locales fall back to English; update each translated strings.xml (the same
locale files where you already added the warning strings) to include localized
versions of local_saves_only and local_saves_only_description, or if these
labels must remain identical across locales mark them as translatable="false" on
the <string> entries; ensure you update the string names local_saves_only and
local_saves_only_description consistently across all locale resource files.
app/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/app/gamenative/ui/screen/library/LibraryAppScreen.kt
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
1 issue found across 9 files (changes from recent commits).
Prompt for AI agents (all 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/utils/ContainerUtils.kt">
<violation number="1" location="app/src/main/java/app/gamenative/utils/ContainerUtils.kt:285">
P2: Legacy localSavesOnly extras can keep the flag permanently true because the new OR fallback still reads the extra, while applyToContainer no longer clears/updates it. This makes disabling local-only saves impossible for containers created under the old scheme.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Comment on lines
375
to
378
| SteamService.closeApp(gameId, isOffline.value) { prefix -> | ||
| PathType.from(prefix).toAbsPath(context, gameId, SteamService.userSteamId!!.accountID) | ||
| }.await() | ||
| } |
Owner
There was a problem hiding this comment.
we should add a check for steam game source as well here, it'll do this for custom games for instance.
| return | ||
| } | ||
|
|
||
| if (ContainerUtils.extractGameSourceFromContainerId(appId) == GameSource.GOG) { |
Owner
There was a problem hiding this comment.
Let's put ContainerUtils.extractGameSourceFromContainerId(appId) in a variable so we don't have to extract it multiple times
Contributor
Author
There was a problem hiding this comment.
using gamesource now!
| return emptyList() | ||
| } | ||
|
|
||
| val isLocalSavesOnly = ContainerUtils.isLocalSavesOnly(context, appId) |
Owner
There was a problem hiding this comment.
Is this used? if not we should remove it
Contributor
Author
There was a problem hiding this comment.
oh nope, removed
| } | ||
|
|
||
| private suspend fun handleExitCloudSync(context: Context, appId: String, gameId: Int) { | ||
| if (ContainerUtils.isLocalSavesOnly(context, appId)) { |
Owner
There was a problem hiding this comment.
We should likely add || isOfflineMode here too
Owner
|
@xXJSONDeruloXx i've left some comments |
# Conflicts: # app/src/main/java/app/gamenative/ui/PluviaMain.kt
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.
Summary by cubic
Adds a per-game “Local saves only” setting to disable cloud save sync for Steam, GOG, and Epic. Shows a Steam client sync warning when launching the Steam client with local-only enabled.
New Features
Bug Fixes
Written for commit eac5d74. Summary will update on new commits.
Summary by CodeRabbit
New Features
UI
Behavior
Strings