Conversation
replace two-phase (download all chunks → assemble all files) with a unified loop that assembles files front-to-back as their chunks land and deletes consumed chunks immediately. peak disk usage drops from ~2x install size to ~1x. - StreamingAssembly: shared pure logic for chunk ordering, last-file tracking, readiness checks, and safe deletion decisions - EpicDownloadManager: downloadAndAssembleEpicChunks replaces separate download+assembly phases for both base game and DLC - GOGDownloadManager: downloadAndAssembleChunks with secure link refresh, used by both main game and dependency downloads. removed dead downloadChunksSimple and assembleFiles. - 14 unit tests covering ordering, deduplication, shared chunks, cleanup safety, and full batch-loop simulations
mirrors the GOG fix — when all files have zero chunks, the chunk loop never executes and assembly never runs without a trailing pass.
… better, made downloading UI progress smoother
📝 WalkthroughWalkthroughAdds a streaming download-and-assembly pipeline: new StreamingAssembly utilities for chunk ordering and last-consumer logic, rewrites Epic/GOG download managers to download-and-assemble chunks incrementally with dynamic parallelism and per-chunk URL candidates, adds an OkHttp factory for parallel downloads, small UI/resume tweaks, and unit tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Manager as DownloadManager
participant Net as Net.httpClient
participant Disk as ChunkStorage
participant Assembler as FileAssembler
participant UI as ProgressReporter
Manager->>Net: request batch of chunk URL candidates
Net-->>Manager: stream chunk data (progress events)
Net->>Disk: write chunk.part (compute MD5 while streaming)
Disk-->>Net: confirm write + MD5
Manager->>Assembler: notify chunk available
Assembler->>Manager: check readiness for nextFileIndex (StreamingAssembly.countReadyFiles)
alt file ready
Manager->>Assembler: assemble file from available chunks
Assembler->>Disk: read chunks, write file
Assembler->>Manager: file assembled (nextFileIndex++)
Manager->>Disk: delete chunks where lastConsumer == assembledFileIndex
Manager->>UI: update (downloadedChunks/total, nextFile/total)
else not ready
Manager->>UI: update progress only
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Possibly related issues
Possibly related PRs
Suggested Reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
# Conflicts: # app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt # app/src/main/java/app/gamenative/service/gog/api/GOGManifestParser.kt
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.kt (1)
904-945: Epic still assembles at batch boundaries, not on chunk completion.
awaitAll()holds assembly until the slowest chunk in the batch finishes, so ready files cannot be assembled and last-use chunks cannot be deleted as soon as they arrive. Mirroring the per-completion channel loop used inGOGDownloadManager.downloadAndAssembleChunks(...)would preserve more of the streaming disk-usage win here.🤖 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/epic/EpicDownloadManager.kt` around lines 904 - 945, The current loop waits on chunkBatch.map { ... }.awaitAll(), which delays assembly until the slowest chunk in the batch completes; change the logic to process each chunk as it completes (like GOGDownloadManager.downloadAndAssembleChunks) by launching async downloads for each chunk (using downloadChunkWithRetry) and consuming their Deferred results via a completion channel or select so you can immediately mark downloadedChunkIds, attempt assembleFile for any files whose chunkParts are now all present, and delete last-use chunk files (using chunkLastFile and chunkCacheDir) as soon as their last file is assembled; ensure you preserve progress/updateStatusMessage and the same failure handling by short-circuiting on the first failed Deferred (returning a Result.failure with the same exceptions).
🤖 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/epic/EpicDownloadManager.kt`:
- Around line 893-908: The Epic download loop uses the default OkHttp dispatcher
which limits same-host concurrency; replace the current OkHttp client used by
Epic downloads with Net.httpForParallelDownloads(parallelDownloads) (where
parallelDownloads is computed from PrefManager.downloadSpeed) so the dispatcher
is tuned for the actual parallelism. Locate where the Epic HTTP
client/okHttpClient is created or passed into downloadChunkWithRetry and
instantiate or inject the client via
Net.httpForParallelDownloads(parallelDownloads), then ensure
downloadChunkWithRetry and any callers (e.g., the chunk download loop over
chunkQueue and functions using cdnUrls) use that client instance so
parallelDownloads is effective.
In `@app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt`:
- Around line 880-900: The retry path after refreshSecureLinks currently obtains
retryResults but does not update downloadedChunkIds or trigger assembleReady(),
so chunks that succeeded on retry remain unrecorded; update the logic after
retryResults are awaited to iterate retryResults (associated with chunkMd5 keys
from chunkBatch/currentChunkUrlCandidates), collect successfully downloaded
chunk IDs and merge them into downloadedChunkIds (same structure/population used
by the initial batch), and then invoke the same assembleReady() /
assembly-triggering flow (or re-run the existing assembleReady check) so retried
successes are counted and assembly can complete; touch functions/variables:
expiredLinkFailures, refreshSecureLinks(...), currentChunkUrlCandidates,
retryResults, downloadChunkWithRetry(...), downloadedChunkIds, and
assembleReady() to ensure parity with the initial-success handling.
---
Nitpick comments:
In `@app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.kt`:
- Around line 904-945: The current loop waits on chunkBatch.map { ...
}.awaitAll(), which delays assembly until the slowest chunk in the batch
completes; change the logic to process each chunk as it completes (like
GOGDownloadManager.downloadAndAssembleChunks) by launching async downloads for
each chunk (using downloadChunkWithRetry) and consuming their Deferred results
via a completion channel or select so you can immediately mark
downloadedChunkIds, attempt assembleFile for any files whose chunkParts are now
all present, and delete last-use chunk files (using chunkLastFile and
chunkCacheDir) as soon as their last file is assembled; ensure you preserve
progress/updateStatusMessage and the same failure handling by short-circuiting
on the first failed Deferred (returning a Result.failure with the same
exceptions).
🪄 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: 8014e270-0ca6-4c43-ba7f-85e81dec7c56
📒 Files selected for processing (8)
app/src/main/java/app/gamenative/service/StreamingAssembly.ktapp/src/main/java/app/gamenative/service/epic/EpicDownloadManager.ktapp/src/main/java/app/gamenative/service/gog/GOGDownloadManager.ktapp/src/main/java/app/gamenative/service/gog/api/GOGManifestParser.ktapp/src/main/java/app/gamenative/ui/screen/library/LibraryAppScreen.ktapp/src/main/java/app/gamenative/ui/screen/library/appscreen/GOGAppScreen.ktapp/src/main/java/app/gamenative/utils/NetworkUtils.ktapp/src/test/java/app/gamenative/service/StreamingAssemblyTest.kt
| val parallelDownloads = PrefManager.downloadSpeed.coerceAtLeast(1) | ||
| val downloadedChunkIds = mutableSetOf<String>() | ||
| var nextFileToAssemble = 0 | ||
|
|
||
| downloadInfo.setProgress(0.0f) | ||
|
|
||
| for (chunkBatch in chunkQueue.chunked(parallelDownloads)) { | ||
| if (!downloadInfo.isActive()) { | ||
| return@withContext Result.failure(Exception("Download cancelled")) | ||
| } | ||
|
|
||
| val results = chunkBatch.map { chunk -> | ||
| async { | ||
| downloadChunkWithRetry(chunk, chunkCacheDir, chunkDir, cdnUrls, downloadInfo) | ||
| } | ||
| }.awaitAll() |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What are the default values of OkHttp Dispatcher.maxRequestsandDispatcher.maxRequestsPerHost, and do they cap concurrent requests to the same host when a custom dispatcher is not configured?
💡 Result:
The default values of OkHttp Dispatcher's maxRequests and maxRequestsPerHost are 64 and 5, respectively. Yes, these values cap concurrent requests to the same host when a custom dispatcher is not configured, as the Dispatcher is used by default in OkHttpClient and enforces these limits by checking running calls per host before promoting new async calls to execution.
Citations:
- 1: https://github.com/square/okhttp/blob/942ba2c48fe715dcf67af305900aeacfef7ee70e/okhttp/src/commonJvmAndroid/kotlin/okhttp3/Dispatcher.kt
- 2: https://square.github.io/okhttp/5.x/okhttp/okhttp3/-dispatcher/max-requests-per-host.html
- 3: https://android.googlesource.com/platform/external/okhttp/+/c6bd683/okhttp/src/main/java/com/squareup/okhttp/Dispatcher.java?autodive=0%2F%2F%2F%2F%2F%2F%2F%2F%2F%2F%2F%2F%2F%2F
- 4: https://square.github.io/okhttp/features/calls
🏁 Script executed:
# Find okHttpClient instantiation in EpicDownloadManager
rg -A 5 "okHttpClient" app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.ktRepository: utkarshdalal/GameNative
Length of output: 767
🏁 Script executed:
# Search for Net.httpForParallelDownloads to see what it does
find . -type f -name "*.kt" | head -20 | xargs rg -l "httpForParallelDownloads"Repository: utkarshdalal/GameNative
Length of output: 51
🏁 Script executed:
# Search for the class definition and dispatcher setup
rg -B 5 -A 10 "class EpicDownloadManager" app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.kt | head -30Repository: utkarshdalal/GameNative
Length of output: 671
🏁 Script executed:
# Search for Net class and httpForParallelDownloads function
rg -r "httpForParallelDownloads" --type ktRepository: utkarshdalal/GameNative
Length of output: 463
🏁 Script executed:
# Look for Net class definition
find . -type f -name "*.kt" | xargs rg -l "object Net|class Net" | head -5Repository: utkarshdalal/GameNative
Length of output: 228
🏁 Script executed:
# Search in GOGDownloadManager for dispatcher configuration
rg -B 5 -A 5 "Dispatcher\|httpForParallelDownloads" app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt | head -40Repository: utkarshdalal/GameNative
Length of output: 49
🏁 Script executed:
# Read NetworkUtils.kt to find httpForParallelDownloads
cat app/src/main/java/app/gamenative/utils/NetworkUtils.ktRepository: utkarshdalal/GameNative
Length of output: 1997
🏁 Script executed:
# Search GOG more carefully for any dispatcher config
rg "Dispatcher" app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.ktRepository: utkarshdalal/GameNative
Length of output: 722
🏁 Script executed:
# Check if GOG uses Net.httpForParallelDownloads
rg "Net\.httpForParallelDownloads\|httpForParallelDownloads" app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.ktRepository: utkarshdalal/GameNative
Length of output: 49
🏁 Script executed:
# Check how Epic constructs chunk URLs - are they same host?
rg -B 3 -A 3 "cdnUrls" app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.kt | head -30Repository: utkarshdalal/GameNative
Length of output: 1386
🏁 Script executed:
# Check GOG's okHttpClient setup
rg -B 5 -A 10 "private val okHttpClient" app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.ktRepository: utkarshdalal/GameNative
Length of output: 49
🏁 Script executed:
# Check if GOG uses Net.http or creates its own
rg "okHttpClient\|Net\.http" app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt | head -20Repository: utkarshdalal/GameNative
Length of output: 49
🏁 Script executed:
# Search for downloadChunk method in GOG
rg -B 2 -A 10 "fun downloadChunk" app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt | head -40Repository: utkarshdalal/GameNative
Length of output: 978
🏁 Script executed:
# Check GOG imports to see what it uses for HTTP
head -30 app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.ktRepository: utkarshdalal/GameNative
Length of output: 1180
🏁 Script executed:
# Find where GOG's downloadChunk methods are called to see what httpClient is passed
rg -B 5 "downloadChunkWithRetry\|downloadChunk" app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt | grep -A 5 "downloadChunk\|httpClient\|Net\|parallelDownloads" | head -50Repository: utkarshdalal/GameNative
Length of output: 49
🏁 Script executed:
# Look for the main download orchestration in GOG to see how httpClient is created/passed
rg -B 3 -A 10 "fun download.*Result" app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt | head -80Repository: utkarshdalal/GameNative
Length of output: 571
🏁 Script executed:
# Find the main download function in GOG
rg -B 2 -A 20 "suspend fun download" app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt | head -100Repository: utkarshdalal/GameNative
Length of output: 4396
🏁 Script executed:
# Search for where Net.httpForParallelDownloads is actually called in the codebase
rg -B 3 -A 3 "Net\.httpForParallelDownloads" app/src/main/java/app/gamenative/service/gog/Repository: utkarshdalal/GameNative
Length of output: 980
Use Net.httpForParallelDownloads(parallelDownloads) for the Epic HTTP client.
The current okHttpClient uses OkHttp's default dispatcher, which caps same-host concurrency to 5 requests. With parallelDownloads potentially much higher, this becomes a bottleneck. GOG already uses Net.httpForParallelDownloads() to tune the dispatcher—apply the same pattern to Epic to make the parallelism setting effective.
🤖 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/epic/EpicDownloadManager.kt` around
lines 893 - 908, The Epic download loop uses the default OkHttp dispatcher which
limits same-host concurrency; replace the current OkHttp client used by Epic
downloads with Net.httpForParallelDownloads(parallelDownloads) (where
parallelDownloads is computed from PrefManager.downloadSpeed) so the dispatcher
is tuned for the actual parallelism. Locate where the Epic HTTP
client/okHttpClient is created or passed into downloadChunkWithRetry and
instantiate or inject the client via
Net.httpForParallelDownloads(parallelDownloads), then ensure
downloadChunkWithRetry and any callers (e.g., the chunk download loop over
chunkQueue and functions using cdnUrls) use that client instance so
parallelDownloads is effective.
There was a problem hiding this comment.
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/service/gog/GOGDownloadManager.kt`:
- Around line 1256-1261: The debug log prints full signed CDN URLs
(ranked.joinToString()) which exposes bearer tokens; update the logging in
GOGDownloadManager (the block that computes `ranked` from `scored` and calls
`Timber.tag("GOG").d(...)`) to redact query parameters before logging—e.g., map
each URL in `ranked` to a sanitized form by stripping or replacing the query
string (take substring before '?' or parse via URI and omit getQuery()) and then
join those sanitized entries for the log message instead of the raw
`ranked.joinToString()`.
- Around line 893-917: The current refresh+retry logic replays the whole
chunkBatch via chunkBatch.map { async { ... downloadChunkWithRetry(...) } }
which re-downloads already-consumed chunks; change it to only retry failed chunk
IDs: after awaiting the initial batch, compute failedChunkIds =
retryResults.zip(chunkBatch).filter { it.first.isFailure }.map { corresponding
chunkMd5 } (use retryResults and chunkBatch to correlate), then only call
downloadChunkWithRetry for those failedChunkIds (using currentChunkUrlCandidates
and downloadChunkWithRetry) and only add successfully retried IDs to
downloadedChunkIds (do not re-add already-successful chunks); keep the existing
assembleReady() and progress update but derive progress from
downloadedChunkIds.size so bytesDownloaded and immediate deletion behave
correctly.
🪄 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: 455e9b19-df89-43e9-bc32-9b205f9dd0ea
📒 Files selected for processing (1)
app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt">
<violation number="1" location="app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt:894">
P1: Retrying only expired-link chunks drops other failures in the same batch. If one chunk needs a secure-link refresh and another failed for a different reason, the second failure is no longer retried or reported here, so the install can continue until it later fails with a generic incomplete-assembly error.</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 880-930: The code currently ignores non-expired failures when
expiredLinkFailures triggers a secure-link refresh; before calling
refreshSecureLinks (and likewise after retryResults), check for any failures in
results whose chunkMd5 is not in expiredLinkFailures (i.e., non-expired
failures) and return@withContext Result.failure(...) immediately; specifically,
compute nonExpiredFailures from results by filtering isFailure and chunkMd5 not
in expiredLinkFailures.map { ... }, and if nonExpiredFailures.isNotEmpty()
return the first failure's exception (or a wrapped Exception) so that
refreshSecureLinks and retry logic (refreshSecureLinks, retryResults,
failedChunkIds, downloadChunkWithRetry, assembleReady) are skipped when there
are non-expired errors.
🪄 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: df777a02-7d45-4c19-853b-36dd2700534a
📒 Files selected for processing (1)
app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt (1)
1246-1273: Consider parallel CDN probing for faster startup.The sequential HEAD probes work correctly, but for N CDN endpoints, startup latency is O(N × avg_probe_time). Parallel probing would reduce this to O(max_probe_time).
♻️ Optional: Parallel probe implementation
private suspend fun rankBaseUrlsByProbe(baseUrls: List<String>, label: String): List<String> = withContext(Dispatchers.IO) { val urls = baseUrls.map { it.trim() }.filter { it.isNotEmpty() }.distinct() if (urls.size <= 1) return@withContext urls - val scored = urls.map { url -> + val scored = urls.map { url -> + async { val start = System.nanoTime() val success = try { val request = Request.Builder() .url(url) .head() .header("User-Agent", "GOG Galaxy") .build() Net.http.newCall(request).execute().use { response -> response.code in 200..499 } } catch (_: Exception) { false } val elapsedMs = (System.nanoTime() - start) / 1_000_000 Triple(url, success, elapsedMs) + } - } + }.awaitAll() val ranked = scored .sortedWith(compareByDescending<Triple<String, Boolean, Long>> { it.second }.thenBy { it.third }) .map { it.first } ranked }🤖 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 1246 - 1273, rankBaseUrlsByProbe currently probes CDN baseUrls sequentially causing O(N) startup latency; change it to run probes in parallel by launching coroutines for each url (e.g., inside coroutineScope { ... } using async(Dispatchers.IO) for each url) so each probe measures success and elapsedMs concurrently, collect all Deferred results with awaitAll(), then sort by success desc then elapsedMs asc as before and return the ordered url list; ensure each probe still uses the same Request/Net.http call and catches exceptions per-probe to produce (url, success, elapsedMs).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt`:
- Around line 1246-1273: rankBaseUrlsByProbe currently probes CDN baseUrls
sequentially causing O(N) startup latency; change it to run probes in parallel
by launching coroutines for each url (e.g., inside coroutineScope { ... } using
async(Dispatchers.IO) for each url) so each probe measures success and elapsedMs
concurrently, collect all Deferred results with awaitAll(), then sort by success
desc then elapsedMs asc as before and return the ordered url list; ensure each
probe still uses the same Request/Net.http call and catches exceptions per-probe
to produce (url, success, elapsedMs).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 10e8d926-353e-4d53-a24e-378447090426
📒 Files selected for processing (1)
app/src/main/java/app/gamenative/service/gog/GOGDownloadManager.kt
Description
Recording
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
Switches Epic and GOG installs to streaming download+assembly to cut peak disk usage to ~1x and speed up resumes. Adds CDN ranking and smoother, byte-based progress for GOG, plus one‑click resume from the game screen.
New Features
Refactors and Performance
StreamingAssemblywith unit tests; unified streaming loop inEpicDownloadManager/GOGDownloadManager; parallelism viaPrefManager.downloadSpeedandNet.httpForParallelDownloads..part+ MD5 verify; total expected bytes include resumed data; better cancellation propagation.GOGManifestParserbuilds ordered URL candidate lists (keeps back‑compat helpers); UI tweaks: resume logic inGOGAppScreenand single‑line percent label inLibraryAppScreen.Written for commit c0eaa24. Summary will update on new commits.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Tests