Workshop Update: Manual Mod Folder Dialog#1072
Workshop Update: Manual Mod Folder Dialog#1072Nightwalker743 wants to merge 1 commit intoutkarshdalal:masterfrom
Conversation
📝 WalkthroughWalkthroughThis PR adds a manual workshop mod-folder override with a folder-picker UI, persists per-game overrides, enriches workshop item metadata, adds ZIP-in-place extraction for single-archive mods, and updates symlink detection/creation behavior with a new 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 browse roots & show breadcrumb
User->>Picker: Navigate & Select folder
Picker->>Screen: onModPathChanged(selectedPath)
Screen->>Screen: Update workshopModPath state
Screen->>Container: putExtra("workshopModPath", selectedPath)
Container->>Container: 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 |
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
Show resolved
Hide resolved
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
6deb3c3 to
bcc000e
Compare
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
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 a manual mod folder picker, stronger mod path detection, and smarter ISteamUGC handling to reduce duplicates and edge cases. Also extracts single-archive uploads and fixes a pre-launch crash in Manage Workshop.
New Features
workshopModPathand shown in the UI..zip_extractedmarker; plus CKM extraction (Skyrim), LZMA decompression with concurrency, and magic‑byte extension fixing. Adds disk space checks before downloads.mods.jsonnow includesdescription,tags,preview_url, and aworkshop_item_url.WorkshopItemgainsdescriptionandtags.Bug Fixes
Written for commit bcc000e. Summary will update on new commits.
Summary by CodeRabbit
New Features
Improvements