Aligned epic downloads to GOG to make it faster from eg India#1220
Aligned epic downloads to GOG to make it faster from eg India#1220utkarshdalal merged 2 commits intomasterfrom
Conversation
|
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 as they are similar to previous changes (1)
📝 WalkthroughWalkthroughConsolidates CDN URL ranking logic into a shared utility that probes base URLs via HTTP HEAD requests, refactors Epic chunk download/assembly to stream decompression directly from HTTP responses using Channel-based concurrent completion, and updates Epic UI to manage download states based on actual download tracking and partial download detection. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant EpicDLMgr as EpicDownloadManager
participant HttpClient
participant Decompressor as decompressStreamingChunkToFile
participant Channel
participant Assembler as assembleReady()
Client->>EpicDLMgr: downloadAndAssembleEpicChunks()
EpicDLMgr->>EpicDLMgr: rankCdnUrlsByProbe (HEAD requests)
par Concurrent Chunk Downloads
loop For each chunk
EpicDLMgr->>HttpClient: GET chunk (HEAD probe ranked URL)
HttpClient-->>EpicDLMgr: InputStream response
EpicDLMgr->>Decompressor: stream InputStream directly
Decompressor->>Decompressor: inflate + write to file<br/>emit updateBytesDownloaded
Decompressor-->>Channel: push ready chunk completion
end
end
loop Channel consumes completions
Channel->>Assembler: per-chunk incremental assembly
Assembler->>Assembler: assemble ready chunks/files
Assembler-->>Client: emit progressChange (streaming)
end
EpicDLMgr-->>Client: final assembly + status update
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
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 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 |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.kt (1)
601-627:⚠️ Potential issue | 🟠 MajorUse
readFullyor a loop for Epic chunk header reads.The code assumes
InputStream.read(byte[])fills the entire buffer in one call. According to Java documentation,read()can legally return fewer bytes than requested—it returns "some number of bytes" at most equal to the buffer length, not a guarantee of the full amount. This causes intermittent failures on network streams that return short reads.Both header reads (lines 604 and 626) need a
readFullyloop:Proposed fix
inputStream.buffered().use { input -> + fun InputStream.readFully(buffer: ByteArray) { + var offset = 0 + while (offset < buffer.size) { + val read = read(buffer, offset, buffer.size - offset) + if (read == -1) throw Exception("Unexpected EOF while reading Epic chunk header") + offset += read + } + } + // Read the entire header - determine size dynamically val headerStart = ByteArray(12) - if (input.read(headerStart) != 12) { - throw Exception("Failed to read chunk header start") - } + input.readFully(headerStart) @@ val remainingSize = headerSize - 12 val remainingBytes = ByteArray(remainingSize) - if (input.read(remainingBytes) != remainingSize) { - throw Exception("Failed to read remaining header: expected $remainingSize bytes") - } + input.readFully(remainingBytes)🤖 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 601 - 627, The header reads in EpicDownloadManager (the InputStream.buffered().use block) incorrectly assume InputStream.read fills the buffer; update the two reads that fill headerStart and remainingBytes to use a readFully approach (either wrap the stream in DataInputStream and call readFully, or implement a loop that repeatedly calls read(...) until the requested number of bytes are read or EOF) so the checks for "!= expected" are reliable and intermittent short reads from network streams are handled.
🤖 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 121-123: The current code drops all Cloudflare mirrors by calling
manifestData.cdnUrls.filter {
!it.baseUrl.startsWith("https://cloudflare.epicgamescdn.com") } before ranking,
which can produce an empty cdnUrls list; change this to prefer non-Cloudflare
entries but fall back to the original manifest list if the filtered result is
empty: compute preferred = manifestData.cdnUrls.filter {
!it.baseUrl.startsWith("https://https://cloudflare.epicgamescdn.com".removePrefix("https://"))
} (or simply the original predicate) then use
rankCdnUrlsByProbe(preferred.ifEmpty { manifestData.cdnUrls }) to populate
cdnUrls, and apply the exact same fallback pattern for the DLC-manifest path
where cdnUrls is similarly built (the other occurance around lines 344-346).
- Around line 696-701: The progress accounting increments via
downloadInfo.updateBytesDownloaded() before chunk validation, so on retryable
failures the bytes remain counted; modify the logic in EpicDownloadManager
around updateBytesDownloaded, emitProgressChange, and
STREAM_PROGRESS_TIME_INTERVAL_MS so bytes are only committed after the chunk
passes size/hash verification (or rollback the provisional bytes on every
failure path where the output file is deleted). Locate the three occurrences
(around the current block and the other spots referenced near lines 739-744 and
755-771) and either (1) keep a provisional counter per-chunk and call
downloadInfo.updateBytesDownloaded(finalVerifiedBytes) only after verification,
or (2) subtract the previously added bytes from downloadInfo when a chunk fails
and is retried, and ensure emitProgressChange reflects the corrected value.
Ensure all failure branches that delete the output file call the rollback path.
In
`@app/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.kt`:
- Around line 353-362: The resume branch for partial downloads uses
performDownload(... selectedGameIds = listOf(libraryItem.gameId)) which drops
any previously selected DLCs; change it to retrieve and reuse the prior selected
DLC IDs (e.g., from libraryItem, a persisted downloadState, or the download
manager) and pass that list into performDownload's selectedGameIds instead of a
single-item list; apply the same fix to the other partial-resume branch (the
similar code around the 440-448 region) or alternatively route the resume
through the manager dialog so the previously chosen DLC IDs are restored and
passed through performDownload.
- Around line 342-348: The current cancel path calls downloadInfo?.cancel() and
immediately calls EpicService.cleanupDownload(context, gameId); change it to
mirror the delete-download dialog by first signaling cancel on the DownloadInfo
(downloadInfo?.cancel()), then await the worker completion via
downloadInfo?.awaitCompletion() (or the existing awaitCompletion() helper)
before invoking EpicService.cleanupDownload(context, gameId), ensuring both
handlers (the block around isDownloading and the one at lines ~433-439) use the
same ordering so cleanup runs only after the download job has fully finished.
In `@app/src/main/java/app/gamenative/utils/CdnRankingUtils.kt`:
- Around line 13-38: The current probe loop maps over urls serially, so each
blocking httpClient.newCall(request).execute() delays the next; change to run
probes concurrently by wrapping the work in coroutineScope and launching per-url
async tasks (using Dispatchers.IO for the blocking execute call) then awaitAll
to collect Triples like Triple(url, success, elapsedMs). Replace the urls.map {
... } block with async tasks that perform the same Request building,
execute().use { ... } success check and elapsed time measurement, then sort the
resulting list the same way; keep the same URL trimming/distinct logic and final
sorted.map { it.first } result.
---
Outside diff comments:
In `@app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.kt`:
- Around line 601-627: The header reads in EpicDownloadManager (the
InputStream.buffered().use block) incorrectly assume InputStream.read fills the
buffer; update the two reads that fill headerStart and remainingBytes to use a
readFully approach (either wrap the stream in DataInputStream and call
readFully, or implement a loop that repeatedly calls read(...) until the
requested number of bytes are read or EOF) so the checks for "!= expected" are
reliable and intermittent short reads from network streams are handled.
🪄 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: 684751f9-5946-4c24-bed6-02922f34a852
📒 Files selected for processing (6)
app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.ktapp/src/main/java/app/gamenative/service/epic/EpicManager.ktapp/src/main/java/app/gamenative/service/gog/GOGDownloadManager.ktapp/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.ktapp/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.ktapp/src/main/java/app/gamenative/utils/CdnRankingUtils.kt
| downloadInfo.updateBytesDownloaded(bytesRead.toLong()) | ||
| val now = System.currentTimeMillis() | ||
| if (now - lastProgressEmitAt >= STREAM_PROGRESS_TIME_INTERVAL_MS) { | ||
| downloadInfo.emitProgressChange() | ||
| lastProgressEmitAt = now | ||
| } |
There was a problem hiding this comment.
Retryable chunk failures permanently inflate progress accounting.
updateBytesDownloaded() runs while compressed bytes are still being streamed, before the chunk passes size and hash validation. Any failure after that point deletes the output file but leaves the counted bytes behind, so retries count the same chunk again and progress can pin far earlier than actual completion. Roll those bytes back on every failure path, or keep them provisional until verification succeeds.
Also applies to: 739-744, 755-771
🤖 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 696 - 701, The progress accounting increments via
downloadInfo.updateBytesDownloaded() before chunk validation, so on retryable
failures the bytes remain counted; modify the logic in EpicDownloadManager
around updateBytesDownloaded, emitProgressChange, and
STREAM_PROGRESS_TIME_INTERVAL_MS so bytes are only committed after the chunk
passes size/hash verification (or rollback the provisional bytes on every
failure path where the output file is deleted). Locate the three occurrences
(around the current block and the other spots referenced near lines 739-744 and
755-771) and either (1) keep a provisional counter per-chunk and call
downloadInfo.updateBytesDownloaded(finalVerifiedBytes) only after verification,
or (2) subtract the previously added bytes from downloadInfo when a chunk fails
and is retried, and ensure emitProgressChange reflects the corrected value.
Ensure all failure branches that delete the output file call the rollback path.
| ): 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 start = System.nanoTime() | ||
| val success = try { | ||
| val request = Request.Builder() | ||
| .url(url) | ||
| .head() | ||
| .header("User-Agent", userAgent) | ||
| .build() | ||
| httpClient.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) | ||
| } | ||
|
|
||
| scored | ||
| .sortedWith(compareByDescending<Triple<String, Boolean, Long>> { it.second }.thenBy { it.third }) | ||
| .map { it.first } |
There was a problem hiding this comment.
Mirror probes still run one-by-one.
withContext(Dispatchers.IO) only moves this work off the caller thread; urls.map { ... execute() } still serializes every HEAD request. One slow mirror now delays ranking of all others, which adds latency to every GOG/Epic probe step instead of reducing it.
⚡ Proposed fix
import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.async
+import kotlinx.coroutines.awaitAll
+import kotlinx.coroutines.coroutineScope
import kotlinx.coroutines.withContext
@@
- val scored = urls.map { url ->
- val start = System.nanoTime()
- val success = try {
- val request = Request.Builder()
- .url(url)
- .head()
- .header("User-Agent", userAgent)
- .build()
- httpClient.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)
- }
+ val scored = coroutineScope {
+ urls.map { url ->
+ async {
+ val start = System.nanoTime()
+ val success = try {
+ val request = Request.Builder()
+ .url(url)
+ .head()
+ .header("User-Agent", userAgent)
+ .build()
+ httpClient.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()
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/app/gamenative/utils/CdnRankingUtils.kt` around lines 13 -
38, The current probe loop maps over urls serially, so each blocking
httpClient.newCall(request).execute() delays the next; change to run probes
concurrently by wrapping the work in coroutineScope and launching per-url async
tasks (using Dispatchers.IO for the blocking execute call) then awaitAll to
collect Triples like Triple(url, success, elapsedMs). Replace the urls.map { ...
} block with async tasks that perform the same Request building, execute().use {
... } success check and elapsed time measurement, then sort the resulting list
the same way; keep the same URL trimming/distinct logic and final sorted.map {
it.first } result.
There was a problem hiding this comment.
2 issues found across 6 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/utils/CdnRankingUtils.kt">
<violation number="1" location="app/src/main/java/app/gamenative/utils/CdnRankingUtils.kt:28">
P2: Do not swallow coroutine cancellation in the broad `catch (Exception)` block; rethrow `CancellationException` so cancellation propagates correctly.</violation>
</file>
<file name="app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt:3373">
P2: Existing EOS override entries are not normalized to `n,b`, so incorrect prior values remain and can prevent the intended native-first DLL loading.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| httpClient.newCall(request).execute().use { response -> | ||
| response.code in 200..499 | ||
| } | ||
| } catch (_: Exception) { |
There was a problem hiding this comment.
P2: Do not swallow coroutine cancellation in the broad catch (Exception) block; rethrow CancellationException so cancellation propagates correctly.
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/utils/CdnRankingUtils.kt, line 28:
<comment>Do not swallow coroutine cancellation in the broad `catch (Exception)` block; rethrow `CancellationException` so cancellation propagates correctly.</comment>
<file context>
@@ -0,0 +1,40 @@
+ httpClient.newCall(request).execute().use { response ->
+ response.code in 200..499
+ }
+ } catch (_: Exception) {
+ false
+ }
</file context>
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
Improved Epic download speed and progress stability by ranking CDN mirrors, streaming chunk decompression, and assembling files as chunks complete. Users in regions like India should see faster installs and smoother progress.
New Features
CdnRankingUtils; prefer fastest mirrors and skip the Cloudflare endpoint.Refactors
Net.httpForParallelDownloads(...)for manifest and chunk requests; stream batch completions and assemble files as soon as ready.CdnRankingUtilsand updateGOGDownloadManagerto use it.Written for commit c74b89f. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Performance
User Experience