Fix installs on adopted primary storage - #1821
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughExternal volume discovery now uses shared eligibility checks. The checks include non-primary volumes and qualifying adopted or removable primary storage. UUID resolution varies by Android API level, with legacy fallback handling and unit test coverage. ChangesExternal install target detection
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The change centralizes adopted-primary storage eligibility and keeps settings selection aligned with install discovery; no actionable merge-blocking risk remains beyond normal checks and review. Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
There was a problem hiding this comment.
Pull request overview
This PR fixes external-install target discovery on Android devices where primary shared storage has been migrated to adopted (SD-backed) storage. It centralizes the “is this a valid install target?” predicate in StorageUtils and uses storage UUID signals to allow adopted-primary volumes while continuing to hide built-in primary storage.
Changes:
- Added
StorageUtils.isExternalInstallTarget(...)andisNonDefaultPrimaryStorage(...)to consistently identify adopted-primary vs built-in primary storage. - Updated Settings volume selector and
DownloadServiceinit-time volume discovery to use the centralized predicate. - Added unit regression coverage for default primary, adopted primary, physical primary fallback, and legacy UUID fallback behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| app/src/main/java/app/gamenative/utils/StorageUtils.kt | Adds centralized install-target eligibility logic based on StorageVolume UUID/flags with version-aware fallbacks. |
| app/src/main/java/app/gamenative/ui/screen/settings/SettingsGroupInterface.kt | Switches Settings volume filtering to use StorageUtils.isExternalInstallTarget(...) so adopted-primary can be shown. |
| app/src/main/java/app/gamenative/service/DownloadService.kt | Aligns background install discovery / path enumeration with the same install-target predicate as Settings. |
| app/src/test/java/app/gamenative/utils/StorageUtilsTest.kt | Adds unit tests for adopted-primary detection and relevant fallback/precedence rules. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| fun isExternalInstallTarget(storageManager: StorageManager?, appFilesDir: File): Boolean { | ||
| val volume = storageManager?.getStorageVolume(appFilesDir) | ||
| ?: return runCatching { Environment.isExternalStorageRemovable(appFilesDir) }.getOrDefault(false) | ||
| if (!volume.isPrimary) return true | ||
|
|
||
| val resolvedStorageUuid = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) { | ||
| volume.storageUuid | ||
| } else { | ||
| try { | ||
| storageManager.getUuidForPath(appFilesDir) | ||
| } catch (_: Exception) { | ||
| null | ||
| } | ||
| } |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
|
Thanks for making this. I don't think this is enough to make games actually run? Have you tested? Can you please provide a recording? |
|
@utkarshdalal Would it be possible to provide an officially signed test APK containing the changes from PR #1821? I've successfully confirmed that the test build detects adopted storage, downloads games to it, and launches executables from the SD-backed path. However, my separately installed beta uses a different package ID. Some native GameNative components contain hardcoded references to Ideally, the test APK would:
This would let me test the complete flow and provide a recording. |
|
@danielbuva - it's building here: https://github.com/utkarshdalal/GameNative/actions/runs/32151437027 |
|
@utkarshdalal The build succeeded, but the hosted runner lost communication during universal APK extraction, likely due to memory or disk pressure. Could you rerun it or build just the single variant needed for testing? |
|
Damn it, I re-ran it now. |
|
I am interested in testing this PR as I have a RP6 with adopted storage. There still doesn't seem to be a signed APK available to test though, either via the linked action from @utkarshdalal or checking for more recently run actions. It seems like it still failed on the re-run. If I'm wrong and am just missing a completed build somewhere I'll gladly download it and give it a try. |
|
https://github.com/utkarshdalal/GameNative/actions/runs/33328287574 @DeviPotato @danielbuva - i fixed the ad hoc action and rebuilt. please try |
Description
Android can migrate primary shared storage onto adopted SD media. GameNative currently rejects every primary
StorageVolumein both Settings andDownloadService, so adopted-primary storage is hidden even though it is the device's large writable backing volume.This change:
StorageUtilsStorageVolume.storageUuidon Android 12+ andStorageManager.getUuidForPathon Android 8-11 to distinguish adopted backing storage fromUUID_DEFAULTRelated discussion: #1718
Related prior work: #1585 (volume enumeration), #809 (toggle-off discovery), and #1768 (modern external storage)
Android background: https://source.android.com/docs/core/storage/adoptable
Recording
Not available yet because this requires a physical device with adopted storage migrated to primary. Please run the Ad-hoc Signed Build workflow for this PR so affected users can install it over their existing GameNative app and validate the fix without clearing app data or redownloading their libraries.
Type of Change
Testing
./gradlew :app:testLegacyDebugUnitTest --tests app.gamenative.utils.StorageUtilsTest./gradlew :app:testModernDebugUnitTest --tests app.gamenative.utils.StorageUtilsTestgit diff --checkReal-device validation is still required before merge.
Scope and non-goals
This supports adopted media when Android exposes it as the backing volume for primary shared storage. It does not synthesize private paths such as
/mnt/expand/<UUID>, request broader storage permissions, or claim support for an adopted private volume that Android does not return through the app's external-files APIs.Device test plan
Test target: any GameNative-supported Android device where a microSD card has been formatted as adopted/internal storage and migrated to primary shared storage.
Regression controls: a built-in-only primary volume must remain excluded, while existing portable SD and USB targets must remain available.
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
Fixes install discovery on devices where primary shared storage is migrated to adopted SD. Previously we hid all primary volumes; now we expose adopted-primary as an install target while keeping built-in primary hidden.
StorageUtils.isExternalInstallTarget, usingStorageVolume.storageUuidon Android 12+ andStorageManager.getUuidForPathon Android 8–11, with fallbacks for physical primary and conservative behavior when UUIDs are unavailable.DownloadServiceand the settings volume selector, so selection and background discovery match and existing installs remain discoverable.StorageUtilsTestfor built-in primary, adopted primary, physical primary, legacy fallback, and typed-UUID precedence.Written for commit a94949d. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Tests