Speed up external - #1779
Merged
Merged
Conversation
added 6 commits
July 25, 2026 21:34
Contributor
📝 WalkthroughWalkthroughThe change adds public install-root mapping and legacy-directory migration, persists and discovers those paths across storage settings and downloads, updates provider records and container A: drive resolution, adds storage-aware FFP launch variables, and replaces the redirect payload while bumping the image filesystem version. ChangesStorage and runtime migration
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/app/gamenative/utils/ContainerUtils.kt (1)
1046-1063: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winApply drive migration to temporary-override launches too.
This block only runs through
getOrCreateContainer.getOrCreateContainerWithOverridebypasses it for both existing containers and its directcreateNewContainerpath, leaving those launches mapped to the legacy/Android/data/...directory. Extract this reconciliation into a shared helper and invoke it from both public container-acquisition flows.🤖 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/utils/ContainerUtils.kt` around lines 1046 - 1063, Extract the A: drive reconciliation currently in getOrCreateContainer into a shared helper that migrates the resolved game folder and updates container.drives. Invoke this helper from both getOrCreateContainer and getOrCreateContainerWithOverride, including existing-container and direct createNewContainer paths, so temporary-override launches also use the migrated directory.
🤖 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/ui/screen/settings/SettingsGroupInterface.kt`:
- Around line 570-575: Update the selectedIndex state in the rememberSaveable
block to recompute when dirs changes, ensuring the saved externalStoragePath is
matched against the newly discovered directories rather than retaining the
initial empty-list index. Preserve the existing fallback to index 0 and
selection-matching logic.
---
Outside diff comments:
In `@app/src/main/java/app/gamenative/utils/ContainerUtils.kt`:
- Around line 1046-1063: Extract the A: drive reconciliation currently in
getOrCreateContainer into a shared helper that migrates the resolved game folder
and updates container.drives. Invoke this helper from both getOrCreateContainer
and getOrCreateContainerWithOverride, including existing-container and direct
createNewContainer paths, so temporary-override launches also use the migrated
directory.
🪄 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 Plus
Run ID: ceb6395f-7f39-4993-8e9c-b0935407da2f
⛔ Files ignored due to path filters (1)
app/src/modern/assets/libredirect-bionic-wx.sois excluded by!**/*.so
📒 Files selected for processing (7)
app/src/main/assets/redirect.tzstapp/src/main/java/app/gamenative/service/DownloadService.ktapp/src/main/java/app/gamenative/ui/screen/settings/SettingsGroupInterface.ktapp/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.ktapp/src/main/java/app/gamenative/utils/ContainerUtils.ktapp/src/main/java/app/gamenative/utils/StorageUtils.ktapp/src/main/java/com/winlator/xenvironment/ImageFsInstaller.java
![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 8 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/DownloadService.kt">
<violation number="1" location="app/src/main/java/app/gamenative/service/DownloadService.kt:60">
P1: Existing external GOG, Epic, and Amazon installs become unreachable after startup migration: only the preference moves, while game directories stay under `Android/data/.../files`. Migrate those directories (or retain legacy roots for each service) before changing the persisted root.</violation>
</file>
<file name="app/src/main/java/app/gamenative/utils/StorageUtils.kt">
<violation number="1" location="app/src/main/java/app/gamenative/utils/StorageUtils.kt:116">
P1: Modern builds cannot create the proposed `<volume>/GameNative` root under scoped storage, so `ensureInstallRoot` falls back to the slow `Android/data/.../files` location and this optimization is ineffective. Use a user-granted SAF tree/MediaStore-compatible location, or retain an app-specific directory for these builds.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| val public = StorageUtils.publicInstallRoot(File(pref)) ?: return | ||
| if (StorageUtils.ensureInstallRoot(public)) { | ||
| Timber.i("Migrating external install root from $pref to ${public.absolutePath}") | ||
| PrefManager.externalStoragePath = public.absolutePath |
Contributor
There was a problem hiding this comment.
P1: Existing external GOG, Epic, and Amazon installs become unreachable after startup migration: only the preference moves, while game directories stay under Android/data/.../files. Migrate those directories (or retain legacy roots for each service) before changing the persisted root.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/app/gamenative/service/DownloadService.kt, line 60:
<comment>Existing external GOG, Epic, and Amazon installs become unreachable after startup migration: only the preference moves, while game directories stay under `Android/data/.../files`. Migrate those directories (or retain legacy roots for each service) before changing the persisted root.</comment>
<file context>
@@ -37,10 +38,27 @@ object DownloadService {
+ val public = StorageUtils.publicInstallRoot(File(pref)) ?: return
+ if (StorageUtils.ensureInstallRoot(public)) {
+ Timber.i("Migrating external install root from $pref to ${public.absolutePath}")
+ PrefManager.externalStoragePath = public.absolutePath
+ }
}
</file context>
| } | ||
|
|
||
| fun ensureInstallRoot(dir: File): Boolean { | ||
| if (!dir.isDirectory && !dir.mkdirs()) return false |
Contributor
There was a problem hiding this comment.
P1: Modern builds cannot create the proposed <volume>/GameNative root under scoped storage, so ensureInstallRoot falls back to the slow Android/data/.../files location and this optimization is ineffective. Use a user-granted SAF tree/MediaStore-compatible location, or retain an app-specific directory for these builds.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/app/gamenative/utils/StorageUtils.kt, line 116:
<comment>Modern builds cannot create the proposed `<volume>/GameNative` root under scoped storage, so `ensureInstallRoot` falls back to the slow `Android/data/.../files` location and this optimization is ineffective. Use a user-granted SAF tree/MediaStore-compatible location, or retain an app-specific directory for these builds.</comment>
<file context>
@@ -98,6 +98,55 @@ object StorageUtils {
+ }
+
+ fun ensureInstallRoot(dir: File): Boolean {
+ if (!dir.isDirectory && !dir.mkdirs()) return false
+ runCatching { File(dir, ".nomedia").createNewFile() }
+ return true
</file context>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/app/gamenative/ui/screen/settings/SettingsGroupInterface.kt (1)
563-565: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winMove public-root preparation off the UI thread.
StorageUtils.preferredInstallRoot(...)may create the install root. Calling it synchronously from the toggle and dropdown callbacks can block the main thread on slow external storage and risk UI freezes/ANRs. Resolve the path in a serializedDispatchers.IOjob, then persist it after completion so older selections cannot overwrite newer ones.Also applies to: 582-585
🤖 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/settings/SettingsGroupInterface.kt` around lines 563 - 565, Update the toggle and dropdown callbacks around StorageUtils.preferredInstallRoot to resolve the install root in a serialized Dispatchers.IO job rather than on the UI thread. After the job completes, persist the result to PrefManager.externalStoragePath, ensuring stale asynchronous selections cannot overwrite the latest selection.
🤖 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/utils/ContainerUtils.kt`:
- Around line 1046-1063: Make legacy-directory migration retryable or atomic by
ensuring provider metadata is durably persisted before completing the remap in
ContainerUtils around resolvedGameFolderPath and the GameSource update branches.
Update AmazonService, EpicService, and GOGService path-update methods to persist
through a guaranteed mechanism or return an explicit failure instead of silently
no-oping when the service instance is unavailable; propagate that failure so
migration is not finalized until the provider path update succeeds. Affected
sites: app/src/main/java/app/gamenative/utils/ContainerUtils.kt:1046-1063
requires migration gating;
app/src/main/java/app/gamenative/service/amazon/AmazonService.kt:319-327,
app/src/main/java/app/gamenative/service/epic/EpicService.kt:369-376, and
app/src/main/java/app/gamenative/service/gog/GOGService.kt:304-310 require
guaranteed persistence or explicit failure.
---
Outside diff comments:
In
`@app/src/main/java/app/gamenative/ui/screen/settings/SettingsGroupInterface.kt`:
- Around line 563-565: Update the toggle and dropdown callbacks around
StorageUtils.preferredInstallRoot to resolve the install root in a serialized
Dispatchers.IO job rather than on the UI thread. After the job completes,
persist the result to PrefManager.externalStoragePath, ensuring stale
asynchronous selections cannot overwrite the latest selection.
🪄 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 Plus
Run ID: 198b9017-311b-4a9a-9c37-4f6ae68dabb0
📒 Files selected for processing (6)
app/src/main/java/app/gamenative/service/amazon/AmazonService.ktapp/src/main/java/app/gamenative/service/epic/EpicService.ktapp/src/main/java/app/gamenative/service/gog/GOGService.ktapp/src/main/java/app/gamenative/ui/screen/settings/SettingsGroupInterface.ktapp/src/main/java/app/gamenative/utils/ContainerUtils.ktapp/src/main/java/app/gamenative/utils/StorageUtils.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/app/gamenative/utils/StorageUtils.kt
Comment on lines
+1046
to
+1063
| val resolvedGameFolderPath = if (gameSource == GameSource.CUSTOM_GAME) { | ||
| gameFolderPath | ||
| } else { | ||
| StorageUtils.resolveLegacyGameDir(gameFolderPath) | ||
| } | ||
|
|
||
| if (resolvedGameFolderPath != null && resolvedGameFolderPath != gameFolderPath) { | ||
| when (gameSource) { | ||
| GameSource.GOG -> | ||
| GOGService.updateInstallPath(extractGameIdFromContainerId(appId).toString(), resolvedGameFolderPath) | ||
| GameSource.EPIC -> | ||
| EpicService.updateInstallPath(extractGameIdFromContainerId(appId), resolvedGameFolderPath) | ||
| GameSource.AMAZON -> | ||
| runCatching { extractGameIdFromContainerId(appId) }.getOrNull() | ||
| ?.let { AmazonService.updateInstallPath(it, resolvedGameFolderPath) } | ||
| else -> {} | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Make legacy-directory migration and provider metadata updates atomic or retryable.
The directory can be moved successfully while the provider update silently no-ops because its service is not running, leaving the container path and database path inconsistent.
app/src/main/java/app/gamenative/utils/ContainerUtils.kt#L1046-L1063: do not remap A: or complete migration until the provider path is durably updated.app/src/main/java/app/gamenative/service/amazon/AmazonService.kt#L319-L327: replace the nullable service-instance update with guaranteed persistence or an explicit failure.app/src/main/java/app/gamenative/service/epic/EpicService.kt#L369-L376: replace the nullable service-instance update with guaranteed persistence or an explicit failure.app/src/main/java/app/gamenative/service/gog/GOGService.kt#L304-L310: replace the nullable service-instance update with guaranteed persistence or an explicit failure.
📍 Affects 4 files
app/src/main/java/app/gamenative/utils/ContainerUtils.kt#L1046-L1063(this comment)app/src/main/java/app/gamenative/service/amazon/AmazonService.kt#L319-L327app/src/main/java/app/gamenative/service/epic/EpicService.kt#L369-L376app/src/main/java/app/gamenative/service/gog/GOGService.kt#L304-L310
🤖 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/utils/ContainerUtils.kt` around lines 1046 -
1063, Make legacy-directory migration retryable or atomic by ensuring provider
metadata is durably persisted before completing the remap in ContainerUtils
around resolvedGameFolderPath and the GameSource update branches. Update
AmazonService, EpicService, and GOGService path-update methods to persist
through a guaranteed mechanism or return an explicit failure instead of silently
no-oping when the service instance is unavailable; propagate that failure so
migration is not finalized until the provider path update succeeds. Affected
sites: app/src/main/java/app/gamenative/utils/ContainerUtils.kt:1046-1063
requires migration gating;
app/src/main/java/app/gamenative/service/amazon/AmazonService.kt:319-327,
app/src/main/java/app/gamenative/service/epic/EpicService.kt:369-376, and
app/src/main/java/app/gamenative/service/gog/GOGService.kt:304-310 require
guaranteed persistence or explicit failure.
Contributor
There was a problem hiding this comment.
2 issues found across 6 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/service/epic/EpicService.kt">
<violation number="1" location="app/src/main/java/app/gamenative/service/epic/EpicService.kt:369">
P2: Double getInstance() call creates a silent-failure window if the service is destroyed between the two calls. Capture the instance once at the top of the runBlocking block and reuse it, same as the AmazonService.updateInstallPath pattern.</violation>
</file>
<file name="app/src/main/java/app/gamenative/service/amazon/AmazonService.kt">
<violation number="1" location="app/src/main/java/app/gamenative/service/amazon/AmazonService.kt:324">
P1: `updateInstallPath` accesses the mutable `instance` field twice via safe-call — the second read can silently miss the `markInstalled` call if the service is destroyed between reads. Capture `instance` into a local val (as `isUpdatePending` does) so the DB update is consistent with the on-disk migration.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| runBlocking(Dispatchers.IO) { | ||
| val game = instance?.amazonManager?.getGameByAppId(appId) ?: return@runBlocking | ||
| if (game.isInstalled && game.installPath != path) { | ||
| instance?.amazonManager?.markInstalled(game.productId, path, game.installSize, game.versionId) |
Contributor
There was a problem hiding this comment.
P1: updateInstallPath accesses the mutable instance field twice via safe-call — the second read can silently miss the markInstalled call if the service is destroyed between reads. Capture instance into a local val (as isUpdatePending does) so the DB update is consistent with the on-disk migration.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/app/gamenative/service/amazon/AmazonService.kt, line 324:
<comment>`updateInstallPath` accesses the mutable `instance` field twice via safe-call — the second read can silently miss the `markInstalled` call if the service is destroyed between reads. Capture `instance` into a local val (as `isUpdatePending` does) so the DB update is consistent with the on-disk migration.</comment>
<file context>
@@ -316,6 +316,16 @@ class AmazonService : Service() {
+ runBlocking(Dispatchers.IO) {
+ val game = instance?.amazonManager?.getGameByAppId(appId) ?: return@runBlocking
+ if (game.isInstalled && game.installPath != path) {
+ instance?.amazonManager?.markInstalled(game.productId, path, game.installSize, game.versionId)
+ }
+ }
</file context>
Comment on lines
+369
to
+375
| fun updateInstallPath(appId: Int, path: String) { | ||
| runBlocking(Dispatchers.IO) { | ||
| val game = getInstance()?.epicManager?.getGameById(appId) ?: return@runBlocking | ||
| if (game.installPath != path) { | ||
| getInstance()?.epicManager?.updateGame(game.copy(installPath = path)) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
P2: Double getInstance() call creates a silent-failure window if the service is destroyed between the two calls. Capture the instance once at the top of the runBlocking block and reuse it, same as the AmazonService.updateInstallPath pattern.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/app/gamenative/service/epic/EpicService.kt, line 369:
<comment>Double getInstance() call creates a silent-failure window if the service is destroyed between the two calls. Capture the instance once at the top of the runBlocking block and reuse it, same as the AmazonService.updateInstallPath pattern.</comment>
<file context>
@@ -366,6 +366,15 @@ class EpicService : Service() {
}
}
+ fun updateInstallPath(appId: Int, path: String) {
+ runBlocking(Dispatchers.IO) {
+ val game = getInstance()?.epicManager?.getGameById(appId) ?: return@runBlocking
</file context>
Suggested change
| fun updateInstallPath(appId: Int, path: String) { | |
| runBlocking(Dispatchers.IO) { | |
| val game = getInstance()?.epicManager?.getGameById(appId) ?: return@runBlocking | |
| if (game.installPath != path) { | |
| getInstance()?.epicManager?.updateGame(game.copy(installPath = path)) | |
| } | |
| } | |
| fun updateInstallPath(appId: Int, path: String) { | |
| runBlocking(Dispatchers.IO) { | |
| val instance = getInstance() ?: return@runBlocking | |
| val game = instance.epicManager.getGameById(appId) ?: return@runBlocking | |
| if (game.installPath != path) { | |
| instance.epicManager.updateGame(game.copy(installPath = path)) | |
| } | |
| } | |
| } |
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
1 participant
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
External storage was slow to load games. Took brotato from 5 mins+ to 20 seconds.
Recording
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
Move external installs from Android/data to a public volume root to bypass MediaProvider overhead, cutting external boot times from 5+ minutes to ~20 seconds. Applies to all stores, and now persists new install paths for GOG, Epic, and Amazon after migration.
Refactors
Migration
Written for commit 74b7a7b. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
Improvements