Epic games Integration #431
Conversation
- Epic Games authentication and login flow - Game library management and syncing - Cloud saves support with manifest parsing (binary and JSON formats) - Game installation and download management - Integration with container system - Epic-specific UI components and screens - Database entities and DAOs for Epic games - Comprehensive test coverage for manifest parsing Includes validation for empty manifests and files to prevent upload/download issues.
…how the correct download for the base game. Next up is seeing about downloading the DLC also.
Now it'll just ignore it if the file is empty.
…pload. Also removed EpicConverter since GOGConverter does the same thing.
… epic. And fixed crash with install
…s downloaded. Also fixed the download progress so that it uses the downloadSize for its progress bar.
… into epic-games-integration
| } | ||
| } | ||
|
|
||
| suspend fun getInstalledExe(appId: Int): String { |
There was a problem hiding this comment.
Should be the same format as the SteamService one. And we shouldn't call it with runBlocking. In general, runBlocking should be almost always avoided as it causes ANRs on old and an increasing number of new devices. Xiaomi etc do very aggressive memory handling, and as soon as something blocking happens on a UI thread, it silently kills the app without a crash log, leaving us with a hard-to-trace bug.
There was a problem hiding this comment.
Thanks for raising.
Steam Service with getInstalledExe does call:
val appInfo = getAppInfoOf(appId) ?: return ""
val installDir = appInfo.config.installDir.ifEmpty { appInfo.name } fun getAppInfoOf(appId: Int): SteamApp? {
return runBlocking(Dispatchers.IO) { instance?.appDao?.findApp(appId) }
}
Comparing this to:
suspend fun getInstalledExe(appId: Int): String {
// Strip EPIC_ prefix to get the raw Epic app name
val game = getGameById(appId)
* Get a single game by ID
*/
suspend fun getGameById(appId: Int): EpicGame? {
return withContext(Dispatchers.IO) {
try {
epicGameDao.getById(appId)
} catch (e: Exception) {
Timber.e(e, "Failed to get Epic game by ID: $appId")
null
}
}
}
So it looks to me like both do block the thread. It looks like we might need to make some more in-depth changes to avoid this on all the services.
| // Initialize Epic service | ||
| appScope.launch { | ||
| try { | ||
| app.gamenative.service.epic.EpicService.start(applicationContext) |
There was a problem hiding this comment.
Please make sure that EpicService is also stopped when the app is closed or we log out.
There was a problem hiding this comment.
Added changes now. Will test and make sure it closes correctly
There was a problem hiding this comment.
Services now close correctly for GOG and Epic alike.
| @Query("SELECT * FROM epic_games ORDER BY title ASC") | ||
| fun getAll(): Flow<List<EpicGame>> | ||
|
|
||
| @Query("SELECT * FROM epic_games ORDER BY title ASC") | ||
| suspend fun getAllAsList(): List<EpicGame> |
There was a problem hiding this comment.
We don't need the exclude here that we used in the GOG DAO?
There was a problem hiding this comment.
So with Epic, we have a filter for isDLC instead. But I'd be happy to match the pattern so that we can exclude more than just DLC. Currently Epic only does Games & DLC, whereas GOG does Movies, Music, Packages, DLC etc.
| // Start download in background | ||
| instance.scope.launch { | ||
| try { | ||
| val commonRedistDir = File(installPath, "_CommonRedist") |
There was a problem hiding this comment.
Confirming that this is downloading to /data/data/app.gamenative/Epic and not /data/data/app.gamenative/files/Epic. I believe it's doing the latter for GOG and that isn't correct. It should be beside the Steam folder.
There was a problem hiding this comment.
Yeah I've realized what I did here.
I was using the fileDir.path instead of dataDir.path. Thanks for catching.
There was a problem hiding this comment.
This is now fixed as the baseDir has been adjusted now to give the correct installPath
…XServer Screen functions.
…th removing hard-coded path for the cloud saves.
…e for GOG and Epic to correctly shutdown and restart correctly.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In
`@app/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.kt`:
- Around line 439-443: Replace the hardcoded toast text used in the
android.widget.Toast.makeText(...) call in EpicAppScreen.kt with a localized
string resource; add an entry (e.g. download_cancelled) to strings.xml and call
context.getString(R.string.download_cancelled) (or use stringResource if in a
composable) instead of the literal "Download cancelled" so the toast uses the
localized string.
- Around line 139-166: LaunchedEffect block performs IO work on Dispatchers.IO
but increments the Compose state refreshTrigger inside that IO context, which
can cause "state changed on a different thread" crashes; move the state mutation
out of the withContext(Dispatchers.IO) block back to the Main dispatcher by
applying refreshTrigger++ (or any update to the Compose state) after the IO
block completes (or wrap it in withContext(Dispatchers.Main)), and ensure
EpicService.updateEpicGame(...) can remain in the IO context while the UI state
update (refreshTrigger) runs on the main thread.
In `@app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt`:
- Around line 1751-1754: The code references isSteamGame but it's not declared;
define it before use by adding a boolean like isSteamGame = gameSource ==
GameSource.STEAM (or if GameSource.STEAM doesn't exist, use the intended
negation pattern, e.g. isSteamGame = !isEpicGame). Place this declaration
alongside the existing isEpicGame and gameId declarations (near gameSource and
ContainerUtils.extractGameIdFromContainerId/appId) so the subsequent if
(isSteamGame) branch compiles.
In `@app/src/main/java/app/gamenative/utils/ContainerUtils.kt`:
- Around line 585-602: The Epic branch logs "EPIC GAME FOUND FOR DRIVE" before
checking if EpicService.getEpicGameOf(...) returned null, so failures are silent
and logs misleading; move or change logging and add a failure log: call
extractGameIdFromContainerId(appId) then call EpicService.getEpicGameOf(gameId)
and only emit Timber.tag("Epic").d("EPIC GAME FOUND FOR DRIVE: %s", gameId) and
Timber.tag("Epic").d("EPIC INSTALL PATH FOUND FOR DRIVE: %s", gameInstallPath)
when game != null and installPath non-empty, and add a Timber.tag("Epic").w or
.d error message when game is null or installPath empty before returning
defaultDrives; update references in the GameSource.EPIC branch and use
Container.getNextAvailableDriveLetter(defaultDrives) as currently used.
In `@app/src/main/res/values-fr/strings.xml`:
- Around line 987-990: The French strings should be consistent and the example
must be localized: update the string resource epic_login_auth_example so its
text is in French and uses the same terminology as
epic_login_auth_code_label/epic_login_auth_code_placeholder (e.g., "Exemple :
code d’autorisation : « exemple »"), ensuring punctuation/escaping matches
surrounding entries and that the apostrophe in "d’autorisation" is the correct
typographic character used elsewhere.
- Line 990: Update the French string resource epic_login_auth_code_placeholder
to remove any mention of "URL" so it only asks for the authorization code;
locate the string named epic_login_auth_code_placeholder and replace its value
with a wording that references the code only (preserve proper escaping for the
apostrophe), or if you prefer to keep URL in flow then update the login flow
copy to match—ensure string and UI flow remain consistent.
In `@app/src/main/res/values-pt-rBR/strings.xml`:
- Line 854: The string resource epic_login_auth_example contains English text
("authorization code: \"example\""); replace the English fragment with
Portuguese (e.g., use "código de autorização" and a Portuguese example like
"exemplo") so the entire value is in Portuguese and consistent with the locale
(update the value of epic_login_auth_example accordingly).
In `@app/src/main/res/values-zh-rTW/strings.xml`:
- Line 999: The epic logout string lacks an error placeholder: update the
strings.xml entry for epic_logout_failed to include a "%s" placeholder (matching
gog_logout_failed) and change the call sites that use
context.getString(R.string.epic_logout_failed) to pass the error message (e.g.,
context.getString(R.string.epic_logout_failed, error?.message ?: "Unknown
error")), mirroring the GOG implementation so the user sees the actual error
details.
♻️ Duplicate comments (11)
app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt (1)
1741-1741: Return type should remain non-nullable.This change was already flagged in a previous review, and the maintainer confirmed it should be reverted. The function never returns
null, and the caller at line 1562 uses the result in string concatenation without null handling.Suggested fix
-): String? { +): String {app/src/main/res/values/strings.xml (1)
1015-1015: Grammar consistency for logout failure message.Suggested fix
- <string name="epic_logout_failed">Failure to logout of Epic Games, please try again</string> + <string name="epic_logout_failed">Failed to log out from Epic Games. Please try again.</string>app/src/main/java/app/gamenative/service/epic/EpicCloudSavesManager.kt (6)
221-225: Avoid auto-download when no prior sync exists.With
lastSync == null,cloudTimestamp >= ""always evaluates true, so this path always downloads and can overwrite newer local saves. Compare cloud vs local timestamps instead (or treat as conflict).🛠️ Suggested fix
- // No sync timestamp - just compare cloud vs local - if (cloudTimestamp >= (lastSync ?: "")) { - SyncAction.DOWNLOAD - } else { - SyncAction.NONE - } + // No sync timestamp - compare cloud vs local directly + val cloudMillis = parseTimestamp(cloudTimestamp) + val localMillis = localNewestTimestamp ?: 0L + return@withContext when { + cloudMillis > localMillis -> SyncAction.DOWNLOAD + localMillis > cloudMillis -> SyncAction.UPLOAD + else -> SyncAction.NONE + }
519-522: Log message doesn’t match actual upload behavior.The log says
${toUpload.size}files are being uploaded, but the call uploads all local files. Update the message or passtoUploadif selective upload was intended.🛠️ Suggested fix
- Timber.tag("Epic").i("[Cloud Saves] Uploading ${toUpload.size} files based on timestamp comparison") + Timber.tag("Epic").i("[Cloud Saves] Uploading all local files after conflict resolution (delta: ${toUpload.size} newer/unique)")
831-842: Useresponse.use {}to avoid OkHttp resource leaks.Manual
close()can be skipped if an exception occurs betweenexecute()andclose(). Wrap the response inuse {}like the other calls in this file.🛠️ Suggested fix
- val response = httpClient.newCall(request).execute() - - Timber.tag("Epic").d("[Cloud Saves] Response code: ${response.code}") - - val responseBody = try { - response.body?.string() ?: "" - } catch (e: Exception) { - Timber.tag("Epic").e(e, "[Cloud Saves] Failed to read response body") - "" - } - - response.close() + val response = httpClient.newCall(request).execute() + response.use { + Timber.tag("Epic").d("[Cloud Saves] Response code: ${response.code}") + + val responseBody = try { + response.body?.string() ?: "" + } catch (e: Exception) { + Timber.tag("Epic").e(e, "[Cloud Saves] Failed to read response body") + "" + } + // ... existing handling using responseBody ... + }
499-503: GuardcopyOfRangebounds to handle truncated chunks.If
offset + sizeexceedschunkData.size,copyOfRangethrows. Add explicit bounds checks in both reconstruction paths.🛠️ Suggested fix
- val partData = chunkData.copyOfRange( - chunkPart.offset.toInt(), - (chunkPart.offset + chunkPart.size).toInt(), - ) + val endIndex = (chunkPart.offset + chunkPart.size).toInt() + if (endIndex > chunkData.size) { + Timber.tag("Epic").e( + "[Cloud Saves] Chunk data truncated for ${fileManifest.filename}: expected $endIndex bytes, got ${chunkData.size}" + ) + downloadSuccess = false + return@forEach + } + val partData = chunkData.copyOfRange( + chunkPart.offset.toInt(), + endIndex, + )- val partData = chunkData.copyOfRange( - chunkPart.offset.toInt(), - (chunkPart.offset + chunkPart.size).toInt(), - ) + val endIndex = (chunkPart.offset + chunkPart.size).toInt() + if (endIndex > chunkData.size) { + Timber.tag("Epic").e( + "[Cloud Saves] Chunk data truncated for ${fileManifest.filename}: expected $endIndex bytes, got ${chunkData.size}" + ) + return@forEach + } + val partData = chunkData.copyOfRange( + chunkPart.offset.toInt(), + endIndex, + )Also applies to: 665-668
964-1011: Chunk GUIDs inChunkPartdon’t matchChunkInfo.
ChunkPartGUIDs are generated independently fromfinalizeChunk, so manifests can reference chunks that don’t exist. Generate a GUID once per chunk and pass it through tofinalizeChunk.🛠️ Suggested fix
- var currentChunkData = mutableListOf<Byte>() + var currentChunkData = mutableListOf<Byte>() + var currentChunkGuid: IntArray? = null @@ - if (currentChunkData.size >= chunkSize) { - val chunk = finalizeChunk(currentChunkData.toByteArray(), chunkNum++, packagedFiles) + if (currentChunkData.size >= chunkSize) { + val chunk = finalizeChunk( + currentChunkData.toByteArray(), + chunkNum++, + packagedFiles, + requireNotNull(currentChunkGuid), + ) chunks.add(chunk) currentChunkData.clear() + currentChunkGuid = null } - val guid = if (currentChunkData.isEmpty()) { - generateGuid() - } else { - fileManifest.chunkParts.lastOrNull()?.guid ?: generateGuid() - } + if (currentChunkGuid == null) { + currentChunkGuid = generateGuid() + } + val guid = currentChunkGuid!! @@ - if (currentChunkData.isNotEmpty()) { - val chunk = finalizeChunk(currentChunkData.toByteArray(), chunkNum++, packagedFiles) + if (currentChunkData.isNotEmpty()) { + val chunk = finalizeChunk( + currentChunkData.toByteArray(), + chunkNum++, + packagedFiles, + requireNotNull(currentChunkGuid), + ) chunks.add(chunk) } @@ - private fun finalizeChunk( - data: ByteArray, - chunkNum: Int, - packagedFiles: MutableMap<String, ByteArray>, - ): app.gamenative.service.epic.manifest.ChunkInfo { + private fun finalizeChunk( + data: ByteArray, + chunkNum: Int, + packagedFiles: MutableMap<String, ByteArray>, + guid: IntArray, + ): app.gamenative.service.epic.manifest.ChunkInfo { @@ - chunkInfo.guid = generateGuid() + chunkInfo.guid = guidAlso applies to: 1031-1053
1253-1257: Choose deterministic subdirectory when multiple profiles exist.
firstOrNullcan pick an arbitrary subdir if multiple profiles are present, which risks syncing the wrong save set. Consider matchingaccountIdor choosing the most recently modified subdir with files.🛠️ Suggested approach
- val dirWithFiles = subDirs.firstOrNull { subDir -> - subDir.listFiles()?.any { it.isFile } == true - } + val dirWithFiles = subDirs + .filter { it.listFiles()?.any { it.isFile } == true } + .maxByOrNull { it.lastModified() }app/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.kt (2)
170-208:dlcTitlesis populated but never used.
Either pass it into the dialog or remove it to avoid redundant fetches and recompositions.
455-483: Unmanaged CoroutineScopes remain in uninstall + cloud sync.
These scopes outlive the composable and can leak. Prefer a lifecycle-aware scope (as you now do for downloads).♻️ Suggested pattern
- private fun performUninstall(context: Context, libraryItem: LibraryItem) { + private fun performUninstall(scope: CoroutineScope, context: Context, libraryItem: LibraryItem) { Timber.tag(TAG).i("Uninstalling Epic game: ${libraryItem.appId}") - CoroutineScope(Dispatchers.IO).launch { + scope.launch(Dispatchers.IO) { try { val result = EpicService.deleteGame(context, libraryItem.gameId) ... } catch (e: Exception) { ... } } }- val options = mutableListOf<AppMenuOption>() + val options = mutableListOf<AppMenuOption>() + val scope = rememberCoroutineScope() ... - val scope = CoroutineScope(Dispatchers.Main + SupervisorJob()) - scope.launch { + scope.launch {- performUninstall(context, libraryItem) + performUninstall(scope, context, libraryItem)#!/bin/bash # Find remaining unmanaged CoroutineScope creations in EpicAppScreen. rg -n "CoroutineScope\\(" app/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.ktAlso applies to: 550-580
app/src/main/java/app/gamenative/service/epic/EpicConstants.kt (1)
87-90: Guard against empty install folder names.
If the sanitized title is empty (e.g., non‑ASCII names), installs collapse into the base directory and can collide.🛠️ Suggested fallback
fun getGameInstallPath(context: android.content.Context, gameTitle: String): String { // Sanitize game title for filesystem val sanitizedTitle = gameTitle.replace(Regex("[^a-zA-Z0-9 \\-_]"), "").trim() - return Paths.get(defaultEpicGamesPath(context), sanitizedTitle).toString() + val folderName = if (sanitizedTitle.isNotBlank()) { + sanitizedTitle + } else { + "EpicGame_${gameTitle.hashCode()}" + } + return Paths.get(defaultEpicGamesPath(context), folderName).toString() }
🧹 Nitpick comments (3)
app/src/main/res/values-de/strings.xml (1)
925-925: Mixed language in example text.The example uses English "authorization code" while the rest of the file uses the German term "Autorisierungscode" (see line 927). For consistency, consider using the German term here as well.
Suggested fix
- <string name="epic_login_auth_example">Beispiel: authorization code: "example"</string> + <string name="epic_login_auth_example">Beispiel: Autorisierungscode: "example"</string>app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt (1)
66-66: Unused import.The
Toastimport is not used in the changed code. The existing usage at line 984 uses the fully qualifiedandroid.widget.Toastinstead.Suggested fix
-import android.widget.Toastapp/src/main/res/values/strings.xml (1)
48-56: LGTM — Epic install/uninstall strings align with existing flows.If you want, I can generate AI translations for these new strings across the other locales.
app/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.kt
Show resolved
Hide resolved
app/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.kt
Outdated
Show resolved
Hide resolved
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="app/src/main/java/app/gamenative/MainActivity.kt">
<violation number="1" location="app/src/main/java/app/gamenative/MainActivity.kt:333">
P2: The comment claims to check for 'no downloads in progress' but the code doesn't actually call `GOGService.hasActiveOperations()`. The EpicService handling below correctly checks this condition before stopping. Consider adding the same check for consistency:
```kotlin
if (GOGService.isRunning && !isChangingConfigurations) {
if (!GOGService.hasActiveOperations()) {
Timber.i("Stopping GOG Service")
GOGService.stop()
} else {
Timber.d("GOGService kept running - has active operations")
}
}
```</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.
1 issue found across 5 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="app/src/main/java/app/gamenative/service/gog/GOGService.kt">
<violation number="1" location="app/src/main/java/app/gamenative/service/gog/GOGService.kt:567">
P2: Hardcoded notification text should be moved to a string resource so it can be localized and maintained consistently.
(Based on your team's feedback about avoiding hardcoded UI strings.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| // Start as foreground service | ||
| val notification = notificationHelper.createForegroundNotification("Connected") | ||
| startForeground(1, notification) // Use different ID than SteamService (which uses 1) | ||
| val notification = notificationHelper.createForegroundNotification("GOG Connected") |
There was a problem hiding this comment.
P2: Hardcoded notification text should be moved to a string resource so it can be localized and maintained consistently.
(Based on your team's feedback about avoiding hardcoded UI strings.)
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/service/gog/GOGService.kt, line 567:
<comment>Hardcoded notification text should be moved to a string resource so it can be localized and maintained consistently.
(Based on your team's feedback about avoiding hardcoded UI strings.) </comment>
<file context>
@@ -564,8 +564,8 @@ class GOGService : Service() {
// Start as foreground service
- val notification = notificationHelper.createForegroundNotification("Connected")
- startForeground(1, notification) // Use different ID than SteamService (which uses 1)
+ val notification = notificationHelper.createForegroundNotification("GOG Connected")
+ startForeground(2, notification)
</file context>
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@app/src/main/java/app/gamenative/service/epic/EpicService.kt`:
- Around line 330-340: The call to runBlocking in downloadGame blocks the caller
thread (risking ANR); change the blocking call to use a background dispatcher
(e.g., runBlocking(Dispatchers.IO)) or convert downloadGame to a suspend
function and call epicManager.getGameById from a coroutine; specifically update
the runBlocking { instance.epicManager.getGameById(appId) } invocation in
downloadGame (and any similar synchronous usages) to run on Dispatchers.IO (or
make downloadGame suspend and remove runBlocking) so the main thread is not
blocked.
- Around line 440-451: getAccountId uses kotlinx.coroutines.runBlocking without
a dispatcher which can block the caller (UI) thread; change the call to use a
background dispatcher. Replace kotlinx.coroutines.runBlocking {
EpicAuthManager.getStoredCredentials(context) } with runBlocking(Dispatchers.IO)
{ EpicAuthManager.getStoredCredentials(context) } (ensure Dispatchers is
imported) so getAccountId, which calls getInstance()?.applicationContext and
EpicAuthManager.getStoredCredentials, executes on IO instead of the main thread
while preserving the existing try/catch and return behavior.
In `@app/src/main/java/app/gamenative/service/gog/GOGService.kt`:
- Around line 567-568: GOGService is starting its foreground notification with
startForeground(2, notification) but NotificationHelper.cancel() only cancels a
hardcoded NOTIFICATION_ID (1), causing cross-service cancellation; update
NotificationHelper by adding cancel(id: Int) and change callers in GOGService
and EpicService to pass their notification IDs (the same IDs used in
startForeground) to notificationHelper.cancel(id) so each service cancels its
own notification, and replace the hardcoded "GOG Connected" in GOGService (used
via createForegroundNotification) with a string resource reference (add
gog_connected to strings.xml and load it via
context.getString(R.string.gog_connected) when creating the notification).
In
`@app/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.kt`:
- Around line 551-582: The click handler creates an unmanaged CoroutineScope
using CoroutineScope(Dispatchers.Main + SupervisorJob()) and uses hardcoded
Toast strings; replace the orphan scope with a lifecycle-aware scope (e.g., use
viewModelScope or lifecycleScope from the owning ViewModel/Activity/Fragment
when invoking EpicCloudSavesManager.syncCloudSaves(context, libraryItem.gameId,
...)) so the job is cancelled with the lifecycle, and move all hardcoded
messages passed to Toast.makeText ("Starting cloud save sync...", "Cloud saves
synced successfully", "Cloud save sync failed", "Cloud save sync error: %s")
into strings.xml (use a formatted string for the error message) and reference
them via context.getString(...) when showing toasts; ensure error logging still
logs the exception via Timber.tag(TAG).e(e, ...).
- Around line 456-485: performUninstall currently creates an unmanaged
CoroutineScope(Dispatchers.IO) that can outlive the composable; change the
signature to accept a lifecycle-aware CoroutineScope (e.g., fun
performUninstall(context: Context, libraryItem: LibraryItem, scope:
CoroutineScope)) and replace CoroutineScope(Dispatchers.IO).launch { ... } with
scope.launch(Dispatchers.IO) { ... } (or use scope.launch {
withContext(Dispatchers.IO) { ... } }) to ensure the job is tied to the caller's
lifecycle—mirroring the approach used in performDownload—and update the call
site(s) that invoke performUninstall accordingly.
♻️ Duplicate comments (3)
app/src/main/java/app/gamenative/service/epic/EpicService.kt (1)
376-411: Hardcoded UI strings should be moved tostrings.xml.The toast messages at lines 380, 394, and 408 ("Download completed successfully!", "Download failed:", "Download error:") should use string resources for localization support and maintainability.
app/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.kt (2)
121-122:dlcTitlesstate variable is declared and populated but never used.The state is set at lines 132-133 and 197-198 but never consumed in the returned
GameDisplayInfoor passed to any component. Consider removing it to avoid unnecessary recompositions.
440-444: User-visible toast string isn't localized."Download cancelled" should come from string resources for consistency with other Epic UI copy.
🧹 Nitpick comments (1)
app/src/main/java/app/gamenative/MainActivity.kt (1)
333-349: Minor style inconsistencies between GOGService and EpicService handling.
- Line 335 is missing a space after
if(compare with line 343)- GOGService doesn't log when kept running due to active operations, while EpicService does
Consider aligning these for consistency:
♻️ Suggested fix
// Stop GOGService if running and no downloads in progress if (GOGService.isRunning && !isChangingConfigurations) { - if(!GOGService.hasActiveOperations()) { + if (!GOGService.hasActiveOperations()) { Timber.i("Stopping GOG Service - no active operations") GOGService.stop() + } else { + Timber.d("GOGService kept running - has active operations") } }
| private fun performUninstall(context: Context, libraryItem: LibraryItem) { | ||
| Timber.tag(TAG).i("Uninstalling Epic game: ${libraryItem.appId}") | ||
| CoroutineScope(Dispatchers.IO).launch { | ||
| try { | ||
| val result = EpicService.deleteGame(context, libraryItem.gameId) | ||
|
|
||
| if (result.isSuccess) { | ||
| Timber.tag(TAG).i("Epic game uninstalled successfully: ${libraryItem.appId}") | ||
| } else { | ||
| Timber.e("Failed to uninstall Epic game: ${libraryItem.appId} - ${result.exceptionOrNull()?.message}") | ||
| withContext(Dispatchers.Main) { | ||
| android.widget.Toast.makeText( | ||
| context, | ||
| "Uninstall failed: ${result.exceptionOrNull()?.message}", | ||
| android.widget.Toast.LENGTH_SHORT, | ||
| ).show() | ||
| } | ||
| } | ||
| } catch (e: Exception) { | ||
| Timber.e(e, "Error uninstalling Epic game") | ||
| withContext(Dispatchers.Main) { | ||
| android.widget.Toast.makeText( | ||
| context, | ||
| "Uninstall error: ${e.message}", | ||
| android.widget.Toast.LENGTH_SHORT, | ||
| ).show() | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Unmanaged CoroutineScope in performUninstall.
Similar to the issue previously flagged for performDownload, this function creates an orphan coroutine not tied to any lifecycle. If the composable is destroyed during the uninstall operation, the scope won't be cancelled properly.
Consider accepting a lifecycle-aware CoroutineScope parameter like performDownload now does.
🛠️ Suggested fix
- private fun performUninstall(context: Context, libraryItem: LibraryItem) {
+ private fun performUninstall(scope: CoroutineScope, context: Context, libraryItem: LibraryItem) {
Timber.tag(TAG).i("Uninstalling Epic game: ${libraryItem.appId}")
- CoroutineScope(Dispatchers.IO).launch {
+ scope.launch(Dispatchers.IO) {
try {And update the call site at line 823:
- performUninstall(context, libraryItem)
+ performUninstall(scope, context, libraryItem)🤖 Prompt for AI Agents
In
`@app/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.kt`
around lines 456 - 485, performUninstall currently creates an unmanaged
CoroutineScope(Dispatchers.IO) that can outlive the composable; change the
signature to accept a lifecycle-aware CoroutineScope (e.g., fun
performUninstall(context: Context, libraryItem: LibraryItem, scope:
CoroutineScope)) and replace CoroutineScope(Dispatchers.IO).launch { ... } with
scope.launch(Dispatchers.IO) { ... } (or use scope.launch {
withContext(Dispatchers.IO) { ... } }) to ensure the job is tied to the caller's
lifecycle—mirroring the approach used in performDownload—and update the call
site(s) that invoke performUninstall accordingly.
app/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@app/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.kt`:
- Around line 476-489: Replace the two hardcoded Toast messages in EpicAppScreen
(the uninstall failure block using result.exceptionOrNull()?.message and the
catch block using e.message) with localized string resources: add
epic_uninstall_failed and epic_uninstall_error to strings.xml (both should
accept a single string placeholder for the error message) and call
context.getString(R.string.epic_uninstall_failed,
result.exceptionOrNull()?.message ?: "") and
context.getString(R.string.epic_uninstall_error, e.message ?: "") where the
current literal strings are used in android.widget.Toast.makeText so the
displayed messages are localized.
- Around line 151-166: The code performs IO work on Dispatchers.IO and then
mutates Compose state (epicGame) on that dispatcher, which is unsafe; keep the
fetch and EpicService.updateEpicGame(...) on Dispatchers.IO but move the Compose
state mutation into the Main dispatcher by wrapping the assignment in
withContext(kotlinx.coroutines.Dispatchers.Main) so that after creating
updatedGame (from EpicService.fetchManifestSizes) and persisting it via
EpicService.updateEpicGame(updatedGame) you call withContext(Dispatchers.Main) {
epicGame = updatedGame } to update UI state safely.
♻️ Duplicate comments (3)
app/src/main/res/values/strings.xml (1)
1017-1017: Align Epic logout failure copy with existing phrasing.
"Failure to logout of Epic Games" reads inconsistent with similar messages. Consider reusing the “Failed to logout …” phrasing.✏️ Suggested tweak
- <string name="epic_logout_failed">Failure to logout of Epic Games, please try again</string> + <string name="epic_logout_failed">Failed to logout from Epic Games, please try again</string>app/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.kt (2)
465-493: Tie uninstall work to a lifecycle-aware scope.
performUninstallcreates an unmanagedCoroutineScope, which can outlive the UI. Consider passing in the composable’s scope (already available inAdditionalDialogs).🔧 Suggested adjustment
- private fun performUninstall(context: Context, libraryItem: LibraryItem) { + private fun performUninstall(scope: CoroutineScope, context: Context, libraryItem: LibraryItem) { Timber.tag(TAG).i("Uninstalling Epic game: ${libraryItem.appId}") - CoroutineScope(Dispatchers.IO).launch { + scope.launch(Dispatchers.IO) { try { val result = EpicService.deleteGame(context, libraryItem.gameId) @@ - performUninstall(context, libraryItem) + performUninstall(scope, context, libraryItem)Also applies to: 849-853
553-589: Use lifecycle scope and string resources for cloud-sync toasts.
A newCoroutineScopeis created per click and the toast text is hardcoded. PreferrememberCoroutineScope()andstrings.xmlentries.🛠️ Suggested pattern
- val options = mutableListOf<AppMenuOption>() + val options = mutableListOf<AppMenuOption>() + val scope = rememberCoroutineScope() @@ - val scope = CoroutineScope(Dispatchers.Main + SupervisorJob()) scope.launch { try { Toast.makeText( context, - "Starting cloud save sync...", + context.getString(R.string.epic_cloud_sync_starting), Toast.LENGTH_SHORT, ).show() @@ Toast.makeText( context, - if (result) "Cloud saves synced successfully" else "Cloud save sync failed", + context.getString( + if (result) R.string.epic_cloud_sync_success else R.string.epic_cloud_sync_failed + ), Toast.LENGTH_LONG, ).show() } catch (e: Exception) { Timber.tag(TAG).e(e, "[Cloud Saves] Sync failed") Toast.makeText( context, - "Cloud save sync error: ${e.message}", + context.getString(R.string.epic_cloud_sync_error, e.message ?: ""), Toast.LENGTH_LONG, ).show() }Add
epic_cloud_sync_starting,epic_cloud_sync_success,epic_cloud_sync_failed, andepic_cloud_sync_errortostrings.xml.
🧹 Nitpick comments (1)
app/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.kt (1)
786-805: Remove duplicateCANCEL_APP_DOWNLOADbranch.
Thewhenhas the same enum case twice; the second branch is dead and can hide intended handling for another dialog type.🧹 Cleanup
- app.gamenative.ui.enums.DialogType.CANCEL_APP_DOWNLOAD -> { - { - Timber.tag(TAG).i("Cancelling Epic download for: $gameId") - val downloadInfo = EpicService.getDownloadInfo(gameId) - downloadInfo?.cancel() - EpicService.cleanupDownload(gameId) - BaseAppScreen.hideInstallDialog(appId) - app.gamenative.PluviaApp.events.emit(app.gamenative.events.AndroidEvent.LibraryInstallStatusChanged(gameId)) - } - } - else -> null
| kotlinx.coroutines.withContext(kotlinx.coroutines.Dispatchers.IO) { | ||
| try { | ||
| val sizes = EpicService.fetchManifestSizes(context, game.id) | ||
| if (sizes.installSize > 0L || sizes.downloadSize > 0L) { | ||
| Timber.tag("Epic").i( | ||
| "Fetched sizes for ${game.title}: install=${sizes.installSize} download=${sizes.downloadSize}", | ||
| ) | ||
| // Update database with fetched size | ||
| val updatedGame = game.copy( | ||
| installSize = sizes.installSize, | ||
| downloadSize = sizes.downloadSize, | ||
| ) | ||
| EpicService.updateEpicGame(updatedGame) | ||
| // Update state directly to show updated size | ||
| epicGame = updatedGame | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and inspect the EpicAppScreen.kt file
fd -t f "EpicAppScreen.kt" --exec cat -n {} \; | head -250Repository: utkarshdalal/GameNative
Length of output: 13349
🏁 Script executed:
# Search for other state updates inside withContext(Dispatchers.IO) or similar IO contexts
rg -A 3 "withContext.*Dispatchers\.IO" --type kotlin | head -100Repository: utkarshdalal/GameNative
Length of output: 9236
🏁 Script executed:
# Also check for patterns of state updates after IO operations
rg "withContext\(" --type kotlin -A 5 | grep -E "(withContext|= \w+|state =)" | head -80Repository: utkarshdalal/GameNative
Length of output: 9401
🏁 Script executed:
# Check if there are other instances in EpicAppScreen.kt where state is updated
grep -n "epicGame\s*=" app/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.ktRepository: utkarshdalal/GameNative
Length of output: 343
Update Compose state on the Main dispatcher.
epicGame is mutated inside Dispatchers.IO, which violates Compose snapshot thread-safety. State mutations must occur on the Main dispatcher. Wrap the state update with withContext(Dispatchers.Main).
Suggested fix
- if (sizes.installSize > 0L || sizes.downloadSize > 0L) {
+ if (sizes.installSize > 0L || sizes.downloadSize > 0L) {
Timber.tag("Epic").i(
"Fetched sizes for ${game.title}: install=${sizes.installSize} download=${sizes.downloadSize}",
)
// Update database with fetched size
val updatedGame = game.copy(
installSize = sizes.installSize,
downloadSize = sizes.downloadSize,
)
EpicService.updateEpicGame(updatedGame)
- // Update state directly to show updated size
- epicGame = updatedGame
+ // Update state on Main dispatcher
+ withContext(Dispatchers.Main) {
+ epicGame = updatedGame
+ }
}🤖 Prompt for AI Agents
In
`@app/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.kt`
around lines 151 - 166, The code performs IO work on Dispatchers.IO and then
mutates Compose state (epicGame) on that dispatcher, which is unsafe; keep the
fetch and EpicService.updateEpicGame(...) on Dispatchers.IO but move the Compose
state mutation into the Main dispatcher by wrapping the assignment in
withContext(kotlinx.coroutines.Dispatchers.Main) so that after creating
updatedGame (from EpicService.fetchManifestSizes) and persisting it via
EpicService.updateEpicGame(updatedGame) you call withContext(Dispatchers.Main) {
epicGame = updatedGame } to update UI state safely.
app/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.kt
Show resolved
Hide resolved
|
Note: Hold off for now, found a critical bug with some games having incredibly large chunks (1.5GB+) which are causing OOM issues. I'll need to fix it by using streaming properly with the decompression and compressing (and validation with SHA) |
Also added fix to use IO thread for getting accountId. Now correctly updates UI on downloads finishing again.
|
Update: This is now fixed, downloaded 8 different games with varying sizes and all complete succesfully. Will move onto final end-to-end testing and ensure there are no outlying bugs left. |
There was a problem hiding this comment.
3 issues found across 2 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.kt">
<violation number="1" location="app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.kt:712">
P2: Inconsistent logging: Use `Timber.tag(LOG_TAG).d()` instead of `Log.d()` to match the codebase's logging pattern.</violation>
<violation number="2" location="app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.kt:730">
P2: Inconsistent logging: Use `Timber.tag(LOG_TAG).w()` instead of `Log.w()` to match the codebase's logging pattern.</violation>
<violation number="3" location="app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.kt:752">
P2: Incorrect Timber API usage: `Timber.d(LOG_TAG, message)` treats LOG_TAG as a format string, not a tag. Use `Timber.tag(LOG_TAG).d(message)` to match the rest of the codebase.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.kt
Outdated
Show resolved
Hide resolved
| buffer.position(46) // Jump to where uncompressedSize should be (58-12=46) | ||
| val uncompressedSize = buffer.int // offset 46-49 (file offset 58-61) | ||
|
|
||
| Log.d(LOG_TAG, "Chunk header: magic=0x${magic.toString(16)}, headerSize=$headerSize, compressedSize=$compressedSize, uncompressedSize=$uncompressedSize, storedAs=0x${storedAs.toString(16)}, isCompressed=$isCompressed, expectedSize=$expectedSize") |
There was a problem hiding this comment.
P2: Inconsistent logging: Use Timber.tag(LOG_TAG).d() instead of Log.d() to match the codebase's logging pattern.
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/service/epic/EpicDownloadManager.kt, line 712:
<comment>Inconsistent logging: Use `Timber.tag(LOG_TAG).d()` instead of `Log.d()` to match the codebase's logging pattern.</comment>
<file context>
@@ -646,6 +653,230 @@ class EpicDownloadManager @Inject constructor(
+ buffer.position(46) // Jump to where uncompressedSize should be (58-12=46)
+ val uncompressedSize = buffer.int // offset 46-49 (file offset 58-61)
+
+ Log.d(LOG_TAG, "Chunk header: magic=0x${magic.toString(16)}, headerSize=$headerSize, compressedSize=$compressedSize, uncompressedSize=$uncompressedSize, storedAs=0x${storedAs.toString(16)}, isCompressed=$isCompressed, expectedSize=$expectedSize")
+
+ outputFile.outputStream().buffered().use { output ->
</file context>
| Log.d(LOG_TAG, "Chunk header: magic=0x${magic.toString(16)}, headerSize=$headerSize, compressedSize=$compressedSize, uncompressedSize=$uncompressedSize, storedAs=0x${storedAs.toString(16)}, isCompressed=$isCompressed, expectedSize=$expectedSize") | |
| Timber.tag(LOG_TAG).d("Chunk header: magic=0x${magic.toString(16)}, headerSize=$headerSize, compressedSize=$compressedSize, uncompressedSize=$uncompressedSize, storedAs=0x${storedAs.toString(16)}, isCompressed=$isCompressed, expectedSize=$expectedSize") |
app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.kt`:
- Around line 705-710: The code currently reads uncompressedSize from a
hard-coded offset (buffer.position(46)) which breaks for headers larger than 62
bytes; instead compute the last-4-bytes position from the actual header size and
read from there — e.g. after reading storedAs (using buffer.position(4 + 16 + 8)
and val storedAs/... as before), set buffer.position(buffer.limit() - 4) (or
compute pos = remainingSize - 4 if you have remainingSize) and then read val
uncompressedSize = buffer.int so the uncompressed size is always taken from the
header's final 4 bytes.
- Around line 507-512: The current verification reads the entire decompressed
file into memory via verifyChunkHashBytes(decompressedFile.readBytes(),
chunk.shaHash) which can OOM for large chunks; replace this with a streaming
file-hash verifier (similar to decompressStreamingChunkToFile) by adding a
helper like verifyChunkHashFile(file: File, expectedHash: ByteArray) that
computes SHA-1 by reading the file in a loop (e.g., 64KB buffer), compares
digests, logs mismatches with hex strings via Timber.tag("Epic").e(...) and
returns boolean, then call that helper in the existing block (replace the
verifyChunkHashBytes(...) call) so successful verification still triggers
downloadInfo.updateBytesDownloaded(chunk.fileSize) and returning
Result.success(decompressedFile).
♻️ Duplicate comments (5)
app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.kt (3)
118-121: Guard against an empty CDN list after Cloudflare filtering.Lines 118-121 can yield an empty list when all URLs are Cloudflare, which makes every chunk fail without a clear fallback. Consider falling back to the unfiltered list (or fail fast with a clear error).
🛠️ Proposed fix
- val cdnUrls = manifestData.cdnUrls.filter { !it.baseUrl.startsWith("https://cloudflare.epicgamescdn.com") } + val filteredCdnUrls = + manifestData.cdnUrls.filter { !it.baseUrl.startsWith("https://cloudflare.epicgamescdn.com") } + val cdnUrls = if (filteredCdnUrls.isNotEmpty()) filteredCdnUrls else manifestData.cdnUrls
337-364: FinalizeDownloadInfoeven when returning early.Lines 110-131 and 213-231 return from inside the
try, so the success/catch cleanup at Lines 337-343 doesn’t run;downloadInfocan remain active and leave the UI stuck. Move deactivation intofinallyso it always runs.🧹 Proposed fix
} finally { + if (downloadInfo.isActive()) { + downloadInfo.setActive(false) + downloadInfo.emitProgressChange() + } // Always emit download stopped event val gameId = game.id ?: 0 app.gamenative.PluviaApp.events.emitJava( app.gamenative.events.AndroidEvent.DownloadStatusChanged(gameId, false), ) }
923-934: Handle partial skips and premature EOF when assembling files.Line 925 ignores the return from
InputStream.skip, which may skip fewer bytes, and Line 934 breaks on EOF without error—this can silently truncate assembled files. Loop until the desired offset is skipped and treat unexpected EOF as a failure.🧩 Proposed fix
// Read chunk data at specified offset chunkFile.inputStream().use { input -> - input.skip(chunkPart.offset.toLong()) + var toSkip = chunkPart.offset.toLong() + while (toSkip > 0) { + val skipped = input.skip(toSkip) + if (skipped <= 0) { + throw Exception("Failed to skip to offset ${chunkPart.offset} for ${chunkPart.guidStr}") + } + toSkip -= skipped + } val buffer = ByteArray(65536) // Increased to 64KB for better I/O performance var remaining = chunkPart.size.toLong() while (remaining > 0) { val toRead = minOf(remaining, buffer.size.toLong()).toInt() val bytesRead = input.read(buffer, 0, toRead) - if (bytesRead == -1) break + if (bytesRead == -1) { + throw Exception("Unexpected EOF assembling ${fileManifest.filename} from ${chunkPart.guidStr}") + } output.write(buffer, 0, bytesRead) remaining -= bytesRead } }app/src/main/java/app/gamenative/service/epic/EpicService.kt (2)
330-335: Avoid plainrunBlockingindownloadGame.Line 333 uses
runBlockingwithout a dispatcher; if this is called from the main thread, the DB read runs on the caller thread and can stall the UI. If you keep a synchronous API, run the blocking call onDispatchers.IO(or make the function suspend). Based on learnings, keeping sync APIs is fine, but it shouldn’t run I/O on the caller thread.🛠️ Proposed fix
- val game = runBlocking { instance.epicManager.getGameById(appId) } + val game = runBlocking(Dispatchers.IO) { instance.epicManager.getGameById(appId) }
337-351: MakeactiveDownloadsinsertion atomic.Lines 337-351 use a check-then-put pattern that can race and start duplicate downloads under concurrent calls. Use
putIfAbsent(orcomputeIfAbsent) to ensure single ownership.🔒 Proposed fix
- // Check if already downloading - if (instance.activeDownloads.containsKey(appId)) { - Timber.tag("Epic").w("Download already in progress for $appId") - return Result.success(instance.activeDownloads[appId]!!) - } - // Create DownloadInfo before launching coroutine to avoid race condition val downloadInfo = DownloadInfo( jobCount = 1, gameId = appId, downloadingAppIds = CopyOnWriteArrayList<Int>(), ) - instance.activeDownloads[appId] = downloadInfo - downloadInfo.setActive(true) + val existing = instance.activeDownloads.putIfAbsent(appId, downloadInfo) + if (existing != null) { + Timber.tag("Epic").w("Download already in progress for $appId") + return Result.success(existing) + } + downloadInfo.setActive(true)
| if (decompressedFile.exists() && decompressedFile.length() == chunk.windowSize.toLong()) { | ||
| // Quick verification - only verify if size matches | ||
| if (verifyChunkHashBytes(decompressedFile.readBytes(), chunk.shaHash)) { | ||
| Timber.tag("Epic").d("Chunk ${chunk.guidStr} already exists and verified, skipping") | ||
| downloadInfo.updateBytesDownloaded(chunk.fileSize) | ||
| return@withContext Result.success(decompressedFile) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "EpicDownloadManager.kt" | head -20Repository: utkarshdalal/GameNative
Length of output: 137
🏁 Script executed:
wc -l ./app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.ktRepository: utkarshdalal/GameNative
Length of output: 142
🏁 Script executed:
sed -n '500,520p' ./app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.ktRepository: utkarshdalal/GameNative
Length of output: 1196
🏁 Script executed:
sed -n '1,50p' ./app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.ktRepository: utkarshdalal/GameNative
Length of output: 1712
🏁 Script executed:
rg -n "verifyChunkHashBytes" ./app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.kt -A 10Repository: utkarshdalal/GameNative
Length of output: 1361
🏁 Script executed:
rg -n "chunk\." ./app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.kt | grep -i "size\|window" | head -20Repository: utkarshdalal/GameNative
Length of output: 386
🏁 Script executed:
rg -n "windowSize\|fileSize" ./app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.kt | head -30Repository: utkarshdalal/GameNative
Length of output: 49
🏁 Script executed:
find . -type f -name "*.kt" | xargs rg -l "windowSize|fileSize" | head -10Repository: utkarshdalal/GameNative
Length of output: 737
🏁 Script executed:
rg -n "class.*Chunk\|data class.*Chunk" ./app/src/main/java/app/gamenative/service/epic/ -A 15 | head -50Repository: utkarshdalal/GameNative
Length of output: 49
🏁 Script executed:
find . -type f -name "EpicManifest.kt" -exec cat -n {} \;Repository: utkarshdalal/GameNative
Length of output: 36470
🏁 Script executed:
sed -n '880,920p' ./app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.ktRepository: utkarshdalal/GameNative
Length of output: 1651
🏁 Script executed:
rg -n "readBytes()" ./app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.ktRepository: utkarshdalal/GameNative
Length of output: 159
🏁 Script executed:
rg -n "decompressStreamingChunkToFile|verifyChunkHashFile" ./app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.ktRepository: utkarshdalal/GameNative
Length of output: 256
🏁 Script executed:
sed -n '660,720p' ./app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.ktRepository: utkarshdalal/GameNative
Length of output: 3257
🏁 Script executed:
sed -n '720,800p' ./app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.ktRepository: utkarshdalal/GameNative
Length of output: 4445
🏁 Script executed:
rg -n "windowSize\|fileSize" ./app/src/main/java/app/gamenative/service/epic/manifest/EpicManifest.kt | head -20Repository: utkarshdalal/GameNative
Length of output: 49
Use streaming hash verification instead of readBytes() for large chunks.
Line 509 loads the entire decompressed chunk into memory before verification. For large chunks (GB-scale), this can cause OOM. The codebase already uses streaming hash verification in decompressStreamingChunkToFile (lines 660+); apply the same pattern here.
Proposed fix
- if (verifyChunkHashBytes(decompressedFile.readBytes(), chunk.shaHash)) {
+ if (verifyChunkHashFile(decompressedFile, chunk.shaHash)) {
Timber.tag("Epic").d("Chunk ${chunk.guidStr} already exists and verified, skipping")
downloadInfo.updateBytesDownloaded(chunk.fileSize)
return@withContext Result.success(decompressedFile)
} else {Add this helper method:
private fun verifyChunkHashFile(file: File, expectedHash: ByteArray): Boolean {
return try {
val digest = MessageDigest.getInstance("SHA-1")
file.inputStream().use { input ->
val buffer = ByteArray(65536)
var read = input.read(buffer)
while (read != -1) {
digest.update(buffer, 0, read)
read = input.read(buffer)
}
}
val actualHash = digest.digest()
if (!actualHash.contentEquals(expectedHash)) {
val expectedHex = expectedHash.joinToString("") { "%02x".format(it) }
val actualHex = actualHash.joinToString("") { "%02x".format(it) }
Timber.tag("Epic").e("Hash mismatch: expected $expectedHex, got $actualHex")
false
} else {
true
}
} catch (e: Exception) {
Timber.tag("Epic").e(e, "Hash verification failed")
false
}
}🤖 Prompt for AI Agents
In `@app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.kt` around
lines 507 - 512, The current verification reads the entire decompressed file
into memory via verifyChunkHashBytes(decompressedFile.readBytes(),
chunk.shaHash) which can OOM for large chunks; replace this with a streaming
file-hash verifier (similar to decompressStreamingChunkToFile) by adding a
helper like verifyChunkHashFile(file: File, expectedHash: ByteArray) that
computes SHA-1 by reading the file in a loop (e.g., 64KB buffer), compares
digests, logs mismatches with hex strings via Timber.tag("Epic").e(...) and
returns boolean, then call that helper in the existing block (replace the
verifyChunkHashBytes(...) call) so successful verification still triggers
downloadInfo.updateBytesDownloaded(chunk.fileSize) and returning
Result.success(decompressedFile).
| val compressedSize = buffer.int // offset 0-3 | ||
| buffer.position(4 + 16 + 8) // Skip GUID (16) and hash (8), now at offset 28 | ||
| val storedAs = buffer.get().toInt() and 0xFF // offset 28 | ||
| val isCompressed = (storedAs and 0x1) == 0x1 | ||
| buffer.position(46) // Jump to where uncompressedSize should be (58-12=46) | ||
| val uncompressedSize = buffer.int // offset 46-49 (file offset 58-61) |
There was a problem hiding this comment.
Uncompressed size offset should depend on header size.
Line 709 always seeks to offset 46 (valid for 62-byte headers). For 66-byte headers, the uncompressed size is in the last 4 bytes, so this can misread sizes and break decompression. Use remainingSize - 4 (last 4 bytes) instead.
🛠️ Proposed fix
- buffer.position(46) // Jump to where uncompressedSize should be (58-12=46)
- val uncompressedSize = buffer.int // offset 46-49 (file offset 58-61)
+ val uncompressedOffset = remainingSize - 4
+ buffer.position(uncompressedOffset)
+ val uncompressedSize = buffer.int🤖 Prompt for AI Agents
In `@app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.kt` around
lines 705 - 710, The code currently reads uncompressedSize from a hard-coded
offset (buffer.position(46)) which breaks for headers larger than 62 bytes;
instead compute the last-4-bytes position from the actual header size and read
from there — e.g. after reading storedAs (using buffer.position(4 + 16 + 8) and
val storedAs/... as before), set buffer.position(buffer.limit() - 4) (or compute
pos = remainingSize - 4 if you have remainingSize) and then read val
uncompressedSize = buffer.int so the uncompressed size is always taken from the
header's final 4 bytes.
2 participants
Overview
This PR is to add Epic Games Integration
Features
Architecture
Epic
EpicApiClient - All Auth responsibilities for API Requests (Currently authClient, but will refactor later)
EpicAuthManager - Manages Auth responsibilities
EpicManager - Library Management
EpicService - Interface for all Epic operations
EpicConstants - Constants for Epic-related functionality such as IDs, names etc.
EpicDownloadManager - Handles Downloading Logic
EpicCloudSavesManager - Handles Cloud Saves
Manifest/
EpicManifest - Handles overall Logic for Manifests including Binary Manifest Parsing
JsonManifestParser - Handles parsing of JSON manifests
ManifestUtls - Common utils required for parsing both JSON and Binary manifests
Other Files:
Testing
I have created tests for the most complex areas, mostly manifest parsing.
Summary by cubic
Add Epic Games Store support: login, library sync, installs/downloads, launching, and cloud saves with new UI and database pieces. This brings Epic games into the library with proper filtering, icons, and service wiring.
New Features
Migration
Written for commit 3590554. Summary will update on new commits.
Summary by CodeRabbit
New Features
UI
Infrastructure
Database
Tests
Strings
✏️ Tip: You can customize this high-level summary in your review settings.