Gamefix: Added correct redist launch dep for Moonlighter & En Garde#638
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new GOGDependencyFix that verifies and downloads GOG redistributables from _CommonRedist, a new download API in GOGDownloadManager (downloadDependenciesWithProgress), a dependency-to-path map constant, a GOG fix entry for gameId 2147483047, and updates callers to the new API. Changes
Sequence Diagram(s)sequenceDiagram
participant FixRunner as FixRunner
participant GOGService as GOGService
participant DLManager as GOGDownloadManager
participant FS as Filesystem
FixRunner->>GOGService: request gogDownloadManager for gameId
GOGService->>DLManager: provide manager
FixRunner->>FS: check `_CommonRedist` paths (GOG_DEPENDENCY_INSTALLED_PATH) for dependency files
alt dependencies missing
FixRunner->>DLManager: downloadDependenciesWithProgress(gameId, deps, gameDir, supportDir, onProgress)
DLManager->>DLManager: download dependencies, emit progress
DLManager->>FixRunner: return Result(success/failure)
else all present
FixRunner-->>FixRunner: return already satisfied (true)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
1 issue found across 5 files
Prompt for AI agents (all 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/gamefixes/GOGDependencyFix.kt">
<violation number="1" location="app/src/main/java/app/gamenative/gamefixes/GOGDependencyFix.kt:28">
P2: Game launch fix synchronously blocks while downloading dependencies, which can stall launch (and could ANR if invoked on the main thread).</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt`:
- Around line 968-991: The caller downloadDependenciesForInstalledGame delegates
to downloadDependencies which currently swallows per-depot errors and returns
success; change downloadDependencies to collect per-dependency results and
bubble up a failure when any dependency fails (e.g., return Result.failure with
an aggregated exception or first error) and update
downloadDependenciesForInstalledGame to return that Result as-is; look for
symbols downloadDependencies and DownloadInfo to modify the flow so partial
failures are aggregated and returned instead of silently logged.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/src/main/java/app/gamenative/gamefixes/GOGDependencyFix.ktapp/src/main/java/app/gamenative/gamefixes/GOG_2147483047.ktapp/src/main/java/app/gamenative/gamefixes/GameFixesRegistry.ktapp/src/main/java/app/gamenative/service/gog/GOGDownloadManager.ktapp/src/main/java/app/gamenative/service/gog/GOGService.kt
0ec9c8f to
a188f3a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt`:
- Around line 968-991: The progress listener never receives updates because
DownloadInfo.updateBytesDownloaded() only records speed samples and doesn't
notify listeners; modify the implementation so progress events are emitted
during download—either call DownloadInfo.emitProgressChange() immediately after
each call to downloadChunk()/updateBytesDownloaded() in
downloadDependencies/downloadChunk, or add an emitProgressChange() call inside
DownloadInfo.updateBytesDownloaded() (or have updateBytesDownloaded call
setProgress()) so downloadDependenciesWithProgress's listener (added there)
receives intermediate progress updates rather than only the final
onProgress?.invoke(1f).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/src/main/java/app/gamenative/gamefixes/GOGDependencyFix.ktapp/src/main/java/app/gamenative/gamefixes/GOG_2147483047.ktapp/src/main/java/app/gamenative/gamefixes/GameFixesRegistry.ktapp/src/main/java/app/gamenative/service/gog/GOGDownloadManager.ktapp/src/main/java/app/gamenative/utils/launchdependencies/GogScriptInterpreterDependency.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/app/gamenative/gamefixes/GOG_2147483047.kt
4b2018c to
da4e1ce
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt (1)
968-993: Existing unresolved issues: progress callback never fires; partial dep failures still reported as success.Both issues were flagged on a prior commit and remain in this code:
onProgressis only invoked with1fon success (line 990). Intermediate progress fromdownloadChunk→updateBytesDownloaded()never callsemitProgressChange(), so the attached listener fires only at the end.downloadDependencieslogs-and-continues on per-depot failures and always returnsResult.success(Unit)(line 961), so callers (includingGOGDependencyFix.apply) cannot distinguish a full success from a partial one.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt` around lines 968 - 993, downloadDependenciesWithProgress currently only triggers onProgress once at final success and downloadDependencies swallows per-depot errors; fix by wiring intermediate progress emissions and returning failure on partial failures: ensure DownloadInfo.updateBytesDownloaded (or the code path in downloadChunk) calls DownloadInfo.emitProgressChange (or otherwise invokes listeners) each time bytes are updated so the onProgress listener passed into downloadDependenciesWithProgress receives intermediate updates, and change downloadDependencies to collect per-depot failures instead of logging-and-continuing—if any depot download fails return Result.failure(...) (or aggregate an exception) so callers like GOGDependencyFix.apply can detect partial failures rather than always getting Result.success(Unit).
🤖 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/gamefixes/GOGDependencyFix.kt`:
- Around line 16-21: The function isSatisfied currently treats any dep ID absent
from GOGConstants.GOG_DEPENDENCY_INSTALLED_PATH as not-installed (causing
repeated downloads); update isSatisfied to check dependencyIds and for any id
where pathMap[id] is null, log a warning (use the existing logger) mentioning
the missing mapping and then treat that id as satisfied (skip-and-warn) instead
of returning false; keep the existing File(commonRedist, it).exists() check for
mapped ids and only return false when a mapped path exists but the file is
missing.
---
Duplicate comments:
In `@app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt`:
- Around line 968-993: downloadDependenciesWithProgress currently only triggers
onProgress once at final success and downloadDependencies swallows per-depot
errors; fix by wiring intermediate progress emissions and returning failure on
partial failures: ensure DownloadInfo.updateBytesDownloaded (or the code path in
downloadChunk) calls DownloadInfo.emitProgressChange (or otherwise invokes
listeners) each time bytes are updated so the onProgress listener passed into
downloadDependenciesWithProgress receives intermediate updates, and change
downloadDependencies to collect per-depot failures instead of
logging-and-continuing—if any depot download fails return Result.failure(...)
(or aggregate an exception) so callers like GOGDependencyFix.apply can detect
partial failures rather than always getting Result.success(Unit).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/src/main/java/app/gamenative/gamefixes/GOGDependencyFix.ktapp/src/main/java/app/gamenative/gamefixes/GOG_2147483047.ktapp/src/main/java/app/gamenative/gamefixes/GameFixesRegistry.ktapp/src/main/java/app/gamenative/service/gog/GOGConstants.ktapp/src/main/java/app/gamenative/service/gog/GOGDownloadManager.ktapp/src/main/java/app/gamenative/utils/launchdependencies/GogScriptInterpreterDependency.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- app/src/main/java/app/gamenative/utils/launchdependencies/GogScriptInterpreterDependency.kt
- app/src/main/java/app/gamenative/gamefixes/GameFixesRegistry.kt
There was a problem hiding this comment.
♻️ Duplicate comments (1)
app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt (1)
882-961: Partial dependency failures are still silently swallowed.
downloadDependenciesusescontinueon depot-level failures (manifest fetch, chunk download, assembly) and unconditionally returnsResult.success(Unit)at line 961. For callers likedownloadDependenciesWithProgress(used by game fixes), this means a missing MSVC redist can be silently skipped while the caller believes all dependencies were installed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt` around lines 882 - 961, The loop in downloadDependencies swallows depot-level failures via continue and then always returns Result.success(Unit); change downloadDependencies (and callers like downloadDependenciesWithProgress) to propagate failures: track if any depot failed (capture the exception or create an aggregated error) when apiClient.fetchDependencyDepotManifest, downloadChunksSimple, or assembleFiles return failure (instead of just continuing), and at the end return Result.failure(...) if any depot failed (or return the first failure immediately). Update the loop around filteredDepots (where manifest fetch, downloadChunksSimple, and assembleFiles are checked) to record/throw the error rather than silently continuing so callers receive a non-success Result.
🧹 Nitpick comments (1)
app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt (1)
968-993: Progress resets to 0 for each dependency depot.
downloadDependenciesiterates over multiple filtered depots and callsdownloadChunksSimplefor each one. Each call resets progress to0f(line 1031) and ramps up to ~1.0 for that depot's chunks, so callers ofdownloadDependenciesWithProgresswill see progress jump backward at each depot boundary rather than advancing smoothly from 0 → 1 across all dependencies.A simple fix: compute the total chunk count across all depots upfront, or weight progress by depot index (e.g.,
(depotIndex + depotLocalProgress) / totalDepots).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt` around lines 968 - 993, Progress resets because downloadChunksSimple reports per-depot progress independently; modify downloadDependencies (called by downloadDependenciesWithProgress) to compute total work (e.g., totalChunks across all filtered depots) before iterating and then report aggregated progress to listeners instead of resetting per-depot: either update DownloadInfo (or its addProgressListener) to accept absolute progress and set its jobCount/total accordingly, or pass a depotOffset and depotWeight into downloadChunksSimple so it reports (depotOffset + depotLocalProgress * depotChunkCount) / totalChunks; update calls in downloadDependencies and downloadDependenciesWithProgress so onProgress sees a monotonically increasing 0..1 value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt`:
- Around line 882-961: The loop in downloadDependencies swallows depot-level
failures via continue and then always returns Result.success(Unit); change
downloadDependencies (and callers like downloadDependenciesWithProgress) to
propagate failures: track if any depot failed (capture the exception or create
an aggregated error) when apiClient.fetchDependencyDepotManifest,
downloadChunksSimple, or assembleFiles return failure (instead of just
continuing), and at the end return Result.failure(...) if any depot failed (or
return the first failure immediately). Update the loop around filteredDepots
(where manifest fetch, downloadChunksSimple, and assembleFiles are checked) to
record/throw the error rather than silently continuing so callers receive a
non-success Result.
---
Nitpick comments:
In `@app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt`:
- Around line 968-993: Progress resets because downloadChunksSimple reports
per-depot progress independently; modify downloadDependencies (called by
downloadDependenciesWithProgress) to compute total work (e.g., totalChunks
across all filtered depots) before iterating and then report aggregated progress
to listeners instead of resetting per-depot: either update DownloadInfo (or its
addProgressListener) to accept absolute progress and set its jobCount/total
accordingly, or pass a depotOffset and depotWeight into downloadChunksSimple so
it reports (depotOffset + depotLocalProgress * depotChunkCount) / totalChunks;
update calls in downloadDependencies and downloadDependenciesWithProgress so
onProgress sees a monotonically increasing 0..1 value.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/src/main/java/app/gamenative/gamefixes/GOGDependencyFix.ktapp/src/main/java/app/gamenative/gamefixes/GOG_2147483047.ktapp/src/main/java/app/gamenative/gamefixes/GameFixesRegistry.ktapp/src/main/java/app/gamenative/service/gog/GOGConstants.ktapp/src/main/java/app/gamenative/service/gog/GOGDownloadManager.ktapp/src/main/java/app/gamenative/utils/launchdependencies/GogScriptInterpreterDependency.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- app/src/main/java/app/gamenative/gamefixes/GOGDependencyFix.kt
- app/src/main/java/app/gamenative/utils/launchdependencies/GogScriptInterpreterDependency.kt
da4e1ce to
0f6c076
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
app/src/main/java/app/gamenative/gamefixes/GOGDependencyFix.kt (1)
16-21:isSatisfiedsilently treats unmapped dep IDs as not-installed — forces re-download every launch.This was flagged in a previous review. If a dependency ID is missing from
GOG_DEPENDENCY_INSTALLED_PATH,pathMap[id]returnsnull, causingisSatisfiedto returnfalseand triggering a redundant download on every launch. Consider logging a warning and treating unknown IDs as satisfied.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/gamefixes/GOGDependencyFix.kt` around lines 16 - 21, The isSatisfied function currently treats dependency IDs missing from GOGConstants.GOG_DEPENDENCY_INSTALLED_PATH as not-installed, causing redundant downloads; update isSatisfied to treat unknown IDs as satisfied instead of failing the check, and emit a warning via the existing logger when pathMap[id] is null so maintainers are alerted; locate the method isSatisfied (uses dependencyIds, commonRedist and pathMap) and change the predicate so that dependencyIds.all { id -> pathMap[id]?.let { File(commonRedist, it).exists() } ?: run { logger.warn("Unknown GOG dependency id: $id - assuming satisfied"); true } }.app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt (1)
991-1016: New public API is clean but inherits the partial-failure masking fromdownloadDependencies.The structure of
downloadDependenciesWithProgressis straightforward — creating a scopedDownloadInfo, wiring the progress listener, and delegating. However,downloadDependencies(lines 904-981) usescontinueon per-depot failures and always returnsResult.success(Unit)at line 984. This meansisSuccessat line 1012 will betrueeven when some dependencies failed to download, and the caller (GOGDependencyFix.apply) will report success while required redists are missing.This was flagged in a previous review and remains unaddressed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt` around lines 991 - 1016, downloadDependenciesWithProgress currently delegates to downloadDependencies but downloadDependencies masks per-depot failures and always returns Result.success, causing downloadDependenciesWithProgress to report success even when some dependencies failed; update downloadDependencies (the function handling per-depot loops and returning at the end) to accumulate failures (e.g., a list of errors or a boolean flag) and return a Result.failure (with an informative exception) when any depot fails, and leave downloadDependenciesWithProgress unchanged except to not call onProgress?.invoke(1f) unless the returned Result is success; ensure callers like GOGDependencyFix.apply receive the proper failure Result so missing redists are not reported as installed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@app/src/main/java/app/gamenative/gamefixes/GOGDependencyFix.kt`:
- Around line 16-21: The isSatisfied function currently treats dependency IDs
missing from GOGConstants.GOG_DEPENDENCY_INSTALLED_PATH as not-installed,
causing redundant downloads; update isSatisfied to treat unknown IDs as
satisfied instead of failing the check, and emit a warning via the existing
logger when pathMap[id] is null so maintainers are alerted; locate the method
isSatisfied (uses dependencyIds, commonRedist and pathMap) and change the
predicate so that dependencyIds.all { id -> pathMap[id]?.let {
File(commonRedist, it).exists() } ?: run { logger.warn("Unknown GOG dependency
id: $id - assuming satisfied"); true } }.
In `@app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt`:
- Around line 991-1016: downloadDependenciesWithProgress currently delegates to
downloadDependencies but downloadDependencies masks per-depot failures and
always returns Result.success, causing downloadDependenciesWithProgress to
report success even when some dependencies failed; update downloadDependencies
(the function handling per-depot loops and returning at the end) to accumulate
failures (e.g., a list of errors or a boolean flag) and return a Result.failure
(with an informative exception) when any depot fails, and leave
downloadDependenciesWithProgress unchanged except to not call
onProgress?.invoke(1f) unless the returned Result is success; ensure callers
like GOGDependencyFix.apply receive the proper failure Result so missing redists
are not reported as installed.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/src/main/java/app/gamenative/gamefixes/GOGDependencyFix.ktapp/src/main/java/app/gamenative/gamefixes/GOG_2147483047.ktapp/src/main/java/app/gamenative/gamefixes/GameFixesRegistry.ktapp/src/main/java/app/gamenative/service/gog/GOGConstants.ktapp/src/main/java/app/gamenative/service/gog/GOGDownloadManager.ktapp/src/main/java/app/gamenative/utils/launchdependencies/GogScriptInterpreterDependency.kt
…tkarshdalal#638) * Added GOG redist launch dep fix for moonlighter * Fixed en garde manifest dependencies
Added a patch for Moonlighter. The manifest game has a manifest that doesn't require the redists it actually needs.

(fun fact, so currently this game only works in heroic and GOG galaxy if other games require the redist. )
Once it runs you will have an extra distrbutable in the container which you can manually run(until automatic redist install is fixed on GOG)
Summary by cubic
Adds a launch-time fix for Moonlighter (GOG) to fetch missing MSVC 2017 x86/x64 redistributables so the game starts reliably. Also adds a per-game GOG dependency check with progress and registers fixes for both Moonlighter GOG IDs.
Bug Fixes
New Features
Written for commit dad6e73. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes