Skip to content

Added container config export for all platforms#649

Open
unbelievableflavour wants to merge 1 commit intoutkarshdalal:masterfrom
unbelievableflavour:config-export-for-all-platforms
Open

Added container config export for all platforms#649
unbelievableflavour wants to merge 1 commit intoutkarshdalal:masterfrom
unbelievableflavour:config-export-for-all-platforms

Conversation

@unbelievableflavour
Copy link
Contributor

@unbelievableflavour unbelievableflavour commented Feb 25, 2026

No config import yet, export is already a useful feature to have even without import.

Summary by CodeRabbit

  • New Features

    • Configuration import and export options are now available from the app's config menu for easier management; import remains fully supported and export is consolidated under the unified menu.
  • Refactor

    • Config actions consolidated into a generic menu-driven flow and underlying export workflow streamlined for a smoother, more consistent user experience.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 25, 2026

📝 Walkthrough

Walkthrough

Refactors SteamAppScreen to delegate config actions to BaseAppScreen, adds a ContainerConfigTransfer utility for exporting container JSON, and implements companion-level export request tracking plus UI wiring (document picker launch) in BaseAppScreen.

Changes

Cohort / File(s) Summary
Export Utility
app/src/main/java/app/gamenative/ui/util/ContainerConfigTransfer.kt
New singleton ContainerConfigTransfer with suspend fun exportConfig(context, appId, uri) that obtains container JSON, writes pretty-printed JSON to the given Uri on Dispatchers.IO, and shows success/failure Toasts.
Base App Screen (export flow & wiring)
app/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.kt
Adds companion-state exportConfigRequests, methods requestExportConfig, clearExportConfigRequest, shouldExportConfig; adds getExportConfigOption and getConfigMenuOptions extension points; integrates SnapshotFlow and a CreateDocument launcher to orchestrate JSON export via ContainerConfigTransfer.
Steam App Screen (API surface change)
app/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.kt
Removes prior export-request state and LaunchedEffect export launcher; adds override fun getConfigMenuOptions(context, libraryItem) to supply Import/Export options via BaseAppScreen hook.

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" menu option
    UI->>UI: requestExportConfig(appId) (companion-state)
    UI->>UI: snapshotFlow detects request -> launch Picker with filename
    Picker-->>UI: returns Uri (or cancels)
    alt Uri returned
        UI->>Transfer: exportConfig(context, appId, uri)
        Transfer->>Storage: open OutputStream at Uri, write JSON
        Transfer-->>UI: return success/failure (Boolean)
        UI->>UI: clearExportConfigRequest(appId)
        UI-->>U: show toast via Transfer result
    else Cancelled
        UI->>UI: clearExportConfigRequest(appId)
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • PR #299: Modifies menu-option provider APIs in BaseAppScreen/SteamAppScreen; overlaps with added getConfigMenuOptions hook.
  • PR #272: Adjusts export/config lifecycle and companion state in BaseAppScreen/SteamAppScreen; directly intersects this refactor.
  • PR #334: Changes ContainerUtils usage/behavior that ContainerConfigTransfer.exportConfig relies on.

Poem

🐰 I hopped through code with glee and cheer,

Saved JSON bits both far and near,
BaseScreen hums, Steam steps aside,
Pick a file, and let it glide,
A little rabbit exports with pride.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 26.83% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Added container config export for all platforms' directly corresponds to the main change: implementing container configuration export functionality across all platforms.

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

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 761d93b and f408c7f.

📒 Files selected for processing (5)
  • app/src/main/java/app/gamenative/ui/screen/library/appscreen/CustomGameAppScreen.kt
  • app/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.kt
  • app/src/main/java/app/gamenative/ui/screen/library/appscreen/GOGAppScreen.kt
  • app/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.kt
  • app/src/main/java/app/gamenative/ui/util/ContainerConfigTransfer.kt

@unbelievableflavour unbelievableflavour force-pushed the config-export-for-all-platforms branch from f408c7f to 5047203 Compare February 25, 2026 11:41
Copy link
Contributor

@phobos665 phobos665 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Have you tested for all of the various ones?

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (2)
app/src/main/java/app/gamenative/ui/util/ContainerConfigTransfer.kt (2)

39-59: ⚠️ Potential issue | 🟠 Major

Reorder catches and rethrow coroutine cancellation.

Line 39 catches Exception before IOException (Line 49), so the IO-specific branch is dead. This also swallows coroutine cancellation unless CancellationException is 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 | 🟠 Major

Handle null openOutputStream as 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

📥 Commits

Reviewing files that changed from the base of the PR and between f408c7f and 5047203.

📒 Files selected for processing (3)
  • app/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.kt
  • app/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.kt
  • app/src/main/java/app/gamenative/ui/util/ContainerConfigTransfer.kt

@unbelievableflavour
Copy link
Contributor Author

unbelievableflavour commented Feb 25, 2026

LGTM.

Have you tested for all of the various ones?

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 unbelievableflavour force-pushed the config-export-for-all-platforms branch from 5047203 to 71bf12b Compare February 25, 2026 13:15
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
app/src/main/java/app/gamenative/ui/util/ContainerConfigTransfer.kt (1)

27-32: ⚠️ Potential issue | 🟠 Major

openOutputStream returning null is still silently treated as success

The ?.use { ... } safe-call at line 28 skips the write when openOutputStream returns null, but execution continues to the success Toast and return 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

📥 Commits

Reviewing files that changed from the base of the PR and between 5047203 and 71bf12b.

📒 Files selected for processing (3)
  • app/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.kt
  • app/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.kt
  • app/src/main/java/app/gamenative/ui/util/ContainerConfigTransfer.kt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants