Added container config export/import for all platforms#649
Merged
utkarshdalal merged 3 commits intoutkarshdalal:masterfrom Mar 10, 2026
Merged
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 reusable ContainerConfigTransfer export utility, centralized per-app export-request orchestration and CreateDocument export flow in BaseAppScreen, and moves SteamAppScreen to expose config menu options via getConfigMenuOptions (removing its prior export-state and launcher). Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant UI as BaseAppScreen UI
participant Picker as DocumentPicker (CreateDocument)
participant Transfer as ContainerConfigTransfer
participant Storage as SystemStorage
U->>UI: Select "Export Config" from app menu
UI->>UI: requestExportConfig(appId) (companion state)
UI->>UI: snapshotFlow detects request -> launch Picker (suggested filename)
Picker-->>UI: returns Uri or cancellation
alt Uri returned
UI->>Transfer: exportConfig(context, appId, uri)
Transfer->>Storage: open OutputStream at Uri, write JSON
Transfer-->>UI: return success/failure
UI->>UI: clearExportConfigRequest(appId)
UI-->>U: show Snackbar result
else Cancelled
UI->>UI: clearExportConfigRequest(appId)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
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 |
phobos665
reviewed
Feb 25, 2026
app/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.kt
Outdated
Show resolved
Hide resolved
![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 5 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/util/ContainerConfigTransfer.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/util/ContainerConfigTransfer.kt:27">
P2: `openOutputStream` can return null; the safe-call skips writing and still reports success, causing false-positive export completion.</violation>
</file>
<file name="app/src/main/java/app/gamenative/ui/screen/library/appscreen/GOGAppScreen.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/screen/library/appscreen/GOGAppScreen.kt:669">
P2: exportConfigRequested remains true until the ActivityResult returns, so a configuration change can re-run LaunchedEffect and launch a second file picker. Clear the request before launching to prevent duplicate pickers on activity recreation.</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/GOGAppScreen.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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/app/gamenative/ui/util/ContainerConfigTransfer.kt`:
- Around line 26-31: The write path currently ignores a null return from
context.contentResolver.openOutputStream(uri) and still reports success; update
the code in ContainerConfigTransfer (the suspend function using
withContext(Dispatchers.IO)) to treat openOutputStream(uri) == null as a
failure: check the result of openOutputStream before calling use (replace the
safe-call chain openOutputStream(uri)?.use { ... } with an explicit val out =
context.contentResolver.openOutputStream(uri) and if (out == null) return false
(or throw a handled exception), otherwise use out.use {
it.write(jsonText.toByteArray(Charsets.UTF_8)); it.flush() } so the function
only returns true when the stream was actually opened and written.
- Around line 39-59: The current catch ordering makes the IOException handler
unreachable and silently swallows coroutine cancellations; in the try/catch
around the export logic in ContainerConfigTransfer (the catch (e: Exception) and
catch (e: IOException) blocks), reorder and refine the handlers: first catch
CancellationException and rethrow it, then catch IOException to show the
IO-specific Toast, and finally catch Exception for other errors; ensure each
Toast uses the same string resource but with an appropriate fallback message
(e.g., "IO error" for IOException, "Unknown error" for Exception).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/src/main/java/app/gamenative/ui/screen/library/appscreen/CustomGameAppScreen.ktapp/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.ktapp/src/main/java/app/gamenative/ui/screen/library/appscreen/GOGAppScreen.ktapp/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.ktapp/src/main/java/app/gamenative/ui/util/ContainerConfigTransfer.kt
unbelievableflavour
force-pushed
the
config-export-for-all-platforms
branch
from
f408c7f to
5047203
Compare
phobos665
reviewed
Feb 25, 2026
Contributor
phobos665
left a comment
phobos665
left a comment
There was a problem hiding this comment.
LGTM.
Have you tested for all of the various ones?
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
app/src/main/java/app/gamenative/ui/util/ContainerConfigTransfer.kt (2)
39-59:⚠️ Potential issue | 🟠 MajorReorder catches and rethrow coroutine cancellation.
Line 39 catches
ExceptionbeforeIOException(Line 49), so the IO-specific branch is dead. This also swallows coroutine cancellation unlessCancellationExceptionis rethrown.🛠️ Proposed fix
import java.io.IOException +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext @@ - } catch (e: Exception) { + } catch (e: CancellationException) { + throw e + } catch (e: IOException) { Toast.makeText( context, context.getString( R.string.base_app_export_failed, - e.message ?: "Unknown error", + e.message ?: "IO error", ), Toast.LENGTH_SHORT, ).show() false - } catch (e: IOException) { + } catch (e: Exception) { Toast.makeText( context, context.getString( R.string.base_app_export_failed, - e.message ?: "IO error", + e.message ?: "Unknown error", ), Toast.LENGTH_SHORT, ).show() false }🤖 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/util/ContainerConfigTransfer.kt` around lines 39 - 59, In ContainerConfigTransfer.kt fix the catch ordering and preserve coroutine cancellation: move the catch (e: IOException) block so it comes before the broader catch (e: Exception), and add an explicit catch for kotlinx.coroutines.CancellationException that immediately rethrows (throw e) before other catches; ensure any generic Exception catch does not swallow CancellationException and that toast/error handling remains only in the more specific Exception/IOException handlers.
26-31:⚠️ Potential issue | 🟠 MajorHandle null
openOutputStreamas an export failure.At Line 27, a null stream path currently falls through to success (
true) even though no bytes were written.🛠️ Proposed fix
withContext(Dispatchers.IO) { - context.contentResolver.openOutputStream(uri)?.use { outputStream -> - outputStream.write(jsonText.toByteArray(Charsets.UTF_8)) - outputStream.flush() - } + val outputStream = context.contentResolver.openOutputStream(uri) + ?: throw IOException("Unable to open output stream for URI: $uri") + outputStream.use { + it.write(jsonText.toByteArray(Charsets.UTF_8)) + it.flush() + } }🤖 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/util/ContainerConfigTransfer.kt` around lines 26 - 31, The export currently assumes context.contentResolver.openOutputStream(uri) returns non-null; if it returns null the function still yields success. Update the withContext(Dispatchers.IO) block in ContainerConfigTransfer (the code using context.contentResolver.openOutputStream(uri), jsonText and withContext(Dispatchers.IO)) to check the result of openOutputStream: if it is null treat the export as failed (return false / propagate a failure) rather than proceeding, and only write/flush when a non-null OutputStream is returned; make sure to keep the existing use { ... } resource handling and return the correct boolean result when the stream is null.
🤖 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/BaseAppScreen.kt`:
- Around line 704-714: Replace the custom SupervisorJob usage in the export
config launcher so the coroutine inherits the existing job hierarchy: change the
uiScope.launch(SupervisorJob() + Dispatchers.Main) invocation to use
uiScope.launch { } or uiScope.launch(Dispatchers.Main) { } instead, keeping the
call to ContainerConfigTransfer.exportConfig(context, appId, uri) and the
finally block that calls clearExportConfigRequest(appId) unchanged; this ensures
the coroutine launched for exporting config follows the surrounding composition
scope rather than creating a new SupervisorJob.
---
Duplicate comments:
In `@app/src/main/java/app/gamenative/ui/util/ContainerConfigTransfer.kt`:
- Around line 39-59: In ContainerConfigTransfer.kt fix the catch ordering and
preserve coroutine cancellation: move the catch (e: IOException) block so it
comes before the broader catch (e: Exception), and add an explicit catch for
kotlinx.coroutines.CancellationException that immediately rethrows (throw e)
before other catches; ensure any generic Exception catch does not swallow
CancellationException and that toast/error handling remains only in the more
specific Exception/IOException handlers.
- Around line 26-31: The export currently assumes
context.contentResolver.openOutputStream(uri) returns non-null; if it returns
null the function still yields success. Update the withContext(Dispatchers.IO)
block in ContainerConfigTransfer (the code using
context.contentResolver.openOutputStream(uri), jsonText and
withContext(Dispatchers.IO)) to check the result of openOutputStream: if it is
null treat the export as failed (return false / propagate a failure) rather than
proceeding, and only write/flush when a non-null OutputStream is returned; make
sure to keep the existing use { ... } resource handling and return the correct
boolean result when the stream is null.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.ktapp/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.ktapp/src/main/java/app/gamenative/ui/util/ContainerConfigTransfer.kt
app/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.kt
Outdated
Show resolved
Hide resolved
Contributor
Author
Have tested for Steam, Epic, GOG. Pretty sure it just works for all though. Doesn't do anything platform specific. Just translated the container config to json and writes it to file. Importing is more complex, but that's not in this PR so it's fine. |
unbelievableflavour
force-pushed
the
config-export-for-all-platforms
branch
from
5047203 to
71bf12b
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
app/src/main/java/app/gamenative/ui/util/ContainerConfigTransfer.kt (1)
27-32:⚠️ Potential issue | 🟠 Major
openOutputStreamreturningnullis still silently treated as successThe
?.use { ... }safe-call at line 28 skips the write whenopenOutputStreamreturnsnull, but execution continues to the successToastandreturn true. The fix from the previous review cycle was not applied.🛠️ Proposed fix
withContext(Dispatchers.IO) { - context.contentResolver.openOutputStream(uri)?.use { outputStream -> - outputStream.write(jsonText.toByteArray(Charsets.UTF_8)) - outputStream.flush() - } + context.contentResolver.openOutputStream(uri)?.use { outputStream -> + outputStream.write(jsonText.toByteArray(Charsets.UTF_8)) + outputStream.flush() + } ?: throw IOException("Unable to open output stream for URI: $uri") }
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.ktapp/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.ktapp/src/main/java/app/gamenative/ui/util/ContainerConfigTransfer.kt
utkarshdalal
reviewed
Mar 9, 2026
Comment on lines
-982
to
-993
| AppMenuOption( | ||
| AppOptionMenuType.ImportConfig, | ||
| onClick = { | ||
| requestImportConfig(gameId) | ||
| } | ||
| ), | ||
| AppMenuOption( | ||
| AppOptionMenuType.ExportConfig, | ||
| onClick = { | ||
| requestExportConfig(gameId) | ||
| } | ||
| ) |
Owner
There was a problem hiding this comment.
didn't we just lose the import config functionality here?
Also if we're working on it, let's add import config to all the stores too, should be trivial.
Contributor
Author
There was a problem hiding this comment.
No it's just moved to BaseAppScreen + Added import feature as well now.
utkarshdalal
reviewed
Mar 9, 2026
Comment on lines
+34
to
+59
| Toast.makeText( | ||
| context, | ||
| context.getString(R.string.base_app_exported), | ||
| Toast.LENGTH_SHORT, | ||
| ).show() | ||
| true | ||
| } catch (e: CancellationException) { | ||
| throw e | ||
| } catch (e: IOException) { | ||
| Toast.makeText( | ||
| context, | ||
| context.getString( | ||
| R.string.base_app_export_failed, | ||
| e.message ?: "IO error", | ||
| ), | ||
| Toast.LENGTH_SHORT, | ||
| ).show() | ||
| false | ||
| } catch (e: Exception) { | ||
| Toast.makeText( | ||
| context, | ||
| context.getString( | ||
| R.string.base_app_export_failed, | ||
| e.message ?: "Unknown error", | ||
| ), | ||
| Toast.LENGTH_SHORT, |
Owner
There was a problem hiding this comment.
will need to change this to snackbar to align with new style
Contributor
Author
There was a problem hiding this comment.
Changed it.
unbelievableflavour
force-pushed
the
config-export-for-all-platforms
branch
from
71bf12b to
8ad0a8a
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
app/src/main/java/app/gamenative/ui/util/ContainerConfigTransfer.kt (1)
27-32:⚠️ Potential issue | 🟠 MajorHandle
openOutputStreamreturning null as a failureAt Line 28,
openOutputStream(uri)may returnnull. The current code still returnstrueand shows a success Snackbar even if no file was written.🛠️ Proposed fix
withContext(Dispatchers.IO) { - context.contentResolver.openOutputStream(uri)?.use { outputStream -> + val outputStream = context.contentResolver.openOutputStream(uri) + ?: throw IOException("Unable to open output stream for URI: $uri") + outputStream.use { outputStream.write(jsonText.toByteArray(Charsets.UTF_8)) outputStream.flush() } },
🤖 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/util/ContainerConfigTransfer.kt` around lines 27 - 32, The code calls context.contentResolver.openOutputStream(uri) but doesn't handle the case it returns null, causing the function to report success even if no file was written; modify the write block in ContainerConfigTransfer (the coroutine using withContext(Dispatchers.IO) and jsonText) to capture the result of openOutputStream(uri) into a variable, check for null before calling use/write, and if null return/propagate a failure (and avoid showing the success Snackbar), otherwise proceed to write/flush and return success; ensure any logging or Snackbar for success is only executed after a successful non-null write.
🤖 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/BaseAppScreen.kt`:
- Around line 42-48: The import for Timber in BaseAppScreen.kt is unused; remove
the explicit import line for Timber (symbol: Timber) from the imports at the top
of BaseAppScreen.kt and run an auto-import/organize-imports or rebuild to ensure
no other references rely on it (or replace with a used logger if intended).
In `@app/src/main/java/app/gamenative/ui/util/ContainerConfigTransfer.kt`:
- Around line 21-25: The exportConfig code uses
ContainerUtils.getOrCreateContainer which silently creates and persists a new
container via createNewContainer; change export flow to first call
ContainerUtils.getContainer (or equivalent existence-check) for the given appId,
and if it returns null return/fail with a clear error or user-facing message
instead of calling getOrCreateContainer; only use getOrCreateContainer when an
explicit create is intended. Update the logic in
ContainerConfigTransfer.exportConfig (or the block using getOrCreateContainer)
to avoid the side-effect by checking existence and handling null before
attempting to read container.containerJson.
---
Duplicate comments:
In `@app/src/main/java/app/gamenative/ui/util/ContainerConfigTransfer.kt`:
- Around line 27-32: The code calls
context.contentResolver.openOutputStream(uri) but doesn't handle the case it
returns null, causing the function to report success even if no file was
written; modify the write block in ContainerConfigTransfer (the coroutine using
withContext(Dispatchers.IO) and jsonText) to capture the result of
openOutputStream(uri) into a variable, check for null before calling use/write,
and if null return/propagate a failure (and avoid showing the success Snackbar),
otherwise proceed to write/flush and return success; ensure any logging or
Snackbar for success is only executed after a successful non-null write.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ad54fcbc-9b4e-4226-92d0-284d90d5a0fa
📒 Files selected for processing (3)
app/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.ktapp/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.ktapp/src/main/java/app/gamenative/ui/util/ContainerConfigTransfer.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.kt
app/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.kt
Outdated
Show resolved
Hide resolved
unbelievableflavour
force-pushed
the
config-export-for-all-platforms
branch
from
8ad0a8a to
8038d77
Compare
unbelievableflavour
changed the title
Contributor
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
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/ui/util/ContainerConfigTransfer.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/util/ContainerConfigTransfer.kt:92">
P2: Manifest component installations happen before config validity is confirmed, so malformed or incompatible imports can trigger downloads/installs even though the import later fails.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
unbelievableflavour
force-pushed
the
config-export-for-all-platforms
branch
from
7c30cf0 to
0b234d9
Compare
unbelievableflavour
force-pushed
the
config-export-for-all-platforms
branch
from
5f68f5b to
e5916f3
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
3 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.
No config import yet, export is already a useful feature to have even without import.
Summary by CodeRabbit
New Features
Refactor