Add Nexus Mods support#1545
Open
Nightwalker743 wants to merge 9 commits into
Open
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds Nexus Mods integration: libarchive dependency, Room v23 schema + migrations and ModDao, archive extraction, FOMOD/Bethesda recipe logic, placement/materializer, Nexus API client and import orchestration service, Compose UI and strings, DI/prefs wiring, and tests. ChangesNexus mod management
Sequence Diagram(s)sequenceDiagram
participant UI
participant Service as NexusModImportService
participant Manager as NexusModManager
participant Extractor as ModArchiveExtractor
participant Materializer as ModMaterializer
UI->>Service: enqueue import / resume
Service->>Manager: importNexusFile(request)
Manager->>Extractor: extract archive (zip/7z/rar)
Manager->>Materializer: apply placement recipes
Materializer->>Manager: apply results + overwrite manifests
Manager->>UI: progress updates and final status
Estimated code review effort🎯 5 (Critical) | ⏱️ ~90+ minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
28 issues found across 69 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/mods/ModPlacementPreset.kt">
<violation number="1" location="app/src/main/java/app/gamenative/mods/ModPlacementPreset.kt:55">
P2: Hardcoded user-facing strings should be declared in Android string resources instead of inline literals.</violation>
</file>
<file name="app/src/main/java/app/gamenative/mods/ModDownloadRegistry.kt">
<violation number="1" location="app/src/main/java/app/gamenative/mods/ModDownloadRegistry.kt:26">
P2: `start()` unconditionally clears pending cancel flags, which can lose valid cancel requests in startup races.</violation>
<violation number="2" location="app/src/main/java/app/gamenative/mods/ModDownloadRegistry.kt:34">
P2: Hardcoded UI status string bypasses localization</violation>
<violation number="3" location="app/src/main/java/app/gamenative/mods/ModDownloadRegistry.kt:65">
P2: `canceledImports` may leak stale install IDs because `requestCancel()` blindly adds any ID without checking whether it corresponds to an active download. Cleanup only occurs in `start()` and `finish()`, which are not guaranteed to be called for every ID passed to `requestCancel()` (e.g., invalid IDs, stale UI states, or interrupted flows). This can cause the set to grow unbounded in memory over the app's lifetime.</violation>
</file>
<file name="app/src/main/java/app/gamenative/mods/BethesdaPluginManager.kt">
<violation number="1" location="app/src/main/java/app/gamenative/mods/BethesdaPluginManager.kt:241">
P2: Plugin entries are deduplicated with first-occurrence-wins (`distinctBy`), which can preserve stale enabled/disabled states when duplicate lines exist. A last-occurrence-wins approach is safer for line-based config files.</violation>
</file>
<file name="app/src/main/java/app/gamenative/data/ModInstall.kt">
<violation number="1" location="app/src/main/java/app/gamenative/data/ModInstall.kt:44">
P1: No database-level constraint ensures only one active mod profile per app, risking nondeterministic profile selection.</violation>
</file>
<file name="app/src/main/java/app/gamenative/mods/ModArchiveInstallAssessor.kt">
<violation number="1" location="app/src/main/java/app/gamenative/mods/ModArchiveInstallAssessor.kt:23">
P2: Hardcoded user-visible strings (queueMessage and assessment reasons) should be extracted to strings.xml and referenced via resources instead of inline literals.</violation>
</file>
<file name="app/src/main/java/app/gamenative/mods/FomodAutoSelector.kt">
<violation number="1" location="app/src/main/java/app/gamenative/mods/FomodAutoSelector.kt:73">
P2: Hardcoded English reason strings bypass Android string resources and localization conventions.</violation>
</file>
<file name="app/src/main/java/app/gamenative/mods/ModConflictAnalyzer.kt">
<violation number="1" location="app/src/main/java/app/gamenative/mods/ModConflictAnalyzer.kt:33">
P2: Analyzer performs full in-memory materialization of all planned files, which may not scale for large mods/libraries. Multiple `.toList()` calls and `flatMap` create large intermediate collections eagerly. For large mod packs this can cause high memory pressure and long pauses.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| winePrefix: String, | ||
| ): List<ModFileConflictReport> = withContext(Dispatchers.IO) { | ||
| val installById = installs.associateBy { it.installId } | ||
| val plannedFiles = installs.flatMap { install -> |
Contributor
There was a problem hiding this comment.
P2: Analyzer performs full in-memory materialization of all planned files, which may not scale for large mods/libraries. Multiple .toList() calls and flatMap create large intermediate collections eagerly. For large mod packs this can cause high memory pressure and long pauses.
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/mods/ModConflictAnalyzer.kt, line 33:
<comment>Analyzer performs full in-memory materialization of all planned files, which may not scale for large mods/libraries. Multiple `.toList()` calls and `flatMap` create large intermediate collections eagerly. For large mod packs this can cause high memory pressure and long pauses.</comment>
<file context>
@@ -0,0 +1,108 @@
+ winePrefix: String,
+ ): List<ModFileConflictReport> = withContext(Dispatchers.IO) {
+ val installById = installs.associateBy { it.installId }
+ val plannedFiles = installs.flatMap { install ->
+ val recipes = recipesByInstallId[install.installId].orEmpty()
+ runCatching {
</file context>
Comment on lines
+241
to
+245
| .distinctBy { it.fileName.lowercase() } | ||
| .mapNotNull { plugin -> | ||
| when { | ||
| plugin.enabled -> enabledPluginLine(plugin.fileName, usesMarkers) | ||
| usesMarkers -> plugin.fileName |
Contributor
There was a problem hiding this comment.
P2: Plugin entries are deduplicated with first-occurrence-wins (distinctBy), which can preserve stale enabled/disabled states when duplicate lines exist. A last-occurrence-wins approach is safer for line-based config files.
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/mods/BethesdaPluginManager.kt, line 241:
<comment>Plugin entries are deduplicated with first-occurrence-wins (`distinctBy`), which can preserve stale enabled/disabled states when duplicate lines exist. A last-occurrence-wins approach is safer for line-based config files.</comment>
<file context>
@@ -0,0 +1,522 @@
+ .distinctPluginLines(game)
+ val baseLines = basePlugins.map { enabledPluginLine(it, usesMarkers) }
+ val managedLines = managedPlugins
+ .distinctBy { it.fileName.lowercase() }
+ .mapNotNull { plugin ->
+ when {
</file context>
Suggested change
| .distinctBy { it.fileName.lowercase() } | |
| .mapNotNull { plugin -> | |
| when { | |
| plugin.enabled -> enabledPluginLine(plugin.fileName, usesMarkers) | |
| usesMarkers -> plugin.fileName | |
| private fun readPluginEntries(file: File, game: BethesdaGame?): List<PluginEntry> = | |
| pluginFileVariants(file) | |
| .flatMap { existingFile -> if (existingFile.isFile) existingFile.readLines() else emptyList() } | |
| .mapNotNull { parsePluginLine(it, game) } | |
| .asReversed() | |
| .distinctBy { it.fileName.lowercase() } | |
| .reversed() |
| } | ||
| } | ||
|
|
||
| fun requestCancel(installId: String) { |
Contributor
There was a problem hiding this comment.
P2: canceledImports may leak stale install IDs because requestCancel() blindly adds any ID without checking whether it corresponds to an active download. Cleanup only occurs in start() and finish(), which are not guaranteed to be called for every ID passed to requestCancel() (e.g., invalid IDs, stale UI states, or interrupted flows). This can cause the set to grow unbounded in memory over the app's lifetime.
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/mods/ModDownloadRegistry.kt, line 65:
<comment>`canceledImports` may leak stale install IDs because `requestCancel()` blindly adds any ID without checking whether it corresponds to an active download. Cleanup only occurs in `start()` and `finish()`, which are not guaranteed to be called for every ID passed to `requestCancel()` (e.g., invalid IDs, stale UI states, or interrupted flows). This can cause the set to grow unbounded in memory over the app's lifetime.</comment>
<file context>
@@ -0,0 +1,73 @@
+ }
+ }
+
+ fun requestCancel(installId: String) {
+ synchronized(canceledImports) {
+ canceledImports += installId
</file context>
| installId = installId, | ||
| appId = appId, | ||
| displayName = displayName, | ||
| status = "Starting", |
Contributor
There was a problem hiding this comment.
P2: Hardcoded UI status string bypasses localization
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/mods/ModDownloadRegistry.kt, line 34:
<comment>Hardcoded UI status string bypasses localization</comment>
<file context>
@@ -0,0 +1,73 @@
+ installId = installId,
+ appId = appId,
+ displayName = displayName,
+ status = "Starting",
+ )
+ downloads.update { current -> current + (installId to info) }
</file context>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 16
🧹 Nitpick comments (5)
app/src/test/java/app/gamenative/mods/NexusApiClientTest.kt (1)
29-32: ⚡ Quick winConsider cleaning up the OkHttpClient in tearDown.
The
OkHttpClientcreated insetUp()maintains internal thread pools (dispatcher and connection pool) that are not shut down when each test completes. While the JVM will eventually garbage-collect these resources, explicitly cleaning them up prevents thread accumulation across test executions.♻️ Recommended cleanup
`@After` fun tearDown() { + client.dispatcher.executorService.shutdown() + client.connectionPool.evictAll() server.shutdown() }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/test/java/app/gamenative/mods/NexusApiClientTest.kt` around lines 29 - 32, The tearDown() currently only calls server.shutdown(), but the OkHttpClient created in setUp() must also be cleaned up to avoid lingering threads; in tearDown() (the teardown method) call the OkHttpClient instance's cleanup routines: shut down its dispatcher executor service (client.dispatcher().executorService().shutdown()), evict connections from its connection pool (client.connectionPool().evictAll()), and close its cache if present (client.cache()?.close()), ensuring you reference the OkHttpClient variable created in setUp() so tests fully release resources.app/src/main/java/app/gamenative/data/ModInstall.kt (1)
40-66: ⚡ Quick winConsider adding a unique partial index to enforce single active profile per app.
The schema allows multiple profiles for the same
app_idto haveactive = truesimultaneously, which could lead to ambiguity about which profile is current. WhileModProfileManager.activateProfilelikely handles deactivation in application code, a database-level constraint would provide stronger guarantees.🔒 Suggested partial index to enforce single active profile
Add this to the
indicesarray:`@Entity`( tableName = "mod_profile", indices = [ Index("app_id"), Index(value = ["app_id", "name"], unique = true), + Index(value = ["app_id"], unique = true, where = "active = 1"), ], )Note: Room's
@Indexannotation does not support awhereclause as of Room 2.x. You would need to add this constraint via a migration SQL statement:CREATE UNIQUE INDEX IF NOT EXISTS `index_mod_profile_app_id_active` ON `mod_profile` (`app_id`) WHERE `active` = 1🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/app/gamenative/data/ModInstall.kt` around lines 40 - 66, The ModProfile entity currently allows multiple active profiles per app; add a DB-level unique partial index to enforce a single active profile by creating a migration that runs the SQL: create a unique index on app_id where active = 1 for table mod_profile (index name like index_mod_profile_app_id_active) since Room's `@Index` can't express WHERE; add this migration alongside your existing migrations and ensure ModProfileManager.activateProfile logic remains in sync with the migration so only one profile per app can be active.app/src/main/java/app/gamenative/ui/component/dialog/NexusModsImportSections.kt (2)
319-324: ⚡ Quick winAvoid O(n²) indexing in collection row rendering.
pending.mods.indexOf(mod)runs a linear search for each displayed item. Use indexed iteration to keep rendering work linear.Suggested fix
- filteredMods.forEach { mod -> + filteredMods.forEach { mod -> val key = mod.collectionKey() val queueItem = queueItems[key] val selected = key in selectedKeys - val index = pending.mods.indexOf(mod) + val index = pending.mods.indexOf(mod) // replace by precomputed index map or indexed list// Example approach outside this block: val indexByKey = remember(pending.mods) { pending.mods.mapIndexed { idx, m -> m.collectionKey() to idx }.toMap() }- val index = pending.mods.indexOf(mod) + val index = indexByKey[key] ?: -1🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/app/gamenative/ui/component/dialog/NexusModsImportSections.kt` around lines 319 - 324, The render loop in NexusModsImportSections.kt uses pending.mods.indexOf(mod) inside filteredMods.forEach, causing O(n²) work; replace this by computing indexes once (e.g., with pending.mods.mapIndexed to build an indexByKey map inside a remember block) or iterate using forEachIndexed so you can look up the index in O(1) when rendering each item; update the code that references pending.mods.indexOf(mod) (the line inside the filteredMods.forEach block) to use the precomputed index lookup (indexByKey[mod.collectionKey()] or the forEachIndexed index) and keep existing uses of queueItems, selectedKeys and mod.collectionKey() unchanged.
63-127: 🏗️ Heavy liftMove new user-facing strings into
strings.xml.This section introduces many hardcoded labels/messages, which bypasses localization and makes translation/consistency maintenance difficult for the new Nexus flow.
Also applies to: 144-175, 233-257, 268-281, 337-414, 438-441, 455-463, 520-523, 573-582, 627-628
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/app/gamenative/ui/component/dialog/NexusModsImportSections.kt` around lines 63 - 127, Replace hardcoded user-facing strings in the Nexus Mods composables with string resources: move literals like "Nexus account", the help texts, "Personal API key", "Save and check key", "Add from Nexus Mods", "Mod or collection URL", "Find files" (and the other occurrences noted) into strings.xml and use stringResource(...) in the composable functions (e.g., the top-level Nexus Mods auth UI and ImportSection). Update calls in ImportSection, the NoExtractOutlinedTextField labels, Buttons, Texts and any validation messages to reference those resource IDs instead of hardcoded literals so the UI is localizable and consistent.app/src/main/java/app/gamenative/ui/screen/library/components/GameOptionsPanel.kt (1)
346-348: 💤 Low valueConsider distinct icons for
ManageGameContentandManageMods.Both
ManageGameContent(line 346) andManageMods(line 348) map toIcons.Default.Apps. Since both options appear in the same Help & Info section (lines 401-404), users will see two adjacent menu items with identical icons, which may reduce clarity.Consider assigning a distinct icon to
ManageModsto improve visual differentiation—for example:- AppOptionMenuType.ManageMods -> Icons.Default.Apps + AppOptionMenuType.ManageMods -> Icons.Default.Extension // or another suitable icon🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/app/gamenative/ui/screen/library/components/GameOptionsPanel.kt` around lines 346 - 348, Two menu entries use the same icon: AppOptionMenuType.ManageGameContent and AppOptionMenuType.ManageMods both map to Icons.Default.Apps in GameOptionsPanel.kt; change the mapping for AppOptionMenuType.ManageMods to a distinct, semantically appropriate icon (for example Icons.Default.Extension or Icons.Default.Build) so the Help & Info section shows visually distinct icons; update the branch/switch case that returns the icon for AppOptionMenuType.ManageMods to the chosen alternative and ensure imports remain correct.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/java/app/gamenative/db/dao/ModDao.kt`:
- Around line 142-146: The method replaceOverwriteManifestsForTargets currently
returns early when manifests.isEmpty(), preventing deletion of previously stored
manifests; change it so that when manifests.isEmpty() you still remove existing
entries for that installId (e.g. call a DAO method to delete all overwrite
manifests for the installId such as
deleteOverwriteManifestsForInstall(installId) or add that method if it doesn't
exist), and otherwise proceed to call
deleteOverwriteManifestsForTargets(installId, manifests.map { it.targetPath })
followed by insertOverwriteManifests(manifests); keep the `@Transaction` and
suspend signature on replaceOverwriteManifestsForTargets and reference
ModOverwriteManifest when building the targetPath list.
- Around line 73-79: activateProfile is deactivating all profiles for appId via
clearActiveProfile(appId) but then re-activates by profileId only
(setProfileActive(profileId)), which can toggle a profile belonging to a
different app; change setProfileActive to accept appId (e.g.,
setProfileActive(appId: String, profileId: String, updatedAt: Long =
System.currentTimeMillis())) and update its `@Query` to include "AND app_id =
:appId" (or otherwise constrain the UPDATE by app id), then call
setProfileActive(appId, profileId) from activateProfile so activation is scoped
to the same app (ensure references to setProfileActive and activateProfile are
updated accordingly).
In `@app/src/main/java/app/gamenative/di/DatabaseModule.kt`:
- Around line 34-41: The database builder currently calls
fallbackToDestructiveMigration(true) which will drop non-reconstructable tables
like mod_profile/mod_placement_recipe/mod_overwrite_manifest; remove the
fallbackToDestructiveMigration(...) call from the Room.databaseBuilder chain in
DatabaseModule.kt and ensure you provide explicit Room migrations (e.g., augment
the set passed to .addMigrations(...) — ROOM_MIGRATION_V7_to_V8,
ROOM_MIGRATION_V20_to_V23, ROOM_MIGRATION_V21_to_V23, ROOM_MIGRATION_V22_to_V23)
for any schema version jumps you expect, or throw/handle a migration-missing
error so destructive resets cannot silently occur.
In `@app/src/main/java/app/gamenative/mods/FomodInstaller.kt`:
- Around line 315-330: The generate(...) overload that maps selectedPluginNames
to keys can silently select multiple plugins when names are duplicated; update
generate to detect ambiguous name matches and fail fast: in generate, for each
name in selectedPluginNames use the same lookup logic (inspecting
installer.steps -> step.groups -> group.plugins and pluginKey(stepIndex,
groupIndex, pluginIndex)) but group results by name and if any name maps to more
than one plugin throw an IllegalArgumentException (or return an error result)
instructing callers to use the key-based API (generateForPluginKeys), otherwise
convert unambiguous names to keys and call generateForPluginKeys with that set;
ensure you reference generate, selectedPluginNames, installer, pluginKey and
generateForPluginKeys in the change.
- Around line 300-308: The XML hardening feature calls in secureFactory() are
currently swallowed by runCatching, letting parsing continue with incomplete
protections; change those runCatching wrappers so failures abort: call
setFeature(...) for each of the four feature URIs
(http://apache.org/xml/features/disallow-doctype-decl,
http://xml.org/sax/features/external-general-entities,
http://xml.org/sax/features/external-parameter-entities,
http://apache.org/xml/features/nonvalidating/load-external-dtd) without
suppressing exceptions (or catch and rethrow a ParserConfigurationException with
a clear message) so secureFactory() throws when a feature cannot be enabled and
callers (e.g. the code that invokes parse(moduleConfigXml)) will skip the
installer if XML hardening cannot be applied; keep the isNamespaceAware and
isExpandEntityReferences settings as-is.
In `@app/src/main/java/app/gamenative/mods/ModConflictAnalyzer.kt`:
- Around line 41-43: The grouping step on plannedFiles uses
it.target.canonicalFile.absolutePath which can throw IOException for
non-existent planned targets; wrap the canonicalization in a safe handler (e.g.,
use runCatching or try/catch) when deriving the key so failed canonicalization
yields a fallback path (such as target.path or absolutePath) or filters out that
PlannedFile, and update the groupBy call to use that safe key provider;
specifically adjust the groupBy lambda referencing
PlannedFile.target/.canonicalFile.absolutePath so it never lets IOException
escape the coroutine.
In `@app/src/main/java/app/gamenative/mods/ModTargetResolver.kt`:
- Around line 66-68: The helper File.isInsideOrEqual has a bug when root.path
equals File.separator (filesystem root) because concatenating another separator
produces "//" and breaks startsWith; update isInsideOrEqual to normalize the
root path before checking containment (e.g., treat root.path == File.separator
specially or trim trailing separators) so that the check becomes this == root ||
path.startsWith(normalizedRoot + File.separator) (and handle the case where
normalizedRoot is just File.separator by checking
path.startsWith(File.separator)); ensure you update the implementation in the
isInsideOrEqual extension to use normalizedRoot and keep the existing equality
fallback.
In `@app/src/main/java/app/gamenative/mods/NexusApiClient.kt`:
- Around line 166-171: The retry/fallback loops in NexusApiClient (e.g., the
getCollectionRevisionGraph call inside the withContext block and the subsequent
GraphQL/REST fallback blocks) only catch NexusApiException and therefore abort
on transient I/O, socket, JSON parsing, or other runtime errors; update those
try/catch blocks to also catch transient exceptions (e.g., IOException,
JSONException/JsonParseException, and general Exception as a last resort) while
preserving the current behavior of rethrowing NexusApiException for status codes
401/403/429, and when catching these transient errors set lastError (or the same
accumulator used currently) and continue the loop so the code will try the next
Graph URL or fallback to REST revision paths (apply the same change to the other
similar try/catch blocks that call getCollectionRevisionGraph and REST
resolution).
- Around line 297-315: The code currently calls
fetchCollectionPayload(resolveNexusUrl(...), apiKey) which always uses
baseRequest(url, apiKey) and thus forwards the Nexus API key to any absolute
URL; modify the request logic so collection/download requests do not leak the
API key: update fetchCollectionPayload(url, apiKey) (and any callers like the
block that handles nextLink) to detect whether the target URL is a Nexus-owned
HTTPS host (use the same resolution logic as resolveNexusUrl or parse the URL
host) and only call baseRequest(url, apiKey) for those Nexus HTTPS hosts; for
all other absolute URLs build a request that does NOT include the API key and
enforces HTTPS (or fails if not HTTPS) before calling executeBytes, ensuring
tokens are never sent to external or plain-HTTP hosts.
In `@app/src/main/java/app/gamenative/mods/NexusModManager.kt`:
- Around line 438-441: The saveLastPlacementForApp function currently returns
early when enabledRecipes is empty, leaving any previously stored placement for
appId in PrefManager.nexusLastPlacementJson; change the logic so when
enabledRecipes.isEmpty() you load the existing
JSONObject(PrefManager.nexusLastPlacementJson) (or create one), remove the entry
for the provided appId (or set it to null), and write the modified JSONObject
back to PrefManager.nexusLastPlacementJson so saved placements are cleared for
that app; keep modifications localized to saveLastPlacementForApp and use the
same JSONObject handling already present in that function.
In `@app/src/main/java/app/gamenative/service/NexusModImportService.kt`:
- Around line 294-301: The resumeInterruptedImports function currently calls
ContextCompat.startForegroundService(...) directly which can throw and crash
callers; wrap the call in a try/catch around
ContextCompat.startForegroundService(appContext, Intent(appContext,
NexusModImportService::class.java).apply { action = ACTION_RESUME_IMPORTS }) and
on exception catch it, log the failure with contextual info (e.g., include
ACTION_RESUME_IMPORTS and NexusModImportService) and return/skip without
rethrowing so the caller is not crashed.
In
`@app/src/main/java/app/gamenative/ui/component/dialog/NexusModsDialogHelpers.kt`:
- Around line 345-355: In archiveChildren, the case-insensitive startsWith check
can mismatch the case-sensitive removePrefix call causing incorrect remaining
paths; update the logic in archiveChildren (within the loop handling
path/prefix) to strip the prefix in a case-insensitive-safe way — either compute
remaining via path.substring(prefix.length) after confirming
path.startsWith(prefix, ignoreCase = true), or normalize both path and prefix
(e.g., lowercased) and call removePrefix on the normalized value and use the
corresponding substring from the original path; ensure this change is applied
where remaining is computed so children are correctly resolved.
In
`@app/src/main/java/app/gamenative/ui/component/dialog/NexusModsFomodSections.kt`:
- Around line 279-286: The call to FomodRecipeGenerator.generateForPluginKeys in
NexusModsFomodSections.kt hardcodes targetRelativePath = "Data", causing
incorrect placement for non-Bethesda presets; change that argument to use
baseDraft.targetRelativePath (or a sensible fallback like "Data" when
baseDraft.targetRelativePath is null/blank) so generateForPluginKeys receives
the user-selected placement; update the invocation in the block that builds
result (the call with installId, installer, selectedPluginKeys, targetRoot =
baseDraft.targetRoot, targetRelativePath = "Data", mode =
ModPlacementMode.OVERWRITE_COPY.name) to pass baseDraft.targetRelativePath ?:
"Data" (or equivalent) instead of the literal "Data".
In
`@app/src/main/java/app/gamenative/ui/component/dialog/NexusModsInstalledSections.kt`:
- Around line 129-131: The TextButton currently validates with
name.trim().isNotBlank() but calls onConfirm(name) with the untrimmed value;
update the onClick handler in NexusModsInstalledSections (the TextButton that
uses onConfirm and name) to pass the trimmed string (e.g., call onConfirm with
name.trim()) so stored profile names don't keep accidental leading/trailing
whitespace.
In
`@app/src/main/java/app/gamenative/ui/screen/library/appscreen/AmazonAppScreen.kt`:
- Around line 351-358: The code calls AmazonService.deleteGame(...) before
computing gameRootDir, so uninstall can clear install metadata and make
getInstallPath(...) return null; capture the install path first by calling
getInstallPath(context, libraryItem) and constructing the File (or null) into a
local variable (e.g. gameRootDirFile) before calling AmazonService.deleteGame,
then call DownloadService.invalidateCache() and pass that captured
gameRootDirFile into NexusModManager.deleteInstallsForApp(context, appId =
libraryItem.appId, gameRootDir = gameRootDirFile) so filesystem mod cleanup runs
against the correct path.
In
`@app/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.kt`:
- Around line 489-494: After EpicService.deleteGame() returns success (the
branch where result.isSuccess is true in EpicAppScreen.kt), move the call to
NexusModManager.deleteInstallsForApp(...) out of the main success path so any
exception it throws cannot propagate to the outer uninstall catch; wrap the
NexusModManager.deleteInstallsForApp(...) call in its own try/catch and on
failure log the error (including context like libraryItem.appId and the path
from getInstallPath(...)) or schedule a retry, but do NOT rethrow so the
uninstall remains reported as successful.
---
Nitpick comments:
In `@app/src/main/java/app/gamenative/data/ModInstall.kt`:
- Around line 40-66: The ModProfile entity currently allows multiple active
profiles per app; add a DB-level unique partial index to enforce a single active
profile by creating a migration that runs the SQL: create a unique index on
app_id where active = 1 for table mod_profile (index name like
index_mod_profile_app_id_active) since Room's `@Index` can't express WHERE; add
this migration alongside your existing migrations and ensure
ModProfileManager.activateProfile logic remains in sync with the migration so
only one profile per app can be active.
In
`@app/src/main/java/app/gamenative/ui/component/dialog/NexusModsImportSections.kt`:
- Around line 319-324: The render loop in NexusModsImportSections.kt uses
pending.mods.indexOf(mod) inside filteredMods.forEach, causing O(n²) work;
replace this by computing indexes once (e.g., with pending.mods.mapIndexed to
build an indexByKey map inside a remember block) or iterate using forEachIndexed
so you can look up the index in O(1) when rendering each item; update the code
that references pending.mods.indexOf(mod) (the line inside the
filteredMods.forEach block) to use the precomputed index lookup
(indexByKey[mod.collectionKey()] or the forEachIndexed index) and keep existing
uses of queueItems, selectedKeys and mod.collectionKey() unchanged.
- Around line 63-127: Replace hardcoded user-facing strings in the Nexus Mods
composables with string resources: move literals like "Nexus account", the help
texts, "Personal API key", "Save and check key", "Add from Nexus Mods", "Mod or
collection URL", "Find files" (and the other occurrences noted) into strings.xml
and use stringResource(...) in the composable functions (e.g., the top-level
Nexus Mods auth UI and ImportSection). Update calls in ImportSection, the
NoExtractOutlinedTextField labels, Buttons, Texts and any validation messages to
reference those resource IDs instead of hardcoded literals so the UI is
localizable and consistent.
In
`@app/src/main/java/app/gamenative/ui/screen/library/components/GameOptionsPanel.kt`:
- Around line 346-348: Two menu entries use the same icon:
AppOptionMenuType.ManageGameContent and AppOptionMenuType.ManageMods both map to
Icons.Default.Apps in GameOptionsPanel.kt; change the mapping for
AppOptionMenuType.ManageMods to a distinct, semantically appropriate icon (for
example Icons.Default.Extension or Icons.Default.Build) so the Help & Info
section shows visually distinct icons; update the branch/switch case that
returns the icon for AppOptionMenuType.ManageMods to the chosen alternative and
ensure imports remain correct.
In `@app/src/test/java/app/gamenative/mods/NexusApiClientTest.kt`:
- Around line 29-32: The tearDown() currently only calls server.shutdown(), but
the OkHttpClient created in setUp() must also be cleaned up to avoid lingering
threads; in tearDown() (the teardown method) call the OkHttpClient instance's
cleanup routines: shut down its dispatcher executor service
(client.dispatcher().executorService().shutdown()), evict connections from its
connection pool (client.connectionPool().evictAll()), and close its cache if
present (client.cache()?.close()), ensuring you reference the OkHttpClient
variable created in setUp() so tests fully release resources.
🪄 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: c08df9e0-66c0-42f6-883c-fdfcb81e246b
📒 Files selected for processing (69)
app/build.gradle.ktsapp/schemas/app.gamenative.db.PluviaDatabase/23.jsonapp/src/androidTest/java/app/gamenative/mods/ModArchiveExtractorAndroidTest.ktapp/src/main/AndroidManifest.xmlapp/src/main/java/app/gamenative/PrefManager.ktapp/src/main/java/app/gamenative/data/ModInstall.ktapp/src/main/java/app/gamenative/db/PluviaDatabase.ktapp/src/main/java/app/gamenative/db/dao/ModDao.ktapp/src/main/java/app/gamenative/db/migration/RoomMigration.ktapp/src/main/java/app/gamenative/di/DatabaseModule.ktapp/src/main/java/app/gamenative/mods/BethesdaPlacementRecipeExpander.ktapp/src/main/java/app/gamenative/mods/BethesdaPluginManager.ktapp/src/main/java/app/gamenative/mods/FomodAutoSelector.ktapp/src/main/java/app/gamenative/mods/FomodInstaller.ktapp/src/main/java/app/gamenative/mods/ModArchiveExtractor.ktapp/src/main/java/app/gamenative/mods/ModArchiveInstallAssessor.ktapp/src/main/java/app/gamenative/mods/ModConflictAnalyzer.ktapp/src/main/java/app/gamenative/mods/ModContainerResolver.ktapp/src/main/java/app/gamenative/mods/ModDownloadRegistry.ktapp/src/main/java/app/gamenative/mods/ModMaterializer.ktapp/src/main/java/app/gamenative/mods/ModPathDetector.ktapp/src/main/java/app/gamenative/mods/ModPlacementPreset.ktapp/src/main/java/app/gamenative/mods/ModPlacementSources.ktapp/src/main/java/app/gamenative/mods/ModProfileManager.ktapp/src/main/java/app/gamenative/mods/ModTargetResolver.ktapp/src/main/java/app/gamenative/mods/NexusApiClient.ktapp/src/main/java/app/gamenative/mods/NexusCollectionManifest.ktapp/src/main/java/app/gamenative/mods/NexusCollectionPrioritySuggester.ktapp/src/main/java/app/gamenative/mods/NexusCollectionReusePolicy.ktapp/src/main/java/app/gamenative/mods/NexusImportState.ktapp/src/main/java/app/gamenative/mods/NexusModAutomationPolicy.ktapp/src/main/java/app/gamenative/mods/NexusModManager.ktapp/src/main/java/app/gamenative/mods/NexusUrlParser.ktapp/src/main/java/app/gamenative/service/NexusModImportService.ktapp/src/main/java/app/gamenative/ui/component/dialog/NexusModsBethesdaSections.ktapp/src/main/java/app/gamenative/ui/component/dialog/NexusModsDialog.ktapp/src/main/java/app/gamenative/ui/component/dialog/NexusModsDialogHelpers.ktapp/src/main/java/app/gamenative/ui/component/dialog/NexusModsFomodSections.ktapp/src/main/java/app/gamenative/ui/component/dialog/NexusModsImportSections.ktapp/src/main/java/app/gamenative/ui/component/dialog/NexusModsInstalledSections.ktapp/src/main/java/app/gamenative/ui/component/dialog/NexusModsPlacementSections.ktapp/src/main/java/app/gamenative/ui/component/dialog/NexusModsSearchField.ktapp/src/main/java/app/gamenative/ui/enums/AppOptionMenuType.ktapp/src/main/java/app/gamenative/ui/screen/library/appscreen/AmazonAppScreen.ktapp/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.ktapp/src/main/java/app/gamenative/ui/screen/library/appscreen/CustomGameAppScreen.ktapp/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.ktapp/src/main/java/app/gamenative/ui/screen/library/appscreen/GOGAppScreen.ktapp/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.ktapp/src/main/java/app/gamenative/ui/screen/library/components/GameOptionsPanel.ktapp/src/main/res/values/strings.xmlapp/src/test/java/app/gamenative/mods/BethesdaPlacementRecipeExpanderTest.ktapp/src/test/java/app/gamenative/mods/BethesdaPluginManagerTest.ktapp/src/test/java/app/gamenative/mods/FomodAutoSelectorTest.ktapp/src/test/java/app/gamenative/mods/FomodInstallerTest.ktapp/src/test/java/app/gamenative/mods/ModArchiveExtractorTest.ktapp/src/test/java/app/gamenative/mods/ModArchiveInstallAssessorTest.ktapp/src/test/java/app/gamenative/mods/ModConflictAnalyzerTest.ktapp/src/test/java/app/gamenative/mods/ModMaterializerTest.ktapp/src/test/java/app/gamenative/mods/ModPlacementPresetDetectorTest.ktapp/src/test/java/app/gamenative/mods/ModPlacementSourcesTest.ktapp/src/test/java/app/gamenative/mods/ModTargetResolverTest.ktapp/src/test/java/app/gamenative/mods/NexusApiClientTest.ktapp/src/test/java/app/gamenative/mods/NexusCollectionManifestParserTest.ktapp/src/test/java/app/gamenative/mods/NexusCollectionPrioritySuggesterTest.ktapp/src/test/java/app/gamenative/mods/NexusCollectionReusePolicyTest.ktapp/src/test/java/app/gamenative/mods/NexusImportStateTest.ktapp/src/test/java/app/gamenative/mods/NexusUrlParserTest.ktgradle/libs.versions.toml
Comment on lines
34
to
41
| return Room.databaseBuilder(context, PluviaDatabase::class.java, DATABASE_NAME) | ||
| .addMigrations(ROOM_MIGRATION_V7_to_V8) | ||
| .addMigrations( | ||
| ROOM_MIGRATION_V7_to_V8, | ||
| ROOM_MIGRATION_V20_to_V23, | ||
| ROOM_MIGRATION_V21_to_V23, | ||
| ROOM_MIGRATION_V22_to_V23, | ||
| ) | ||
| .fallbackToDestructiveMigration(true) |
Contributor
There was a problem hiding this comment.
Avoid global destructive fallback now that the DB tracks deployed mod state.
After this PR, the database owns non-reconstructable state like mod_profile, mod_placement_recipe, and mod_overwrite_manifest. If a future version gap hits Line 41, Room will drop that metadata while the deployed files and backups remain on disk, leaving installs unsafe to reconcile or restore.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/main/java/app/gamenative/di/DatabaseModule.kt` around lines 34 - 41,
The database builder currently calls fallbackToDestructiveMigration(true) which
will drop non-reconstructable tables like
mod_profile/mod_placement_recipe/mod_overwrite_manifest; remove the
fallbackToDestructiveMigration(...) call from the Room.databaseBuilder chain in
DatabaseModule.kt and ensure you provide explicit Room migrations (e.g., augment
the set passed to .addMigrations(...) — ROOM_MIGRATION_V7_to_V8,
ROOM_MIGRATION_V20_to_V23, ROOM_MIGRATION_V21_to_V23, ROOM_MIGRATION_V22_to_V23)
for any schema version jumps you expect, or throw/handle a migration-missing
error so destructive resets cannot silently occur.
Comment on lines
+300
to
+308
| private fun secureFactory(): DocumentBuilderFactory = | ||
| DocumentBuilderFactory.newInstance().apply { | ||
| isNamespaceAware = false | ||
| runCatching { setFeature("http://apache.org/xml/features/disallow-doctype-decl", true) } | ||
| runCatching { setFeature("http://xml.org/sax/features/external-general-entities", false) } | ||
| runCatching { setFeature("http://xml.org/sax/features/external-parameter-entities", false) } | ||
| runCatching { setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false) } | ||
| isExpandEntityReferences = false | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file (in case path differs) and print relevant sections around secureFactory and its usage.
FILE="app/src/main/java/app/gamenative/mods/FomodInstaller.kt"
test -f "$FILE" || { echo "Missing $FILE"; exit 1; }
echo "=== secureFactory() around lines 260-360 ==="
sed -n '250,360p' "$FILE" | cat -n
echo "=== Search for ModuleConfig.xml parsing ==="
rg -n "ModuleConfig\.xml|moduleconfig\.xml|ModuleConfig|config\.xml" "$FILE" || true
echo "=== Search for XML parsing usage of DocumentBuilderFactory / secureFactory ==="
rg -n "secureFactory\(|DocumentBuilderFactory|DocumentBuilder|parse\(|newDocumentBuilder\(|DocumentBuilder" "$FILE" || true
echo "=== Search for runCatching { setFeature(...) } usage ==="
rg -n "runCatching\s*\{\s*setFeature" "$FILE" || true
echo "=== Check plugin selection logic for name ambiguity ==="
rg -n "selectedPluginNames|plugin\.name|display name|name-based|map.*plugin|associate.*plugin|group" "$FILE" || true
echo "=== Print generate()/selection-related sections (approx. 360-520) ==="
sed -n '360,540p' "$FILE" | cat -nRepository: utkarshdalal/GameNative
Length of output: 15890
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="app/src/main/java/app/gamenative/mods/FomodInstaller.kt"
test -f "$FILE"
echo "=== parsePlugin() / plugin.name definition ==="
rg -n "fun parsePlugin|parsePlugin\\(|data class FomodPlugin|class FomodPlugin|val name:|plugin\\.name" "$FILE" || true
# Show around parsePlugin if found
LINE=$(rg -n "fun parsePlugin" "$FILE" | head -n1 | cut -d: -f1 || true)
if [ -n "${LINE:-}" ]; then
START=$((LINE-60)); END=$((LINE+120))
sed -n "${START},${END}p" "$FILE" | cat -n
fi
echo "=== selectedPluginNames construction / callers of generate() ==="
rg -n "FomodRecipeGenerator\\.generate\\(" -S . || true
rg -n "selectedPluginNames" -S . || trueRepository: utkarshdalal/GameNative
Length of output: 11261
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "FomodParser\\.parse\\(" -S app/src/main/java app/src/test/java || trueRepository: utkarshdalal/GameNative
Length of output: 903
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the NexusModsDialog parse call sites and surrounding flow.
FILE="app/src/main/java/app/gamenative/ui/component/dialog/NexusModsDialog.kt"
test -f "$FILE"
echo "=== NexusModsDialog.kt around first FomodParser.parse call (line ~1284) ==="
sed -n '1250,1315p' "$FILE" | cat -n
echo "=== NexusModsDialog.kt around second FomodParser.parse call (line ~1523) ==="
sed -n '1485,1555p' "$FILE" | cat -n
echo "=== Search where selectedPluginNames is built/passed ==="
rg -n "selectedPluginNames" "$FILE" || true
echo "=== Search for FomodRecipeGenerator.generate call in NexusModsDialog ==="
rg -n "FomodRecipeGenerator\\.generate\\(" "$FILE" || trueRepository: utkarshdalal/GameNative
Length of output: 8126
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "FomodRecipeGenerator\\.generate\\(" app/src/main/java app/src/test/java || true
rg -n "selectedPluginNames\\s*=" app/src/main/java app/src/test/java || true
rg -n "selectedPluginNames" app/src/main/java app/src/test/java || true
rg -n "FomodAutoSelector" app/src/main/java app/src/test/java || trueRepository: utkarshdalal/GameNative
Length of output: 1764
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="app/src/main/java/app/gamenative/mods/FomodAutoSelector.kt"
test -f "$FILE"
sed -n '1,220p' "$FILE" | cat -nRepository: utkarshdalal/GameNative
Length of output: 3848
Fail closed if XML hardening cannot be enabled.
In FomodInstaller.secureFactory(), the setFeature(...) hardening calls are wrapped in runCatching { ... } and failures are ignored, then the parser is created and parse(moduleConfigXml) runs. If the active XML provider doesn’t support one (or more) of these features, XXE protections may be incomplete. Make feature-setting failures abort parsing (throw) so the caller can skip the installer.
Suggested fix
private fun secureFactory(): DocumentBuilderFactory =
DocumentBuilderFactory.newInstance().apply {
isNamespaceAware = false
- runCatching { setFeature("http://apache.org/xml/features/disallow-doctype-decl", true) }
- runCatching { setFeature("http://xml.org/sax/features/external-general-entities", false) }
- runCatching { setFeature("http://xml.org/sax/features/external-parameter-entities", false) }
- runCatching { setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false) }
+ setFeature("http://apache.org/xml/features/disallow-doctype-decl", true)
+ setFeature("http://xml.org/sax/features/external-general-entities", false)
+ setFeature("http://xml.org/sax/features/external-parameter-entities", false)
+ setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false)
+ isXIncludeAware = false
isExpandEntityReferences = false
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/main/java/app/gamenative/mods/FomodInstaller.kt` around lines 300 -
308, The XML hardening feature calls in secureFactory() are currently swallowed
by runCatching, letting parsing continue with incomplete protections; change
those runCatching wrappers so failures abort: call setFeature(...) for each of
the four feature URIs (http://apache.org/xml/features/disallow-doctype-decl,
http://xml.org/sax/features/external-general-entities,
http://xml.org/sax/features/external-parameter-entities,
http://apache.org/xml/features/nonvalidating/load-external-dtd) without
suppressing exceptions (or catch and rethrow a ParserConfigurationException with
a clear message) so secureFactory() throws when a feature cannot be enabled and
callers (e.g. the code that invokes parse(moduleConfigXml)) will skip the
installer if XML hardening cannot be applied; keep the isNamespaceAware and
isExpandEntityReferences settings as-is.
Contributor
There was a problem hiding this comment.
7 issues found across 84 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/mods/ModPlacementPreset.kt">
<violation number="1" location="app/src/main/java/app/gamenative/mods/ModPlacementPreset.kt:55">
P2: Hardcoded user-facing strings should be declared in Android string resources instead of inline literals.</violation>
</file>
<file name="app/src/main/java/app/gamenative/mods/ModDownloadRegistry.kt">
<violation number="1" location="app/src/main/java/app/gamenative/mods/ModDownloadRegistry.kt:26">
P2: `start()` unconditionally clears pending cancel flags, which can lose valid cancel requests in startup races.</violation>
<violation number="2" location="app/src/main/java/app/gamenative/mods/ModDownloadRegistry.kt:34">
P2: Hardcoded UI status string bypasses localization</violation>
<violation number="3" location="app/src/main/java/app/gamenative/mods/ModDownloadRegistry.kt:65">
P2: `canceledImports` may leak stale install IDs because `requestCancel()` blindly adds any ID without checking whether it corresponds to an active download. Cleanup only occurs in `start()` and `finish()`, which are not guaranteed to be called for every ID passed to `requestCancel()` (e.g., invalid IDs, stale UI states, or interrupted flows). This can cause the set to grow unbounded in memory over the app's lifetime.</violation>
</file>
<file name="app/src/main/java/app/gamenative/mods/BethesdaPluginManager.kt">
<violation number="1" location="app/src/main/java/app/gamenative/mods/BethesdaPluginManager.kt:241">
P2: Plugin entries are deduplicated with first-occurrence-wins (`distinctBy`), which can preserve stale enabled/disabled states when duplicate lines exist. A last-occurrence-wins approach is safer for line-based config files.</violation>
</file>
<file name="app/src/main/java/app/gamenative/data/ModInstall.kt">
<violation number="1" location="app/src/main/java/app/gamenative/data/ModInstall.kt:44">
P1: No database-level constraint ensures only one active mod profile per app, risking nondeterministic profile selection.</violation>
</file>
<file name="app/src/main/java/app/gamenative/mods/ModArchiveInstallAssessor.kt">
<violation number="1" location="app/src/main/java/app/gamenative/mods/ModArchiveInstallAssessor.kt:23">
P2: Hardcoded user-visible strings (queueMessage and assessment reasons) should be extracted to strings.xml and referenced via resources instead of inline literals.</violation>
</file>
<file name="app/src/main/java/app/gamenative/mods/FomodAutoSelector.kt">
<violation number="1" location="app/src/main/java/app/gamenative/mods/FomodAutoSelector.kt:21">
P2: Global typePatterns check unnecessarily disables FOMOD deterministic auto-selection</violation>
<violation number="2" location="app/src/main/java/app/gamenative/mods/FomodAutoSelector.kt:73">
P2: Hardcoded English reason strings bypass Android string resources and localization conventions.</violation>
</file>
<file name="app/src/main/java/app/gamenative/mods/ModConflictAnalyzer.kt">
<violation number="1" location="app/src/main/java/app/gamenative/mods/ModConflictAnalyzer.kt:33">
P2: Analyzer performs full in-memory materialization of all planned files, which may not scale for large mods/libraries. Multiple `.toList()` calls and `flatMap` create large intermediate collections eagerly. For large mod packs this can cause high memory pressure and long pauses.</violation>
</file>
<file name="app/src/main/java/app/gamenative/mods/ModTargetResolver.kt">
<violation number="1" location="app/src/main/java/app/gamenative/mods/ModTargetResolver.kt:19">
P2: Hardcoded English labels in resolver bypass localization when rendered in UI</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| targetRelativePath: String = "Data", | ||
| ): FomodAutoSelectionResult? { | ||
| if (installer.unsupportedWarnings.isNotEmpty()) return null | ||
| if (installer.steps.any { step -> step.groups.any { group -> group.plugins.any { it.typePatterns.isNotEmpty() } } }) { |
Contributor
There was a problem hiding this comment.
P2: Global typePatterns check unnecessarily disables FOMOD deterministic auto-selection
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/mods/FomodAutoSelector.kt, line 21:
<comment>Global typePatterns check unnecessarily disables FOMOD deterministic auto-selection</comment>
<file context>
@@ -0,0 +1,79 @@
+ targetRelativePath: String = "Data",
+ ): FomodAutoSelectionResult? {
+ if (installer.unsupportedWarnings.isNotEmpty()) return null
+ if (installer.steps.any { step -> step.groups.any { group -> group.plugins.any { it.typePatterns.isNotEmpty() } } }) {
+ return null
+ }
</file context>
| fun roots(gameRootDir: File?, winePrefix: String): List<ResolvedModTargetRoot> { | ||
| val result = mutableListOf<ResolvedModTargetRoot>() | ||
| if (gameRootDir?.isDirectory == true) { | ||
| result += ResolvedModTargetRoot(ModTargetRoot.GAME_DIR, "Game Directory", gameRootDir) |
Contributor
There was a problem hiding this comment.
P2: Hardcoded English labels in resolver bypass localization when rendered in UI
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/mods/ModTargetResolver.kt, line 19:
<comment>Hardcoded English labels in resolver bypass localization when rendered in UI</comment>
<file context>
@@ -0,0 +1,72 @@
+ fun roots(gameRootDir: File?, winePrefix: String): List<ResolvedModTargetRoot> {
+ val result = mutableListOf<ResolvedModTargetRoot>()
+ if (gameRootDir?.isDirectory == true) {
+ result += ResolvedModTargetRoot(ModTargetRoot.GAME_DIR, "Game Directory", gameRootDir)
+ }
+ if (winePrefix.isNotBlank()) {
</file context>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@app/src/main/java/app/gamenative/ui/component/dialog/NexusModsDialogHelpers.kt`:
- Around line 62-66: The profileStatus extension currently maps every
non-enabled placeable install to "PROFILE_DISABLED", overwriting true disabled
installs; update ModInstall.profileStatus so it only returns "PROFILE_DISABLED"
when canPlaceFiles() && !enabledInProfile AND the current status is not
ModInstallStatus.DISABLED.name (i.e., preserve status when status ==
ModInstallStatus.DISABLED.name), otherwise return the original status; reference
the ModInstall.profileStatus function, canPlaceFiles(), enabledInProfile and
status symbols when making the change.
🪄 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: eb6147fc-15bd-49f0-a8ba-6cf7f4b48077
📒 Files selected for processing (56)
app/build.gradle.ktsapp/src/main/java/app/gamenative/PrefManager.ktapp/src/main/java/app/gamenative/db/dao/ModDao.ktapp/src/main/java/app/gamenative/mods/BethesdaPlacementRecipeExpander.ktapp/src/main/java/app/gamenative/mods/BethesdaPluginManager.ktapp/src/main/java/app/gamenative/mods/FomodAutoSelector.ktapp/src/main/java/app/gamenative/mods/FomodInstaller.ktapp/src/main/java/app/gamenative/mods/ModArchiveExtractor.ktapp/src/main/java/app/gamenative/mods/ModConflictAnalyzer.ktapp/src/main/java/app/gamenative/mods/ModContainerResolver.ktapp/src/main/java/app/gamenative/mods/ModMaterializer.ktapp/src/main/java/app/gamenative/mods/ModPlacementPreset.ktapp/src/main/java/app/gamenative/mods/ModPlacementSources.ktapp/src/main/java/app/gamenative/mods/ModProfileManager.ktapp/src/main/java/app/gamenative/mods/ModTargetResolver.ktapp/src/main/java/app/gamenative/mods/NexusApiClient.ktapp/src/main/java/app/gamenative/mods/NexusModManager.ktapp/src/main/java/app/gamenative/service/NexusModImportService.ktapp/src/main/java/app/gamenative/ui/component/dialog/NexusModsBethesdaSections.ktapp/src/main/java/app/gamenative/ui/component/dialog/NexusModsDialog.ktapp/src/main/java/app/gamenative/ui/component/dialog/NexusModsDialogHelpers.ktapp/src/main/java/app/gamenative/ui/component/dialog/NexusModsFomodSections.ktapp/src/main/java/app/gamenative/ui/component/dialog/NexusModsImportSections.ktapp/src/main/java/app/gamenative/ui/component/dialog/NexusModsInstalledSections.ktapp/src/main/java/app/gamenative/ui/component/dialog/NexusModsPlacementSections.ktapp/src/main/java/app/gamenative/ui/component/dialog/NexusModsSearchField.ktapp/src/main/java/app/gamenative/ui/screen/library/appscreen/AmazonAppScreen.ktapp/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.ktapp/src/main/java/app/gamenative/ui/screen/library/appscreen/CustomGameAppScreen.ktapp/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.ktapp/src/main/java/app/gamenative/ui/screen/library/appscreen/GOGAppScreen.ktapp/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.ktapp/src/main/java/app/gamenative/ui/screen/library/components/GameOptionsPanel.ktapp/src/main/res/values-da/strings.xmlapp/src/main/res/values-de/strings.xmlapp/src/main/res/values-es/strings.xmlapp/src/main/res/values-fr/strings.xmlapp/src/main/res/values-it/strings.xmlapp/src/main/res/values-ja/strings.xmlapp/src/main/res/values-ko/strings.xmlapp/src/main/res/values-pl/strings.xmlapp/src/main/res/values-pt-rBR/strings.xmlapp/src/main/res/values-ro/strings.xmlapp/src/main/res/values-ru/strings.xmlapp/src/main/res/values-uk/strings.xmlapp/src/main/res/values-zh-rCN/strings.xmlapp/src/main/res/values-zh-rTW/strings.xmlapp/src/main/res/values/strings.xmlapp/src/test/java/app/gamenative/mods/BethesdaPlacementRecipeExpanderTest.ktapp/src/test/java/app/gamenative/mods/FomodAutoSelectorTest.ktapp/src/test/java/app/gamenative/mods/FomodInstallerTest.ktapp/src/test/java/app/gamenative/mods/ModMaterializerTest.ktapp/src/test/java/app/gamenative/mods/ModPlacementPresetDetectorTest.ktapp/src/test/java/app/gamenative/mods/ModProfileManagerTest.ktapp/src/test/java/app/gamenative/mods/NexusApiClientTest.ktgradle/libs.versions.toml
✅ Files skipped from review due to trivial changes (1)
- gradle/libs.versions.toml
🚧 Files skipped from review as they are similar to previous changes (26)
- app/build.gradle.kts
- app/src/main/java/app/gamenative/ui/component/dialog/NexusModsSearchField.kt
- app/src/test/java/app/gamenative/mods/BethesdaPlacementRecipeExpanderTest.kt
- app/src/main/java/app/gamenative/ui/screen/library/components/GameOptionsPanel.kt
- app/src/main/java/app/gamenative/mods/ModContainerResolver.kt
- app/src/main/java/app/gamenative/mods/ModPlacementSources.kt
- app/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.kt
- app/src/test/java/app/gamenative/mods/ModPlacementPresetDetectorTest.kt
- app/src/main/java/app/gamenative/mods/ModConflictAnalyzer.kt
- app/src/main/java/app/gamenative/ui/component/dialog/NexusModsFomodSections.kt
- app/src/main/java/app/gamenative/PrefManager.kt
- app/src/main/java/app/gamenative/mods/BethesdaPlacementRecipeExpander.kt
- app/src/main/java/app/gamenative/mods/FomodInstaller.kt
- app/src/main/java/app/gamenative/mods/FomodAutoSelector.kt
- app/src/main/java/app/gamenative/mods/ModPlacementPreset.kt
- app/src/main/java/app/gamenative/mods/NexusApiClient.kt
- app/src/main/java/app/gamenative/ui/component/dialog/NexusModsInstalledSections.kt
- app/src/main/java/app/gamenative/ui/component/dialog/NexusModsBethesdaSections.kt
- app/src/main/java/app/gamenative/ui/component/dialog/NexusModsImportSections.kt
- app/src/test/java/app/gamenative/mods/ModMaterializerTest.kt
- app/src/main/java/app/gamenative/service/NexusModImportService.kt
- app/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.kt
- app/src/test/java/app/gamenative/mods/NexusApiClientTest.kt
- app/src/main/java/app/gamenative/ui/screen/library/appscreen/CustomGameAppScreen.kt
- app/src/main/java/app/gamenative/mods/ModArchiveExtractor.kt
- app/src/main/java/app/gamenative/mods/NexusModManager.kt
Contributor
There was a problem hiding this comment.
2 issues found across 85 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/mods/ModPlacementPreset.kt">
<violation number="1" location="app/src/main/java/app/gamenative/mods/ModPlacementPreset.kt:55">
P2: Hardcoded user-facing strings should be declared in Android string resources instead of inline literals.</violation>
</file>
<file name="app/src/main/java/app/gamenative/mods/ModDownloadRegistry.kt">
<violation number="1" location="app/src/main/java/app/gamenative/mods/ModDownloadRegistry.kt:26">
P2: `start()` unconditionally clears pending cancel flags, which can lose valid cancel requests in startup races.</violation>
<violation number="2" location="app/src/main/java/app/gamenative/mods/ModDownloadRegistry.kt:34">
P2: Hardcoded UI status string bypasses localization</violation>
<violation number="3" location="app/src/main/java/app/gamenative/mods/ModDownloadRegistry.kt:65">
P2: `canceledImports` may leak stale install IDs because `requestCancel()` blindly adds any ID without checking whether it corresponds to an active download. Cleanup only occurs in `start()` and `finish()`, which are not guaranteed to be called for every ID passed to `requestCancel()` (e.g., invalid IDs, stale UI states, or interrupted flows). This can cause the set to grow unbounded in memory over the app's lifetime.</violation>
</file>
<file name="app/src/main/java/app/gamenative/mods/BethesdaPluginManager.kt">
<violation number="1" location="app/src/main/java/app/gamenative/mods/BethesdaPluginManager.kt:241">
P2: Plugin entries are deduplicated with first-occurrence-wins (`distinctBy`), which can preserve stale enabled/disabled states when duplicate lines exist. A last-occurrence-wins approach is safer for line-based config files.</violation>
</file>
<file name="app/src/main/java/app/gamenative/data/ModInstall.kt">
<violation number="1" location="app/src/main/java/app/gamenative/data/ModInstall.kt:44">
P1: No database-level constraint ensures only one active mod profile per app, risking nondeterministic profile selection.</violation>
</file>
<file name="app/src/main/java/app/gamenative/mods/ModArchiveInstallAssessor.kt">
<violation number="1" location="app/src/main/java/app/gamenative/mods/ModArchiveInstallAssessor.kt:23">
P2: Hardcoded user-visible strings (queueMessage and assessment reasons) should be extracted to strings.xml and referenced via resources instead of inline literals.</violation>
</file>
<file name="app/src/main/java/app/gamenative/mods/FomodAutoSelector.kt">
<violation number="1" location="app/src/main/java/app/gamenative/mods/FomodAutoSelector.kt:21">
P2: Global typePatterns check unnecessarily disables FOMOD deterministic auto-selection</violation>
<violation number="2" location="app/src/main/java/app/gamenative/mods/FomodAutoSelector.kt:73">
P2: Hardcoded English reason strings bypass Android string resources and localization conventions.</violation>
</file>
<file name="app/src/main/java/app/gamenative/mods/ModConflictAnalyzer.kt">
<violation number="1" location="app/src/main/java/app/gamenative/mods/ModConflictAnalyzer.kt:33">
P2: Analyzer performs full in-memory materialization of all planned files, which may not scale for large mods/libraries. Multiple `.toList()` calls and `flatMap` create large intermediate collections eagerly. For large mod packs this can cause high memory pressure and long pauses.</violation>
</file>
<file name="app/src/main/java/app/gamenative/mods/ModTargetResolver.kt">
<violation number="1" location="app/src/main/java/app/gamenative/mods/ModTargetResolver.kt:19">
P2: Hardcoded English labels in resolver bypass localization when rendered in UI</violation>
</file>
<file name="app/src/main/res/values-uk/strings.xml">
<violation number="1" location="app/src/main/res/values-uk/strings.xml:1564">
P2: Ukrainian Nexus strings use ad-hoc '(ів)'/(s) parenthetical hacks instead of Android `<plurals>` for count-dependent messages, producing grammatically incorrect output for many quantities and embedding untranslated English text in the Ukrainian localization.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| @@ -1,4 +1,4 @@ | |||
| <resources> | |||
| <resources> | |||
Contributor
There was a problem hiding this comment.
P2: Ukrainian Nexus strings use ad-hoc '(ів)'/(s) parenthetical hacks instead of Android <plurals> for count-dependent messages, producing grammatically incorrect output for many quantities and embedding untranslated English text in the Ukrainian localization.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/res/values-uk/strings.xml, line 1564:
<comment>Ukrainian Nexus strings use ad-hoc '(ів)'/(s) parenthetical hacks instead of Android `<plurals>` for count-dependent messages, producing grammatically incorrect output for many quantities and embedding untranslated English text in the Ukrainian localization.</comment>
<file context>
@@ -1542,4 +1542,52 @@
+ <string name="nexus_no_current_files">Поточні файли для завантаження не знайдено.</string>
+ <string name="nexus_show_older_files">Показати старі або архівні файли</string>
+ <string name="nexus_collection_title">Колекція Nexus</string>
+ <string name="nexus_collection_ready">%1$d мод(ів) готово. Вибрано: %2$d.</string>
+ <string name="nexus_collection_ready_with_failed">%1$d мод(ів) готово; %2$d не вдалося визначити. Вибрано: %3$d.</string>
+ <string name="nexus_estimated_temp_space">Оцінка місця, потрібного під час імпорту: %1$s; доступно %2$s.</string>
</file context>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds Nexus Mods support to GameNative. This includes importing Nexus mod files and collections, managing installed mods per game, setting mod priority/order, handling file placement, FOMOD installers, Bethesda plugin/load-order checks, storage cleanup, and install health checks.
Main things added:
Import Nexus mod files and collections, with support for resuming after interruptions.
Manage installed mods per game, including profiles that save enable/disable state, and priority order.
Apply mod order into the game folder or container with conflict checks and overwrite backups.
Detect common mod placement presets for Bethesda, BepInEx, MelonLoader, Unreal, REDmod, and custom installs.
Add Bethesda plugin checks for missing masters, missing deployed plugins, and missing BSA/BA2 files.
Add FOMOD support for ModuleConfig-based installers.
Add Nexus storage cleanup and install health checks.
Recording
My recordings are too large, three videos here and some pictures showing a few other games working: https://drive.google.com/drive/folders/1iH226A55hEkOxPDn22OKg90vH02LvgSv?usp=sharing
Type of Change
Checklist
#code-changes, I have discussed this change there and it has been green-lighted. If I do not have access, I have still provided clear context in this PR. 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 first‑class Nexus Mods support: import mods and collections, manage per‑game profiles and load order, and safely apply files with conflict checks and Bethesda plugin validation. Polished localizations and hardened collection import/auth flows; resume import gated behind Nexus sign‑in.
New Features
NexusModImportServicewith live progress.PrefManager.Dependencies
me.zhanghai.android.libarchivevialibs.libarchive.android.Written for commit f1eb842. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
Tests