Skip to content

Workshop implementation, relative mouse movement toggle, and ControlsProfile binding fix - #977

Merged
utkarshdalal merged 25 commits into
utkarshdalal:masterfrom
Nightwalker743:New-Workshop-Implementation
Mar 31, 2026
Merged

Workshop implementation, relative mouse movement toggle, and ControlsProfile binding fix#977
utkarshdalal merged 25 commits into
utkarshdalal:masterfrom
Nightwalker743:New-Workshop-Implementation

Conversation

@Nightwalker743

@Nightwalker743 Nightwalker743 commented Mar 22, 2026

Copy link
Copy Markdown
Contributor

Summary by cubic

Adds Steam Workshop mod support with an in‑app Manage Workshop dialog, background download/sync with pause/resume that survives restarts, and safe install/cleanup for Steam games. State is saved per game in the SteamApp DB (Room v18); downloads start on save and a launch prompt appears if updates exceed 100MB.

  • New Features

    • Manage Workshop: browse with previews/last‑updated; select all/none or specific items; defaults off; Steam‑only; localized; saves enabled_workshop_item_ids/workshop_mods in SteamApp.
    • Background download/sync/cleanup: start on save with progress; pause/resume and queue persist across restarts; post‑processing (CKM extract, LZMA decompress, extension fixes); large‑update prompt at launch.
    • “Delete Workshop Mods” fully cleans content and installed entries (strategy targets and Unity AppData) and clears state on uninstall; content is preserved during container recovery.
  • Bug Fixes

    • Deterministic naming and collision handling for directories and flat files (lowest item ID wins); title collisions disambiguated; sorted iteration for consistent results.
    • Safer linking/copying and cleanup with foreign‑entry checks, symlink safety, and robust disk‑space checks (walk parent dirs; handle 0‑free); proper cleanup when deselecting all mods or after zero subscriptions.
    • Broadened file‑type detection; streaming SHA‑1 hashing in SteamAutoCloud to avoid OOM; offline guard and fetch‑result completeness check to prevent partial/erroneous cleanup.
    • Database: auto migration to v18 adds workshop_mods, enabled_workshop_item_ids, workshop_download_pending; pending‑download queries enable resume on app start.

Written for commit 0143850. Summary will update on new commits.

Summary by CodeRabbit

  • New Features

    • Workshop mods: pre-launch subscription sync with download/progress, per-launch integration into game folders, and in-app “Delete workshop mods” action
    • Relative mouse movement toggle for controller input
  • Improvements

    • Non-blocking background sync with disk-space checks, post-download processing, and graceful error handling that preserves existing mods when fetch fails
    • Container settings persist and copy with duplicated containers
  • UI

    • Updated library empty/login messages, workshop-related strings, and delete confirmation text

Implements workshop mod downloading, symlinking, and configuration for
Steam games. Supports multiple strategies: SymlinkIntoDir, CopyIntoDir,
Source engine, Skyrim, Unity, and Standard ISteamUGC.

Files:
- WorkshopManager.kt: Central orchestrator for download, sync, and config
- WorkshopModPathDetector.kt: 4-heuristic detection of mod directories
- WorkshopModPathStrategy.kt: Strategy types with fan-out policies
- WorkshopSymlinker.kt: Symlink/copy operations with flat-file support
- WorkshopItem.kt: Data model for workshop items
Replace custom OkHttpClient.Builder() in downloadItemViaHttp with the
existing Net.http singleton, which provides DNS-over-HTTPS fallback,
connection retry, and proper timeout configuration. Removes unused
OkHttpClient and TimeUnit imports.
@coderabbitai

coderabbitai Bot commented Mar 22, 2026

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds Steam Workshop sync and relative-mouse input support: new workshop models, path detection and installation strategies, download/post-processing and symlink/copy installer, UI toggles and delete flow, container persistence, and launch-time integration for syncing mods.

Changes

Cohort / File(s) Summary
Launch & Workshop Flow
app/src/main/java/app/gamenative/ui/PluviaMain.kt
Integrates a pre-launch Workshop sync: fetch subscriptions, compute items to sync, disk-space check, download with progress, post-process (extract/decompress/fix timestamps), and call mod installation helpers; errors are logged and non-blocking.
Workshop Core & Sync
app/src/main/java/app/gamenative/workshop/WorkshopItem.kt, .../WorkshopModPathStrategy.kt, .../WorkshopModPathDetector.kt, .../WorkshopSymlinker.kt
Adds WorkshopItem/FetchResult models, path-detection heuristics selecting strategies with confidence, and a symlinker that symlinks or copies items into target dirs with safe cleanup, fingerprinting, and per-item error aggregation.
UI: Config & Dialogs
app/src/main/java/app/gamenative/ui/component/dialog/GeneralTab.kt, .../ContainerConfigDialog.kt, .../ControllerTab.kt
Adds workshop mods toggle and conditional “delete workshop mods” button with confirmation; ContainerConfigDialog accepts onDeleteWorkshopMods callback; adds relative-mouse toggle in controller settings.
Container Persistence & Manager
app/src/main/java/com/winlator/container/Container.java, .../ContainerData.kt, .../ContainerManager.java, app/src/main/java/app/gamenative/utils/ContainerUtils.kt
Adds workshopMods and relativeMouseMovement fields, getters/setters, persists them in JSON and Compose Saver, maps them in ContainerUtils, and copies workshop setting when duplicating containers.
Screens & Input Integration
app/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.kt, .../xserver/XServerScreen.kt
Wires the delete-workshop handler into the config dialog for Steam items and forwards container.relativeMouseMovement to the active XServer.
Misc / Utilities & Strings
app/src/main/java/com/winlator/inputcontrols/ControlsProfile.java, app/src/main/res/values/strings.xml
Optimizes binding parsing in ControlsProfile; adds localized strings for workshop management and relative-mouse descriptions; updates several library "not logged in / empty" messages.
New Workshop UI action
app/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.kt (callback body)
Implements onDeleteWorkshopMods handler executing deletion on IO dispatcher and showing success/error snackbars with Timber logging.

Sequence Diagram(s)

sequenceDiagram
    participant App as PluviaMain (preLaunch)
    participant Steam as SteamClient/Service
    participant WM as WorkshopManager
    participant FS as Filesystem
    participant WS as WorkshopSymlinker

    App->>Steam: query subscribed workshop items (steamClient/steamId)
    Steam-->>App: subscribed item list (or failure)
    alt fetch succeeded and items present
        App->>WM: getItemsNeedingSync(items, workshopContentDir)
        WM-->>App: itemsToSync
        App->>FS: check free space (>= required*2)
        alt space OK
            App->>WM: downloadItems(itemsToSync) with progress callbacks
            WM->>FS: write downloaded files
            App->>WM: post-process (extract/decompress/fix timestamps)
            App->>WS: configureModSymlinks(gameRoot, workshopContentDir, items)
            WS->>FS: create symlinks or copies
        else insufficient space
            App->>App: show insufficient space snackbar
        end
    else fetch succeeded but no items
        App->>WM: deleteWorkshopMods(...) (cleanup)
    else fetch failed
        alt workshopContentDir exists
            App->>WS: configureModSymlinks(..., use existing on-disk items)
        else
            App->>App: continue launch without cloud sync
        end
    end
    App->>App: continue launch
Loading
sequenceDiagram
    participant User as User
    participant UI as ContainerConfigDialog / GeneralTab
    participant App as BaseAppScreen
    participant WM as WorkshopManager
    participant FS as Filesystem

    User->>UI: Click "Delete workshop mods"
    UI->>UI: show confirmation dialog
    User->>UI: Confirm
    UI->>App: invoke onDeleteWorkshopMods()
    App->>WM: deleteWorkshopMods(context, appId) (Dispatchers.IO)
    WM->>FS: remove workshop content directory and symlinks
    FS-->>WM: deletion result
    WM-->>App: success or error
    App-->>UI: show snackbar (success or error)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐰 I dug through binaries and folders deep,
I hopped on Steam to fetch what users keep,
Symlinks stitched, copies snug and neat,
A relative mouse now hops to a new beat,
Hooray — hop, sync, and game on, repeat! 🎮✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 24.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The PR description is auto-generated and very comprehensive but lacks explicit mapping to the template structure, and no recording is visible in the provided description. Clarify whether a recording was attached separately; if required by the template, ensure it is included or confirm it exists in the PR comments.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the three main changes: workshop implementation, relative mouse movement toggle, and ControlsProfile binding fix—all of which are core changes present in the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

11 issues found across 17 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/PluviaMain.kt">

<violation number="1" location="app/src/main/java/app/gamenative/ui/PluviaMain.kt:1783">
P2: Disk-space guard is bypassed when `usableSpace` is 0, so downloads can proceed despite unknown/unavailable free space.</violation>

<violation number="2" location="app/src/main/java/app/gamenative/ui/PluviaMain.kt:1884">
P1: Successful fetch with zero subscriptions skips workshop cleanup/configuration, allowing previous workshop content to persist and diverge from current subscription state.</violation>
</file>

<file name="app/src/main/java/app/gamenative/workshop/WorkshopSymlinker.kt">

<violation number="1" location="app/src/main/java/app/gamenative/workshop/WorkshopSymlinker.kt:140">
P1: Title-based entry names are not made unique, so colliding sanitized titles can map multiple active workshop items to the same path and overwrite each other.</violation>

<violation number="2" location="app/src/main/java/app/gamenative/workshop/WorkshopSymlinker.kt:220">
P2: Flat-file sync silently overwrites same-name files across workshop items, dropping active content without surfacing a conflict.</violation>

<violation number="3" location="app/src/main/java/app/gamenative/workshop/WorkshopSymlinker.kt:350">
P1: `ensureCopy` can recursively delete through a destination symlink because it calls `deleteRecursively()` without checking `Files.isSymbolicLink` first.</violation>
</file>

<file name="app/src/main/java/app/gamenative/ui/component/dialog/ControllerTab.kt">

<violation number="1" location="app/src/main/java/app/gamenative/ui/component/dialog/ControllerTab.kt:106">
P2: Relative-mouse toggle updates config from a captured snapshot, risking lost updates to other fields changed after composition.</violation>
</file>

<file name="app/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.kt">

<violation number="1" location="app/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.kt:789">
P1: Workshop mod deletion is exposed without Steam-source gating, allowing non-Steam app IDs to resolve to Steam workshop paths and delete unintended content.</violation>
</file>

<file name="app/src/main/java/app/gamenative/workshop/WorkshopModPathDetector.kt">

<violation number="1" location="app/src/main/java/app/gamenative/workshop/WorkshopModPathDetector.kt:136">
P2: Config scan over AppData/Documents performs unbounded recursive enumeration and accumulates candidates in memory, which can cause expensive startup I/O and memory pressure on large user datasets.</violation>

<violation number="2" location="app/src/main/java/app/gamenative/workshop/WorkshopModPathDetector.kt:420">
P1: Empty fuzzy token is added for blank developerName, making AppData fuzzy matching always succeed and causing noisy, expensive recursive scanning.</violation>
</file>

<file name="app/src/main/java/app/gamenative/workshop/WorkshopModPathStrategy.kt">

<violation number="1" location="app/src/main/java/app/gamenative/workshop/WorkshopModPathStrategy.kt:44">
P2: `effectiveDirs` can throw `NoSuchElementException` because `targetDirs.first()` is used without enforcing non-empty `targetDirs` in public constructors.</violation>
</file>

<file name="app/src/main/java/app/gamenative/utils/ContainerUtils.kt">

<violation number="1" location="app/src/main/java/app/gamenative/utils/ContainerUtils.kt:296">
P2: New `workshopMods`/`relativeMouseMovement` are wired for container read/apply but omitted from default persistence/new-container default construction, causing inconsistent defaults behavior.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread app/src/main/java/app/gamenative/ui/PluviaMain.kt Outdated
Comment thread app/src/main/java/app/gamenative/workshop/WorkshopSymlinker.kt
Comment thread app/src/main/java/app/gamenative/workshop/WorkshopSymlinker.kt Outdated
Comment thread app/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.kt Outdated
Comment thread app/src/main/java/app/gamenative/workshop/WorkshopSymlinker.kt Outdated
Comment thread app/src/main/java/app/gamenative/ui/component/dialog/ControllerTab.kt Outdated
binaryResult.candidates.forEach { add(it) }
collectModsDirectories(gameInstallDir, 0).forEach { add(it) }
(listOf(gameInstallDir) + appDataRoots.map { it.root })
.forEach { root -> collectFromConfigFiles(root, appDataRoots).forEach { add(it) } }

@cubic-dev-ai cubic-dev-ai Bot Mar 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Config scan over AppData/Documents performs unbounded recursive enumeration and accumulates candidates in memory, which can cause expensive startup I/O and memory pressure on large user datasets.

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/workshop/WorkshopModPathDetector.kt, line 136:

<comment>Config scan over AppData/Documents performs unbounded recursive enumeration and accumulates candidates in memory, which can cause expensive startup I/O and memory pressure on large user datasets.</comment>

<file context>
@@ -0,0 +1,503 @@
+        binaryResult.candidates.forEach { add(it) }
+        collectModsDirectories(gameInstallDir, 0).forEach { add(it) }
+        (listOf(gameInstallDir) + appDataRoots.map { it.root })
+            .forEach { root -> collectFromConfigFiles(root, appDataRoots).forEach { add(it) } }
+        collectFromAppDataFuzzy(appDataRoots, gameName, developerName).forEach { add(it) }
+
</file context>
Fix with Cubic

constructor(targetDir: File) : this(listOf(targetDir), FanOutPolicy.PRIMARY_ONLY)

val effectiveDirs: List<File> get() = when (fanOut) {
FanOutPolicy.PRIMARY_ONLY -> listOf(targetDirs.first())

@cubic-dev-ai cubic-dev-ai Bot Mar 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: effectiveDirs can throw NoSuchElementException because targetDirs.first() is used without enforcing non-empty targetDirs in public constructors.

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/workshop/WorkshopModPathStrategy.kt, line 44:

<comment>`effectiveDirs` can throw `NoSuchElementException` because `targetDirs.first()` is used without enforcing non-empty `targetDirs` in public constructors.</comment>

<file context>
@@ -0,0 +1,64 @@
+        constructor(targetDir: File) : this(listOf(targetDir), FanOutPolicy.PRIMARY_ONLY)
+
+        val effectiveDirs: List<File> get() = when (fanOut) {
+            FanOutPolicy.PRIMARY_ONLY -> listOf(targetDirs.first())
+            FanOutPolicy.ALL_DIRS -> targetDirs
+        }
</file context>
Fix with Cubic

Comment thread app/src/main/java/app/gamenative/utils/ContainerUtils.kt

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

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/inputcontrols/ControlsProfile.java (1)

282-291: ⚠️ Potential issue | 🟡 Minor

Empty bindings can incorrectly enable virtual gamepad mode.

At Line 282, hasGamepadBinding starts as true. If an element has zero bindings, the loop is skipped and Line 291 can set virtualGamepad = true incorrectly.

💡 Proposed fix
-                boolean hasGamepadBinding = true;
+                boolean hasGamepadBinding = bindingsJSONArray.length() > 0;
                 JSONArray bindingsJSONArray = elementJSONObject.getJSONArray("bindings");
                 element.setBindingCount(bindingsJSONArray.length());
                 for (int j = 0; j < bindingsJSONArray.length(); j++) {
                     Binding binding = Binding.fromString(bindingsJSONArray.getString(j));
                     element.setBindingAt(j, binding);
                     if (!binding.isGamepad()) hasGamepadBinding = false;
                 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/src/main/java/com/winlator/inputcontrols/ControlsProfile.java` around
lines 282 - 291, The bug is that hasGamepadBinding is initialized true so
elements with zero bindings can spuriously enable virtualGamepad; change the
logic in the loop that reads elementJSONObject.getJSONArray("bindings") so
hasGamepadBinding starts false (or only set to true when a binding.isGamepad()
is encountered) and/or ensure you check bindingsJSONArray.length() > 0 before
treating a true hasGamepadBinding as meaningful; update the variables around
element.setBindingCount(...), Binding.fromString(...),
element.setBindingAt(...), and the final virtualGamepad assignment so
virtualGamepad is set to true only when at least one binding exists and at least
one binding.isGamepad() returned true.
🧹 Nitpick comments (2)
app/src/main/java/app/gamenative/workshop/WorkshopModPathStrategy.kt (1)

37-47: Consider validating non-empty targetDirs to prevent runtime exception.

effectiveDirs calls targetDirs.first() when fanOut is PRIMARY_ONLY. If targetDirs is somehow empty, this throws NoSuchElementException. While the convenience constructor enforces a single-element list, the primary constructor accepts any List<File>.

🛡️ Optional: Add init validation
 data class SymlinkIntoDir(
     val targetDirs: List<File>,
     val fanOut: FanOutPolicy = FanOutPolicy.PRIMARY_ONLY,
 ) : WorkshopModPathStrategy() {
+    init {
+        require(targetDirs.isNotEmpty()) { "targetDirs must not be empty" }
+    }
     constructor(targetDir: File) : this(listOf(targetDir), FanOutPolicy.PRIMARY_ONLY)

Apply the same to CopyIntoDir.

🤖 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/WorkshopModPathStrategy.kt` around
lines 37 - 47, SymlinkIntoDir's effectiveDirs uses targetDirs.first() which will
throw if the primary constructor received an empty list; add a non-empty
validation in SymlinkIntoDir (e.g., check targetDirs.isNotEmpty() in an init
block and throw an IllegalArgumentException with a clear message) so the class
always has at least one target; apply the same validation pattern to the
analogous CopyIntoDir class to prevent runtime NoSuchElementException when
FanOutPolicy.PRIMARY_ONLY is used.
app/src/main/java/app/gamenative/workshop/WorkshopSymlinker.kt (1)

234-246: Minor redundancy in stale cleanup passes.

This second pass for directory symlinks duplicates some logic from the first pass (lines 224-232). Consider consolidating into a single pass that handles both file and directory symlinks.

♻️ Optional consolidation
-        // Remove stale file symlinks that we created (point into workshopContentBase)
-        targetDir.listFiles()?.forEach { entry ->
-            if (entry.name in expectedFiles) return@forEach
-            if (Files.isSymbolicLink(entry.toPath()) && isOurSymlink(entry, workshopContentBase)) {
-                if (deleteEntry(entry)) {
-                    Timber.tag(TAG).i("  Removed stale flat file (ours): ${entry.name}")
-                    removed++
-                }
-            }
-        }
-
-        // Also clean up any stale directory symlinks from a previous non-flat run
-        targetDir.listFiles()?.forEach { entry ->
-            if (entry.isDirectory || Files.isSymbolicLink(entry.toPath())) {
-                if (Files.isSymbolicLink(entry.toPath()) && isOurSymlink(entry, workshopContentBase)
-                    && entry.name !in expectedFiles
-                ) {
-                    if (deleteEntry(entry)) {
-                        Timber.tag(TAG).i("  Removed stale dir symlink (ours): ${entry.name}")
-                        removed++
-                    }
-                }
-            }
-        }
+        // Remove stale symlinks (file or directory) that we created
+        targetDir.listFiles()?.forEach { entry ->
+            if (entry.name in expectedFiles) return@forEach
+            if (Files.isSymbolicLink(entry.toPath()) && isOurSymlink(entry, workshopContentBase)) {
+                if (deleteEntry(entry)) {
+                    val kind = if (entry.isDirectory) "dir" else "file"
+                    Timber.tag(TAG).i("  Removed stale $kind symlink (ours): ${entry.name}")
+                    removed++
+                }
+            }
+        }
🤖 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/WorkshopSymlinker.kt` around lines
234 - 246, The second pass over targetDir.listFiles() that only handles
directory symlinks duplicates logic from the earlier file-pass; merge the two
passes by updating the initial targetDir.listFiles()?.forEach loop to check both
entry.isDirectory and Files.isSymbolicLink(entry.toPath()), and inside that
unified loop apply the existing checks (Files.isSymbolicLink,
isOurSymlink(entry, workshopContentBase), entry.name !in expectedFiles), call
deleteEntry(entry) and increment removed and log via Timber.tag(TAG).i(...) on
success (preserve the exact deleteEntry, isOurSymlink, expectedFiles, removed,
and log usage), then remove the redundant second loop.
🤖 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/GeneralTab.kt`:
- Around line 339-377: The delete-confirmation flow removes files via
onDeleteWorkshopMods() but leaves the pending config flag workshopMods true, so
the user can re-sync on save; update the confirmButton handler inside the
AlertDialog (where onDeleteWorkshopMods() is called) to also clear the
workshopMods flag in the pending config (e.g. set state.config.value =
config.copy(workshopMods = false) or otherwise update config.workshopMods)
before or after calling onDeleteWorkshopMods(), so deletion is persisted even if
the user doesn't toggle the SettingsSwitch; ensure you reference the
state/config used in this composable (state.config.value, config,
onDeleteWorkshopMods) when making the change.

In
`@app/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.kt`:
- Around line 789-795: Wrap the WorkshopManager.deleteWorkshopMods call inside a
runCatching (or try/catch) within the onDeleteWorkshopMods lambda so IO
exceptions are caught, and on success/failure show user feedback (e.g.,
Toast/Snackbar or existing UI helper) from the UI coroutine; specifically update
the uiScope.launch block that calls WorkshopManager.deleteWorkshopMods(context,
libraryItem.appId) to catch exceptions and call the appropriate success/failure
message helper on the main thread.

In `@app/src/main/java/app/gamenative/utils/ContainerUtils.kt`:
- Around line 470-478: The PrefManager-backed default path is missing
relativeMouseMovement and workshopMods, so update getDefaultContainerData(),
setDefaultContainerData(), and the default ContainerData(...) branch inside
createNewContainer() to read/store/apply those two fields from the PrefManager
defaults; ensure the default ContainerData constructor call used when creating a
new container includes relativeMouseMovement and workshopMods so newly created
containers inherit the user-changed defaults set via setDefaultContainerData().

---

Outside diff comments:
In `@app/src/main/java/com/winlator/inputcontrols/ControlsProfile.java`:
- Around line 282-291: The bug is that hasGamepadBinding is initialized true so
elements with zero bindings can spuriously enable virtualGamepad; change the
logic in the loop that reads elementJSONObject.getJSONArray("bindings") so
hasGamepadBinding starts false (or only set to true when a binding.isGamepad()
is encountered) and/or ensure you check bindingsJSONArray.length() > 0 before
treating a true hasGamepadBinding as meaningful; update the variables around
element.setBindingCount(...), Binding.fromString(...),
element.setBindingAt(...), and the final virtualGamepad assignment so
virtualGamepad is set to true only when at least one binding exists and at least
one binding.isGamepad() returned true.

---

Nitpick comments:
In `@app/src/main/java/app/gamenative/workshop/WorkshopModPathStrategy.kt`:
- Around line 37-47: SymlinkIntoDir's effectiveDirs uses targetDirs.first()
which will throw if the primary constructor received an empty list; add a
non-empty validation in SymlinkIntoDir (e.g., check targetDirs.isNotEmpty() in
an init block and throw an IllegalArgumentException with a clear message) so the
class always has at least one target; apply the same validation pattern to the
analogous CopyIntoDir class to prevent runtime NoSuchElementException when
FanOutPolicy.PRIMARY_ONLY is used.

In `@app/src/main/java/app/gamenative/workshop/WorkshopSymlinker.kt`:
- Around line 234-246: The second pass over targetDir.listFiles() that only
handles directory symlinks duplicates logic from the earlier file-pass; merge
the two passes by updating the initial targetDir.listFiles()?.forEach loop to
check both entry.isDirectory and Files.isSymbolicLink(entry.toPath()), and
inside that unified loop apply the existing checks (Files.isSymbolicLink,
isOurSymlink(entry, workshopContentBase), entry.name !in expectedFiles), call
deleteEntry(entry) and increment removed and log via Timber.tag(TAG).i(...) on
success (preserve the exact deleteEntry, isOurSymlink, expectedFiles, removed,
and log usage), then remove the redundant second loop.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 68de146a-88c5-4556-b423-2f5459fe04a6

📥 Commits

Reviewing files that changed from the base of the PR and between 69cae38 and da34e96.

📒 Files selected for processing (17)
  • app/src/main/java/app/gamenative/ui/PluviaMain.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/ControllerTab.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/GeneralTab.kt
  • app/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.kt
  • app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt
  • app/src/main/java/app/gamenative/utils/ContainerUtils.kt
  • app/src/main/java/app/gamenative/workshop/WorkshopItem.kt
  • app/src/main/java/app/gamenative/workshop/WorkshopManager.kt
  • app/src/main/java/app/gamenative/workshop/WorkshopModPathDetector.kt
  • app/src/main/java/app/gamenative/workshop/WorkshopModPathStrategy.kt
  • app/src/main/java/app/gamenative/workshop/WorkshopSymlinker.kt
  • app/src/main/java/com/winlator/container/Container.java
  • app/src/main/java/com/winlator/container/ContainerData.kt
  • app/src/main/java/com/winlator/container/ContainerManager.java
  • app/src/main/java/com/winlator/inputcontrols/ControlsProfile.java
  • app/src/main/res/values/strings.xml

Comment thread app/src/main/java/app/gamenative/ui/component/dialog/GeneralTab.kt Outdated
Comment thread app/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.kt Outdated
Comment on lines +470 to +478
container.setRelativeMouseMovement(containerData.relativeMouseMovement)
container.setGestureConfig(containerData.gestureConfig)
container.setExternalDisplayMode(containerData.externalDisplayMode)
container.setExternalDisplaySwap(containerData.externalDisplaySwap)
container.setForceDlc(containerData.forceDlc)
container.setSteamOfflineMode(containerData.steamOfflineMode)
container.setUseLegacyDRM(containerData.useLegacyDRM)
container.setUnpackFiles(containerData.unpackFiles)
container.setWorkshopMods(containerData.workshopMods)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

New containers still won’t inherit these two defaults.

This completes the per-container apply path, but the PrefManager-backed default path in this same file still omits relativeMouseMovement and workshopMods (getDefaultContainerData(), setDefaultContainerData(), and the default ContainerData(...) branch in createNewContainer()). Newly created containers will therefore still initialize both flags to false, even after the user changes the app’s default container config.

🤖 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/ContainerUtils.kt` around lines 470 -
478, The PrefManager-backed default path is missing relativeMouseMovement and
workshopMods, so update getDefaultContainerData(), setDefaultContainerData(),
and the default ContainerData(...) branch inside createNewContainer() to
read/store/apply those two fields from the PrefManager defaults; ensure the
default ContainerData constructor call used when creating a new container
includes relativeMouseMovement and workshopMods so newly created containers
inherit the user-changed defaults set via setDefaultContainerData().

- Zero-subscription cleanup: delete workshop content and remove stale
  symlinks when user has unsubscribed from all items
- ensureCopy symlink safety: check Files.isSymbolicLink() before
  deleteRecursively() to avoid following symlinks into other dirs
- Title collision detection: disambiguate entryNameForId when multiple
  items sanitize to the same title by appending _<itemId>
- Steam-source gating: only show delete workshop mods button for
  Steam games (pass null for non-Steam sources)
- Empty token filter: filter out empty strings from buildFuzzyTokens
  to prevent false matches when developerName is blank
- Auto-disable workshopMods toggle after deleting workshop mods
- Error handling: wrap deleteWorkshopMods calls in try-catch with
  snackbar feedback in both BaseAppScreen and PluviaMain

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 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/ui/PluviaMain.kt">

<violation number="1" location="app/src/main/java/app/gamenative/ui/PluviaMain.kt:1051">
P2: User-facing snackbar exposes raw exception text and uses hardcoded, non-localized strings on newly added error/success paths.</violation>

<violation number="2" location="app/src/main/java/app/gamenative/ui/PluviaMain.kt:1893">
P2: Workshop cleanup order is incorrect: deleting workshop content before `configureModSymlinks` causes the cleanup call to no-op due to an existence guard, risking stale symlinks.</violation>
</file>

<file name="app/src/main/java/app/gamenative/workshop/WorkshopSymlinker.kt">

<violation number="1" location="app/src/main/java/app/gamenative/workshop/WorkshopSymlinker.kt:145">
P2: Name disambiguation is not globally unique, so secondary collisions can map different workshop items to the same target entry.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread app/src/main/java/app/gamenative/ui/PluviaMain.kt Outdated
Comment thread app/src/main/java/app/gamenative/ui/PluviaMain.kt Outdated
Comment thread app/src/main/java/app/gamenative/workshop/WorkshopSymlinker.kt Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 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/PluviaMain.kt`:
- Around line 1043-1054: The delete action is currently passed unconditionally
via onDeleteWorkshopMods and may run deleteWorkshopMods for non‑Steam
containers; before wiring or invoking onDeleteWorkshopMods (where
openContainerConfigForAppId/appId is used), fetch the container config (via
openContainerConfigForAppId or the same lookup used to render the dialog) and
only provide/execute the WorkshopManager.deleteWorkshopMods(appId) block when
the container type equals Steam; otherwise omit or disable onDeleteWorkshopMods
so the Snackbar path cannot call WorkshopManager.deleteWorkshopMods for
GOG/Epic/Amazon/custom containers.
- Around line 1757-1912: When container.isWorkshopMods is false the code never
removes previously managed mod placements; add a cleanup branch executed when
container.isWorkshopMods is false that locates the workshopContentDir via
WorkshopManager.getWorkshopContentDir(winePrefix, gameId) (use
ImageFs.find(context).wineprefix for winePrefix) and the gameRootDir via
File(SteamService.getAppDirPath(gameId)), delete or prune workshopContentDir
(e.g., deleteRecursively or remove managed files) and then call
WorkshopManager.configureModSymlinks(...) with items = emptyList(), winePrefix
and gameName (SteamService.getAppInfoOf(gameId)?.name ?: "") to remove stale
symlinks so disabling the toggle actually removes managed mods.

In
`@app/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.kt`:
- Around line 789-799: The delete action currently only calls
WorkshopManager.deleteWorkshopMods(context, libraryItem.appId) which removes the
workshop content dir but leaves symlinks/copies in the game tree; update the
onDeleteWorkshopMods handler to also remove installed mod entries by calling a
cleanup method (either extend WorkshopManager.deleteWorkshopMods to also remove
installed entries or add/ call a new method like
WorkshopManager.removeInstalledWorkshopEntries(context, libraryItem.appId) that
traverses the game install tree and deletes symlinks/copies), perform this work
on Dispatchers.IO inside the existing try block, and surface success/failure via
SnackbarManager as already done.

In `@app/src/main/java/app/gamenative/workshop/WorkshopSymlinker.kt`:
- Around line 137-146: The current disambiguation (rawEntryNameForId ->
entryNameForId) can still produce collisions (e.g., "foo", "foo", "foo_42") so
ensure final entry names are globally unique: iterate over rawEntryNameForId
entries and build entryNameForId while tracking a usedNames set; for each id
compute a baseName = sanitizeFileName(itemTitles[id]) or id.toString(), then
attempt candidate names (baseName, baseName + "_" + id, baseName + "_" + id +
"_1", incrementing a counter) until you find a name not in usedNames, assign
that to entryNameForId[id] and add to usedNames. Update the logic that produces
entryNameForId (referencing rawEntryNameForId, sanitizeFileName, activeItemDirs,
itemTitles) to use this uniqueness loop so no final-name collisions occur.
- Around line 119-131: The flat single-file fast path is only executed when
useSymlinks is true, causing items to be copied as directories under
WorkshopModPathStrategy.CopyIntoDir; move the allSingleFile detection out of the
useSymlinks guard so it runs regardless of linking mode and invoke
syncFlatFilesIntoDir with the current useSymlinks value (e.g., add a parameter
useSymlinks to syncFlatFilesIntoDir or overload it) so that syncFlatFilesIntoDir
will create a symlink when useSymlinks is true or copy the file into place when
false; update the implementation of syncFlatFilesIntoDir to branch on the new
useSymlinks flag and perform file copy behavior for CopyIntoDir.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 863b05b8-6585-45be-b7f2-9fc6d83a047b

📥 Commits

Reviewing files that changed from the base of the PR and between da34e96 and 63f1916.

📒 Files selected for processing (5)
  • app/src/main/java/app/gamenative/ui/PluviaMain.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/GeneralTab.kt
  • app/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.kt
  • app/src/main/java/app/gamenative/workshop/WorkshopModPathDetector.kt
  • app/src/main/java/app/gamenative/workshop/WorkshopSymlinker.kt
✅ Files skipped from review due to trivial changes (1)
  • app/src/main/java/app/gamenative/workshop/WorkshopModPathDetector.kt
🚧 Files skipped from review as they are similar to previous changes (1)
  • app/src/main/java/app/gamenative/ui/component/dialog/GeneralTab.kt

Comment thread app/src/main/java/app/gamenative/ui/PluviaMain.kt Outdated
Comment on lines +1757 to +1912
// Workshop mod sync: download subscribed mods and configure symlinks
if (container.isWorkshopMods) {
try {
setLoadingMessage("Checking Workshop mods...")
setLoadingProgress(-1f)
val steamClient = SteamService.instance?.steamClient
val steamId = SteamService.userSteamId
if (steamClient != null && steamId != null) {
val imageFs = ImageFs.find(context)
val winePrefix = imageFs.wineprefix
val fetchResult = WorkshopManager.getSubscribedItems(gameId, steamClient, steamId)
val items = fetchResult.items
if (items.isNotEmpty()) {
Timber.tag("Workshop").i("Found ${items.size} subscribed workshop items for appId=$gameId")
val workshopContentDir = WorkshopManager.getWorkshopContentDir(winePrefix, gameId)

if (fetchResult.succeeded) {
WorkshopManager.cleanupUnsubscribedItems(items, workshopContentDir)
} else {
Timber.tag("Workshop").w(
"Subscription fetch incomplete, skipping cleanup to preserve existing mods"
)
}
val itemsToSync = WorkshopManager.getItemsNeedingSync(items, workshopContentDir)

if (itemsToSync.isNotEmpty()) {
// Check available disk space before downloading
// Use 2x safety margin to account for LZMA decompression
// and CKM extraction creating temp files alongside originals
val requiredBytes = itemsToSync.sumOf { it.fileSizeBytes } * 2
val availableBytes = workshopContentDir.usableSpace
if (requiredBytes > 0 && availableBytes > 0 && requiredBytes > availableBytes) {
val reqMB = String.format("%.0f", requiredBytes / 1_048_576.0)
val avlMB = String.format("%.0f", availableBytes / 1_048_576.0)
Timber.tag("Workshop").e(
"Insufficient disk space: need ${reqMB}MB, have ${avlMB}MB"
)
SnackbarManager.show(
"Not enough space for workshop mods (need ${reqMB} MB, have ${avlMB} MB)"
)
} else {
Timber.tag("Workshop").i("Downloading ${itemsToSync.size} workshop items...")
setLoadingMessage("Downloading Workshop Mods (0/${itemsToSync.size})")
val licenses = SteamService.getLicensesFromDb()

// Track item-level progress so byte callbacks can include it
var currentCompleted = 0
var currentTitle = itemsToSync.firstOrNull()?.title ?: ""

val successCount = WorkshopManager.downloadItems(
items = itemsToSync,
steamClient = steamClient,
licenses = licenses,
workshopContentDir = workshopContentDir,
onItemProgress = { completed, total, title ->
currentCompleted = completed
currentTitle = title
Timber.tag("Workshop").d("Progress: $completed/$total - $title")
setLoadingMessage("Downloading Workshop Mods ($completed/$total)\n$title")
},
onBytesProgress = { downloaded, total ->
if (total > 0) {
setLoadingProgress(downloaded.toFloat() / total.toFloat())
val dlMB = String.format("%.1f", downloaded / 1_048_576.0)
val totalMB = String.format("%.1f", total / 1_048_576.0)
setLoadingMessage(
"Downloading Workshop Mods ($currentCompleted/${itemsToSync.size})\n" +
"$currentTitle\n" +
"${dlMB} MB / ${totalMB} MB"
)
}
},
onOverallProgress = { progress ->
Timber.tag("Workshop").d("Overall: ${(progress * 100).toInt()}%")
},
)

val failedCount = itemsToSync.size - successCount
if (failedCount > 0) {
Timber.tag("Workshop").w("$failedCount workshop mod(s) failed to download")
SnackbarManager.show(
"$failedCount workshop mod(s) failed to download"
)
}

WorkshopManager.fixItemFileNames(itemsToSync, workshopContentDir)
}
}

// Post-processing runs every launch (not just after downloads) so
// that files from a previous broken run get fixed retroactively.
// Order matters: decompress LZMA first so fixFileExtensions can
// read the real magic bytes (e.g. GMAD) instead of LZMA's 0x5D.
WorkshopManager.extractCkmFiles(workshopContentDir)
WorkshopManager.decompressLzmaFiles(workshopContentDir)
WorkshopManager.fixFileExtensions(workshopContentDir)

WorkshopManager.updateMarkerTimestamps(items, workshopContentDir)

setLoadingMessage("Configuring Workshop mods...")
setLoadingProgress(-1f)
val gameRootDir = File(SteamService.getAppDirPath(gameId))
val gameName = SteamService.getAppInfoOf(gameId)?.name ?: ""
WorkshopManager.configureModSymlinks(
gameRootDir = gameRootDir,
workshopContentDir = workshopContentDir,
items = items,
winePrefix = winePrefix,
gameName = gameName,
)
Timber.tag("Workshop").i("Workshop mod sync complete for appId=$gameId")
} else if (!fetchResult.succeeded) {
// Fetch failed and returned no items — don't wipe existing mods.
// Still configure symlinks so previously downloaded mods work offline.
Timber.tag("Workshop").w(
"Workshop fetch failed for appId=$gameId, preserving existing on-disk mods"
)
val workshopContentDir = WorkshopManager.getWorkshopContentDir(winePrefix, gameId)
if (workshopContentDir.exists()) {
setLoadingMessage("Configuring Workshop mods...")
setLoadingProgress(-1f)
val gameRootDir = File(SteamService.getAppDirPath(gameId))
val gameName = SteamService.getAppInfoOf(gameId)?.name ?: ""
WorkshopManager.configureModSymlinks(
gameRootDir = gameRootDir,
workshopContentDir = workshopContentDir,
items = emptyList(),
winePrefix = winePrefix,
gameName = gameName,
)
}
} else {
Timber.tag("Workshop").d("No subscribed workshop items for appId=$gameId")
// Clean up any previously downloaded mods and remove stale symlinks
val workshopContentDir = WorkshopManager.getWorkshopContentDir(winePrefix, gameId)
if (workshopContentDir.exists()) {
workshopContentDir.deleteRecursively()
Timber.tag("Workshop").i("Deleted workshop content for appId=$gameId")
}
val gameRootDir = File(SteamService.getAppDirPath(gameId))
val gameName = SteamService.getAppInfoOf(gameId)?.name ?: ""
WorkshopManager.configureModSymlinks(
gameRootDir = gameRootDir,
workshopContentDir = workshopContentDir,
items = emptyList(),
winePrefix = winePrefix,
gameName = gameName,
)
}
} else {
Timber.tag("Workshop").w("Steam client or Steam ID not available, skipping workshop sync")
}
} catch (e: Exception) {
Timber.tag("Workshop").e(e, "Workshop mod sync failed, continuing without mods")
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Disabling workshopMods never removes existing managed placements.

All workshop cleanup/configuration lives inside the if (container.isWorkshopMods) branch. After one enabled launch, flipping the toggle off skips any call that would remove the managed symlinks/copies, so those entries stay in the game directories and the mods keep loading.

Cleanup path to add
-        if (container.isWorkshopMods) {
+        if (container.isWorkshopMods) {
             try {
                 setLoadingMessage("Checking Workshop mods...")
                 setLoadingProgress(-1f)
                 val steamClient = SteamService.instance?.steamClient
                 val steamId = SteamService.userSteamId
@@
             } catch (e: Exception) {
                 Timber.tag("Workshop").e(e, "Workshop mod sync failed, continuing without mods")
             }
+        } else {
+            val imageFs = ImageFs.find(context)
+            val winePrefix = imageFs.wineprefix
+            val workshopContentDir = WorkshopManager.getWorkshopContentDir(winePrefix, gameId)
+            val gameRootDir = File(SteamService.getAppDirPath(gameId))
+            val gameName = SteamService.getAppInfoOf(gameId)?.name ?: ""
+            WorkshopManager.configureModSymlinks(
+                gameRootDir = gameRootDir,
+                workshopContentDir = workshopContentDir,
+                items = emptyList(),
+                winePrefix = winePrefix,
+                gameName = gameName,
+            )
         }
🧰 Tools
🪛 detekt (1.23.8)

[warning] 1789-1789: String.format("%.0f", requiredBytes / 1_048_576.0) uses implicitly default locale for string formatting.

(detekt.potential-bugs.ImplicitDefaultLocale)


[warning] 1790-1790: String.format("%.0f", availableBytes / 1_048_576.0) uses implicitly default locale for string formatting.

(detekt.potential-bugs.ImplicitDefaultLocale)


[warning] 1820-1820: String.format("%.1f", downloaded / 1_048_576.0) uses implicitly default locale for string formatting.

(detekt.potential-bugs.ImplicitDefaultLocale)


[warning] 1821-1821: String.format("%.1f", total / 1_048_576.0) uses implicitly default locale for string formatting.

(detekt.potential-bugs.ImplicitDefaultLocale)

🤖 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/PluviaMain.kt` around lines 1757 - 1912,
When container.isWorkshopMods is false the code never removes previously managed
mod placements; add a cleanup branch executed when container.isWorkshopMods is
false that locates the workshopContentDir via
WorkshopManager.getWorkshopContentDir(winePrefix, gameId) (use
ImageFs.find(context).wineprefix for winePrefix) and the gameRootDir via
File(SteamService.getAppDirPath(gameId)), delete or prune workshopContentDir
(e.g., deleteRecursively or remove managed files) and then call
WorkshopManager.configureModSymlinks(...) with items = emptyList(), winePrefix
and gameName (SteamService.getAppInfoOf(gameId)?.name ?: "") to remove stale
symlinks so disabling the toggle actually removes managed mods.

Comment thread app/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.kt Outdated
Comment thread app/src/main/java/app/gamenative/workshop/WorkshopSymlinker.kt
Comment thread app/src/main/java/app/gamenative/workshop/WorkshopSymlinker.kt Outdated
- Fix zero-subscription cleanup: use deleteWorkshopMods with game tree
  cleanup instead of separate content deletion + configureModSymlinks
  (which no-ops when content dir is already deleted)
- Enhance deleteWorkshopMods to also clean up installed mod entries:
  gbe_fork steam_settings/mods/ symlinks, strategy-detected game dirs,
  and Unity AppData targets
- Fix name collision disambiguation: use iterative uniqueness loop with
  usedNames set to prevent secondary collisions (e.g. 'foo', 'foo',
  'foo_42' scenario)
- Remove raw exception text from user-facing snackbar messages; log
  full exception via Timber instead
- Pass gameRootDir and gameName to deleteWorkshopMods from BaseAppScreen
  for full game tree cleanup on delete

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues 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/workshop/WorkshopSymlinker.kt">

<violation number="1" location="app/src/main/java/app/gamenative/workshop/WorkshopSymlinker.kt:141">
P2: Entry naming depends on `activeItemDirs` iteration order, which is derived from `listFiles()` and can vary. For colliding titles, different runs can assign different IDs to the unsuffixed base name, changing `expectedNames` and causing unnecessary stale cleanup/recreation.</violation>
</file>

<file name="app/src/main/java/app/gamenative/workshop/WorkshopManager.kt">

<violation number="1">
P2: Unity AppData cleanup only uses detectUnityModTargets (install-dir based), so auto-created AppData subtargets like Dreams (created during routing) are never cleaned and can leave stale mod links behind.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread app/src/main/java/app/gamenative/workshop/WorkshopSymlinker.kt Outdated
Comment thread app/src/main/java/app/gamenative/workshop/WorkshopManager.kt

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/PluviaMain.kt`:
- Around line 1790-1791: Update the String.format calls in PluviaMain.kt to use
an explicit Locale to avoid locale-dependent decimal separators: replace the two
usages producing reqMB and avlMB (String.format("%.0f", requiredBytes /
1_048_576.0) and String.format("%.0f", availableBytes / 1_048_576.0)) and the
download progress format calls at the other location with
String.format(Locale.US, ...) (or another explicit Locale of your choice) so the
output is consistent; ensure you import java.util.Locale and apply the same
change to the progress formatting code that produces the download percentage.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b2ab75a9-f7d7-45b4-805c-7a124ae0bb63

📥 Commits

Reviewing files that changed from the base of the PR and between 63f1916 and 70a6e1a.

📒 Files selected for processing (4)
  • app/src/main/java/app/gamenative/ui/PluviaMain.kt
  • app/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.kt
  • app/src/main/java/app/gamenative/workshop/WorkshopManager.kt
  • app/src/main/java/app/gamenative/workshop/WorkshopSymlinker.kt
✅ Files skipped from review due to trivial changes (1)
  • app/src/main/java/app/gamenative/workshop/WorkshopSymlinker.kt
🚧 Files skipped from review as they are similar to previous changes (1)
  • app/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.kt

Comment thread app/src/main/java/app/gamenative/ui/PluviaMain.kt Outdated
- Sort activeItemDirs.keys in WorkshopSymlinker to ensure deterministic
  naming for colliding titles across runs (lowest ID gets base name)
- Extend Phase 3 Unity AppData cleanup to find auto-created Dreams/
  peer directories next to Models/ targets
- Add explicit Locale.US to all String.format calls in PluviaMain to
  prevent locale-dependent decimal separators

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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/PluviaMain.kt`:
- Around line 1788-1798: The disk-space check currently reads usableSpace from
workshopContentDir which returns 0 when the directory doesn't exist, letting the
guard availableBytes > 0 bypass validation; fix the check in the block around
itemsToSync.sumOf { it.fileSizeBytes } by selecting an existing ancestor to
measure (e.g., use workshopContentDir if it exists, otherwise use
workshopContentDir.parentFile or walk up parents until an existing directory is
found) and call usableSpace on that fallback; then use that availableBytes in
the same comparison/logging/Snackbar logic so the insufficent-space path cannot
be skipped when the target directory is not yet created.

In `@app/src/main/java/app/gamenative/workshop/WorkshopSymlinker.kt`:
- Around line 228-234: The current collection into expectedFiles silently
overwrites files with the same name because expectedFiles is a map from filename
→ File; change it to record provenance and/or emit a warning on collisions:
update expectedFiles (referenced in WorkshopSymlinker.kt) to hold the source
item id along with the File (e.g., map value becomes a pair or small data class
like (File, itemId)) and when inserting in the loop over activeItemDirs detect
if expectedFiles already contains the filename and then log a clear warning
(including both item IDs and file path) or aggregate the conflict for later
reporting so collisions are not silently lost.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4c164fd1-e1f4-417c-bd68-247484ce1470

📥 Commits

Reviewing files that changed from the base of the PR and between 70a6e1a and 0110cfd.

📒 Files selected for processing (3)
  • app/src/main/java/app/gamenative/ui/PluviaMain.kt
  • app/src/main/java/app/gamenative/workshop/WorkshopManager.kt
  • app/src/main/java/app/gamenative/workshop/WorkshopSymlinker.kt

Comment thread app/src/main/java/app/gamenative/ui/PluviaMain.kt Outdated
Comment thread app/src/main/java/app/gamenative/workshop/WorkshopSymlinker.kt Outdated
- Walk up parent dirs for usableSpace when workshopContentDir does not
  yet exist, preventing the space guard from being silently skipped
- Log a warning when flat-file sync overwrites a filename contributed
  by a different workshop item, with both source paths for diagnosis

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 2 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/workshop/WorkshopSymlinker.kt">

<violation number="1" location="app/src/main/java/app/gamenative/workshop/WorkshopSymlinker.kt:234">
P2: Flat-file filename collisions are resolved by unsorted map iteration, so duplicate filenames can pick different source mods across runs.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread app/src/main/java/app/gamenative/workshop/WorkshopSymlinker.kt Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

♻️ Duplicate comments (3)
app/src/main/java/app/gamenative/ui/PluviaMain.kt (2)

1044-1056: ⚠️ Potential issue | 🟠 Major

Make this delete action Steam-only and pass the game root.

At Line 1048 this only passes context + appId, so deleteWorkshopMods() skips cleanupInstalledModEntries(...); CopyIntoDir placements in the game tree survive the delete. Because the callback is still wired unconditionally here, a non-Steam container can also trigger deletion against the same numeric app ID.

🤖 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/PluviaMain.kt` around lines 1044 - 1056,
The onDeleteWorkshopMods callback should only run for Steam containers and must
pass the game's root path so deleteWorkshopMods can perform
cleanupInstalledModEntries and remove CopyIntoDir placements; update the lambda
that calls WorkshopManager.deleteWorkshopMods(context, appId) to first check the
container/source is Steam (e.g., using the same predicate used elsewhere for
Steam-only actions) and call WorkshopManager.deleteWorkshopMods(context, appId,
gameRoot) (or the overload that accepts the game root) so
cleanupInstalledModEntries is executed; ensure the action is not wired for
non-Steam containers.

1760-1760: ⚠️ Potential issue | 🟠 Major

Disabling workshopMods still leaves old placements behind.

Everything that removes or reconfigures managed workshop entries is inside the enabled branch. After one launch with workshop mods on, flipping the toggle off skips any cleanup call, so existing symlinks/copies in the game directories keep loading.

🤖 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/PluviaMain.kt` at line 1760, The code
only runs workshop cleanup while container.isWorkshopMods is true, so toggling
it off leaves old placements; move or extract the cleanup/reconfiguration logic
out of the enabled-only block and ensure it runs when the toggle becomes false
(either in an else branch or immediately after detecting the change). Locate the
code inside the if (container.isWorkshopMods) block and invoke the existing
cleanup routine (e.g., the method that removes managed workshop entries /
symlinks/copies—search for names like removeManagedWorkshopEntries,
cleanupWorkshopPlacements, or similar) when the toggle is turned off so
symlinks/copies in game directories are removed or reverted. Ensure the cleanup
runs atomically on toggle-off and is not skipped by the early branch.
app/src/main/java/app/gamenative/workshop/WorkshopSymlinker.kt (1)

119-130: ⚠️ Potential issue | 🟠 Major

Handle flat single-file items in copy mode too.

Line 119 still gates the loose-file path behind useSymlinks. Under WorkshopModPathStrategy.CopyIntoDir, a single-file item gets copied as <target>/<title-or-id>/file.ext instead of the loose file the game scans for, so those mods remain undiscoverable.

🤖 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/WorkshopSymlinker.kt` around lines
119 - 130, The current logic only treats "allSingleFile" items as loose files
when useSymlinks is true, causing single-file mods to be nested under
<target>/<title-or-id>/file.ext in CopyIntoDir mode and become undiscoverable;
update the condition around the allSingleFile/syncFlatFilesIntoDir block (the
useSymlinks guard near function WorkshopSymlinker.kt that computes allSingleFile
and calls syncFlatFilesIntoDir) so that the flat-file path is applied not only
when useSymlinks is true but also when the chosen path strategy is
WorkshopModPathStrategy.CopyIntoDir (or simply remove the useSymlinks gate),
ensuring syncFlatFilesIntoDir is invoked for single-file items in copy mode as
well.
🤖 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/PluviaMain.kt`:
- Around line 1792-1801: The guard incorrectly treats availableBytes == 0 as
"unknown" and skips the insufficient-space branch; change how unknown is
represented and the conditional: set availableBytes = spaceDir?.usableSpace ?:
-1L and update the check to if (requiredBytes > 0 && availableBytes >= 0 &&
requiredBytes > availableBytes) so a truly full volume (0 bytes) triggers the
insufficient-space handling while a null/unknown spaceDir remains
distinguishable; use the existing symbols spaceDir, availableBytes,
requiredBytes, Timber and SnackbarManager to locate and update the code.
- Around line 1775-1776: The code calls
WorkshopManager.cleanupUnsubscribedItems(...) when fetchResult.succeeded, but
fetchResult.succeeded is set true after the first successful page in
WorkshopManager.getSubscribedItems(), so cleanup may run on a partial list;
modify the logic so cleanup only runs after the full subscription fetch
completes successfully: either (A) change WorkshopManager.getSubscribedItems()
to only set fetchResult.succeeded (or return a new flag like
fetchResult.isComplete) after all pages are fetched without error, or (B) update
the caller in PluviaMain.kt to check a new/available indicator (e.g.
fetchResult.isComplete or fetchResult.error == null &&
!fetchResult.hasMorePages) before calling
WorkshopManager.cleanupUnsubscribedItems(items, workshopContentDir); ensure you
reference fetchResult.succeeded, WorkshopManager.getSubscribedItems, and
cleanupUnsubscribedItems in the change.

In `@app/src/main/java/app/gamenative/workshop/WorkshopSymlinker.kt`:
- Around line 27-31: Replace the duplicated local set MOD_CONTAINER_NAMES in
WorkshopSymlinker with a reference to the detector's canonical set
WorkshopModPathDetector.HIGH_CONFIDENCE_NAMES so you don't drift; specifically
remove or stop maintaining the local MOD_CONTAINER_NAMES and use
WorkshopModPathDetector.HIGH_CONFIDENCE_NAMES wherever MOD_CONTAINER_NAMES is
used by functions/methods in WorkshopSymlinker (e.g., any place that currently
checks MOD_CONTAINER_NAMES) to ensure entries like "resourcepacks" and
"resource_packs" are handled consistently.
- Around line 329-340: In WorkshopSymlinker (the block that checks
Files.isSymbolicLink(linkPath) and the similar code around lines handling
mismatched symlink/directory), guard deletions so you do not remove foreign
user-managed entries: before Files.delete(linkPath) (and before removing
directories in the analogous block at 366-374), verify the existing entry
actually points into workshopContentBase (or resolves under workshopContentBase)
or contains the COPY_SENTINEL marker; if neither condition is true, return a
safe result (e.g., skip/reject) instead of deleting or recreating; update the
logic in the methods handling linkPath/currentTarget and the
directory-replacement branch to perform these checks and bail out when the entry
is external to workshopContentBase or lacks the sentinel.

---

Duplicate comments:
In `@app/src/main/java/app/gamenative/ui/PluviaMain.kt`:
- Around line 1044-1056: The onDeleteWorkshopMods callback should only run for
Steam containers and must pass the game's root path so deleteWorkshopMods can
perform cleanupInstalledModEntries and remove CopyIntoDir placements; update the
lambda that calls WorkshopManager.deleteWorkshopMods(context, appId) to first
check the container/source is Steam (e.g., using the same predicate used
elsewhere for Steam-only actions) and call
WorkshopManager.deleteWorkshopMods(context, appId, gameRoot) (or the overload
that accepts the game root) so cleanupInstalledModEntries is executed; ensure
the action is not wired for non-Steam containers.
- Line 1760: The code only runs workshop cleanup while container.isWorkshopMods
is true, so toggling it off leaves old placements; move or extract the
cleanup/reconfiguration logic out of the enabled-only block and ensure it runs
when the toggle becomes false (either in an else branch or immediately after
detecting the change). Locate the code inside the if (container.isWorkshopMods)
block and invoke the existing cleanup routine (e.g., the method that removes
managed workshop entries / symlinks/copies—search for names like
removeManagedWorkshopEntries, cleanupWorkshopPlacements, or similar) when the
toggle is turned off so symlinks/copies in game directories are removed or
reverted. Ensure the cleanup runs atomically on toggle-off and is not skipped by
the early branch.

In `@app/src/main/java/app/gamenative/workshop/WorkshopSymlinker.kt`:
- Around line 119-130: The current logic only treats "allSingleFile" items as
loose files when useSymlinks is true, causing single-file mods to be nested
under <target>/<title-or-id>/file.ext in CopyIntoDir mode and become
undiscoverable; update the condition around the
allSingleFile/syncFlatFilesIntoDir block (the useSymlinks guard near function
WorkshopSymlinker.kt that computes allSingleFile and calls syncFlatFilesIntoDir)
so that the flat-file path is applied not only when useSymlinks is true but also
when the chosen path strategy is WorkshopModPathStrategy.CopyIntoDir (or simply
remove the useSymlinks gate), ensuring syncFlatFilesIntoDir is invoked for
single-file items in copy mode as well.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 417e3d82-7438-4995-9227-ac0de18d9450

📥 Commits

Reviewing files that changed from the base of the PR and between 0110cfd and 1ac4d72.

📒 Files selected for processing (2)
  • app/src/main/java/app/gamenative/ui/PluviaMain.kt
  • app/src/main/java/app/gamenative/workshop/WorkshopSymlinker.kt

Comment thread app/src/main/java/app/gamenative/ui/PluviaMain.kt Outdated
Comment thread app/src/main/java/app/gamenative/ui/PluviaMain.kt Outdated
Comment thread app/src/main/java/app/gamenative/workshop/WorkshopSymlinker.kt Outdated
Comment thread app/src/main/java/app/gamenative/workshop/WorkshopSymlinker.kt
Sort flat-file sync iteration by item ID so the lowest ID consistently
wins when multiple workshop items contribute files with the same name.
Skip duplicates instead of overwriting, matching the directory symlink
naming convention.
…dup constants

- Use -1L sentinel for unknown available space so a truly-full volume
  (0 bytes free) still triggers the insufficient-space guard
- Add isComplete flag to WorkshopFetchResult; only run cleanup when all
  subscription pages were fetched without error, not just the first page
- Guard ensureSymlink/ensureCopy against foreign entries: verify symlinks
  point into workshopContentBase and directories have our copy sentinel
  before replacing or deleting them
- Replace duplicated MOD_CONTAINER_NAMES with reference to
  WorkshopModPathDetector.HIGH_CONFIDENCE_NAMES canonical set
- Pass gameRootDir/gameName to deleteWorkshopMods in container config
  dialog so cleanupInstalledModEntries runs properly

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue 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/ui/PluviaMain.kt">

<violation number="1" location="app/src/main/java/app/gamenative/ui/PluviaMain.kt:1048">
P2: Workshop delete parses `appId` as raw Int, which fails for container IDs and can skip game-tree mod cleanup.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread app/src/main/java/app/gamenative/ui/PluviaMain.kt Outdated
Container IDs like STEAM_1234567_12345 fail toIntOrNull(), causing
gameRootDir to be null and skipping cleanupInstalledModEntries. Use
ContainerUtils.extractGameIdFromContainerId to properly extract the
numeric game ID from the container ID string.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd recommend removing the relative mouse functionality & controlsPorfile bindng changes and put them into a different PR.

It makes this PR more difficult to review and overall, we should have 1 intent per PR.

@Nightwalker743 Nightwalker743 Mar 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, should be good to go now, sorry about that!

@Nightwalker743 Nightwalker743 Mar 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@phobos665

Just FYI I feel like this disclaimer should be mentioned somewhere in the discord in regards to workshop support to hopefully help with "customer support" lol:

*** DISCLAIMER *** If workshop mods don't appear to work for you, you could try going to Open Container then navigating to C:/Program Files x86/Steam/steamapps/workshop/content/game's id/ and copy all of the mod folders there that are just a bunch of numbers then move them to wherever your game's mod folder is if there is one. Look up instructions on how to mod your game, mods might be in the A drive (your game's directory) in a Mods folder. Or it might be in C:/Users/xuser/Appdata then in either Local, LocalLow or Roaming somewhere but you have to find the game folder there yourself since it might be under the studio's name as an example. Workshop support can be very picky sometimes files need to be decompressed, or renamed to a different file type, so don't expect mods to work perfectly for every game. Some game's workshop support will not work without internet connection and/or steam client running, Portal 2 and Killing Floor are two examples.

That being said, these are games that I've confirmed work for me on a Fold 7:

Age of Empires 2 HD

Banished

Broforce

Command and Conquer Generals Zero Hour

Dont Starve Together

Garry's Mod

Going Medieval

Insurgency (mostly, issue with text in the UI being re-named. Singleplayer renamed to #GameUI_Singleplayer for instance. Still works though)

Left 4 Dead 2

Mount and Blade Warband

Rimworld

Star Wars Knights of the Old Republic 2

Starbound

Superliminal

Terraria

Skyrim

Tooth and Tail

Total Tank Simulator

Viscera Cleanup Detail

Will not work:

Portal 2

Killing Floor, it seems like the mods download correctly and are listed in the main menu but you can not click on "Steam Workshop content/mods", seemingly because you don't have an internet connection or steam isn't on.

Half Life 2, seems like it needs to be online too?? Strangely enough...

Men of War Assault Squad 2

These features are being split into separate PRs:
- Relative mouse movement toggle
- ControlsProfile binding deserialization fix

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/com/winlator/container/Container.java">

<violation number="1">
P2: Per-container relative mouse movement persistence was removed without migration/replacement, causing previously saved user setting to be lost.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread app/src/main/java/com/winlator/container/Container.java
Comment on lines +1832 to +1847
onBytesProgress = { downloaded, total ->
if (total > 0) {
setLoadingProgress(downloaded.toFloat() / total.toFloat())
val dlMB = String.format(Locale.US, "%.1f", downloaded / 1_048_576.0)
val totalMB = String.format(Locale.US, "%.1f", total / 1_048_576.0)
setLoadingMessage(
"Downloading Workshop Mods ($currentCompleted/${itemsToSync.size})\n" +
"$currentTitle\n" +
"${dlMB} MB / ${totalMB} MB"
)
}
},
onOverallProgress = { progress ->
Timber.tag("Workshop").d("Overall: ${(progress * 100).toInt()}%")
},
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we already have stringUtils for doing calculations for download progress. Best check there! (Or network utils, I forget exactly).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll have a look tonight, thanks for the suggestion!

Comment on lines +536 to +541
private val KNOWN_EXTENSIONS = setOf(
"gma", "vpk", "bsp", "zip", "rar", "7z",
"bsa", "esp", "esm", "ckm", "pak", "bin",
"txt", "cfg", "lua", "mdl", "vmt", "vtf",
"wav", "mp3", "ogg", "png", "jpg", "jpeg",
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this could probably go into your WorkshopItem.kt instead so this file is more for business logic

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll have a look at this as well tonight, thanks!

@phobos665

Copy link
Copy Markdown
Contributor

@utkarshdalal this might be a wild suggestion, but how much of this logic should we pull into JavaSteam and then expose to GN?

I'll need to read more as it's a chunky boy

@Pepelespooder

Pepelespooder commented Mar 23, 2026

Copy link
Copy Markdown

Just as an aside I worked on some of this last night and i ran into an issue with Godot There has to be a For instance P drive symlink that bypasses (x86) parenthesis as it fails no matter what when (x86) is inside the path. I haven't gone too deep yet but figured id make a note here.


You also have to race condition the Goldberg DLL as gbe and regular goldberg are both used and they require different mod setups


Sometimes Steam returns First instead of community and in my implementation i had to edit the javasteam implementation to include First as 1:1 for community

@Nightwalker743

Copy link
Copy Markdown
Contributor Author

@utkarshdalal Sure, this is a video showing it working, figured I'd show it updating some mods rather than installing a bunch since that might take a while, lol.

https://drive.google.com/file/d/1BEgo_a8A9sgbeBV7zhkjx1YglygTOhhk/view?usp=drivesdk

@utkarshdalal

Copy link
Copy Markdown
Owner

Hm, I think putting it in the general tab is not how we should do this (inconsistent UX, and also it'll need to go in the default container config dialog which will open a new can of worms)

I think we do it like we have manage DLC - we can rename "Manage DLC" to "DLC & Workshop" or add a new "Workshop" button:

  • On initial install, show the workshop mods as a list of checkboxes that can be included by the user (it could be in the same manage DLC modal)
  • Similarly, if they were not checked on install, the user can later click "DLC & Workshop" and check the workshop items, they will install
  • For deletion, we can disable deletion of workshop items (like we do for DLC) by greying out the checkboxes once installed, or by allowing deletion like you seem to have already. It's totally fine to not allow deletion.

Thoughts? We can speak on Discord if that's easier.

@Nightwalker743

Nightwalker743 commented Mar 26, 2026

Copy link
Copy Markdown
Contributor Author

Updated video after these updates:

https://drive.google.com/file/d/1NgdhTAdNZnmb429XSZn0NL0v02wSjvOE/view?usp=drivesdk

In regards to deleting mods, when a mod is unchecked in the list, it's marked for deletion on the next game launch. You can see on my second launch in this link that I uncheck a couple mods and they're gone from the in-game mod manager (since they were deleted)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes to this file are setting off alarm bells for me!
Are these changes related to workshop? If not, please undo

Timber.tag("Workshop").i(
"Workshop launch check: workshopMods=${container.isWorkshopMods}, enabledIds=${enabledWorkshopIds.size}"
)
if (container.isWorkshopMods && enabledWorkshopIds.isNotEmpty()) {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this will run on every single launch?
Seems excessive to do this check every time, and seems a little unintuitive that the workshop mods get installed on boot rather than when the dialog is closed (like what happens with the DLC manager).

We should trigger the download when the dialog is closed and new workshop items are selected.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

revert these changes please, seems like just rearranging strings?

Comment on lines +299 to +300
workshopMods = container.isWorkshopMods,
enabledWorkshopItemIds = container.enabledWorkshopItemIds,

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm, i'm not sure that storing these in the container is a good idea. If someone imports a config from the compatibility list, these will get overwritten. Similarly if someone exports a config with the workshop mods and someone else imports it, they'll be included.
Probably better to add a column to steamapp.kt and SteamAppDao.kt for storing installed workshop mods. That is more persistent, and probably less lines of code.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 20 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/ContainerUtils.kt">

<violation number="1" location="app/src/main/java/app/gamenative/utils/ContainerUtils.kt:746">
P2: If restore rename fails, the preserved workshop directory is deleted, which can permanently lose workshop data after a successful preservation step.</violation>
</file>

<file name="app/src/main/java/app/gamenative/service/SteamService.kt">

<violation number="1" location="app/src/main/java/app/gamenative/service/SteamService.kt:3360">
P2: Auto-resumed workshop downloads bypass the Wi‑Fi-only guard; `resumePendingWorkshopDownloads()` starts downloads directly and `WorkshopManager.startWorkshopDownload` has no connectivity check, so users with `downloadOnWifiOnly=true` can start downloads on cellular after login.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread app/src/main/java/app/gamenative/utils/ContainerUtils.kt Outdated
Comment thread app/src/main/java/app/gamenative/service/SteamService.kt
…ve, cleanup fixes

- Move workshop_mods/enabled_workshop_item_ids from Container to SteamApp DB
  (Room v15->v17 with AutoMigrations) per @utkarshdalal review
- Download-on-save instead of blocking launch download per @utkarshdalal review
- Launch-time update check with 100MB threshold prompt for large updates
- Revert unrelated strings.xml rearrangements per @utkarshdalal review
- Workshop download pause/resume and queue persistence across app restarts
- Properly clean up symlinks when user deselects all mods (fixes @coderabbitai issue)
- Preview image support for workshop mod thumbnails
- Consolidate duplicated toRealPath pattern in WorkshopSymlinker
- Preserve workshop content during container corruption recovery
- Default mods to unselected (user must opt-in)
- Clear workshop state on app uninstall
- 35 unit tests for WorkshopManager
@Nightwalker743
Nightwalker743 force-pushed the New-Workshop-Implementation branch from 6a0d6c6 to 1bd82ee Compare March 29, 2026 00:59

@utkarshdalal utkarshdalal Mar 30, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add the new strings to all the translation files please? AI translations are fine

…ementation

# Conflicts:
#	app/schemas/app.gamenative.db.PluviaDatabase/17.json
#	app/src/main/java/app/gamenative/db/PluviaDatabase.kt
Removed note about upstream disabling auto-migration from 16 to 17 due to duplicate column. - Not needed.
- Replace our 17.json with upstream's (no workshop columns) to match
  users who upgraded via PR utkarshdalal#1048
- Disable AutoMigration(16→17) per upstream (duplicate ufs_parse_version)
- Add manual no-op Migration(16,17) so v16 users get a clean path
  to v17 without destructive fallback
- AutoMigration(17→18) adds all 3 workshop columns in one step

Migration paths:
  v16 → 17 (manual no-op) → 18 (auto: workshop columns)
  v17 → 18 (auto: workshop columns)
Comment on lines -563 to -564
} else if (downloadProgress in 0f..1f && downloadProgress < 1f) {
downloadStatusMessage?.takeUnless { it.isBlank() } ?: ""

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did we remove this? It'll remove the messages like "fetching manifest" at the beginning etc if I'm not wrong? Or will the code at like 813 preserve that?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code at 813 preserves it, I re-downloaded a game and I still get the downloading manifest status

Image

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's undo this please, unless there was a reason to do this? Worried that it could break something since it's for downloads that are in progress.

try {
// Skip workshop sync if Steam isn't fully connected yet
// (can happen if the user taps Play immediately after opening the app).
if (!SteamService.isLoggedIn) {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line should probably be if (isOffline || !SteamService.isConnected || !SteamService.isLoggedIn) { to align with cloud saves.
This would break on Steam offline mode or when the user doesn't have internet (we should skip checking for workshop updates if the user doesn't have internet.

Comment on lines +720 to +746
// Preserve workshop content before deleting the corrupt container
val workshopBase = File(containerDir, ".wine/drive_c/Program Files (x86)/Steam/steamapps/workshop")
val tempWorkshop = if (workshopBase.exists() && workshopBase.isDirectory) {
val temp = File(context.cacheDir, "workshop_preserve_$containerId")
temp.deleteRecursively()
if (workshopBase.renameTo(temp)) {
Timber.i("Preserved workshop content from corrupted container: $containerId")
temp
} else null
} else null

FileUtils.delete(containerDir)
// Retry container creation after cleanup
container = containerManager.createContainerFuture(containerId, data).get()

// Restore preserved workshop content into the fresh container
if (tempWorkshop != null && tempWorkshop.exists() && container != null) {
val newWorkshopBase = File(
container.rootDir,
".wine/drive_c/Program Files (x86)/Steam/steamapps/workshop",
)
newWorkshopBase.parentFile?.mkdirs()
if (tempWorkshop.renameTo(newWorkshopBase)) {
Timber.i("Restored workshop content into new container: $containerId")
} else {
Timber.w("Failed to restore workshop content for $containerId, preserved at: ${tempWorkshop.absolutePath}")
}

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious if you ever faced this happening? That the workshop mods needed to be preserved on a corrupted container?
When did you see containers getting corrupted?

val steamClient = SteamService.instance?.steamClient ?: return null
val steamId = SteamService.userSteamId ?: return null

val fetchResult = getSubscribedItems(appId, steamClient, steamId)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (!fetchResult.succeeded || !fetchResult.isComplete) {
    Timber.tag(TAG).w("Workshop fetch incomplete/failed for appId=$appId; skipping update check")
    return null
}

Does it make sense to add something like that here?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a lot of the code in here to find the steam_settings folder, writing mods.json, and then writing the mod files belongs in SteamUtils.kt under ensureSteamSettings and replaceSteamApi.

That way, we won't need to look for the steam_settings folder, we just create the files under the steam_settings folder we create under ensureSteamSettings.

Is that possible to do? Or not because at the time of calling replaceSteamApi, we don't know which mods are being used?

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues 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/workshop/WorkshopManager.kt">

<violation number="1">
P2: Update checks are skipped entirely when subscriptions exceed the MAX_PAGES*PAGE_SIZE cap, because isComplete stays false and the new guard returns null. Heavy users (>5000 items) will never get update checks despite having partial results.</violation>
</file>

<file name="app/src/main/java/app/gamenative/utils/ContainerUtils.kt">

<violation number="1">
P1: Corrupted-container recovery now deletes the whole container without preserving/restoring `.steamapps/workshop`, causing workshop mod data loss on recovery.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

@@ -0,0 +1,3279 @@
package app.gamenative.workshop

@cubic-dev-ai cubic-dev-ai Bot Mar 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Update checks are skipped entirely when subscriptions exceed the MAX_PAGES*PAGE_SIZE cap, because isComplete stays false and the new guard returns null. Heavy users (>5000 items) will never get update checks despite having partial results.

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/workshop/WorkshopManager.kt, line 3195:

<comment>Update checks are skipped entirely when subscriptions exceed the MAX_PAGES*PAGE_SIZE cap, because isComplete stays false and the new guard returns null. Heavy users (>5000 items) will never get update checks despite having partial results.</comment>

<file context>
@@ -3191,6 +3191,12 @@ object WorkshopManager {
 
         val fetchResult = getSubscribedItems(appId, steamClient, steamId)
+
+        if (!fetchResult.succeeded || !fetchResult.isComplete) {
+            Timber.tag(TAG).w("Workshop fetch incomplete/failed for appId=$appId; skipping update check")
+            return null
</file context>
Fix with Cubic

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not needed, no one is using >5000 mods. And if they are, certainly not on a phone/mobile device.

@utkarshdalal
utkarshdalal merged commit 0c687e3 into utkarshdalal:master Mar 31, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants