Fix List layout crash with external storage - #1758
Open
Meloon33 wants to merge 8 commits into
Open
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough
ChangesLibrary and container integration
Estimated code review effort: 3 (Moderate) | ~25 minutes 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
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/com/winlator/container/ContainerManager.java (1)
35-49: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winEnforce the
ContainerManagersingleton for all callers.
getInstance()creates the managed instance, but multiple existing directnew ContainerManager(context)calls create stale/independentcontainerslists. Make the constructor private and update the remaining call sites inAdrenotoolsManager,ContainerUtils,PluviaMain,XServerScreen, andImageFsInstallerto usegetInstance(), or explicitly define synchronization for independently created managers.🤖 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/com/winlator/container/ContainerManager.java` around lines 35 - 49, Make ContainerManager’s constructor private so all access goes through getInstance(Context) and shares the managed containers state. Update direct construction sites in AdrenotoolsManager, ContainerUtils, PluviaMain, XServerScreen, and ImageFsInstaller to call ContainerManager.getInstance(...) instead, preserving each caller’s available context.
🤖 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/library/components/LibraryListCard.kt`:
- Around line 118-123: Update ListViewCard to accept the existing
imageRefreshCounter and pass it into the card’s icon-resolution flow. Include
imageRefreshCounter as an additional key for the produceState that initializes
iconUrl, so custom-game icon resolution retries after a refresh while preserving
the current appId and clientIconUrl keys.
In `@app/src/main/java/com/winlator/container/ContainerManager.java`:
- Around line 35-42: Make ContainerManager.getInstance(Context) thread-safe by
synchronizing singleton initialization, or by making instance volatile and
implementing correct double-checked locking. Ensure only one ContainerManager is
constructed and its ArrayList<Container> is fully initialized before
publication, so concurrent callers receive the same instance and container list.
---
Outside diff comments:
In `@app/src/main/java/com/winlator/container/ContainerManager.java`:
- Around line 35-49: Make ContainerManager’s constructor private so all access
goes through getInstance(Context) and shares the managed containers state.
Update direct construction sites in AdrenotoolsManager, ContainerUtils,
PluviaMain, XServerScreen, and ImageFsInstaller to call
ContainerManager.getInstance(...) instead, preserving each caller’s available
context.
🪄 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
Run ID: 600508b7-2518-4139-886b-2ae20e5276a7
📒 Files selected for processing (8)
app/src/main/java/app/gamenative/data/LibraryItem.ktapp/src/main/java/app/gamenative/service/SteamService.ktapp/src/main/java/app/gamenative/ui/screen/library/appscreen/CustomGameAppScreen.ktapp/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.ktapp/src/main/java/app/gamenative/ui/screen/library/components/LibraryListCard.ktapp/src/main/java/app/gamenative/ui/screen/library/components/LibraryListPane.ktapp/src/main/java/app/gamenative/utils/CustomGameScanner.ktapp/src/main/java/com/winlator/container/ContainerManager.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.
1 issue 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/utils/CustomGameScanner.kt">
<violation number="1" location="app/src/main/java/app/gamenative/utils/CustomGameScanner.kt:176">
P2: Concurrent icon resolutions can both observe a null singleton and each rescan `home/`, defeating this change's per-item scan avoidance. Make `ContainerManager.getInstance` safely synchronized (or initialize it atomically) so all callers share one loaded manager.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| // 2) Try extracting from the selected container executable | ||
| try { | ||
| val cm = ContainerManager(context) | ||
| val cm = ContainerManager.getInstance(context) |
Contributor
There was a problem hiding this comment.
P2: Concurrent icon resolutions can both observe a null singleton and each rescan home/, defeating this change's per-item scan avoidance. Make ContainerManager.getInstance safely synchronized (or initialize it atomically) so all callers share one loaded manager.
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/CustomGameScanner.kt, line 176:
<comment>Concurrent icon resolutions can both observe a null singleton and each rescan `home/`, defeating this change's per-item scan avoidance. Make `ContainerManager.getInstance` safely synchronized (or initialize it atomically) so all callers share one loaded manager.</comment>
<file context>
@@ -173,7 +173,7 @@ object CustomGameScanner {
// 2) Try extracting from the selected container executable
try {
- val cm = ContainerManager(context)
+ val cm = ContainerManager.getInstance(context)
if (cm.hasContainer(appId)) {
val container = cm.getContainerById(appId)
</file context>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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
@.artifacts/9266bc76-2492-4ea3-87e8-b690e9961ac9/implementation_plan.artifact.md:
- Around line 14-31: Replace all machine-local file:///E:/workspace/... links
with repository-relative links in
.artifacts/9266bc76-2492-4ea3-87e8-b690e9961ac9/implementation_plan.artifact.md
lines 14-31 and 37-46, and
.artifacts/9266bc76-2492-4ea3-87e8-b690e9961ac9/walkthrough.artifact.md lines
9-20; preserve each link’s referenced repository path and section target while
making them portable for all reviewers.
In @.artifacts/9266bc76-2492-4ea3-87e8-b690e9961ac9/task.artifact.md:
- Around line 1-10: Update the task tracker in task.artifact.md to reflect the
delivered changes: mark all Step 1 subtasks as complete, and mark Steps 2 and 3
as complete because the LibraryListCard asynchronous icon loading and
LibraryListPane layout fixes are included. Keep the existing task names and
structure unchanged.
🪄 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: b37c6298-f49c-45aa-8005-34b522f2b0ee
📒 Files selected for processing (13)
.artifacts/9266bc76-2492-4ea3-87e8-b690e9961ac9/implementation_plan.artifact.md.artifacts/9266bc76-2492-4ea3-87e8-b690e9961ac9/task.artifact.md.artifacts/9266bc76-2492-4ea3-87e8-b690e9961ac9/walkthrough.artifact.mdapp/src/main/java/app/gamenative/ui/PluviaMain.ktapp/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.ktapp/src/main/java/app/gamenative/ui/screen/library/components/LibraryAppItem.ktapp/src/main/java/app/gamenative/ui/screen/library/components/LibraryListCard.ktapp/src/main/java/app/gamenative/ui/screen/library/components/LibraryListPane.ktapp/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.ktapp/src/main/java/app/gamenative/utils/ContainerUtils.ktapp/src/main/java/com/winlator/container/ContainerManager.javaapp/src/main/java/com/winlator/contents/AdrenotoolsManager.javaapp/src/main/java/com/winlator/xenvironment/ImageFsInstaller.java
💤 Files with no reviewable changes (1)
- app/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- app/src/main/java/app/gamenative/ui/screen/library/components/LibraryListCard.kt
- app/src/main/java/com/winlator/container/ContainerManager.java
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 13 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…_plan.artifact.md Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
…orage Crash - Enforce ContainerManager singleton (final class, private constructor) - Fix race condition in CustomGameCache with synchronization - Fix overlapping horizontal dividers in LIST layout - Add robust exists() checks and error handling for external storage - Increase KDoc/Javadoc coverage to >80% - Replace absolute machine paths with relative links
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/CustomGameCache.kt`:
- Around line 63-73: Update getOrRebuildCache to snapshot
getManualFolders().toSet() once, use that snapshot for the cache-change
comparison, pass it into buildCache instead of calling getManualFolders again,
and assign the same snapshot to cacheManualFolders.
🪄 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: 5fc9f8ac-38a2-40c6-86d8-48ff6a1efa3f
📒 Files selected for processing (8)
.artifacts/9266bc76-2492-4ea3-87e8-b690e9961ac9/implementation_plan.artifact.md.artifacts/9266bc76-2492-4ea3-87e8-b690e9961ac9/task.artifact.md.artifacts/9266bc76-2492-4ea3-87e8-b690e9961ac9/walkthrough.artifact.mdapp/src/main/java/app/gamenative/ui/screen/library/components/LibraryListCard.ktapp/src/main/java/app/gamenative/ui/screen/library/components/LibraryListPane.ktapp/src/main/java/app/gamenative/utils/CustomGameCache.ktapp/src/main/java/app/gamenative/utils/CustomGameScanner.ktapp/src/main/java/com/winlator/container/ContainerManager.java
🚧 Files skipped from review as they are similar to previous changes (4)
- app/src/main/java/app/gamenative/ui/screen/library/components/LibraryListPane.kt
- app/src/main/java/com/winlator/container/ContainerManager.java
- app/src/main/java/app/gamenative/utils/CustomGameScanner.kt
- app/src/main/java/app/gamenative/ui/screen/library/components/LibraryListCard.kt
Comment on lines
63
to
73
| fun getOrRebuildCache( | ||
| getManualFolders: () -> Set<String>, | ||
| readGameIdFromFile: (File) -> Int? | ||
| ): Map<Int, String> { | ||
| ): Map<Int, String> = synchronized(this) { | ||
| val currentManualFolders = getManualFolders() | ||
| val cachedManual = cacheManualFolders | ||
|
|
||
| // Rebuild if manual folders changed or cache is null | ||
| if (appIdCache == null || cachedManual != currentManualFolders) { | ||
| appIdCache = buildCache(getManualFolders, readGameIdFromFile) | ||
| cacheManualFolders = currentManualFolders |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Build and fingerprint the cache from one folder snapshot.
getManualFolders() is evaluated at Line 67 and again inside buildCache() at Line 72. During an external-storage update, those calls can observe different sets, leaving appIdCache inconsistent with cacheManualFolders. Snapshot once with getManualFolders().toSet(), use that snapshot for the build, and store the same snapshot.
Proposed fix
- val currentManualFolders = getManualFolders()
+ val currentManualFolders = getManualFolders().toSet()
...
- appIdCache = buildCache(getManualFolders, readGameIdFromFile)
+ appIdCache = buildCache({ currentManualFolders }, readGameIdFromFile)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| fun getOrRebuildCache( | |
| getManualFolders: () -> Set<String>, | |
| readGameIdFromFile: (File) -> Int? | |
| ): Map<Int, String> { | |
| ): Map<Int, String> = synchronized(this) { | |
| val currentManualFolders = getManualFolders() | |
| val cachedManual = cacheManualFolders | |
| // Rebuild if manual folders changed or cache is null | |
| if (appIdCache == null || cachedManual != currentManualFolders) { | |
| appIdCache = buildCache(getManualFolders, readGameIdFromFile) | |
| cacheManualFolders = currentManualFolders | |
| fun getOrRebuildCache( | |
| getManualFolders: () -> Set<String>, | |
| readGameIdFromFile: (File) -> Int? | |
| ): Map<Int, String> = synchronized(this) { | |
| val currentManualFolders = getManualFolders().toSet() | |
| val cachedManual = cacheManualFolders | |
| // Rebuild if manual folders changed or cache is null | |
| if (appIdCache == null || cachedManual != currentManualFolders) { | |
| appIdCache = buildCache({ currentManualFolders }, readGameIdFromFile) | |
| cacheManualFolders = currentManualFolders |
🤖 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/CustomGameCache.kt` around lines 63 -
73, Update getOrRebuildCache to snapshot getManualFolders().toSet() once, use
that snapshot for the cache-change comparison, pass it into buildCache instead
of calling getManualFolders again, and assign the same snapshot to
cacheManualFolders.
Contributor
There was a problem hiding this comment.
3 issues found across 8 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/CustomGameCache.kt">
<violation number="1" location="app/src/main/java/app/gamenative/utils/CustomGameCache.kt:22">
P3: Generated documentation now contains duplicate consecutive KDoc blocks for `buildCache` and `getOrRebuildCache`, making this small API harder to maintain. Keep the expanded block and remove the older duplicate blocks.</violation>
<violation number="2" location="app/src/main/java/app/gamenative/utils/CustomGameCache.kt:33">
P2: `getManualFolders()` is evaluated once here to populate `currentManualFolders`, and then the same lambda is passed through to `buildCache()` which invokes it again independently. If external storage changes between those two evaluations, the cache will be built from a different folder set than what gets stored in `cacheManualFolders`, breaking the staleness check on subsequent calls. Consider passing the already-captured `currentManualFolders` (e.g., as `{ currentManualFolders }`) to `buildCache` so the same snapshot is used consistently.</violation>
</file>
<file name="app/src/main/java/com/winlator/container/ContainerManager.java">
<violation number="1" location="app/src/main/java/com/winlator/container/ContainerManager.java:33">
P3: The class-level Javadoc uses `[getInstance(Context)]` with square brackets, but this is not standard Javadoc link syntax — brackets render as literal text. Use `{@link #getInstance(Context)}` to generate a proper hyperlink to the method.
(Kotlin uses bracket references in KDoc, but this is a Java file and standard Javadoc requires `{@link ...}` syntax.)</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| @@ -10,19 +10,29 @@ import timber.log.Timber | |||
| */ | |||
Contributor
There was a problem hiding this comment.
P2: getManualFolders() is evaluated once here to populate currentManualFolders, and then the same lambda is passed through to buildCache() which invokes it again independently. If external storage changes between those two evaluations, the cache will be built from a different folder set than what gets stored in cacheManualFolders, breaking the staleness check on subsequent calls. Consider passing the already-captured currentManualFolders (e.g., as { currentManualFolders }) to buildCache so the same snapshot is used consistently.
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/CustomGameCache.kt, line 33:
<comment>`getManualFolders()` is evaluated once here to populate `currentManualFolders`, and then the same lambda is passed through to `buildCache()` which invokes it again independently. If external storage changes between those two evaluations, the cache will be built from a different folder set than what gets stored in `cacheManualFolders`, breaking the staleness check on subsequent calls. Consider passing the already-captured `currentManualFolders` (e.g., as `{ currentManualFolders }`) to `buildCache` so the same snapshot is used consistently.</comment>
<file context>
@@ -10,19 +10,29 @@ import timber.log.Timber
getManualFolders: () -> Set<String>,
readGameIdFromFile: (File) -> Int?
- ): Map<Int, String> {
+ ): Map<Int, String> = synchronized(this) {
val cache = mutableMapOf<Int, String>()
-
</file context>
| * Builds the appId cache by scanning all Custom Game manual folders. | ||
| * Returns a map of appId (Int) -> folder path (String). | ||
| */ | ||
| /** |
Contributor
There was a problem hiding this comment.
P3: Generated documentation now contains duplicate consecutive KDoc blocks for buildCache and getOrRebuildCache, making this small API harder to maintain. Keep the expanded block and remove the older duplicate blocks.
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/CustomGameCache.kt, line 22:
<comment>Generated documentation now contains duplicate consecutive KDoc blocks for `buildCache` and `getOrRebuildCache`, making this small API harder to maintain. Keep the expanded block and remove the older duplicate blocks.</comment>
<file context>
@@ -10,19 +10,29 @@ import timber.log.Timber
* Builds the appId cache by scanning all Custom Game manual folders.
* Returns a map of appId (Int) -> folder path (String).
*/
+ /**
+ * Builds the appId cache by scanning all Custom Game manual folders.
+ * Returns a map of appId (Int) -> folder path (String).
</file context>
| public class ContainerManager { | ||
| /** | ||
| * Manages Wine containers, including their creation, duplication, and lifecycle. | ||
| * This class follows the Singleton pattern and should be accessed via [getInstance(Context)]. |
Contributor
There was a problem hiding this comment.
P3: The class-level Javadoc uses [getInstance(Context)] with square brackets, but this is not standard Javadoc link syntax — brackets render as literal text. Use {@link #getInstance(Context)} to generate a proper hyperlink to the method.
(Kotlin uses bracket references in KDoc, but this is a Java file and standard Javadoc requires {@link ...} syntax.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/com/winlator/container/ContainerManager.java, line 33:
<comment>The class-level Javadoc uses `[getInstance(Context)]` with square brackets, but this is not standard Javadoc link syntax — brackets render as literal text. Use `{@link #getInstance(Context)}` to generate a proper hyperlink to the method.
(Kotlin uses bracket references in KDoc, but this is a Java file and standard Javadoc requires `{@link ...}` syntax.)</comment>
<file context>
@@ -28,12 +28,22 @@
-public class ContainerManager {
+/**
+ * Manages Wine containers, including their creation, duplication, and lifecycle.
+ * This class follows the Singleton pattern and should be accessed via [getInstance(Context)].
+ */
+public final class ContainerManager {
</file context>
Suggested change
| * This class follows the Singleton pattern and should be accessed via [getInstance(Context)]. | |
| * This class follows the Singleton pattern and should be accessed via {@link #getInstance(Context)}. |
Contributor
There was a problem hiding this comment.
1 issue found across 4 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/CustomGameCache.kt">
<violation number="1" location="app/src/main/java/app/gamenative/utils/CustomGameCache.kt:67">
P3: Cached lookups now allocate and copy the full manual-folder set unnecessarily. `PrefManager.customGameManualFolders` already returns a fresh decoded snapshot; retaining it avoids extra work on the hot lookup path.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| ): Map<Int, String> { | ||
| val currentManualFolders = getManualFolders() | ||
| ): Map<Int, String> = synchronized(this) { | ||
| val currentManualFolders = getManualFolders().toSet() |
Contributor
There was a problem hiding this comment.
P3: Cached lookups now allocate and copy the full manual-folder set unnecessarily. PrefManager.customGameManualFolders already returns a fresh decoded snapshot; retaining it avoids extra work on the hot lookup 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/utils/CustomGameCache.kt, line 67:
<comment>Cached lookups now allocate and copy the full manual-folder set unnecessarily. `PrefManager.customGameManualFolders` already returns a fresh decoded snapshot; retaining it avoids extra work on the hot lookup path.</comment>
<file context>
@@ -64,7 +64,7 @@ internal object CustomGameCache {
readGameIdFromFile: (File) -> Int?
): Map<Int, String> = synchronized(this) {
- val currentManualFolders = getManualFolders()
+ val currentManualFolders = getManualFolders().toSet()
val cachedManual = cacheManualFolders
</file context>
Suggested change
| val currentManualFolders = getManualFolders().toSet() | |
| val currentManualFolders = getManualFolders() |
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
Resolved a critical application crash occurring specifically in the "List" layout when custom games from external storage were added or accessed. The primary cause was synchronous filesystem I/O performed on the UI thread within the
LibraryItemdata model, which triggered ANRs and crashes—especially during external storage connection/disconnection events.Key Changes:
produceStateblock inLibraryListCard. This ensures the library UI remains responsive regardless of storage speed or mount status.ContainerManagerto use a singleton pattern (getInstance). This avoids redundant disk scans of the internalhome/directory for every list item, significantly reducing overhead in the "List" layout.LibraryListPanewhere horizontal dividers were drawn inside the same grid cell as item cards, causing visual overlap and touch interference.CustomGameScannerandContainerManagerwith lifecycle safeguards to gracefully handle missing paths or pending permissions when external drives are hot-plugged.LibraryItemby removing logic from its getters, preventing accidental main-thread I/O during recompositions.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.CONTRIBUTING.md.Summary by cubic
Fixes a crash in the List layout when external storage is connected by moving icon loading off the UI thread, improving storage error handling, and fixing list dividers. Also makes
ContainerManagera thread-safe singleton and synchronizesCustomGameCacheto prevent redundant scans and race conditions.Bug Fixes
LibraryListCard(start empty; keyed byimageRefreshCounter) and block duplicate icon extractions.LibraryItemfor custom games; resolve icons in UI only.HorizontalDivideroutside item cards with horizontal padding and theme color (also for skeletons) to prevent overlap and focus/touch issues.CustomGameScanner(manual folders, metadata writes, scanning) to handle hot-plugged storage safely.appInfo.isInstalledin list badge to avoid unnecessary status recomputation.Performance
ContainerManagera synchronized singleton (finalclass, private constructor,getInstance) with thread-safe container list; update usages across the app (SteamService,PluviaMain,XServerScreen,AdrenotoolsManager,ImageFsInstaller, etc.).CustomGameCache(@Volatilefields; synchronizedgetOrRebuildCache,addEntry,invalidate) to stop concurrent cache rebuilds and redundant disk scans.Written for commit 56de92b. Summary will update on new commits.
Summary by CodeRabbit