Workshop Update: Manual Mod Folder Dialog#1072
Open
Nightwalker743 wants to merge 5 commits intoutkarshdalal:masterfrom
Open
Workshop Update: Manual Mod Folder Dialog#1072Nightwalker743 wants to merge 5 commits intoutkarshdalal:masterfrom
Nightwalker743 wants to merge 5 commits intoutkarshdalal:masterfrom
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:
📝 WalkthroughWalkthroughAdds a per-game workshop mod-folder override with a folder-picker UI and persistence, enriches workshop item metadata, auto-extracts single-archive mods in-place, and updates symlink detection/creation and strategy caching with a new stdSeen signal. Changes
Sequence DiagramsequenceDiagram
participant User
participant UI as WorkshopManagerDialog
participant Picker as FolderPickerDialog
participant Screen as SteamAppScreen
participant Container as Container Extras
participant Manager as WorkshopManager
User->>UI: Click "Override Mod Folder"
UI->>Picker: Open (gameRootDir, winePrefix)
Picker->>Picker: Build roots, list dirs, show breadcrumb
User->>Picker: Navigate & Select folder
Picker->>Screen: onModPathChanged(selectedPath)
Screen->>Container: putExtra("workshopModPath", selectedPath) / saveData()
Screen->>Manager: configureSymlinksForApp(..., workshopModPath=selectedPath)
Manager->>Manager: Bypass auto-detection, create symlinks into override path
Manager->>Manager: extractZipMods() may detect & extract ZIPs in-place
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
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.
4 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/ui/component/dialog/WorkshopManagerDialog.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/component/dialog/WorkshopManagerDialog.kt:597">
P2: Folder picker performs unbounded full directory listing/sort without protective error handling, risking stalls and unstable behavior on large/restricted folders.</violation>
</file>
<file name="app/src/main/java/app/gamenative/workshop/WorkshopManager.kt">
<violation number="1" location="app/src/main/java/app/gamenative/workshop/WorkshopManager.kt:552">
P1: ZIP extraction of external workshop content has no decompression limits, allowing zip-bomb style disk/IO exhaustion.</violation>
<violation number="2" location="app/src/main/java/app/gamenative/workshop/WorkshopManager.kt:3178">
P2: Stale symlink cleanup uses unresolved symlink target strings, so relative/normalized links to workshop content may be missed and left behind.</violation>
<violation number="3" location="app/src/main/java/app/gamenative/workshop/WorkshopManager.kt:3399">
P1: Container-creation failure is ignored, so download continues into a path that can later be orphan-cleaned and delete freshly downloaded workshop data.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
app/src/main/java/app/gamenative/ui/component/dialog/WorkshopManagerDialog.kt
Outdated
Show resolved
Hide resolved
![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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/app/gamenative/workshop/WorkshopModPathDetector.kt (1)
46-61:⚠️ Potential issue | 🟠 MajorThese folder names are too generic to drive an install-path decision.
collectModsDirectories()and the binary path matcher both treatALL_MOD_DIR_NAMEShits as actionable. Withtextures,sounds,audio,content,assets, and especiallydatain that set, ordinary asset references now become candidates; becauseresolveInstallPath()returns the first matching segment, a path likeData/Mods/...now resolves toinstallDir/Datainstead of the actualModsfolder. Keep these names out of the fast path, or require a second corroborating signal before returningSymlinkIntoDir.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/workshop/WorkshopModPathDetector.kt` around lines 46 - 61, The list ALL_MOD_DIR_NAMES contains overly generic entries (e.g. "textures", "sounds", "audio", "content", "assets", "data") that are causing false positives in collectModsDirectories() and resolveInstallPath() — update the code to remove these generic names from the fast-path sets (MEDIUM_CONFIDENCE_NAMES/LOW_CONFIDENCE_NAMES or at least exclude them from ALL_MOD_DIR_NAMES) or change the decision logic in collectModsDirectories() / resolveInstallPath() so that a match on a low-confidence name requires a second corroborating signal (e.g., presence of a high-confidence sibling like "Mods"/"workshop" or a binary path matcher hit) before returning SymlinkIntoDir; locate symbols MEDIUM_CONFIDENCE_NAMES, LOW_CONFIDENCE_NAMES, ALL_MOD_DIR_NAMES, collectModsDirectories, resolveInstallPath, and SymlinkIntoDir to implement the change.
🧹 Nitpick comments (1)
app/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.kt (1)
1246-1257: Avoid a second container-ID source of truth here.The rest of
SteamAppScreenalready treatslibraryItem.appIdas the canonical key for this container. ReconstructingSTEAM_$gameIdjust for the new workshop-path flow makes the override state depend on a separate ID format.♻️ Suggested cleanup
- val containerId = "STEAM_$gameId" + val containerId = libraryItem.appIdAlso applies to: 1265-1268, 1310-1313
🤖 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/library/appscreen/SteamAppScreen.kt` around lines 1246 - 1257, The code creates a local containerId ("STEAM_$gameId") causing a second source of truth; replace uses of the reconstructed containerId with the canonical libraryItem.appId to ensure the workshop-path override state ties to the same key as the rest of SteamAppScreen. Update places that set containerId and calls to ContainerUtils.getContainer(context, containerId) (used when computing wsWinePrefix and any other workshop-path logic around workshopModPath/wsGameRootDir) to use libraryItem.appId instead, removing the duplicated "STEAM_$gameId" construction.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app/src/main/java/app/gamenative/ui/component/dialog/WorkshopManagerDialog.kt`:
- Around line 563-570: The current userDir selection in WorkshopManagerDialog.kt
can pick an unstable first entry from usersDir.listFiles(); change the logic to
prefer the explicit "steamuser" directory under drive_c/users if it exists and
is a directory, and only if it does not exist fall back to scanning
usersDir.listFiles() for the first directory not named "Public" (or default to
File(usersDir, "steamuser") if none found); update the userDir assignment that
uses usersDir, winePrefix and userDir to implement this deterministic
preference.
In
`@app/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.kt`:
- Around line 1309-1315: The Timber info log in the CoroutineScope block (inside
SteamAppScreen.kt where ContainerUtils.getContainer and
container.putExtra("workshopModPath", ...) are used) writes the full newPath
which may contain PII; change the log to not include the absolute path—log only
that the override was set or cleared (e.g., "workshop mod path override set" vs
"cleared"), or if you need a hint log a sanitized basename derived from newPath
(use the filename component only) instead of the full path in the
Timber.tag("Workshop").i(...) call.
In `@app/src/main/java/app/gamenative/workshop/WorkshopManager.kt`:
- Around line 512-525: The ZIP extraction gate currently requires a single
non-preview file and a permanent .zip_extracted marker; change it to detect
actual archives (look for any file whose name endsWith(".zip", ignoreCase=true)
or whose content matches ZIP magic) instead of requiring exactly one payload,
and move invocation so extraction runs after fixFileExtensions() within
runPostProcessing(); ensure File(itemDir, ".zip_extracted") is only created
after a successful extraction by extractZip(...) and treat the marker as stale
if the archive's lastModified is newer than the marker (or if extraction
previously failed) so updates/retries aren’t skipped (update logic around
File(itemDir, ".zip_extracted"), the filtering that builds contentFiles/zipFile,
and where runPostProcessing() calls fixFileExtensions()).
- Around line 2168-2202: The try/catch around symlink creation (working with
targetDir, modDirs, Files.createSymbolicLink) logs failures but leaves the
global/manual override flag in an incorrect state (willUseFilesystemMods),
causing the method to skip auto-sync; fix by capturing the current
willUseFilesystemMods value before the try, and in the catch restore that prior
value (or unset the manual override) in addition to logging via
Timber.tag(TAG).w; apply the same pattern to the other similar blocks referenced
(the catch blocks around the symlink steps at the other locations) so any
failure does not permanently force manual mode.
---
Outside diff comments:
In `@app/src/main/java/app/gamenative/workshop/WorkshopModPathDetector.kt`:
- Around line 46-61: The list ALL_MOD_DIR_NAMES contains overly generic entries
(e.g. "textures", "sounds", "audio", "content", "assets", "data") that are
causing false positives in collectModsDirectories() and resolveInstallPath() —
update the code to remove these generic names from the fast-path sets
(MEDIUM_CONFIDENCE_NAMES/LOW_CONFIDENCE_NAMES or at least exclude them from
ALL_MOD_DIR_NAMES) or change the decision logic in collectModsDirectories() /
resolveInstallPath() so that a match on a low-confidence name requires a second
corroborating signal (e.g., presence of a high-confidence sibling like
"Mods"/"workshop" or a binary path matcher hit) before returning SymlinkIntoDir;
locate symbols MEDIUM_CONFIDENCE_NAMES, LOW_CONFIDENCE_NAMES, ALL_MOD_DIR_NAMES,
collectModsDirectories, resolveInstallPath, and SymlinkIntoDir to implement the
change.
---
Nitpick comments:
In
`@app/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.kt`:
- Around line 1246-1257: The code creates a local containerId ("STEAM_$gameId")
causing a second source of truth; replace uses of the reconstructed containerId
with the canonical libraryItem.appId to ensure the workshop-path override state
ties to the same key as the rest of SteamAppScreen. Update places that set
containerId and calls to ContainerUtils.getContainer(context, containerId) (used
when computing wsWinePrefix and any other workshop-path logic around
workshopModPath/wsGameRootDir) to use libraryItem.appId instead, removing the
duplicated "STEAM_$gameId" construction.
🪄 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: 99e79017-36dc-48d2-8ab6-dfedac631bf0
📒 Files selected for processing (5)
app/src/main/java/app/gamenative/ui/component/dialog/WorkshopManagerDialog.ktapp/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.ktapp/src/main/java/app/gamenative/workshop/WorkshopItem.ktapp/src/main/java/app/gamenative/workshop/WorkshopManager.ktapp/src/main/java/app/gamenative/workshop/WorkshopModPathDetector.kt
app/src/main/java/app/gamenative/ui/component/dialog/WorkshopManagerDialog.kt
Show resolved
Hide resolved
app/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.kt
Show resolved
Hide resolved
- Full-screen workshop manager dialog with hero image, mod list, select-all/deselect-all, and per-mod toggle switches - FolderPickerDialog for manually choosing mod installation paths when automatic detection fails - Improved mod path detection: binary scanning, config file parsing, AppData walking with fuzzy matching - CKM (Creation Kit Module) extraction for Skyrim mods - LZMA decompression with concurrent processing - ZIP extraction for single-archive workshop uploads - Magic-byte file type detection and extension fixing - Disk space checking before downloads - allSelected derivedStateOf optimization (removed wasteful toMap key) - Added KNOWN_EXTENSIONS to WorkshopItem for file type validation
Nightwalker743
force-pushed
the
workshop-update
branch
from
6deb3c3 to
bcc000e
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/app/gamenative/workshop/WorkshopModPathDetector.kt (1)
46-61:⚠️ Potential issue | 🟠 MajorKeep generic asset folders out of the candidate-name sets.
textures,sounds,audio,content,assets, and especiallydataare ordinary game resource directories; some are even treated as infrastructure inCONFIG_SKIP_DIRSbelow. With these additions the install/binary heuristics can now returnSymlinkIntoDir(<game>/Data|Textures|Audio...), and Phase 6 later materializes that result, which risks dropping workshop items into the base asset tree instead of a real mod folder.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/workshop/WorkshopModPathDetector.kt` around lines 46 - 61, The MEDIUM_CONFIDENCE_NAMES and LOW_CONFIDENCE_NAMES sets in WorkshopModPathDetector.kt include generic asset folders that should not be treated as mod candidates; remove "textures", "sounds", "audio" from MEDIUM_CONFIDENCE_NAMES and remove "content", "assets", "data" (and any other generic resource names like "textures"/"audio" if duplicated) from LOW_CONFIDENCE_NAMES so they are no longer part of ALL_MOD_DIR_NAMES, and instead ensure these generic names are present in CONFIG_SKIP_DIRS (or an equivalent skip list) so the installer/binary heuristics won't treat them as mod install targets; keep the ALL_MOD_DIR_NAMES union as-is so it reflects the corrected sets.
♻️ Duplicate comments (3)
app/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.kt (1)
1307-1315:⚠️ Potential issue | 🟠 MajorDon't log the full override path.
This still emits the user-selected absolute path at info level. That path can include the Wine username and other local directory names; log only set/cleared or a sanitized basename.
🤖 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/library/appscreen/SteamAppScreen.kt` around lines 1307 - 1315, The current onModPathChanged lambda stores the full user-selected absolute path (workshopModPath) and logs it via Timber.tag("Workshop").i(...); change the logging so it never emits the full absolute path: after setting workshopModPath and saving via ContainerUtils.getContainer(...)/container.putExtra(...)/container.saveData(), log only whether the override was set or cleared or log a sanitized basename (e.g., File(newPath).name) instead of the full newPath; update the Timber call in the onModPathChanged block to use that sanitized value or a "set/cleared" message.app/src/main/java/app/gamenative/workshop/WorkshopManager.kt (2)
2100-2202:⚠️ Potential issue | 🟠 MajorUnset the manual override when manual symlink creation fails.
If
targetDiris invalid or unwritable, the catch only logs.hasManualModPathstays true, so later logic still suppresses Phase 6 filesystem sync and stale-symlink cleanup. That leaves the game with neither the manual links nor the old auto-detected links.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/workshop/WorkshopManager.kt` around lines 2100 - 2202, The try/catch around creating symlinks for the manual path currently only logs errors (catch block) and leaves the manual override set (workshopModPath / hasManualModPath), which prevents Phase 6 sync and stale-symlink cleanup; modify the catch(e: Exception) in the symlink creation block so that on failure you clear/unset the manual override (e.g., reset workshopModPath to empty or call the existing routine that disables manual mode) and update any related state (hasManualModPath) and logs so the code will fall back to auto-detection and allow the normal cleanup/sync to run. Ensure you reference the same symbols used here: targetDir, modDirs, workshopModPath, hasManualModPath, and the catch block that currently calls Timber.tag(TAG).w(...).
505-560:⚠️ Potential issue | 🟠 MajorZIP extraction still skips updates and extension-repair cases.
extractZipMods()still runs beforefixFileExtensions(), so archives that only become recognizable after magic-byte repair are left unextracted until a later run. The permanent.zip_extractedmarker also survives re-downloads, so updated archives are skipped entirely.Also applies to: 3276-3283
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/workshop/WorkshopManager.kt` around lines 505 - 560, extractZipMods currently runs before fixFileExtensions and uses a permanent ".zip_extracted" marker that causes updated or extension-repaired archives to be skipped; change the flow so extraction occurs after fixFileExtensions (run fixFileExtensions before calling extractZipMods) and modify extractZipMods to treat the marker as versioned: instead of a permanent file, record/check a stamp tied to the ZIP (e.g., store marker named ".zip_extracted_<zipLastModified>" or compute a checksum) or compare the zipFile.lastModified() against the marker timestamp so re-downloaded/modified archives are re-extracted; update code paths involving extractZipMods and the marker creation logic (references: extractZipMods(), fixFileExtensions(), the ".zip_extracted" marker handling, and the zipFile delete/createNewFile sequence) to implement this behavior.
🤖 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/workshop/WorkshopManager.kt`:
- Around line 3393-3406: The catch for ContainerUtils.getOrCreateContainer(...)
currently only logs and allows workshop download to proceed, which reintroduces
the orphan-container/data-loss path; instead fail-closed by aborting/propagating
the error when container bootstrap fails: replace the catch-with-log with logic
that either rethrows the exception or returns from the surrounding method (so no
subsequent writes to getContainerWinePrefix(...) occur), and keep or augment the
Timber.tag(TAG).w(...) message to include the reason before exiting; reference
ContainerUtils.getOrCreateContainer, the containerId = "STEAM_$appId" local, and
subsequent use of getContainerWinePrefix(...) to locate where to stop progress.
---
Outside diff comments:
In `@app/src/main/java/app/gamenative/workshop/WorkshopModPathDetector.kt`:
- Around line 46-61: The MEDIUM_CONFIDENCE_NAMES and LOW_CONFIDENCE_NAMES sets
in WorkshopModPathDetector.kt include generic asset folders that should not be
treated as mod candidates; remove "textures", "sounds", "audio" from
MEDIUM_CONFIDENCE_NAMES and remove "content", "assets", "data" (and any other
generic resource names like "textures"/"audio" if duplicated) from
LOW_CONFIDENCE_NAMES so they are no longer part of ALL_MOD_DIR_NAMES, and
instead ensure these generic names are present in CONFIG_SKIP_DIRS (or an
equivalent skip list) so the installer/binary heuristics won't treat them as mod
install targets; keep the ALL_MOD_DIR_NAMES union as-is so it reflects the
corrected sets.
---
Duplicate comments:
In
`@app/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.kt`:
- Around line 1307-1315: The current onModPathChanged lambda stores the full
user-selected absolute path (workshopModPath) and logs it via
Timber.tag("Workshop").i(...); change the logging so it never emits the full
absolute path: after setting workshopModPath and saving via
ContainerUtils.getContainer(...)/container.putExtra(...)/container.saveData(),
log only whether the override was set or cleared or log a sanitized basename
(e.g., File(newPath).name) instead of the full newPath; update the Timber call
in the onModPathChanged block to use that sanitized value or a "set/cleared"
message.
In `@app/src/main/java/app/gamenative/workshop/WorkshopManager.kt`:
- Around line 2100-2202: The try/catch around creating symlinks for the manual
path currently only logs errors (catch block) and leaves the manual override set
(workshopModPath / hasManualModPath), which prevents Phase 6 sync and
stale-symlink cleanup; modify the catch(e: Exception) in the symlink creation
block so that on failure you clear/unset the manual override (e.g., reset
workshopModPath to empty or call the existing routine that disables manual mode)
and update any related state (hasManualModPath) and logs so the code will fall
back to auto-detection and allow the normal cleanup/sync to run. Ensure you
reference the same symbols used here: targetDir, modDirs, workshopModPath,
hasManualModPath, and the catch block that currently calls
Timber.tag(TAG).w(...).
- Around line 505-560: extractZipMods currently runs before fixFileExtensions
and uses a permanent ".zip_extracted" marker that causes updated or
extension-repaired archives to be skipped; change the flow so extraction occurs
after fixFileExtensions (run fixFileExtensions before calling extractZipMods)
and modify extractZipMods to treat the marker as versioned: instead of a
permanent file, record/check a stamp tied to the ZIP (e.g., store marker named
".zip_extracted_<zipLastModified>" or compute a checksum) or compare the
zipFile.lastModified() against the marker timestamp so re-downloaded/modified
archives are re-extracted; update code paths involving extractZipMods and the
marker creation logic (references: extractZipMods(), fixFileExtensions(), the
".zip_extracted" marker handling, and the zipFile delete/createNewFile sequence)
to implement this behavior.
🪄 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: 4bd047f0-aeb3-46d1-912a-903fb50bf0ed
📒 Files selected for processing (5)
app/src/main/java/app/gamenative/ui/component/dialog/WorkshopManagerDialog.ktapp/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.ktapp/src/main/java/app/gamenative/workshop/WorkshopItem.ktapp/src/main/java/app/gamenative/workshop/WorkshopManager.ktapp/src/main/java/app/gamenative/workshop/WorkshopModPathDetector.kt
✅ Files skipped from review due to trivial changes (1)
- app/src/main/java/app/gamenative/workshop/WorkshopItem.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/app/gamenative/ui/component/dialog/WorkshopManagerDialog.kt
Owner
|
conflicts :( |
# Conflicts: # app/src/main/java/app/gamenative/ui/component/dialog/WorkshopManagerDialog.kt
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/src/main/java/app/gamenative/ui/component/dialog/WorkshopManagerDialog.kt (1)
161-166: Switch the workshop list toLazyColumn.Line 165 makes the whole dialog a scrolling
Column, and Lines 406-478 then compose every filtered row and preview image eagerly. That will get expensive fast on accounts with large Workshop libraries and is likely to cause jank/memory pressure in exactly the screen that can grow unbounded.Also applies to: 406-478
🤖 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/component/dialog/WorkshopManagerDialog.kt` around lines 161 - 166, The dialog currently uses a full-screen scrollable Column (Modifier.verticalScroll(scrollState)) in WorkshopManagerDialog which eagerly composes every filtered row and preview (lines ~406-478); replace the outer Column with a LazyColumn (use rememberLazyListState) and move filtering/iteration into LazyColumn.items with stable keys so only visible items are composed, and ensure per-item composables (the workshop row/preview functions) load images lazily to avoid holding all bitmaps in memory; update modifiers (fillMaxSize(), background(Color.Black)) to the LazyColumn and remove verticalScroll(scrollState).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app/src/main/java/app/gamenative/ui/component/dialog/WorkshopManagerDialog.kt`:
- Around line 612-617: The breadcrumb construction in the remember block
(variables breadcrumb, currentDir, currentRootLabel, roots) flattens path parts
by replacing separators with spaces; instead, split the relative path on both
'/' and '\' into segments and rejoin with " / " so segments become "Root / Mods
/ Foo / Bar"; update the rel computation to derive segments (e.g.,
rel.split(Regex("[/\\\\]+")).filterNot { it.isBlank() }) and use joinToString("
/ ") combined with currentRootLabel when building the final breadcrumb string.
---
Nitpick comments:
In
`@app/src/main/java/app/gamenative/ui/component/dialog/WorkshopManagerDialog.kt`:
- Around line 161-166: The dialog currently uses a full-screen scrollable Column
(Modifier.verticalScroll(scrollState)) in WorkshopManagerDialog which eagerly
composes every filtered row and preview (lines ~406-478); replace the outer
Column with a LazyColumn (use rememberLazyListState) and move
filtering/iteration into LazyColumn.items with stable keys so only visible items
are composed, and ensure per-item composables (the workshop row/preview
functions) load images lazily to avoid holding all bitmaps in memory; update
modifiers (fillMaxSize(), background(Color.Black)) to the LazyColumn and remove
verticalScroll(scrollState).
🪄 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: 536e4862-b862-4140-8ec1-ba8f8962b839
📒 Files selected for processing (2)
app/src/main/java/app/gamenative/ui/component/dialog/WorkshopManagerDialog.ktapp/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.kt
✅ Files skipped from review due to trivial changes (1)
- app/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.kt
app/src/main/java/app/gamenative/ui/component/dialog/WorkshopManagerDialog.kt
Show resolved
Hide resolved
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/ui/component/dialog/WorkshopManagerDialog.kt`:
- Around line 595-608: The directory enumeration in LaunchedEffect(currentDir)
can throw or return null and currently loading may never be reset; wrap the
withContext(Dispatchers.IO) read in a try/catch/finally so that any exceptions
from dir.listFiles() are caught, set subDirs = emptyList() on error, and always
set loading = false in the finally block; specifically modify the LaunchedEffect
block that references currentDir, loading, subDirs, withContext(Dispatchers.IO)
and dir.listFiles() to perform the guarded read (filter/sort) inside the try,
handle null/exception by assigning emptyList(), and ensure loading is cleared
regardless of success or failure.
🪄 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: a8fcffd9-82cf-4d04-84f8-95aed1e98d7c
📒 Files selected for processing (1)
app/src/main/java/app/gamenative/ui/component/dialog/WorkshopManagerDialog.kt
app/src/main/java/app/gamenative/ui/component/dialog/WorkshopManagerDialog.kt
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (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/ui/component/dialog/WorkshopManagerDialog.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/component/dialog/WorkshopManagerDialog.kt:606">
P2: Broad `Exception` catch in `LaunchedEffect` swallows coroutine cancellation and can apply stale UI state after directory changes.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
app/src/main/java/app/gamenative/ui/component/dialog/WorkshopManagerDialog.kt
Outdated
Show resolved
Hide resolved
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
3 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.
Description
Right now the logic for automatic mod folder symlink placement isn't perfect (and it never will be without full steam client), so this adds the ability to choose the mod folder yourself where mods will be symlinked to in case if automatic detection fails. This should help with workshop support compatibility quite a bit.
Recording
Screen_Recording_20260402_004755_GameNative.mp4
Checklist
#code-changes, I have discussed this change there and it has been green-lighted. If I do not have access, I have still provided clear context in this PR. If I skip both, I accept that this change may face delays in review, may not be reviewed at all, or may be closed.CONTRIBUTING.md.Summary by cubic
Adds a full-screen Workshop manager with an in-app folder picker for manual mod placement, safer
ISteamUGCrouting to prevent duplicates, and enrichedmods.jsonfor better item details. Also extracts single-archive uploads and hardens container and folder‑picker behavior to avoid crashes.New Features
workshopModPath.mods.jsonalongside..zip_extractedmarker; plus CKM extraction, LZMA decompression, and magic‑byte extension fixing. Adds disk space checks before downloads.ISteamUGCwhen the binary signals Workshop API usage and a HIGH‑confidence mod dir exists; forcesISteamUGCfor Starbound.mods.jsonnow includesdescription,tags,preview_url, andworkshop_item_url.WorkshopItemgainsdescriptionandtags.Bug Fixes
CancellationExceptionto avoid stuck states.ISteamUGCpath is preferred to prevent duplicate mods.Written for commit 79fc8e6. Summary will update on new commits.
Summary by CodeRabbit
New Features
Improvements