feat: container storage manager#816
Closed
xXJSONDeruloXx wants to merge 4 commits intoutkarshdalal:masterfrom
Closed
feat: container storage manager#816xXJSONDeruloXx wants to merge 4 commits intoutkarshdalal:masterfrom
xXJSONDeruloXx wants to merge 4 commits intoutkarshdalal:masterfrom
Conversation
Contributor
📝 WalkthroughWalkthroughAdds a new container storage management feature: UI dialog, backend ContainerStorageManager (scan, sizes, move, remove, uninstall), Settings integration, string resources, DAO helpers, StorageUtils move refactor, and unit tests for container utilities. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant SettingsUI as "Settings UI"
participant Dialog as "ContainerStorageManagerDialog"
participant Manager as "ContainerStorageManager"
participant FS as "File System"
participant Sources as "Game Sources"
User->>SettingsUI: Tap "Manage Containers"
SettingsUI->>Dialog: show(visible=true)
Dialog->>Manager: loadEntries(context)
Manager->>FS: list container directories
FS-->>Manager: container dirs
Manager->>Sources: resolve game metadata & install paths
Sources-->>Manager: game info
Manager->>FS: compute directory sizes
FS-->>Manager: size bytes
Manager-->>Dialog: List<Entry>
Dialog->>User: render entries
User->>Dialog: Tap "Remove"/"Uninstall"/"Move"
Dialog->>User: show confirmation
User->>Dialog: confirm
Dialog->>Manager: removeContainer / uninstallGameAndContainer / startMove
Manager->>Sources: source-specific uninstall or update
Manager->>FS: delete/move files (progress events)
FS-->>Manager: success/failure
Manager-->>Dialog: result
Dialog->>Manager: loadEntries(context) [reload]
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 5 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/utils/ContainerStorageManager.kt">
<violation number="1" location="app/src/main/java/app/gamenative/utils/ContainerStorageManager.kt:132">
P2: Uninstall flow reports success even when container deletion fails because `removeContainer`'s boolean result is ignored.</violation>
<violation number="2" location="app/src/main/java/app/gamenative/utils/ContainerStorageManager.kt:194">
P2: Container loading does full recursive size scans of container/install directories on each reload, which scales poorly for large libraries and causes repeated heavy I/O.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![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/utils/ContainerStorageManager.kt (1)
348-366: Consider documenting symlink coordination with ContainerMigrator.Both
relinkActiveSymlinkIfNeededhere andContainerMigrator(at lines 131-137 per context snippet) modify the active container symlink athomeDir/xuser. While these are unlikely to run concurrently in practice (migration runs at startup, removal is user-initiated), documenting this shared resource would help future maintainers understand the coordination requirements.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/utils/ContainerStorageManager.kt` around lines 348 - 366, Add a clear comment near relinkActiveSymlinkIfNeeded (and mirror it in ContainerMigrator around the code that updates the same symlink) explaining that both methods modify the active container symlink at homeDir/ImageFs.USER, describe the expected execution ordering (ContainerMigrator runs at startup; relinkActiveSymlinkIfNeeded runs on user-initiated removal), state the concurrency assumption (they are not expected to run concurrently), and call out that if this assumption changes in the future the symlink updates should be protected (e.g., a file/lock or centralized helper) to avoid races; include references to the functions ContainerStorageManager.relinkActiveSymlinkIfNeeded and ContainerMigrator where the shared resource is touched.app/src/main/java/app/gamenative/ui/screen/settings/ContainerStorageManagerDialog.kt (1)
67-70: Consider usingrememberSaveablefor pending dialog state.The
pendingRemovalandpendingUninstallstates useremember, which means if the user triggers a confirmation dialog and then rotates the device, the dialog will disappear. While not critical, usingrememberSaveablewith a customSaver(or storing just thecontainerIdand re-resolving the entry) would provide a smoother UX.For
entriesandisLoading, usingrememberis fine sinceLaunchedEffect(Unit)will reload them anyway.🤖 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/settings/ContainerStorageManagerDialog.kt` around lines 67 - 70, Change pending dialog state from ephemeral remember to saved state: replace remember for pendingRemoval and pendingUninstall with rememberSaveable and provide a Saver or persist a simple identifier; specifically, keep the UI state as pendingRemoval and pendingUninstall (types ContainerStorageManager.Entry?) but store only a stable key (e.g., containerId) via rememberSaveable and on restore resolve it back to ContainerStorageManager.Entry (or implement a custom Saver for ContainerStorageManager.Entry); leave entries and isLoading as-is since they reload in LaunchedEffect.
🤖 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/ContainerStorageManager.kt`:
- Around line 126-136: The SteamService.deleteApp call performs DAO deletions in
a launched coroutine and returns immediately, causing removeContainer in
ContainerStorageManager (inside the GameSource.STEAM branch) to run before DB
work completes; fix by making SteamService.deleteApp provide a completion signal
(e.g., make it a suspend function that performs DAO deletions directly or return
a Deferred/Job you can await) and then await its completion in the uninstall
flow before calling removeContainer(context, entry.containerId); propagate any
failure from the awaited deleteApp into the Result.failure path so container
removal only happens on confirmed success.
---
Nitpick comments:
In
`@app/src/main/java/app/gamenative/ui/screen/settings/ContainerStorageManagerDialog.kt`:
- Around line 67-70: Change pending dialog state from ephemeral remember to
saved state: replace remember for pendingRemoval and pendingUninstall with
rememberSaveable and provide a Saver or persist a simple identifier;
specifically, keep the UI state as pendingRemoval and pendingUninstall (types
ContainerStorageManager.Entry?) but store only a stable key (e.g., containerId)
via rememberSaveable and on restore resolve it back to
ContainerStorageManager.Entry (or implement a custom Saver for
ContainerStorageManager.Entry); leave entries and isLoading as-is since they
reload in LaunchedEffect.
In `@app/src/main/java/app/gamenative/utils/ContainerStorageManager.kt`:
- Around line 348-366: Add a clear comment near relinkActiveSymlinkIfNeeded (and
mirror it in ContainerMigrator around the code that updates the same symlink)
explaining that both methods modify the active container symlink at
homeDir/ImageFs.USER, describe the expected execution ordering
(ContainerMigrator runs at startup; relinkActiveSymlinkIfNeeded runs on
user-initiated removal), state the concurrency assumption (they are not expected
to run concurrently), and call out that if this assumption changes in the future
the symlink updates should be protected (e.g., a file/lock or centralized
helper) to avoid races; include references to the functions
ContainerStorageManager.relinkActiveSymlinkIfNeeded and ContainerMigrator where
the shared resource is touched.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 071426dd-bee1-426c-a34b-656fc41bd749
📒 Files selected for processing (5)
app/src/main/java/app/gamenative/ui/screen/settings/ContainerStorageManagerDialog.ktapp/src/main/java/app/gamenative/ui/screen/settings/SettingsGroupInterface.ktapp/src/main/java/app/gamenative/utils/ContainerStorageManager.ktapp/src/main/res/values/strings.xmlapp/src/test/java/app/gamenative/utils/ContainerStorageManagerTest.kt
Comment on lines
+126
to
+136
| val result = when (gameSource) { | ||
| GameSource.STEAM -> { | ||
| val deleted = SteamService.deleteApp(gameId) | ||
| if (!deleted) { | ||
| Result.failure(Exception("Failed to uninstall Steam game")) | ||
| } else { | ||
| removeContainer(context, entry.containerId) | ||
| PluviaApp.events.emitJava(AndroidEvent.LibraryInstallStatusChanged(gameId)) | ||
| Result.success(Unit) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify if SteamService.deleteApp has any mechanism to await completion
ast-grep --pattern $'fun deleteApp($_, $_): $_ {
$$$
}'
# Also check if there's a suspend variant
rg -n "suspend.*deleteApp" --type kotlinRepository: utkarshdalal/GameNative
Length of output: 267
🏁 Script executed:
# Find and examine SteamService.deleteApp implementation
fd -n "SteamService.kt" --type f | head -1 | xargs wc -lRepository: utkarshdalal/GameNative
Length of output: 316
🏁 Script executed:
# Get the SteamService.deleteApp implementation
rg -n "fun deleteApp" app/src/main/java/app/gamenative/service/SteamService.kt -A 30Repository: utkarshdalal/GameNative
Length of output: 1592
🏁 Script executed:
# Also check how ContainerStorageManager calls it
rg -n "SteamService.deleteApp" app/src/main/java/app/gamenative/utils/ContainerStorageManager.kt -B 5 -A 10Repository: utkarshdalal/GameNative
Length of output: 742
Race condition: Steam database deletion is asynchronous but not awaited before container removal.
SteamService.deleteApp() launches database operations (appInfoDao.deleteApp(), changeNumbersDao.deleteByAppId(), etc.) asynchronously via scope.launch without awaiting them, then returns immediately after file deletion. At line 132, removeContainer() is called without waiting for the pending database deletions to complete, risking inconsistent state if the database operations fail or are significantly delayed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/app/gamenative/utils/ContainerStorageManager.kt` around
lines 126 - 136, The SteamService.deleteApp call performs DAO deletions in a
launched coroutine and returns immediately, causing removeContainer in
ContainerStorageManager (inside the GameSource.STEAM branch) to run before DB
work completes; fix by making SteamService.deleteApp provide a completion signal
(e.g., make it a suspend function that performs DAO deletions directly or return
a Deferred/Job you can await) and then await its completion in the uninstall
flow before calling removeContainer(context, entry.containerId); propagate any
failure from the awaited deleteApp into the Result.failure path so container
removal only happens on confirmed success.
Owner
|
Thanks for this, shared on Discord as well but dropping it here too: The reason I was averse to it is that the codebase is already huge and unwieldy. My focus right now is on adding features only if they're absolutely necessary, and in the minimal possible way. If the goal here is to remove orphaned containers, my personal opinion is that we should fix bugs so that there aren't orphaned containers - half installed games for all stores should show a "delete" option. If they are not showing (which afaik should no longer be happening after the clean ups to GOG/Epic/Amazon), then that's what we need to fix. |
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/utils/ContainerStorageManager.kt`:
- Around line 195-220: The delete branches for GameSource.GOG, GameSource.EPIC
and GameSource.AMAZON call service-specific methods (GOGService.deleteGame,
EpicService.deleteGame, AmazonService.deleteGame) that already invoke
ContainerUtils.deleteContainer, so remove the redundant local
removeContainer(context, entry.containerId) calls in those branches; keep
returning the service result (result) and ensure you do not reintroduce
removeContainer using the raw entry.containerId—if you must call local cleanup
instead, use normalizedContainerId consistently (e.g., normalizedContainerId) to
match the service call.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 64f59a20-8724-484d-ab88-63ba9fd07d8f
📒 Files selected for processing (5)
app/src/main/java/app/gamenative/db/dao/EpicGameDao.ktapp/src/main/java/app/gamenative/db/dao/SteamAppDao.ktapp/src/main/java/app/gamenative/ui/screen/settings/ContainerStorageManagerDialog.ktapp/src/main/java/app/gamenative/utils/ContainerStorageManager.ktapp/src/main/res/values/strings.xml
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/res/values/strings.xml
Comment on lines
+195
to
+220
| GameSource.GOG -> { | ||
| val result = GOGService.deleteGame( | ||
| context, | ||
| LibraryItem( | ||
| appId = normalizedContainerId, | ||
| name = entry.displayName, | ||
| gameSource = GameSource.GOG, | ||
| ), | ||
| ) | ||
| if (result.isSuccess && entry.hasContainer) removeContainer(context, entry.containerId) | ||
| result | ||
| } | ||
|
|
||
| GameSource.EPIC -> { | ||
| val result = EpicService.deleteGame(context, gameId) | ||
| if (result.isSuccess && entry.hasContainer) removeContainer(context, entry.containerId) | ||
| result | ||
| } | ||
|
|
||
| GameSource.AMAZON -> { | ||
| val productId = AmazonService.getProductIdByAppId(gameId) | ||
| ?: return@withContext Result.failure(Exception("Amazon product id not found")) | ||
| val result = AmazonService.deleteGame(context, productId) | ||
| if (result.isSuccess && entry.hasContainer) removeContainer(context, entry.containerId) | ||
| result | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify ContainerUtils.deleteContainer is called within each service's deleteGame method
echo "=== GOGService.deleteGame ==="
rg -n "ContainerUtils.deleteContainer" app/src/main/java/app/gamenative/service/gog/GOGManager.kt -B2 -A2
echo "=== EpicService.deleteGame ==="
rg -n "ContainerUtils.deleteContainer" app/src/main/java/app/gamenative/service/epic/EpicService.kt -B2 -A2
echo "=== AmazonService.deleteGame ==="
rg -n "ContainerUtils.deleteContainer" app/src/main/java/app/gamenative/service/amazon/AmazonService.kt -B2 -A2Repository: utkarshdalal/GameNative
Length of output: 869
🏁 Script executed:
#!/bin/bash
# Check the removeContainer method implementation
echo "=== removeContainer method ==="
rg -n "fun removeContainer" app/src/main/java/app/gamenative/utils/ContainerStorageManager.kt -A25
# Check what entry.containerId is and how it relates to appId/productId
echo -e "\n=== Checking entry.containerId usage ==="
rg -n "entry\.containerId" app/src/main/java/app/gamenative/utils/ContainerStorageManager.kt | head -20
# Check if there's a relationship between normalizedContainerId and containerId
echo -e "\n=== normalizedContainerId vs containerId ==="
rg -n "normalizedContainerId\|entry\.containerId" app/src/main/java/app/gamenative/utils/ContainerStorageManager.kt -B1 -A1 | head -40Repository: utkarshdalal/GameNative
Length of output: 2219
🏁 Script executed:
#!/bin/bash
# Check ContainerUtils.deleteContainer implementation
echo "=== ContainerUtils.deleteContainer ==="
rg -n "fun deleteContainer" app/src/main/java/app/gamenative/utils/ContainerUtils.kt -A20
# Check normalizeContainerId function
echo -e "\n=== normalizeContainerId function ==="
rg -n "fun normalizeContainerId" app/src/main/java/app/gamenative/utils/ContainerStorageManager.kt -A10Repository: utkarshdalal/GameNative
Length of output: 1535
🏁 Script executed:
#!/bin/bash
# Check GOGService.deleteGame to see how it uses the passed appId
echo "=== GOGService.deleteGame method ==="
rg -n "fun deleteGame" app/src/main/java/app/gamenative/service/gog/GOGManager.kt -A30 | head -50
# Check what format entry.containerId has - look at the LibraryItem or database model
echo -e "\n=== Searching for LibraryItem definition ==="
fd "LibraryItem" app/src/main/java --type f | head -5
# Check ContainerManager to see if getContainerById normalizes
echo -e "\n=== ContainerManager.getContainerById ==="
fd "ContainerManager" app/src/main/java --type f | head -3Repository: utkarshdalal/GameNative
Length of output: 1973
🏁 Script executed:
#!/bin/bash
# Check GOGService.deleteGame around the ContainerUtils.deleteContainer call
echo "=== GOGService.deleteGame full method ==="
sed -n '464,510p' app/src/main/java/app/gamenative/service/gog/GOGManager.ktRepository: utkarshdalal/GameNative
Length of output: 2409
🏁 Script executed:
#!/bin/bash
# Check how Epic and Amazon services are called in ContainerStorageManager
echo "=== ContainerStorageManager delete logic for Epic ==="
sed -n '207,211p' app/src/main/java/app/gamenative/utils/ContainerStorageManager.kt
echo -e "\n=== ContainerStorageManager delete logic for Amazon ==="
sed -n '215,218p' app/src/main/java/app/gamenative/utils/ContainerStorageManager.kt
# Check EpicService.deleteGame signature
echo -e "\n=== EpicService.deleteGame ==="
rg -n "fun deleteGame" app/src/main/java/app/gamenative/service/epic/EpicService.kt -A5
# Check AmazonService.deleteGame signature
echo -e "\n=== AmazonService.deleteGame ==="
rg -n "fun deleteGame" app/src/main/java/app/gamenative/service/amazon/AmazonService.kt -A5Repository: utkarshdalal/GameNative
Length of output: 1429
🏁 Script executed:
#!/bin/bash
# Check how entry is created and what containerId format is
echo "=== Finding where entry comes from in deleteGameAndContainer ==="
sed -n '160,175p' app/src/main/java/app/gamenative/utils/ContainerStorageManager.kt
# Check LibraryItem to understand appId and containerId relationship
echo -e "\n=== LibraryItem.kt ==="
cat app/src/main/java/app/gamenative/data/LibraryItem.ktRepository: utkarshdalal/GameNative
Length of output: 3181
🏁 Script executed:
#!/bin/bash
# Check what Entry is - likely from database
echo "=== Searching for Entry data class ==="
fd "class Entry" app/src/main/java --type f
rg -n "data class Entry" app/src/main/java
# Check if entry.containerId is the normalized format already or if it includes duplicates
echo -e "\n=== Check ContainerStorageManager Entry usage ==="
rg -n "class Entry\|data class Entry" app/src/main/java/app/gamenative/utils/ContainerStorageManager.kt -A10 | head -30Repository: utkarshdalal/GameNative
Length of output: 425
🏁 Script executed:
#!/bin/bash
# Get the Entry definition from ContainerStorageManager
echo "=== ContainerStorageManager Entry definition ==="
sed -n '54,70p' app/src/main/java/app/gamenative/utils/ContainerStorageManager.kt
# Also check how entry is populated
echo -e "\n=== How entry.containerId is set ==="
rg -n "entry\.containerId\s*=" app/src/main/java/app/gamenative/utils/ContainerStorageManager.kt -B2 -A2 | head -20
# Check the complete GOG branch again more carefully
echo -e "\n=== Full GOG branch with variable names ==="
sed -n '195,205p' app/src/main/java/app/gamenative/utils/ContainerStorageManager.ktRepository: utkarshdalal/GameNative
Length of output: 1346
🏁 Script executed:
#!/bin/bash
# Let me trace through the exact IDs being used:
# For GOG: check what happens with duplicate indices
echo "=== Verify normalizeContainerId removes duplicates properly ==="
python3 << 'EOF'
def normalize_container_id(container_id: str) -> str:
return container_id.split("(")[0]
# Test cases
test_ids = ["GOG_123", "GOG_123(1)", "GOG_123(2)"]
for test_id in test_ids:
normalized = normalize_container_id(test_id)
print(f"{test_id} -> {normalized}")
EOF
echo -e "\n=== Check if entry.containerId has duplicates format ==="
rg -n "containerId.*\(" app/src/main/java/app/gamenative/utils/ContainerStorageManager.kt
# Check where entry comes from - does it preserve the duplicate suffix?
echo -e "\n=== Where ContainerStorageManager gets entries from ==="
rg -n "fun.*Entry\|return.*Entry" app/src/main/java/app/gamenative/utils/ContainerStorageManager.kt | head -15Repository: utkarshdalal/GameNative
Length of output: 1037
🏁 Script executed:
#!/bin/bash
# Check ContainerManager to understand how it finds containers
echo "=== ContainerManager.getContainerById ==="
rg -n "fun getContainerById" app/src/main/java/com/winlator/container/ContainerManager.java -A15
# Check if ContainerManager uses normalized IDs
echo -e "\n=== ContainerManager container lookup ==="
rg -n "hasContainer\|getContainerById" app/src/main/java/com/winlator/container/ContainerManager.java | head -20
# Check where containers are listed to understand if duplicate suffix is preserved
echo -e "\n=== How containers are discovered/listed ==="
rg -n "fun getContainers\|\.listFiles" app/src/main/java/com/winlator/container/ContainerManager.java | head -10Repository: utkarshdalal/GameNative
Length of output: 197
🏁 Script executed:
#!/bin/bash
# ContainerManager might be in a different location or named differently
echo "=== Find ContainerManager or similar ==="
fd "Container" app/src/main/java/com/winlator --type f | head -10
# Try searching for the actual deleteContainer implementation more carefully
echo -e "\n=== Look at ContainerUtils.deleteContainer again ==="
sed -n '1027,1037p' app/src/main/java/app/gamenative/utils/ContainerUtils.kt
# Check what ContainerManager does
echo -e "\n=== Find ContainerManager class ==="
find app/src/main/java -name "*.java" -o -name "*.kt" | xargs grep -l "class ContainerManager" 2>/dev/null
# Check if there's normalization happening in ContainerManager
echo -e "\n=== ContainerManager implementation ==="
rg -n "class ContainerManager" app/src/main/java --type java -A30 | head -50Repository: utkarshdalal/GameNative
Length of output: 4174
🏁 Script executed:
#!/bin/bash
# Check hasContainer and getContainerById methods
echo "=== ContainerManager.hasContainer and getContainerById ==="
rg -n "public.*hasContainer\|public.*getContainerById" app/src/main/java/com/winlator/container/ContainerManager.java -A10Repository: utkarshdalal/GameNative
Length of output: 125
🏁 Script executed:
#!/bin/bash
# Search more broadly for ContainerManager methods
echo "=== All public methods in ContainerManager ==="
rg -n "public" app/src/main/java/com/winlator/container/ContainerManager.java | grep "(" | head -30Repository: utkarshdalal/GameNative
Length of output: 1169
🏁 Script executed:
#!/bin/bash
# Check hasContainer and getContainerById implementations
echo "=== hasContainer method ==="
sed -n '274,280p' app/src/main/java/com/winlator/container/ContainerManager.java
echo -e "\n=== getContainerById method ==="
sed -n '279,290p' app/src/main/java/com/winlator/container/ContainerManager.javaRepository: utkarshdalal/GameNative
Length of output: 1007
🏁 Script executed:
#!/bin/bash
# This is critical - let me verify what entry.containerId contains
# by checking how entries are created/populated
echo "=== How entries are populated in ContainerStorageManager ==="
rg -n "Entry(" app/src/main/java/app/gamenative/utils/ContainerStorageManager.kt | head -20
# Check the getEntries function or similar
echo -e "\n=== Function that creates Entry objects ==="
rg -n "fun.*Entry\|Entry(" app/src/main/java/app/gamenative/utils/ContainerStorageManager.kt -B5 | grep -A5 "fun\|Entry(" | head -40Repository: utkarshdalal/GameNative
Length of output: 339
🏁 Script executed:
#!/bin/bash
# Fix the regex - escape the parentheses
echo "=== How entries are populated in ContainerStorageManager ==="
rg -n "Entry\(" app/src/main/java/app/gamenative/utils/ContainerStorageManager.kt | head -20
# Check the complete method that creates entries
echo -e "\n=== getStorageManagerEntries or similar ==="
rg -n "fun get.*Entry\|suspend fun.*Entry" app/src/main/java/app/gamenative/utils/ContainerStorageManager.kt -A2Repository: utkarshdalal/GameNative
Length of output: 558
Use consistent container IDs across service deletion calls and local cleanup.
The service-specific deleteGame methods call ContainerUtils.deleteContainer internally, making the subsequent removeContainer calls redundant. Additionally, for GOG, the service receives the normalized container ID (with duplicate suffix removed) while removeContainer is called with the raw entry.containerId, potentially targeting the wrong container if duplicates exist.
Either remove the redundant removeContainer calls or ensure both deletion paths use the same container ID format (preferably normalizedContainerId for all sources to match the service method calls).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/app/gamenative/utils/ContainerStorageManager.kt` around
lines 195 - 220, The delete branches for GameSource.GOG, GameSource.EPIC and
GameSource.AMAZON call service-specific methods (GOGService.deleteGame,
EpicService.deleteGame, AmazonService.deleteGame) that already invoke
ContainerUtils.deleteContainer, so remove the redundant local
removeContainer(context, entry.containerId) calls in those branches; keep
returning the service result (result) and ensure you do not reintroduce
removeContainer using the raw entry.containerId—if you must call local cleanup
instead, use normalizedContainerId consistently (e.g., normalizedContainerId) to
match the service call.
Contributor
There was a problem hiding this comment.
2 issues found across 5 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/db/dao/SteamAppDao.kt">
<violation number="1" location="app/src/main/java/app/gamenative/db/dao/SteamAppDao.kt:45">
P2: New DAO method performs an unbounded eager `List` load of Steam apps, creating scalable performance/memory risk in the storage manager path.</violation>
</file>
<file name="app/src/main/java/app/gamenative/utils/ContainerStorageManager.kt">
<violation number="1" location="app/src/main/java/app/gamenative/utils/ContainerStorageManager.kt:462">
P2: Installed-only entries are marked uninstallable without validating integer game ID parseability, but uninstall path hard-fails on non-numeric IDs.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| "AND type != 0 " + | ||
| "ORDER BY LOWER(name)", | ||
| ) | ||
| suspend fun getAllOwnedAppsAsList( |
Contributor
There was a problem hiding this comment.
P2: New DAO method performs an unbounded eager List load of Steam apps, creating scalable performance/memory risk in the storage manager path.
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/db/dao/SteamAppDao.kt, line 45:
<comment>New DAO method performs an unbounded eager `List` load of Steam apps, creating scalable performance/memory risk in the storage manager path.</comment>
<file context>
@@ -35,6 +35,17 @@ interface SteamAppDao {
+ "AND type != 0 " +
+ "ORDER BY LOWER(name)",
+ )
+ suspend fun getAllOwnedAppsAsList(
+ invalidPkgId: Int = INVALID_PKG_ID,
+ ): List<SteamApp>
</file context>
xXJSONDeruloXx
force-pushed
the
feat/container-storage-manager
branch
from
f6dd100 to
7cd1455
Compare
Contributor
There was a problem hiding this comment.
1 issue found across 7 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/utils/StorageUtils.kt">
<violation number="1" location="app/src/main/java/app/gamenative/utils/StorageUtils.kt:228">
P2: `moveGamesFromOldPath` ignores `moveDirectory` failure result and always signals completion, masking migration errors.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| ) | ||
|
|
||
| withContext(Dispatchers.Main) { | ||
| onComplete() |
Contributor
There was a problem hiding this comment.
P2: moveGamesFromOldPath ignores moveDirectory failure result and always signals completion, masking migration errors.
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 228:
<comment>`moveGamesFromOldPath` ignores `moveDirectory` failure result and always signals completion, masking migration errors.</comment>
<file context>
@@ -191,27 +187,45 @@ object StorageUtils {
+ )
+
+ withContext(Dispatchers.Main) {
+ onComplete()
}
}
</file context>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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/service/amazon/AmazonConstants.kt`:
- Around line 54-64: internalAmazonGamesPath and externalAmazonGamesPath
currently create directories as a side-effect and externalAmazonGamesPath builds
a path from an uninitialized PrefManager.externalStoragePath; remove the
File(path).mkdirs() calls from both functions so they only compute and return
the path (no I/O), and change externalAmazonGamesPath to validate
PrefManager.externalStoragePath (e.g., null/blank) and reject it by throwing an
IllegalStateException (or returning null) instead of constructing a bogus path;
update callers that need directories to perform mkdirs() at the actual
install/write sites.
In
`@app/src/main/java/app/gamenative/ui/screen/settings/ContainerStorageManagerDialog.kt`:
- Around line 391-393: The three filesystem-heavy calls
ContainerStorageManager.getStorageLocation(context, entry),
ContainerStorageManager.canMoveToExternal(context, entry) and
ContainerStorageManager.canMoveToInternal(context, entry) are being executed on
every recomposition; wrap them in a single remember(entry) block (or precompute
them when loading entries) so normalizePath()/AmazonConstants file operations
run once per entry instead of repeatedly during composition; compute a
tuple/object with storageLocation, canMoveToExternal and canMoveToInternal
inside that remember and replace the three direct calls in ContainerStorageRow
with the remembered values.
In `@app/src/main/java/app/gamenative/utils/ContainerStorageManager.kt`:
- Around line 165-168: The UI and backend currently block moves back to internal
because both canMoveToInternal and moveGame gate actions on
isExternalStorageConfigured(); remove that dependency so moves back are allowed
even if external storage is disabled. Specifically, update canMoveToInternal to
return canMoveGame(entry) && getStorageLocation(context, entry) ==
StorageLocation.EXTERNAL (drop the isExternalStorageConfigured() check), and
make the corresponding change in moveGame so logic that permits moving an entry
from StorageLocation.EXTERNAL to internal does not require
isExternalStorageConfigured(); continue to use getStorageLocation(...) and
StorageLocation.EXTERNAL to detect external installs and preserve existing
validation in canMoveGame(entry).
- Around line 171-305: moveGame currently performs DAO reads/updates and
filesystem operations (calls like StorageUtils.moveDirectory,
entryPoint.gogGameDao().getById, update, markAsInstalled, etc.) without catching
exceptions, so thrown errors bypass the Result path; wrap the body of moveGame
after validations (the section starting from resolving targetRoot/targetDir
through post-move DAO updates and the StorageUtils.moveDirectory call) in a
try/catch that catches Throwable and returns Result.failure(th) (or a wrapped
Exception) so callers always receive a Result on unexpected errors; ensure the
catch logs the error (Timber) and returns Result.failure to surface failures to
the UI.
- Around line 329-330: Before removing containerDir, detect if activeLink was
pointing at that container by checking the symlink target (use
java.nio.file.Files.isSymbolicLink(activeLink.toPath()) and
Files.readSymbolicLink(activeLink.toPath()) or equivalent) and compare that
target to containerDir (or its name/path); store that boolean (e.g.,
wasActiveTarget) and after deletion call relinkActiveSymlinkIfNeeded(homeDir,
containerDir) only if wasActiveTarget is true. Also update
relinkActiveSymlinkIfNeeded (and the other occurrence around the 846-860 block)
to rely on Files.isSymbolicLink/readSymbolicLink rather than activeLink.exists()
so dangling symlinks are detected correctly.
In `@app/src/main/java/app/gamenative/utils/StorageUtils.kt`:
- Around line 215-229: The wrapper moveGamesFromOldPath currently discards the
outcome of moveDirectory and always invokes onComplete, so change
moveGamesFromOldPath to return the migration result (e.g., Boolean or a Result
type) by capturing the return value from moveDirectory (val success =
moveDirectory(...)), call onComplete() only when success is true (wrap that call
in withContext(Dispatchers.Main) and conditionally invoke), and return success
to callers; then update callers (e.g., SteamAppScreen) to inspect the returned
Boolean/Result to distinguish success vs failure and dismiss or surface errors
accordingly.
In `@app/src/main/res/values/strings.xml`:
- Line 816: The current string resource "container_storage_summary" uses a
hardcoded "%1$d items" which doesn't handle singular/plural; replace this with a
<plurals> resource (e.g. name="container_storage_summary") that provides
singular and plural forms and moves the "%2$s" (the secondary string) into the
appropriate quantity strings, then update callers to use
pluralStringResource(...) (or resources.getQuantityString(...) in non-Compose
code) to select the right form and pass the count and the second argument so "1
item" renders correctly; locate usages of "container_storage_summary" and change
them to use plural selection.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b86a96f4-9f4e-46fa-86d3-0cd93e3850e9
📒 Files selected for processing (7)
app/src/main/java/app/gamenative/service/amazon/AmazonConstants.ktapp/src/main/java/app/gamenative/ui/screen/settings/ContainerStorageManagerDialog.ktapp/src/main/java/app/gamenative/ui/screen/settings/SettingsGroupInterface.ktapp/src/main/java/app/gamenative/utils/ContainerStorageManager.ktapp/src/main/java/app/gamenative/utils/StorageUtils.ktapp/src/main/res/values/strings.xmlpi-session-2026-03-11T20-41-45-419Z_567dcdd3-d91f-4691-b7fc-e5f96c819fcb.html
Comment on lines
+54
to
+64
| fun internalAmazonGamesPath(context: Context): String { | ||
| val path = Paths.get(context.dataDir.path, "Amazon").toString() | ||
| File(path).mkdirs() | ||
| return path | ||
| } | ||
|
|
||
| fun externalAmazonGamesPath(): String { | ||
| val path = Paths.get(PrefManager.externalStoragePath, "Amazon", "games").toString() | ||
| File(path).mkdirs() | ||
| return path | ||
| } |
Contributor
There was a problem hiding this comment.
Keep these path helpers side-effect free.
internalAmazonGamesPath() and externalAmazonGamesPath() are reused from storage-location checks, so the mkdirs() calls here make a read-only inventory scan create empty Amazon / Amazon/games directories. externalAmazonGamesPath() should also reject an uninitialized PrefManager.externalStoragePath instead of constructing a path from it. Create the directory only at the actual install/write call sites.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/app/gamenative/service/amazon/AmazonConstants.kt` around
lines 54 - 64, internalAmazonGamesPath and externalAmazonGamesPath currently
create directories as a side-effect and externalAmazonGamesPath builds a path
from an uninitialized PrefManager.externalStoragePath; remove the
File(path).mkdirs() calls from both functions so they only compute and return
the path (no I/O), and change externalAmazonGamesPath to validate
PrefManager.externalStoragePath (e.g., null/blank) and reject it by throwing an
IllegalStateException (or returning null) instead of constructing a bogus path;
update callers that need directories to perform mkdirs() at the actual
install/write sites.
Comment on lines
+165
to
+168
| fun canMoveToInternal(context: Context, entry: Entry): Boolean { | ||
| return canMoveGame(entry) && | ||
| isExternalStorageConfigured() && | ||
| getStorageLocation(context, entry) == StorageLocation.EXTERNAL |
Contributor
There was a problem hiding this comment.
Allow moves back to internal without the external-storage toggle.
Both canMoveToInternal() and moveGame() require isExternalStorageConfigured(). If a user disables external storage while games still live there, the move-back button disappears and the backend rejects the move, leaving those installs stranded.
💡 Minimal fix
fun canMoveToInternal(context: Context, entry: Entry): Boolean {
return canMoveGame(entry) &&
- isExternalStorageConfigured() &&
getStorageLocation(context, entry) == StorageLocation.EXTERNAL
}
@@
- if (!isExternalStorageConfigured()) {
+ if (target == MoveTarget.EXTERNAL && !isExternalStorageConfigured()) {
return@withContext Result.failure(IllegalStateException("External storage is not enabled"))
}Also applies to: 184-186
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/app/gamenative/utils/ContainerStorageManager.kt` around
lines 165 - 168, The UI and backend currently block moves back to internal
because both canMoveToInternal and moveGame gate actions on
isExternalStorageConfigured(); remove that dependency so moves back are allowed
even if external storage is disabled. Specifically, update canMoveToInternal to
return canMoveGame(entry) && getStorageLocation(context, entry) ==
StorageLocation.EXTERNAL (drop the isExternalStorageConfigured() check), and
make the corresponding change in moveGame so logic that permits moving an entry
from StorageLocation.EXTERNAL to internal does not require
isExternalStorageConfigured(); continue to use getStorageLocation(...) and
StorageLocation.EXTERNAL to detect external installs and preserve existing
validation in canMoveGame(entry).
Comment on lines
+171
to
+305
| suspend fun moveGame( | ||
| context: Context, | ||
| entry: Entry, | ||
| target: MoveTarget, | ||
| onProgressUpdate: (currentFile: String, fileProgress: Float, movedFiles: Int, totalFiles: Int) -> Unit, | ||
| ): Result<Unit> = withContext(Dispatchers.IO) { | ||
| val normalizedContainerId = normalizeContainerId(entry.containerId) | ||
| val gameSource = entry.gameSource ?: detectGameSource(normalizedContainerId) | ||
| ?: return@withContext Result.failure(IllegalArgumentException("Unknown game source")) | ||
|
|
||
| if (!canMoveGame(entry)) { | ||
| return@withContext Result.failure(IllegalArgumentException("This game cannot be moved")) | ||
| } | ||
| if (!isExternalStorageConfigured()) { | ||
| return@withContext Result.failure(IllegalStateException("External storage is not enabled")) | ||
| } | ||
|
|
||
| val sourceDir = File(entry.installPath.orEmpty()) | ||
| if (!sourceDir.exists() || !sourceDir.isDirectory) { | ||
| return@withContext Result.failure(IllegalArgumentException("Game install folder does not exist")) | ||
| } | ||
|
|
||
| val currentLocation = getStorageLocation(context, gameSource, sourceDir.absolutePath) | ||
| when (target) { | ||
| MoveTarget.EXTERNAL -> if (currentLocation != StorageLocation.INTERNAL) { | ||
| return@withContext Result.failure(IllegalStateException("Game is not stored internally")) | ||
| } | ||
|
|
||
| MoveTarget.INTERNAL -> if (currentLocation != StorageLocation.EXTERNAL) { | ||
| return@withContext Result.failure(IllegalStateException("Game is not stored externally")) | ||
| } | ||
| } | ||
|
|
||
| val targetRoot = resolveTargetInstallRoot(context, gameSource, target) | ||
| ?: return@withContext Result.failure(IllegalArgumentException("Unsupported game source")) | ||
| val targetDir = File(targetRoot, sourceDir.name) | ||
|
|
||
| if (samePath(sourceDir, targetDir)) { | ||
| return@withContext Result.success(Unit) | ||
| } | ||
| if (targetDir.exists()) { | ||
| return@withContext Result.failure( | ||
| IllegalStateException("Destination already exists: ${targetDir.absolutePath}"), | ||
| ) | ||
| } | ||
|
|
||
| val entryPoint = EntryPointAccessors.fromApplication( | ||
| context.applicationContext, | ||
| StorageManagerDaoEntryPoint::class.java, | ||
| ) | ||
|
|
||
| val steamGameId = if (gameSource == GameSource.STEAM) { | ||
| extractGameId(normalizedContainerId) | ||
| ?: return@withContext Result.failure(IllegalArgumentException("Invalid Steam game id")) | ||
| } else { | ||
| null | ||
| } | ||
| val gogGame = if (gameSource == GameSource.GOG) { | ||
| val gameId = extractGameId(normalizedContainerId)?.toString() | ||
| ?: return@withContext Result.failure(IllegalArgumentException("Invalid GOG game id")) | ||
| entryPoint.gogGameDao().getById(gameId) | ||
| ?: return@withContext Result.failure(IllegalStateException("GOG game not found in database")) | ||
| } else { | ||
| null | ||
| } | ||
| val epicGame = if (gameSource == GameSource.EPIC) { | ||
| val gameId = extractGameId(normalizedContainerId) | ||
| ?: return@withContext Result.failure(IllegalArgumentException("Invalid Epic game id")) | ||
| entryPoint.epicGameDao().getById(gameId) | ||
| ?: return@withContext Result.failure(IllegalStateException("Epic game not found in database")) | ||
| } else { | ||
| null | ||
| } | ||
| val amazonGame = if (gameSource == GameSource.AMAZON) { | ||
| val gameId = extractGameId(normalizedContainerId) | ||
| ?: return@withContext Result.failure(IllegalArgumentException("Invalid Amazon game id")) | ||
| entryPoint.amazonGameDao().getByAppId(gameId) | ||
| ?: return@withContext Result.failure(IllegalStateException("Amazon game not found in database")) | ||
| } else { | ||
| null | ||
| } | ||
|
|
||
| Timber.tag("ContainerStorageManager").i( | ||
| "Moving game %s from %s to %s", | ||
| entry.containerId, | ||
| sourceDir.absolutePath, | ||
| targetDir.absolutePath, | ||
| ) | ||
|
|
||
| val moveResult = StorageUtils.moveDirectory( | ||
| sourceDir = sourceDir.absolutePath, | ||
| targetDir = targetDir.absolutePath, | ||
| onProgressUpdate = onProgressUpdate, | ||
| ) | ||
| if (moveResult.isFailure) { | ||
| return@withContext moveResult | ||
| } | ||
|
|
||
| val installSize = entry.gameInstallSizeBytes ?: StorageUtils.getFolderSize(targetDir.absolutePath) | ||
| when (gameSource) { | ||
| GameSource.STEAM -> { | ||
| steamGameId?.let { PluviaApp.events.emitJava(AndroidEvent.LibraryInstallStatusChanged(it)) } | ||
| } | ||
|
|
||
| GameSource.GOG -> { | ||
| gogGame?.let { | ||
| entryPoint.gogGameDao().update(it.copy(installPath = targetDir.absolutePath, installSize = installSize)) | ||
| } | ||
| } | ||
|
|
||
| GameSource.EPIC -> { | ||
| epicGame?.let { | ||
| entryPoint.epicGameDao().update(it.copy(installPath = targetDir.absolutePath, installSize = installSize)) | ||
| } | ||
| } | ||
|
|
||
| GameSource.AMAZON -> { | ||
| amazonGame?.let { | ||
| entryPoint.amazonGameDao().markAsInstalled(it.productId, targetDir.absolutePath, installSize, it.versionId) | ||
| } | ||
| } | ||
|
|
||
| GameSource.CUSTOM_GAME -> { | ||
| return@withContext Result.failure(UnsupportedOperationException("Custom games are not supported")) | ||
| } | ||
| } | ||
|
|
||
| Timber.tag("ContainerStorageManager").i( | ||
| "Moved game %s successfully to %s", | ||
| entry.containerId, | ||
| targetDir.absolutePath, | ||
| ) | ||
|
|
||
| Result.success(Unit) | ||
| } |
Contributor
There was a problem hiding this comment.
Return Result.failure for unexpected move errors too.
After the early validation branches, moveGame() still does DAO reads/updates and filesystem work without a catch. If any of that throws, the caller never receives a Result, so the snackbar path is skipped and the migration dialog can stay stuck open.
💡 Minimal fix
suspend fun moveGame(
context: Context,
entry: Entry,
target: MoveTarget,
onProgressUpdate: (currentFile: String, fileProgress: Float, movedFiles: Int, totalFiles: Int) -> Unit,
): Result<Unit> = withContext(Dispatchers.IO) {
- val normalizedContainerId = normalizeContainerId(entry.containerId)
- val gameSource = entry.gameSource ?: detectGameSource(normalizedContainerId)
- ?: return@withContext Result.failure(IllegalArgumentException("Unknown game source"))
+ try {
+ val normalizedContainerId = normalizeContainerId(entry.containerId)
+ val gameSource = entry.gameSource ?: detectGameSource(normalizedContainerId)
+ ?: return@withContext Result.failure(IllegalArgumentException("Unknown game source"))
- if (!canMoveGame(entry)) {
- return@withContext Result.failure(IllegalArgumentException("This game cannot be moved"))
- }
- if (!isExternalStorageConfigured()) {
- return@withContext Result.failure(IllegalStateException("External storage is not enabled"))
- }
+ if (!canMoveGame(entry)) {
+ return@withContext Result.failure(IllegalArgumentException("This game cannot be moved"))
+ }
+ if (!isExternalStorageConfigured()) {
+ return@withContext Result.failure(IllegalStateException("External storage is not enabled"))
+ }
- val sourceDir = File(entry.installPath.orEmpty())
- if (!sourceDir.exists() || !sourceDir.isDirectory) {
- return@withContext Result.failure(IllegalArgumentException("Game install folder does not exist"))
- }
+ val sourceDir = File(entry.installPath.orEmpty())
+ if (!sourceDir.exists() || !sourceDir.isDirectory) {
+ return@withContext Result.failure(IllegalArgumentException("Game install folder does not exist"))
+ }
- val currentLocation = getStorageLocation(context, gameSource, sourceDir.absolutePath)
- when (target) {
- MoveTarget.EXTERNAL -> if (currentLocation != StorageLocation.INTERNAL) {
- return@withContext Result.failure(IllegalStateException("Game is not stored internally"))
- }
+ val currentLocation = getStorageLocation(context, gameSource, sourceDir.absolutePath)
+ when (target) {
+ MoveTarget.EXTERNAL -> if (currentLocation != StorageLocation.INTERNAL) {
+ return@withContext Result.failure(IllegalStateException("Game is not stored internally"))
+ }
- MoveTarget.INTERNAL -> if (currentLocation != StorageLocation.EXTERNAL) {
- return@withContext Result.failure(IllegalStateException("Game is not stored externally"))
+ MoveTarget.INTERNAL -> if (currentLocation != StorageLocation.EXTERNAL) {
+ return@withContext Result.failure(IllegalStateException("Game is not stored externally"))
+ }
}
- }
- val targetRoot = resolveTargetInstallRoot(context, gameSource, target)
- ?: return@withContext Result.failure(IllegalArgumentException("Unsupported game source"))
- val targetDir = File(targetRoot, sourceDir.name)
+ val targetRoot = resolveTargetInstallRoot(context, gameSource, target)
+ ?: return@withContext Result.failure(IllegalArgumentException("Unsupported game source"))
+ val targetDir = File(targetRoot, sourceDir.name)
- if (samePath(sourceDir, targetDir)) {
- return@withContext Result.success(Unit)
- }
- if (targetDir.exists()) {
- return@withContext Result.failure(
- IllegalStateException("Destination already exists: ${targetDir.absolutePath}"),
- )
- }
+ if (samePath(sourceDir, targetDir)) {
+ return@withContext Result.success(Unit)
+ }
+ if (targetDir.exists()) {
+ return@withContext Result.failure(
+ IllegalStateException("Destination already exists: ${targetDir.absolutePath}"),
+ )
+ }
- val entryPoint = EntryPointAccessors.fromApplication(
- context.applicationContext,
- StorageManagerDaoEntryPoint::class.java,
- )
+ val entryPoint = EntryPointAccessors.fromApplication(
+ context.applicationContext,
+ StorageManagerDaoEntryPoint::class.java,
+ )
- ...
+ ...
- Result.success(Unit)
+ Result.success(Unit)
+ } catch (e: Exception) {
+ Timber.tag("ContainerStorageManager").e(e, "Failed to move game %s", entry.containerId)
+ Result.failure(e)
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/app/gamenative/utils/ContainerStorageManager.kt` around
lines 171 - 305, moveGame currently performs DAO reads/updates and filesystem
operations (calls like StorageUtils.moveDirectory,
entryPoint.gogGameDao().getById, update, markAsInstalled, etc.) without catching
exceptions, so thrown errors bypass the Result path; wrap the body of moveGame
after validations (the section starting from resolving targetRoot/targetDir
through post-move DAO updates and the StorageUtils.moveDirectory call) in a
try/catch that catches Throwable and returns Result.failure(th) (or a wrapped
Exception) so callers always receive a Result on unexpected errors; ensure the
catch logs the error (Timber) and returns Result.failure to surface failures to
the UI.
Comment on lines
+329
to
+330
| if (deleted) { | ||
| relinkActiveSymlinkIfNeeded(homeDir, containerDir) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Java/Kotlin on Android, does java.io.File.exists() return false for a dangling symbolic link?
💡 Result:
Yes. On Android, java.io.File.exists() is implemented as Libcore.os.access(path, F_OK) and returns false if that call throws (any) ErrnoException (including ENOENT).[1] The underlying access(2) syscall dereferences symlinks, and explicitly documents ENOENT for “a dangling symbolic link”, so a broken/dangling symlink makes exists() return false.[2]
Sources:
[1] Android libcore java.io.File (exists() → doAccess(F_OK) → Libcore.os.access(...), exceptions mapped to false) (android.googlesource.com)
[2] access(2) man page (ENOENT: “does not exist or is a dangling symbolic link”; symlinks are dereferenced) (man7.org)
Citations:
- 1: https://android.googlesource.com/platform/libcore.git/%2B/android-4.2.2_r1/luni/src/main/java/java/io/File.java
- 2: https://man7.org/linux/man-pages/man2/access.2.html
🏁 Script executed:
# Find and examine the file
fd -t f "ContainerStorageManager.kt"Repository: utkarshdalal/GameNative
Length of output: 132
🏁 Script executed:
# Read the file to check size and context around lines 329-330
wc -l app/src/main/java/app/gamenative/utils/ContainerStorageManager.ktRepository: utkarshdalal/GameNative
Length of output: 136
🏁 Script executed:
# Get context around lines 329-330
sed -n '320,340p' app/src/main/java/app/gamenative/utils/ContainerStorageManager.ktRepository: utkarshdalal/GameNative
Length of output: 867
🏁 Script executed:
# Get context around lines 846-860
sed -n '840,865p' app/src/main/java/app/gamenative/utils/ContainerStorageManager.ktRepository: utkarshdalal/GameNative
Length of output: 957
🏁 Script executed:
# Find the implementation of relinkActiveSymlinkIfNeeded
rg -n "fun relinkActiveSymlinkIfNeeded" app/src/main/java/app/gamenative/utils/ContainerStorageManager.ktRepository: utkarshdalal/GameNative
Length of output: 158
Check if activeLink points to the active container before deleting containerDir.
The current implementation calls relinkActiveSymlinkIfNeeded() after containerDir is deleted. Inside that function, the check activeLink.exists() returns false if activeLink is a dangling symlink—which it will be if it was pointing to the now-deleted container. This causes the function to return early at line 852 without fixing the broken symlink.
Per Android's implementation, java.io.File.exists() calls the access(2) syscall which dereferences symlinks and returns false for dangling symlinks (ENOENT).
Determine whether activeLink points to the container being deleted before deletion, then perform the relink operation if needed.
Also applies to: 846-860
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/app/gamenative/utils/ContainerStorageManager.kt` around
lines 329 - 330, Before removing containerDir, detect if activeLink was pointing
at that container by checking the symlink target (use
java.nio.file.Files.isSymbolicLink(activeLink.toPath()) and
Files.readSymbolicLink(activeLink.toPath()) or equivalent) and compare that
target to containerDir (or its name/path); store that boolean (e.g.,
wasActiveTarget) and after deletion call relinkActiveSymlinkIfNeeded(homeDir,
containerDir) only if wasActiveTarget is true. Also update
relinkActiveSymlinkIfNeeded (and the other occurrence around the 846-860 block)
to rely on Files.isSymbolicLink/readSymbolicLink rather than activeLink.exists()
so dangling symlinks are detected correctly.
Comment on lines
+215
to
229
| suspend fun moveGamesFromOldPath( | ||
| sourceDir: String, | ||
| targetDir: String, | ||
| onProgressUpdate: (currentFile: String, fileProgress: Float, movedFiles: Int, totalFiles: Int) -> Unit, | ||
| onComplete: () -> Unit, | ||
| ) = withContext(Dispatchers.IO) { | ||
| moveDirectory( | ||
| sourceDir = sourceDir, | ||
| targetDir = targetDir, | ||
| onProgressUpdate = onProgressUpdate, | ||
| ) | ||
|
|
||
| withContext(Dispatchers.Main) { | ||
| onComplete() | ||
| } |
Contributor
There was a problem hiding this comment.
Don't drop the migration result in moveGamesFromOldPath().
This wrapper always fires onComplete() and still returns Unit, so callers cannot distinguish “cleanup finished” from “move succeeded.” The current SteamAppScreen caller only uses that callback to dismiss the dialog, which makes failed migrations look successful.
💡 Minimal fix
suspend fun moveGamesFromOldPath(
sourceDir: String,
targetDir: String,
onProgressUpdate: (currentFile: String, fileProgress: Float, movedFiles: Int, totalFiles: Int) -> Unit,
onComplete: () -> Unit,
- ) = withContext(Dispatchers.IO) {
- moveDirectory(
+ ): Result<Unit> = withContext(Dispatchers.IO) {
+ val result = moveDirectory(
sourceDir = sourceDir,
targetDir = targetDir,
onProgressUpdate = onProgressUpdate,
)
withContext(Dispatchers.Main) {
onComplete()
}
+
+ result
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/app/gamenative/utils/StorageUtils.kt` around lines 215 -
229, The wrapper moveGamesFromOldPath currently discards the outcome of
moveDirectory and always invokes onComplete, so change moveGamesFromOldPath to
return the migration result (e.g., Boolean or a Result type) by capturing the
return value from moveDirectory (val success = moveDirectory(...)), call
onComplete() only when success is true (wrap that call in
withContext(Dispatchers.Main) and conditionally invoke), and return success to
callers; then update callers (e.g., SteamAppScreen) to inspect the returned
Boolean/Result to distinguish success vs failure and dismiss or surface errors
accordingly.
| <string name="settings_storage_manage_subtitle">Review game and container storage in one place</string> | ||
| <string name="container_storage_title">Container & Storage Manager</string> | ||
| <string name="container_storage_subtitle">Review game and container storage, including installed games without containers</string> | ||
| <string name="container_storage_summary">%1$d items • %2$s</string> |
Contributor
There was a problem hiding this comment.
Use a plural resource for the summary count.
%1$d items will render as 1 items in the singular case. This should be a <plurals> entry and be selected with pluralStringResource(...).
🤖 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` at line 816, The current string resource
"container_storage_summary" uses a hardcoded "%1$d items" which doesn't handle
singular/plural; replace this with a <plurals> resource (e.g.
name="container_storage_summary") that provides singular and plural forms and
moves the "%2$s" (the secondary string) into the appropriate quantity strings,
then update callers to use pluralStringResource(...) (or
resources.getQuantityString(...) in non-Compose code) to select the right form
and pass the count and the second argument so "1 item" renders correctly; locate
usages of "container_storage_summary" and change them to use plural selection.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/src/main/java/app/gamenative/utils/StorageUtils.kt (2)
98-100:⚠️ Potential issue | 🟠 MajorDon't treat
visitFileFailed()as success.Continuing here lets the migration skip unreadable files and still return
Result.success(Unit).ContainerStorageManager.moveGame()then updates install metadata as if the move completed, even though part of the game stayed behind in the source tree.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/utils/StorageUtils.kt` around lines 98 - 100, The visitor currently treats visitFileFailed(file, exc) as success by returning FileVisitResult.CONTINUE which allows the move to report Result.success(Unit) even when files were unreadable; change visitFileFailed in the FileVisitor used by moveGame to stop traversal and propagate the error (e.g., return FileVisitResult.TERMINATE and surface or rethrow the IOException so Files.walkFileTree fails), and then ensure ContainerStorageManager.moveGame catches that and returns a failure Result instead of Result.success(Unit) when an I/O error occurs.
124-156:⚠️ Potential issue | 🔴 CriticalDrain the buffer fully before deleting the source file.
FileChannel.write()can write fewer bytes than remain in the buffer—the API contract explicitly allows partial writes. The current code callswrite()once per read cycle and does not drain remaining buffer data after the main loop exits, risking silent file truncation followed by permanent deletion of the source copy.💡 Minimal fix
FileChannel.open(sourceFilePath, StandardOpenOption.READ).use { sourceChannel -> FileChannel.open( targetFilePath, StandardOpenOption.CREATE, StandardOpenOption.WRITE, StandardOpenOption.TRUNCATE_EXISTING, ).use { targetChannel -> val buffer = ByteBuffer.allocateDirect(8 * 1024 * 1024) var bytesRead: Int while (sourceChannel.read(buffer).also { bytesRead = it } > 0) { buffer.flip() + while (buffer.hasRemaining()) { + targetChannel.write(buffer) + } - targetChannel.write(buffer) buffer.compact() @@ } } + + buffer.flip() + while (buffer.hasRemaining()) { + targetChannel.write(buffer) + } targetChannel.force(true) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/utils/StorageUtils.kt` around lines 124 - 156, The loop reads from sourceChannel into a direct ByteBuffer and calls targetChannel.write(buffer) once per read, but FileChannel.write may perform partial writes so leftover buffer data can remain unwritten when the read loop exits; ensure you drain the buffer fully before deleting the source by, after each read (or after the loop), repeatedly calling targetChannel.write(buffer) until buffer.hasRemaining() is false (or using a do-while to flush after the loop ends), then call targetChannel.force(true) and only then Files.delete(sourceFilePath); refer to the FileChannel.open usages, the ByteBuffer buffer, targetChannel.write(buffer), targetChannel.force(true), and Files.delete(sourceFilePath) in StorageUtils.kt and preserve existing bytesCopied and onProgressUpdate behavior.
♻️ Duplicate comments (6)
app/src/main/java/app/gamenative/utils/StorageUtils.kt (1)
215-229:⚠️ Potential issue | 🟠 MajorReturn the migration result instead of
Unit.Because the last expression here is
withContext(Dispatchers.Main) { onComplete() }, Kotlin infers this wrapper as returningUnit; theResult<Unit>frommoveDirectory()is discarded. Callers can dismiss their UI, but they still cannot distinguish “move failed” from “cleanup finished.”💡 Minimal fix
suspend fun moveGamesFromOldPath( sourceDir: String, targetDir: String, onProgressUpdate: (currentFile: String, fileProgress: Float, movedFiles: Int, totalFiles: Int) -> Unit, onComplete: () -> Unit, - ) = withContext(Dispatchers.IO) { - moveDirectory( + ): Result<Unit> = withContext(Dispatchers.IO) { + val result = moveDirectory( sourceDir = sourceDir, targetDir = targetDir, onProgressUpdate = onProgressUpdate, ) withContext(Dispatchers.Main) { onComplete() } + + result }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/utils/StorageUtils.kt` around lines 215 - 229, The function moveGamesFromOldPath currently discards the Result returned by moveDirectory because the final expression is withContext(Dispatchers.Main) { onComplete() } which yields Unit; change moveGamesFromOldPath to return the Result<Unit> from moveDirectory (or wrap/propagate it) so callers can inspect success/failure, and ensure you still invoke onComplete() on the Main dispatcher after moveDirectory completes (but do not let the withContext block be the function's final expression that hides the Result). Locate moveGamesFromOldPath and moveDirectory to implement this propagation and adjust the function signature/return type accordingly.app/src/main/java/app/gamenative/utils/ContainerStorageManager.kt (4)
370-394:⚠️ Potential issue | 🟠 MajorDon't delete the container twice in the non-Steam uninstall paths.
GOGService.deleteGame(),EpicService.deleteGame(), andAmazonService.deleteGame()already remove the linked container. The extraremoveContainer(context, entry.containerId)is redundant, and in the GOG branch it can target a different container because the service receivesnormalizedContainerIdwhile the local cleanup uses the rawentry.containerId(for example, duplicate-suffixed IDs).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/utils/ContainerStorageManager.kt` around lines 370 - 394, The uninstall branches for GameSource.GOG, GameSource.EPIC, and GameSource.AMAZON are calling removeContainer(context, entry.containerId) after invoking GOGService.deleteGame(), EpicService.deleteGame(), and AmazonService.deleteGame(), but those service methods already remove the linked container (and GOG uses normalizedContainerId), so remove the redundant local container removal: delete the conditional removeContainer(...) calls in the GOG, EPIC, and AMAZON branches (while preserving the entry.hasContainer checks if still needed for other flows) to avoid double-deletion and potential mismatched container IDs (normalizedContainerId vs entry.containerId).
171-305:⚠️ Potential issue | 🟠 MajorWrap the move/update path in
try/catchso this always returns aResult.After the early validation branches, this function still does DAO reads/updates and filesystem work without a catch. Any thrown exception bypasses the
Resultcontract entirely, which meansContainerStorageManagerDialog.startMove()can fail before it clearsmovingEntryNameor shows an error snackbar.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/utils/ContainerStorageManager.kt` around lines 171 - 305, The moveGame suspend function currently performs filesystem operations and DAO updates (StorageUtils.moveDirectory, StorageUtils.getFolderSize, entryPoint.gogGameDao().update, entryPoint.epicGameDao().update, entryPoint.amazonGameDao().markAsInstalled, PluviaApp.events.emitJava, Timber logs) without catching exceptions; wrap the main move/update block (starting before calling StorageUtils.moveDirectory through the final Timber success log and Result.success) in a try/catch (catch Throwable) that logs the error and returns Result.failure(exception) so any thrown exception is converted into a Result and the caller’s contract is preserved.
165-168:⚠️ Potential issue | 🟠 MajorAllow moves back to internal even when external storage is disabled.
These checks hide and reject the only recovery path for games that are already on external storage. Only moves to external should depend on
isExternalStorageConfigured().💡 Minimal fix
fun canMoveToInternal(context: Context, entry: Entry): Boolean { return canMoveGame(entry) && - isExternalStorageConfigured() && getStorageLocation(context, entry) == StorageLocation.EXTERNAL } @@ - if (!isExternalStorageConfigured()) { + if (target == MoveTarget.EXTERNAL && !isExternalStorageConfigured()) { return@withContext Result.failure(IllegalStateException("External storage is not enabled")) }Also applies to: 184-185
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/utils/ContainerStorageManager.kt` around lines 165 - 168, The canMoveToInternal check wrongly blocks moving games back to internal when external storage is disabled; change the function canMoveToInternal to remove the isExternalStorageConfigured() requirement so it returns canMoveGame(entry) && getStorageLocation(context, entry) == StorageLocation.EXTERNAL, and apply the same fix to the duplicate check elsewhere (the other occurrence of the move-to-internal logic) so moves back to internal are allowed regardless of isExternalStorageConfigured().
322-330:⚠️ Potential issue | 🟠 MajorDetermine whether the active symlink points at this container before deleting it.
relinkActiveSymlinkIfNeeded()usesactiveLink.exists(), which is false for a dangling symlink. AftercontainerDiris deleted, a link that used to point to it is already dangling, sopointsToDeletedbecomes false and the brokenhome/<user>symlink is left behind.Also applies to: 846-852
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/utils/ContainerStorageManager.kt` around lines 322 - 330, Before deleting the containerDir, determine whether the active symlink currently points to this container (accounting for dangling symlinks) and pass that info to relinkActiveSymlinkIfNeeded; specifically, replace the current post-delete check that relies on activeLink.exists() with pre-delete logic that checks Files.isSymbolicLink(activeLink) and resolves/reads its target (e.g., Files.readSymbolicLink or activeLink.toRealPath/resolve) to compare to containerDir (or its canonical/normalized Path) so pointsToDeleted is computed before FileUtils.delete(containerDir) and then use that value when calling relinkActiveSymlinkIfNeeded(homeDir, containerDir, pointsToDeleted) or otherwise call relinkActiveSymlinkIfNeededConditionally if it accepts the flag.app/src/main/java/app/gamenative/service/amazon/AmazonConstants.kt (1)
54-64:⚠️ Potential issue | 🟠 MajorKeep these path helpers side-effect free.
internalAmazonGamesPath()/externalAmazonGamesPath()are reused from storage-location checks, so themkdirs()calls turn a read-only inventory scan into writes and can create emptyAmazondirectories while the dialog is just rendering.externalAmazonGamesPath()also still builds a path fromPrefManager.externalStoragePathwithout rejecting blank/uninitialized values. Move directory creation to the actual install/move call sites and validate the configured external root before constructing the path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/service/amazon/AmazonConstants.kt` around lines 54 - 64, internalAmazonGamesPath() and externalAmazonGamesPath() should be side‑effect free: remove the File(...).mkdirs() calls from both helpers so they only build and return paths, and do not create directories during read/scan operations; additionally, in externalAmazonGamesPath() validate PrefManager.externalStoragePath (reject if null/blank) before constructing/returning the path (return an empty/null/optional result to signal invalid config), and move any directory creation logic into the actual install/move call sites that perform writes so those callers explicitly create dirs when needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@app/src/main/java/app/gamenative/utils/StorageUtils.kt`:
- Around line 98-100: The visitor currently treats visitFileFailed(file, exc) as
success by returning FileVisitResult.CONTINUE which allows the move to report
Result.success(Unit) even when files were unreadable; change visitFileFailed in
the FileVisitor used by moveGame to stop traversal and propagate the error
(e.g., return FileVisitResult.TERMINATE and surface or rethrow the IOException
so Files.walkFileTree fails), and then ensure ContainerStorageManager.moveGame
catches that and returns a failure Result instead of Result.success(Unit) when
an I/O error occurs.
- Around line 124-156: The loop reads from sourceChannel into a direct
ByteBuffer and calls targetChannel.write(buffer) once per read, but
FileChannel.write may perform partial writes so leftover buffer data can remain
unwritten when the read loop exits; ensure you drain the buffer fully before
deleting the source by, after each read (or after the loop), repeatedly calling
targetChannel.write(buffer) until buffer.hasRemaining() is false (or using a
do-while to flush after the loop ends), then call targetChannel.force(true) and
only then Files.delete(sourceFilePath); refer to the FileChannel.open usages,
the ByteBuffer buffer, targetChannel.write(buffer), targetChannel.force(true),
and Files.delete(sourceFilePath) in StorageUtils.kt and preserve existing
bytesCopied and onProgressUpdate behavior.
---
Duplicate comments:
In `@app/src/main/java/app/gamenative/service/amazon/AmazonConstants.kt`:
- Around line 54-64: internalAmazonGamesPath() and externalAmazonGamesPath()
should be side‑effect free: remove the File(...).mkdirs() calls from both
helpers so they only build and return paths, and do not create directories
during read/scan operations; additionally, in externalAmazonGamesPath() validate
PrefManager.externalStoragePath (reject if null/blank) before
constructing/returning the path (return an empty/null/optional result to signal
invalid config), and move any directory creation logic into the actual
install/move call sites that perform writes so those callers explicitly create
dirs when needed.
In `@app/src/main/java/app/gamenative/utils/ContainerStorageManager.kt`:
- Around line 370-394: The uninstall branches for GameSource.GOG,
GameSource.EPIC, and GameSource.AMAZON are calling removeContainer(context,
entry.containerId) after invoking GOGService.deleteGame(),
EpicService.deleteGame(), and AmazonService.deleteGame(), but those service
methods already remove the linked container (and GOG uses
normalizedContainerId), so remove the redundant local container removal: delete
the conditional removeContainer(...) calls in the GOG, EPIC, and AMAZON branches
(while preserving the entry.hasContainer checks if still needed for other flows)
to avoid double-deletion and potential mismatched container IDs
(normalizedContainerId vs entry.containerId).
- Around line 171-305: The moveGame suspend function currently performs
filesystem operations and DAO updates (StorageUtils.moveDirectory,
StorageUtils.getFolderSize, entryPoint.gogGameDao().update,
entryPoint.epicGameDao().update, entryPoint.amazonGameDao().markAsInstalled,
PluviaApp.events.emitJava, Timber logs) without catching exceptions; wrap the
main move/update block (starting before calling StorageUtils.moveDirectory
through the final Timber success log and Result.success) in a try/catch (catch
Throwable) that logs the error and returns Result.failure(exception) so any
thrown exception is converted into a Result and the caller’s contract is
preserved.
- Around line 165-168: The canMoveToInternal check wrongly blocks moving games
back to internal when external storage is disabled; change the function
canMoveToInternal to remove the isExternalStorageConfigured() requirement so it
returns canMoveGame(entry) && getStorageLocation(context, entry) ==
StorageLocation.EXTERNAL, and apply the same fix to the duplicate check
elsewhere (the other occurrence of the move-to-internal logic) so moves back to
internal are allowed regardless of isExternalStorageConfigured().
- Around line 322-330: Before deleting the containerDir, determine whether the
active symlink currently points to this container (accounting for dangling
symlinks) and pass that info to relinkActiveSymlinkIfNeeded; specifically,
replace the current post-delete check that relies on activeLink.exists() with
pre-delete logic that checks Files.isSymbolicLink(activeLink) and resolves/reads
its target (e.g., Files.readSymbolicLink or activeLink.toRealPath/resolve) to
compare to containerDir (or its canonical/normalized Path) so pointsToDeleted is
computed before FileUtils.delete(containerDir) and then use that value when
calling relinkActiveSymlinkIfNeeded(homeDir, containerDir, pointsToDeleted) or
otherwise call relinkActiveSymlinkIfNeededConditionally if it accepts the flag.
In `@app/src/main/java/app/gamenative/utils/StorageUtils.kt`:
- Around line 215-229: The function moveGamesFromOldPath currently discards the
Result returned by moveDirectory because the final expression is
withContext(Dispatchers.Main) { onComplete() } which yields Unit; change
moveGamesFromOldPath to return the Result<Unit> from moveDirectory (or
wrap/propagate it) so callers can inspect success/failure, and ensure you still
invoke onComplete() on the Main dispatcher after moveDirectory completes (but do
not let the withContext block be the function's final expression that hides the
Result). Locate moveGamesFromOldPath and moveDirectory to implement this
propagation and adjust the function signature/return type accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 48fe8b18-1e26-4d96-b929-f457af62a8ce
📒 Files selected for processing (6)
app/src/main/java/app/gamenative/service/amazon/AmazonConstants.ktapp/src/main/java/app/gamenative/ui/screen/settings/ContainerStorageManagerDialog.ktapp/src/main/java/app/gamenative/ui/screen/settings/SettingsGroupInterface.ktapp/src/main/java/app/gamenative/utils/ContainerStorageManager.ktapp/src/main/java/app/gamenative/utils/StorageUtils.ktapp/src/main/res/values/strings.xml
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/res/values/strings.xml
Contributor
Author
|
drafting for now, in case we go with the already combined version |
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.
also can identify and clean up orphaned containers!
Summary by cubic
Adds a Container & Storage Manager to review, move, and reclaim space across containers and game installs, with uninstall options. Open from Settings > Downloads > Storage Manager.
New Features
Bug Fixes
Written for commit 7cd1455. Summary will update on new commits.
Summary by CodeRabbit
New Features
Chores
Tests