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.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughThis PR introduces comprehensive Epic Games Store integration to the app, including OAuth authentication, game library management with database persistence, manifest-based game installation, cloud saves synchronization with conflict resolution, and full UI support for Epic game discovery, installation, and launch via Wine containers. Changes
Sequence DiagramssequenceDiagram
participant User
participant UI as SettingsGroupInterface
participant Auth as EpicAuthManager
participant Client as EpicAuthClient
participant Store as File Storage
participant Service as EpicService
User->>UI: Enters OAuth code
UI->>Auth: authenticateWithCode(code)
Auth->>Auth: extractCodeFromInput()
Auth->>Client: authenticateWithCode(code)
Client->>Client: POST to Epic token endpoint
Client-->>Auth: Result<EpicAuthResponse>
Auth->>Store: saveCredentials(epicCredentials.json)
Auth-->>UI: Result<EpicCredentials>
UI->>Service: initialize()
Service->>Service: Background sync library
Service-->>User: Login success
sequenceDiagram
participant User
participant UI as EpicAppScreen
participant Service as EpicService
participant Manager as EpicDownloadManager
participant Manifest as ManifestUtils
participant HTTP as OkHttp CDN Client
participant Disk as File System
User->>UI: Click Download Game
UI->>Service: downloadGame(appName, installPath)
Service->>Manager: fetchManifestFromEpic()
Manager->>HTTP: GET manifest binary
HTTP-->>Manager: manifestBytes
Manager->>Manifest: detect() / parse manifest
Manifest-->>Manager: EpicManifest with chunks
Manager->>Manager: Download chunks in parallel batches
Manager->>HTTP: GET chunk from CDN (retry on fail)
HTTP-->>Manager: chunkData
Manager->>Disk: decompress, verify hash
Manager->>Disk: assemble files from chunks
Disk-->>Manager: installation complete
Manager-->>Service: Result<Unit>
Service->>Service: Update DB, emit event
Service-->>UI: Progress updates
UI-->>User: Install complete
sequenceDiagram
participant User
participant UI as EpicAppScreen
participant Service as EpicService
participant CloudMgr as EpicCloudSavesManager
participant Local as Local Save Folder
participant API as Epic Cloud API
participant HTTP as OkHttp Client
User->>UI: Click Cloud Saves
UI->>CloudMgr: syncCloudSaves(appId)
CloudMgr->>Local: List local save files
CloudMgr->>API: listCloudSaves()
HTTP-->>CloudMgr: CloudSaveFiles list
CloudMgr->>CloudMgr: determineSyncAction(local, cloud)
alt Needs Download
CloudMgr->>API: downloadFile() + manifest
HTTP-->>CloudMgr: manifestBytes + chunks
CloudMgr->>CloudMgr: decompressChunks()
CloudMgr->>Local: reconstruct files
else Needs Upload
CloudMgr->>Local: read saves, chunk into 1MB
CloudMgr->>CloudMgr: packageSaveFiles()
CloudMgr->>API: requestWriteLinks()
HTTP-->>CloudMgr: writeLink mappings
CloudMgr->>HTTP: upload chunks
CloudMgr->>HTTP: upload manifest last
else Conflict
CloudMgr->>CloudMgr: resolveConflict(timestamps)
end
CloudMgr->>CloudMgr: setSyncTimestamp()
CloudMgr-->>UI: Boolean result
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 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. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
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/ui/screen/library/components/LibraryAppItem.kt (1)
339-346: Align GOG install checks inAppItemandGameInfoBlockYou're using different identifiers for GOG install checks:
- In
AppItem(grid hero/capsule install flag):GameSource.GOG -> GOGService.isGameInstalled(appInfo.gameId.toString()) GameSource.EPIC -> EpicService.isGameInstalled(appInfo.appId.removePrefix("EPIC_"))
- In
GameInfoBlock(status text for GOG/Epic):GameSource.GOG -> GOGService.isGameInstalled(appInfo.appId) GameSource.EPIC -> EpicService.isGameInstalled(appInfo.appId.removePrefix("EPIC_"))Everywhere else in this file, GOG uses
gameId(orgameId.toString()) withGOGService.isGameInstalled, while Epic consistently strips"EPIC_"fromappInfo.appId.Using
appInfo.appIdfor GOG will never match the expected ID and cause GOG titles to show as "not installed" inGameInfoBlock, even when the install marker exists in the grid view.Change the GOG branch in
GameInfoBlockto:GameSource.GOG -> GOGService.isGameInstalled(appInfo.gameId.toString())so install status is computed consistently across the UI.
Also applies to: 352-359
🤖 Fix all issues with AI Agents
In
@app/src/androidTest/java/app/gamenative/service/epic/manifest/test/ManifestParseTest.kt:
- Around line 1-39: The file defines a top-level CLI main()
(ManifestParseTest.main) placed under androidTest which never runs under
AndroidJUnitRunner; remove or relocate this manual tool: either delete
ManifestParseTest.kt from the instrumentation tests, move the main() utility
into a separate tools/util module or docs, or convert its logic into a proper
JUnit test method inside ManifestParseValidationTest (calling
ManifestUtils.loadFromFile and ManifestTestSerializer.createManifestSummary) so
it runs as part of tests; ensure any references to ManifestUtils and
ManifestTestSerializer are updated/imported in the chosen location.
In
@app/src/androidTest/java/app/gamenative/service/epic/manifest/test/ManifestTestSerializer.kt:
- Around line 155-158: The bytesToHex function misformats bytes >= 128 because
signed bytes are passed directly to String.format, producing negative ints and
multi-byte hex; update bytesToHex (the function named bytesToHex) to convert
each Byte to an unsigned value before formatting (e.g., call toInt() and mask
with 0xFF) so each byte always formats as a two-digit hex value, preserving
hashes like shaHash/hashMd5/hashSha256.
In @app/src/main/java/app/gamenative/data/EpicGame.kt:
- Around line 180-186: EpicCredentials currently holds sensitive tokens in
plaintext; before persisting instances of EpicCredentials (wherever you
serialize/save them) encrypt accessToken and refreshToken using the existing
Crypto.encrypt() and decrypt on load with Crypto.decrypt(), or replace the
storage with EncryptedSharedPreferences; update the code that creates/reads
EpicCredentials (the save/load methods that produce the credentials.json) to
store encrypted strings for accessToken/refreshToken while keeping
accountId/displayName as-is and ensure serialization/deserialization maps to the
encrypted fields.
In @app/src/main/java/app/gamenative/data/LibraryItem.kt:
- Around line 29-30: The capsuleHash property in LibraryItem is declared
nullable (String?) but given a default "" which is inconsistent; decide whether
null should represent "no value" or use empty-string sentinel and make the type
consistent: either change capsuleHash to non-nullable String with default "" to
match heroHash, or change its default to null to keep it nullable. Update the
property declaration (capsuleHash) accordingly and adjust any callers that
assume non-null to handle the chosen representation.
In @app/src/main/java/app/gamenative/db/converters/EpicConverter.kt:
- Around line 19-24: The toStringList function in EpicConverter calls
Json.decodeFromString without handling parse errors; wrap the decoding in a
try/catch around Json.decodeFromString<List<String>>(value) inside toStringList,
catch serialization/JSON exceptions (e.g.,
SerializationException/JsonDecodingException/Exception), optionally log the
error with context (function name and the raw value), and return a safe fallback
such as emptyList() when parsing fails so Room does not crash on malformed JSON.
In @app/src/main/java/app/gamenative/service/epic/EpicAuthClient.kt:
- Around line 47-55: The current authenticateWithCode (and the similar
authenticateWithRefreshToken) builds form bodies via string interpolation which
fails to URL-encode reserved characters; replace the raw string formBody
creation with an OkHttp FormBody built via FormBody.Builder().add("grant_type",
"authorization_code").add("code", authorizationCode).add("token_type",
"eg1").build() (and for refresh use "grant_type"="refresh_token" and
"refresh_token"=refreshToken), then use that FormBody as the request body so
parameters are correctly URL-encoded; keep the existing Credentials.basic(...)
header logic unchanged.
In @app/src/main/java/app/gamenative/service/epic/EpicAuthManager.kt:
- Around line 118-163: getStoredCredentials currently returns the old
credentials when EpicAuthClient.refreshAccessToken fails, which can hand callers
an expired token; update the error path in getStoredCredentials so that when
refreshResult.isFailure you log the error and return Result.failure(...)
(include the refreshResult exception/message) instead of
Result.success(credentials), so callers receive a clear failure and can prompt
re‑auth; reference getStoredCredentials and EpicAuthClient.refreshAccessToken
when making this change and ensure any logging uses Timber.e with the refresh
error details.
In @app/src/main/java/app/gamenative/service/epic/EpicCloudSavesManager.kt:
- Around line 1106-1111: serializeManifest currently returns manifest.data which
is emptied by BinaryManifest.parseContents(), so uploads get zero bytes; update
serializeManifest (in EpicCloudSavesManager) to rebuild the binary manifest from
the parsed EpicManifest fields (e.g., meta, chunkDataList, fileManifestList,
customFields) into the expected binary format, or change the parsing flow to
preserve raw bytes (store original raw bytes before
BinaryManifest.parseContents() is called). Locate serializeManifest and
EpicManifest/BinaryManifest.parseContents to implement reconstruction of the
binary layout or to capture and return the preserved raw data instead of
manifest.data.
- Around line 211-217: The string comparison of ISO timestamps in
EpicCloudSavesManager (the branch returning SyncAction.DOWNLOAD when
`cloudTimestamp >= (lastSync ?: "")`) is unsafe; parse the timestamp strings to
temporal objects (e.g., java.time.Instant via Instant.parse or convert to epoch
millis) and compare Instants/longs instead, and treat a null `lastSync`
explicitly (e.g., compare cloudInstant to Instant.EPOCH or handle as "no prior
sync") rather than falling back to an empty string; update the logic in the
method that decides SyncAction (the block using `cloudTimestamp` and `lastSync`)
to perform those parsed temporal comparisons and return SyncAction.DOWNLOAD/NONE
accordingly.
- Around line 992-997: The current padding logic in EpicCloudSavesManager.kt
(the paddedData assignment) pads every chunk to 1MB which bloats uploads; remove
the forced padding by using the original data directly (replace the paddedData
calculation in the function where paddedData is set with paddedData = data or
just use data wherever paddedData is referenced) and ensure any downstream
upload/chunking logic in the same class (e.g., methods handling chunk boundaries
and upload requests) accepts variable-size chunks instead of assuming 1MB fixed
size.
- Around line 227-234: The parseTimestamp function swallows parsing exceptions;
modify its catch block to log the caught exception and the offending timestamp
before returning 0L so issues can be diagnosed: in parseTimestamp, replace the
empty catch with a log call (e.g., logger.warn/error("Failed to parse timestamp:
$timestamp", e) or use the project logger instance) and then return 0L, adding
an import or logger instance if needed; keep Instant.parse usage and the 0L
fallback behavior.
In @app/src/main/java/app/gamenative/service/epic/EpicConstants.kt:
- Around line 77-84: INTERNAL_BASE_PATH and the computed property
internalEpicGamesPath assume a fixed package path which breaks for other build
variants; change the API so the hardcoded INTERNAL_BASE_PATH is removed and
internalEpicGamesPath is made to accept or compute the base from the app Context
(or be provided via a constructor/parameter). Specifically, replace the constant
INTERNAL_BASE_PATH and the val internalEpicGamesPath with a function or property
that takes a Context (or a base File/string) and returns File(context.filesDir,
"Epic/games").absolutePath (or uses the injected base path), updating callers to
pass Context.filesDir or the injected base so path resolution no longer depends
on the hardcoded package name.
In @app/src/main/java/app/gamenative/service/epic/EpicManager.kt:
- Around line 254-277: The batch loop mixes collections and drops the processed
count: replace the boundary check using gamesList.size with newGamesList.size
(i.e., use newGamesList.size - 1 or compare to newGamesList.lastIndex) so the
last-batch condition aligns with the loop over newGamesList; also avoid
returning epicGames.size after epicGames.clear() — track a separate
processedCount (increment when an epicGame is added or when a fetch succeeds)
and return Result.success(processedCount) instead of epicGames.size; update
references in the loop around REFRESH_BATCH_SIZE, epicGames,
epicGameDao.upsertPreservingInstallStatus and the final Result.success call.
- Around line 766-780: The early unconditional Result.success(Unit)
short-circuits the background sync, so remove that stray return and its
accompanying debug TODO/Timbre if desired; call refreshLibrary(context), handle
its Result as intended (log success count via Timber.tag("Epic") and return
Result.success(Unit) on success or Result.failure(...) with the exception on
failure), ensuring the function returns the outcome derived from the
refreshLibrary(...) Result rather than the premature Result.success(Unit).
- Around line 665-693: Delete the two unused private helper methods
extractImageUrl and parseJsonArray from EpicManager.kt: remove the entire
extractImageUrl(keyImages: JSONArray?, imageType: String): String function and
the parseJsonArray(jsonArray: JSONArray?): List<String> function, and also
remove any now-unused imports (e.g., org.json.JSONArray / JSONObject) that
become dead after deleting these methods; ensure no remaining callers reference
extractImageUrl or parseJsonArray before committing.
In @app/src/main/java/app/gamenative/service/epic/EpicService.kt:
- Around line 362-446: The race is caused because downloadGame launches a
coroutine then immediately returns instance.activeDownloads[appName] before the
coroutine has created and inserted the DownloadInfo; move creation and insertion
of the DownloadInfo out of the coroutine so it is created (e.g., val
downloadInfo = DownloadInfo(); downloadInfo.setActive(true);
instance.activeDownloads[appName] = downloadInfo) before calling
instance.scope.launch, and then use that same downloadInfo inside the launched
block and for the immediate Result.success return; adjust any duplicate-download
check to use instance.activeDownloads and ensure finally still removes the same
key.
- Around line 236-246: The use of runBlocking in getEpicGameOf and getDLCForGame
blocks the calling thread and can cause ANR; change both functions
(getEpicGameOf and getDLCForGame) to be suspend functions and remove runBlocking
so they directly call getInstance()?.epicManager?.getGameByAppName(appName) and
getInstance()?.epicManager?.getDLCForTitle(appId), then propagate suspend to
callers (or update call sites to run on a background coroutine); alternatively,
if you cannot change callers, add clear documentation/@WorkerThread annotation
and ensure all call sites are moved off the Main thread.
In @app/src/main/java/app/gamenative/service/epic/manifest/EpicManifest.kt:
- Around line 344-349: The current lazy property guidNum in EpicManifest
attempts to pack four 32-bit parts into a single ULong using shifts beyond 64
bits, causing overflow; fix by replacing guidNum with a representation that can
hold 128 bits (e.g., change its type to Pair<ULong, ULong> or a custom data
class like Guid128) and compute high and low 64-bit halves by combining
guid[0]..guid[1] into the high ULong and guid[2]..guid[3] into the low ULong, or
if only 64-bit is required, explicitly define guidLow: ULong that uses just the
lower two parts (guid[2], guid[3]) so no shift exceeds 63 bits; update all
usages of guidNum to the new representation (Guid128 or guidLow) throughout the
class.
In
@app/src/main/java/app/gamenative/service/epic/manifest/JsonManifestParser.kt:
- Around line 17-40: In parse(jsonData: ByteArray) inside
JsonManifestParser.parse, wrap the JSONObject(jsonString) construction and
subsequent JSON-dependent parsing (parseManifestMeta, parseChunkDataList,
parseFileManifestList, parseCustomFields) in a try-catch that catches
JSONException (and optionally any other parsing exceptions) and rethrows an
IllegalArgumentException (or a custom parsing exception) with a clear message
like "Failed to parse manifest JSON" including the original exception as the
cause so callers can inspect the root error; this prevents a raw JSONException
from bubbling up and gives a descriptive, traceable error while preserving the
original exception.
In @app/src/main/java/app/gamenative/ui/PluviaMain.kt:
- Around line 1186-1193: The Epic branch currently skips Steam/cloud sync;
instead, perform the same cloud-save download sequence used for GOG before
launching: detect Epic via ContainerUtils.extractGameSourceFromContainerId(...)
== GameSource.EPIC, call the existing cloud-download routine (the same
method/flow invoked in the GOG branch around the GOG pre-launch block), wait for
it to complete, log via Timber.tag("preLaunchApp"), hide the loading dialog with
setLoadingDialogVisible(false) only after download finishes, then call
onSuccess(context, appId); keep the existing Epic upload-on-exit behavior in
MainViewModel unchanged.
In
@app/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.kt:
- Line 640: The onProgressChanged callback updates the Compose state
downloadProgressState from a background thread; wrap the assignment in a
main-thread coroutine using the existing scope and Dispatchers.Main (match the
WineProtonManagerDialog pattern) so the update happens on the UI thread, e.g.
use scope.launch(Dispatchers.Main) { downloadProgressState = progress } inside
the onProgressChanged lambda.
In
@app/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.kt:
- Around line 362-380: The onPauseResumeClick handler is using the raw
libraryItem.appId (including the "EPIC_" prefix) so EpicService.getEpicGameOf
usually returns null; strip the prefix like other methods before calling
EpicService: compute val epicId = libraryItem.appId.removePrefix("EPIC_"), then
call EpicService.getEpicGameOf(epicId) and EpicService.getDownloadInfo(appName)
using the resulting appName/epicId as appropriate; keep the rest of the logic
(isDownloading check, EpicService.cleanupDownload, downloadInfo.cancel, and
calling onDownloadInstallClick) but use the stripped id to ensure the
pause/resume path actually finds the Epic game.
- Around line 122-131: The current LaunchedEffect in EpicAppScreen
(installListener subscribed via app.gamenative.PluviaApp.events.on in the
getGameDisplayInfo path) never unsubscribes listeners which leaks and causes
duplicate handling; replace the LaunchedEffect(appId) block with a
DisposableEffect(appId) that registers the same installListener with
PluviaApp.events.on<app.gamenative.events.AndroidEvent.LibraryInstallStatusChanged,
Unit>(installListener) and calls
PluviaApp.events.off<app.gamenative.events.AndroidEvent.LibraryInstallStatusChanged,
Unit>(installListener) inside onDispose so listeners are removed when the
composable is disposed or appId changes.
In
@app/src/main/java/app/gamenative/ui/screen/library/components/LibraryAppItem.kt:
- Line 505: The line declaring `var alpha = remember(Int) { 1f }` is invalid and
unused; delete that statement from the `GameInfoBlock`/`LibraryAppItem`
composable. If you actually need local float state, replace it with a correct
Compose state usage such as `val alpha by remember { mutableFloatStateOf(1f) }`
or reuse the existing outer `alpha` defined earlier, but do not keep the
`remember(Int)` form.
In
@app/src/main/java/app/gamenative/ui/screen/library/components/LibraryBottomSheet.kt:
- Around line 121-132: The Epic FlowFilterChip is using the wrong state
variable: it currently sets selected = showGOG which will reflect GOG selection
for the Epic chip; update the Epic chip to use selected = showEpic so the chip
displays the correct selection state (locate the FlowFilterChip that calls
onSourceToggle(GameSource.EPIC) and replace showGOG with showEpic).
In
@app/src/main/java/app/gamenative/ui/screen/library/components/LibraryListPane.kt:
- Line 459: Fix the formatting of the parameter assignment for showEpic: add a
space after the equals sign in the assignment that references
state.showEpicInLibrary so it reads with a space between '=' and state; locate
the call or component prop where showEpic is set (symbol: showEpic and
state.showEpicInLibrary) and update the spacing.
In @app/src/main/res/values/strings.xml:
- Line 984: Replace the incorrect XML comment "<!-- GOG Login Dialog -->" with
"<!-- Epic Games Login Dialog -->" so the comment accurately describes the
following Epic Games login strings in the strings.xml file; update the
single-line XML comment that precedes the Epic Games login string resources (the
comment containing "GOG Login Dialog") to read "Epic Games Login Dialog".
In @app/src/test/java/app/gamenative/service/epic/EpicManagerTest.kt:
- Around line 120-126: The test incorrectly compares the Boolean property
isEAManaged to a string; update the assertion so that thirdPartyManagedApp still
equals "The EA App" but isEAManaged is asserted as true (use an assertion that
checks the Boolean truth of isEAManaged rather than comparing it to the string),
replacing the current assertEquals for isEAManaged with an assert that validates
game.isEAManaged is true.
🟠 Major comments (12)
app/src/main/java/app/gamenative/service/epic/manifest/JsonManifestParser.kt-17-40 (1)
17-40: Add exception handling for JSON parsing.Line 27 parses JSON without error handling. If
jsonStringcontains malformed JSON,JSONObject(jsonString)will throwJSONException, causing a crash. Consider wrapping the parsing logic in a try-catch block and providing a more descriptive error message.🔎 Proposed fix with error handling
fun parse(jsonData: ByteArray): EpicManifest { if (jsonData.isEmpty()) { throw IllegalArgumentException("Cannot parse empty manifest data") } val jsonString = String(jsonData, Charsets.UTF_8) if (jsonString.isBlank()) { throw IllegalArgumentException("Manifest contains only whitespace") } - val json = JSONObject(jsonString) + val json = try { + JSONObject(jsonString) + } catch (e: org.json.JSONException) { + throw IllegalArgumentException("Invalid JSON manifest format", e) + } val manifest = JsonManifest() manifest.version = blobToNum(json.optString("ManifestFileVersion", "013000000000")) manifest.storedAs = 0 // JSON manifests are never compressed // Parse components manifest.meta = parseManifestMeta(json) manifest.chunkDataList = parseChunkDataList(json, manifest.version) manifest.fileManifestList = parseFileManifestList(json) manifest.customFields = parseCustomFields(json) return manifest }app/src/main/java/app/gamenative/db/converters/EpicConverter.kt-19-24 (1)
19-24: Add error handling for malformed JSON.The
toStringListconverter lacks exception handling for malformed JSON. If the database contains corrupted or invalid JSON data,Json.decodeFromStringwill throw an uncaught exception, potentially causing crashes. Room TypeConverters should handle deserialization failures gracefully.🔎 Proposed fix with error handling
@TypeConverter fun toStringList(value: String): List<String> { if (value.isEmpty()) { return emptyList() } - return Json.decodeFromString<List<String>>(value) + return try { + Json.decodeFromString<List<String>>(value) + } catch (e: Exception) { + // Log the error and return empty list for corrupted data + timber.log.Timber.e(e, "Failed to deserialize string list from database") + emptyList() + } }app/src/main/java/app/gamenative/ui/PluviaMain.kt-1186-1193 (1)
1186-1193: Missing Epic cloud save download before launch.Epic games bypass cloud save synchronization before launch, but MainViewModel.kt shows Epic only syncs (uploads) on exit (lines 310-330). This is inconsistent with GOG, which downloads saves before launch (lines 1162-1184) and uploads on exit (lines 289-309).
Without downloading cloud saves before launch, users may lose progress if newer saves exist in the cloud. The PR objectives mention Epic supports "cloud saves (download and upload)", suggesting both operations should occur.
🔎 Suggested fix to add Epic cloud save download
Replace the Epic bypass with cloud save synchronization, mirroring the GOG pattern:
- // For Epic Games, bypass Steam Cloud operations entirely and proceed to launch - val isEpicGame = ContainerUtils.extractGameSourceFromContainerId(appId) == GameSource.EPIC - if (isEpicGame) { - Timber.tag("preLaunchApp").i("Epic Game detected for $appId — skipping Steam Cloud sync and launching container") - setLoadingDialogVisible(false) - onSuccess(context, appId) - return@launch - } + // For Epic Games, sync cloud saves before launch + val isEpicGame = ContainerUtils.extractGameSourceFromContainerId(appId) == GameSource.EPIC + if (isEpicGame) { + Timber.tag("Epic").i("[Cloud Saves] Epic Game detected for $appId — syncing cloud saves before launch") + + // Sync cloud saves (download latest saves before playing) + Timber.tag("Epic").d("[Cloud Saves] Starting pre-game download sync for $appId") + val syncSuccess = app.gamenative.service.epic.EpicCloudSavesManager.syncCloudSaves( + context = context, + appId = appId.removePrefix("EPIC_"), + preferredAction = "download" + ) + + if (!syncSuccess) { + Timber.tag("Epic").w("[Cloud Saves] Download sync failed for $appId, proceeding with launch anyway") + // Don't block launch on sync failure - log warning and continue + } else { + Timber.tag("Epic").i("[Cloud Saves] Download sync completed successfully for $appId") + } + + setLoadingDialogVisible(false) + onSuccess(context, appId) + return@launch + }Committable suggestion skipped: line range outside the PR's diff.
app/src/test/java/app/gamenative/service/epic/EpicManagerTest.kt-120-126 (1)
120-126: Fix EA test assertion forisEAManaged
isEAManagedis a Boolean property, but the test compares it to the string"The EA App":assertEquals("The EA App", game.thirdPartyManagedApp) assertEquals("The EA App", game.isEAManaged)Change the second assertion to:
assertEquals("The EA App", game.thirdPartyManagedApp) assertTrue(game.isEAManaged)app/src/androidTest/java/app/gamenative/service/epic/manifest/test/ManifestTestSerializer.kt-155-158 (1)
155-158: FixbytesToHexto handle all byte values correctlyThe current implementation corrupts hash strings when bytes have values ≥ 128. In Kotlin/Java, a negative byte (e.g., 0xFF = -1) gets sign-extended to a negative Int when passed to
String.format("%02x", byte), producing 8 hex digits instead of 2.Since hashes inherently contain bytes across the full 0-255 range, this corrupts every hash serialization:
shaHash,hash,hashMd5, andhashSha256.Use an unsigned mask when formatting:
private fun bytesToHex(bytes: ByteArray): String { if (bytes.isEmpty()) return "" return bytes.joinToString("") { "%02x".format(it.toInt() and 0xFF) } }This produces consistent 2-digit hex for all byte values and ensures hash comparisons remain reliable.
app/src/main/java/app/gamenative/service/epic/EpicAuthManager.kt-118-163 (1)
118-163: Returning expired credentials when refresh fails may violate caller expectations
getStoredCredentialsattempts to refresh if the access token is near expiry, but on refresh failure it logs and returns the old (likely expired) credentials:if (refreshResult.isFailure) { Timber.e("Failed to refresh token, returning expired credentials") return Result.success(credentials) }Callers of
getStoredCredentialstypically expect a valid token or a failure, not a silently expired one, so this can lead to confusing downstream 401s.Suggested behavior change
Consider failing explicitly so the UI can prompt the user to re‑authenticate:
- if (refreshResult.isFailure) { - Timber.e("Failed to refresh token, returning expired credentials") - return Result.success(credentials) - } + if (refreshResult.isFailure) { + Timber.e("Failed to refresh token; treating stored credentials as invalid") + return Result.failure( + refreshResult.exceptionOrNull() ?: Exception("Token refresh failed") + ) + }If you want a more nuanced behavior, you could differentiate between transient network errors (retry) and hard auth errors (force login), but returning a clearly failed
Resultis still preferable to silently handing out expired tokens.app/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.kt-362-380 (1)
362-380:onPauseResumeClickuses unstrippedappIdand likely never finds the Epic gameOther methods (e.g.,
isInstalled,onDeleteDownloadClick) call:EpicService.getEpicGameOf(libraryItem.appId.removePrefix("EPIC_"))But
onPauseResumeClickdoes:val epicGame = EpicService.getEpicGameOf(libraryItem.appId) val appName = epicGame?.appName ?: returnIf
EpicService.getEpicGameOfexpects the raw Epic app name (withoutEPIC_), this call will usually returnnull, and the pause/resume action becomes a no‑op.Proposed fix for consistent ID handling
- val epicGame = EpicService.getEpicGameOf(libraryItem.appId) + val epicGame = EpicService.getEpicGameOf(libraryItem.appId.removePrefix("EPIC_"))This aligns
onPauseResumeClickwithisInstalled,isDownloading,getDownloadProgress,onDeleteDownloadClick, etc.app/src/main/java/app/gamenative/service/epic/EpicAuthClient.kt-47-55 (1)
47-55: Form body should URL‑encodeauthorizationCode/refreshTokeninstead of raw string interpolation
formBodyis built via string concatenation:val formBody = "grant_type=authorization_code&code=$authorizationCode&token_type=eg1" ... val formBody = "grant_type=refresh_token&refresh_token=$refreshToken&token_type=eg1"If the code or refresh token ever contains
+,&,=, or other reserved characters, the request becomes invalid. Please build the body withFormBody.Builder()(or equivalent) so parameters are correctly URL‑encoded.Proposed change using OkHttp FormBody
-import okhttp3.MediaType.Companion.toMediaTypeOrNull +import okhttp3.FormBody @@ - val formBody = "grant_type=authorization_code&code=$authorizationCode&token_type=eg1" - val requestBody = formBody.toRequestBody("application/x-www-form-urlencoded".toMediaTypeOrNull()) + val requestBody = FormBody.Builder() + .add("grant_type", "authorization_code") + .add("code", authorizationCode) + .add("token_type", "eg1") + .build() @@ - val formBody = "grant_type=refresh_token&refresh_token=$refreshToken&token_type=eg1" - val requestBody = formBody.toRequestBody("application/x-www-form-urlencoded".toMediaTypeOrNull()) + val requestBody = FormBody.Builder() + .add("grant_type", "refresh_token") + .add("refresh_token", refreshToken) + .add("token_type", "eg1") + .build()Also applies to: 103-111
app/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.kt-122-131 (1)
122-131: Event listener ingetGameDisplayInfois never unsubscribed; preferDisposableEffectwith on/off pairingHere you subscribe to
PluviaApp.eventsinside aLaunchedEffect:LaunchedEffect(appId) { val installListener: (AndroidEvent.LibraryInstallStatusChanged) -> Unit = { ... } PluviaApp.events.on<AndroidEvent.LibraryInstallStatusChanged, Unit>(installListener) }Unlike other parts of the codebase (e.g.,
XServerScreen), there is no correspondingoffcall, so listeners will accumulate as composables are recomposed or recreated, and every install event will fan out to stale listeners.Suggested pattern using `DisposableEffect`
DisposableEffect(appId) { val installListener: (AndroidEvent.LibraryInstallStatusChanged) -> Unit = { event -> if (event.appId == libraryItem.appId) { refreshTrigger++ } } PluviaApp.events.on<AndroidEvent.LibraryInstallStatusChanged, Unit>(installListener) onDispose { PluviaApp.events.off<AndroidEvent.LibraryInstallStatusChanged, Unit>(installListener) } }This matches the existing pattern used in
XServerScreenand avoids leaking listeners onto the global event bus.app/src/main/java/app/gamenative/service/epic/EpicCloudSavesManager.kt-211-217 (1)
211-217: String comparison of ISO timestamps may produce incorrect results.Lines 213 and 201-202 compare ISO timestamp strings lexicographically. While ISO 8601 format (
2024-01-15T10:30:00Z) is designed to be lexicographically sortable, the comparisoncloudTimestamp >= (lastSync ?: "")with an empty string fallback is error-prone. IflastSyncis null, the conditioncloudTimestamp >= ""is always true.🔎 Proposed fix using epoch milliseconds
- // No sync timestamp - just compare cloud vs local - if (cloudTimestamp >= (lastSync ?: "")) { - SyncAction.DOWNLOAD - } else { - SyncAction.NONE - } + // No sync timestamp - download cloud saves since we have no baseline + SyncAction.DOWNLOADapp/src/main/java/app/gamenative/service/epic/EpicCloudSavesManager.kt-992-997 (1)
992-997: Padding small chunks to 1MB is wasteful for cloud saves.Cloud save files are typically small. Padding every chunk to 1MB significantly inflates upload size. For example, a 50KB save file would be padded to 1MB.
🔎 Proposed fix - remove padding for cloud saves
private fun finalizeChunk( data: ByteArray, chunkNum: Int, packagedFiles: MutableMap<String, ByteArray> ): app.gamenative.service.epic.manifest.ChunkInfo { - // Pad to 1 MB if needed - val paddedData = if (data.size < 1024 * 1024) { - data + ByteArray(1024 * 1024 - data.size) - } else { - data - } + // Use data as-is for cloud saves (no padding needed) + val chunkData = data // Compress chunk - val compressedData = compressChunk(paddedData) + val compressedData = compressChunk(chunkData) // Calculate hashes - val shaHash = java.security.MessageDigest.getInstance("SHA-1").digest(paddedData) - val rollingHash = calculateRollingHash(paddedData) + val shaHash = java.security.MessageDigest.getInstance("SHA-1").digest(chunkData) + val rollingHash = calculateRollingHash(chunkData) // ... rest unchanged, but update windowSize: - chunkInfo.windowSize = paddedData.size + chunkInfo.windowSize = chunkData.sizeapp/src/main/java/app/gamenative/service/epic/EpicService.kt-236-246 (1)
236-246:runBlockingmay cause ANR if called from Main thread.
getEpicGameOfandgetDLCForGameuserunBlockingwhich blocks the calling thread. If invoked from the UI thread, this could cause ANR. Consider either making these suspend functions or documenting that they must be called from a background thread.🔎 Suggested approach: convert to suspend functions
-fun getEpicGameOf(appName: String): EpicGame? { - return runBlocking { - getInstance()?.epicManager?.getGameByAppName(appName) - } -} +suspend fun getEpicGameOf(appName: String): EpicGame? { + return getInstance()?.epicManager?.getGameByAppName(appName) +} -fun getDLCForGame(appId: String): List<EpicGame> { - return runBlocking { - getInstance()?.epicManager?.getDLCForTitle(appId) ?: emptyList() - } -} +suspend fun getDLCForGame(appId: String): List<EpicGame> { + return getInstance()?.epicManager?.getDLCForTitle(appId) ?: emptyList() +}Based on learnings,
runBlockingis acceptable if performance is verified, but callers should be audited to ensure they're not on the Main thread.Committable suggestion skipped: line range outside the PR's diff.
🟡 Minor comments (7)
app/src/main/java/app/gamenative/data/LibraryItem.kt-29-30 (1)
29-30: Consider the nullability design for capsuleHash.
capsuleHashis declared as nullable (String?) but defaults to an empty string. This creates ambiguity:
- If null represents "no value", the default should be
nullinstead of""- If empty string is the sentinel value, the type should be non-nullable
StringThis inconsistency may lead to unnecessary null checks. Consider aligning with
heroHash's pattern (non-nullable with empty default) unless there's a specific reason to distinguish between null and empty.app/src/main/res/values/strings.xml-984-984 (1)
984-984: Fix the comment copy-paste error.The comment says
<!-- GOG Login Dialog -->but this section contains Epic Games login strings. Update to<!-- Epic Games Login Dialog -->.🔎 Proposed fix
- <!-- GOG Login Dialog --> + <!-- Epic Games Login Dialog -->app/src/androidTest/java/app/gamenative/service/epic/manifest/test/ManifestParseTest.kt-1-39 (1)
1-39: Remove or relocate this manual CLI tool from the instrumentation test directory.This file contains only a
main()function inandroidTest, which is reserved for Android instrumentation tests. The test runner (AndroidJUnitRunner) only executes methods annotated with@Testand ignores top-level functions, making this code unreachable as written.The actual test logic belongs in
ManifestParseValidationTest.kt(which is properly configured with@Testmethods). IfManifestParseTestserves as a manual debugging utility, move it to a separate utility module or documentation, or refactor it as a proper@Testmethod.app/src/main/java/app/gamenative/ui/screen/library/components/LibraryListPane.kt-459-459 (1)
459-459: Fix the formatting: add space after=.Missing space between
=andstatein the parameter assignment.🔎 Proposed fix
- showEpic =state.showEpicInLibrary, + showEpic = state.showEpicInLibrary,app/src/main/java/app/gamenative/service/epic/EpicConstants.kt-77-84 (1)
77-84: Hardcoded internal path assumes fixed package name.
INTERNAL_BASE_PATHhardcodes/data/data/app.gamenative/files. If the package name changes (e.g., debug builds, flavors), this path will be incorrect. Consider passingContext.filesDirfrom calling code instead.// Alternative: Remove hardcoded path and require context fun getInternalEpicGamesPath(context: Context): String { return File(context.filesDir, "Epic/games").absolutePath }app/src/main/java/app/gamenative/service/epic/EpicCloudSavesManager.kt-227-234 (1)
227-234: Log the swallowed exception for debugging.The static analysis flagged this as a swallowed exception. While returning
0Lis a safe fallback, logging the exception helps diagnose timestamp parsing issues.🔎 Proposed fix
private fun parseTimestamp(timestamp: String): Long { return try { val instant = Instant.parse(timestamp) instant.toEpochMilli() } catch (e: Exception) { + Timber.tag("Epic").w(e, "[Cloud Saves] Failed to parse timestamp: $timestamp") 0L } }app/src/main/java/app/gamenative/service/epic/EpicManager.kt-665-693 (1)
665-693: Remove unused private methods.
extractImageUrlandparseJsonArrayare dead code with no callers. Delete them from EpicManager.kt.
| data class EpicCredentials( | ||
| val accessToken: String, | ||
| val refreshToken: String, | ||
| val accountId: String, | ||
| val displayName: String, | ||
| val expiresAt: Long = 0, | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n -A 10 'EpicCredentials' --type=kotlin | head -150Repository: utkarshdalal/GameNative
Length of output: 13725
🏁 Script executed:
# Search for SharedPreferences usage with EpicCredentials
rg -n 'SharedPreferences|EncryptedSharedPreferences' --type=kotlin | grep -i epicRepository: utkarshdalal/GameNative
Length of output: 505
🏁 Script executed:
# Search for Room database usage related to credentials
rg -n '@Entity|@Dao|@Database' --type=kotlin | grep -A 5 -B 5 -i 'epic\|credential'Repository: utkarshdalal/GameNative
Length of output: 1569
🏁 Script executed:
# Check for any encryption/security related code
rg -n 'EncryptedSharedPreferences|encrypt|Cipher|SecureStorage' --type=kotlin | head -50Repository: utkarshdalal/GameNative
Length of output: 6470
🏁 Script executed:
rg -n 'getCredentialsFilePath' --type=kotlin -A 5Repository: utkarshdalal/GameNative
Length of output: 3878
🏁 Script executed:
# Check the exact file storage location
rg -n 'credentialsFilePath\|filesDir\|cacheDir' app/src/main/java/app/gamenative/service/epic/ --type=kotlin -B 2 -A 2Repository: utkarshdalal/GameNative
Length of output: 49
Encrypt EpicCredentials before storage.
accessToken and refreshToken in EpicCredentials are currently stored as plaintext JSON in context.filesDir/epic/credentials.json. This is inconsistent with security best practices and the app's handling of Steam tokens (which uses Crypto.encrypt()). Encrypt these sensitive credentials before writing to disk, or use EncryptedSharedPreferences.
🤖 Prompt for AI Agents
In @app/src/main/java/app/gamenative/data/EpicGame.kt around lines 180 - 186,
EpicCredentials currently holds sensitive tokens in plaintext; before persisting
instances of EpicCredentials (wherever you serialize/save them) encrypt
accessToken and refreshToken using the existing Crypto.encrypt() and decrypt on
load with Crypto.decrypt(), or replace the storage with
EncryptedSharedPreferences; update the code that creates/reads EpicCredentials
(the save/load methods that produce the credentials.json) to store encrypted
strings for accessToken/refreshToken while keeping accountId/displayName as-is
and ensure serialization/deserialization maps to the encrypted fields.
app/src/main/java/app/gamenative/service/epic/EpicCloudSavesManager.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/app/gamenative/ui/screen/library/components/LibraryAppItem.kt
Show resolved
Hide resolved
app/src/main/java/app/gamenative/ui/screen/library/components/LibraryBottomSheet.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI Agents
In
@app/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.kt:
- Around line 371-381: The cancel flow calls EpicService.cleanupDownload before
downloadInfo.cancel, which can make cancel a no-op; swap the calls so
downloadInfo.cancel() is invoked first, then
EpicService.cleanupDownload(appName) to ensure the download is cancelled before
cleanup. Apply the same change in the onDeleteDownloadClick path (reverse the
order of downloadInfo.cancel() and EpicService.cleanupDownload(appName)) so both
cancel/delete flows first cancel the DownloadInfo and only then perform
EpicService cleanup.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/src/main/java/app/gamenative/data/EpicGame.ktapp/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.kt
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-28T13:56:06.888Z
Learnt from: utkarshdalal
Repo: utkarshdalal/GameNative PR: 176
File: app/src/main/java/app/gamenative/service/SteamService.kt:179-180
Timestamp: 2025-09-28T13:56:06.888Z
Learning: In the GameNative project, the AppInfo table (with AppInfoDao) tracks local game installation state including which apps are downloaded and which depots were installed. This data should NOT be cleared during logout in clearDatabase() because games remain physically installed on the device and users should see their installed games when logging back in. Only user-specific Steam account data should be cleared on logout.
Applied to files:
app/src/main/java/app/gamenative/data/EpicGame.kt
🔇 Additional comments (8)
app/src/main/java/app/gamenative/data/EpicGame.kt (3)
12-125: LGTM - Well-structured Room entity.The
EpicGameentity is comprehensive with appropriate column definitions. The default values are sensible, and the documentation comments clearly explain the purpose of each field.
126-170: Computed properties are well-implemented.The fallback chains for
primaryImageUrlandiconUrlhandle missing art assets gracefully. TherequiresOriginandrequiresUbisoftchecks properly handle case-insensitivity withlowercase().
186-213: Supporting data classes look good.
EpicDownloadInfo,EpicDLCInfo, andGameTokenare appropriately structured for their purposes.app/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.kt (5)
46-106: Companion object dialog state management is appropriate.Using
mutableStateListOffor shared dialog state works for this use case. TheformatBytesutility is correctly implemented.
575-654: Well-implemented observer pattern with proper cleanup.The
observeGameStatefunction correctly tracks disposables and returns a cleanup function. This pattern should be followed for the event listener ingetGameDisplayInfo.
659-779: Dialog implementation follows Compose patterns correctly.The use of
snapshotFlowto observe companion object state and the dialog lifecycle handling are appropriate.
230-266: Install status methods are correctly implemented.Proper null handling and logging throughout these methods.
468-488: Container support properly delegates to ContainerUtils.The container management methods correctly use the shared utility class.
app/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.kt
Outdated
Show resolved
Hide resolved
Now it'll just ignore it if the file is empty.
|
Things left to do:
|
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI Agents
In @app/src/main/java/app/gamenative/service/epic/EpicAuthManager.kt:
- Around line 39-42: The catch block in EpicAuthManager that logs failures to
clear credentials uses the wrong product name ("GOG") in the Timber.e call;
update the log message to reference "Epic" instead, e.g., change the message
string in the catch inside the method that clears Epic credentials (the
try/catch in EpicAuthManager where Timber.e(e, "...")) so it reads "Failed to
clear Epic credentials" while keeping the exception parameter and returning
false unchanged.
- Around line 130-135: The code in EpicAuthManager that calls
EpicAuthClient.refreshAccessToken currently treats a refresh failure as a
success by returning the expired credentials; instead, change the failure path
to return a failure Result (e.g., Result.failure(...) with a clear exception) so
callers can detect and handle auth errors; update the block around
EpicAuthClient.refreshAccessToken in EpicAuthManager.kt to propagate the refresh
error (or implement a limited retry before returning Result.failure) rather than
wrapping expired credentials in Result.success.
In @app/src/main/java/app/gamenative/service/epic/EpicCloudSavesManager.kt:
- Around line 951-958: The manifest is being uploaded with manifest.data empty
because BinaryManifest only implements read()—add a serialization method (e.g.,
write()/serialize()/toBytes()) on BinaryManifest that encodes meta, chunks,
files and customFields into the binary format, and call it after
createManifest() to set manifest.data (e.g., manifest.data =
manifest.serialize()) before building manifestName and adding to packagedFiles;
ensure the new method mirrors the existing read() structure and populates the
same fields used by createManifest().
In @app/src/main/java/app/gamenative/service/epic/EpicConstants.kt:
- Around line 74-81: INTERNAL_BASE_PATH and the derived internalEpicGamesPath
are using a hardcoded "/data/data/app.gamenative/files" which will break across
package names/devices; update internalEpicGamesPath to resolve using a Context
at runtime (e.g., accept a Context parameter or provide a setter) or convert it
to a lazily-initialized property that uses context.filesDir to build
Paths.get(context.filesDir.absolutePath, "Epic", "games"). Replace usages of
INTERNAL_BASE_PATH with the new context-based resolver and remove the hardcoded
const to ensure paths reflect the actual app package at runtime.
- Around line 115-119: In getGameInstallPath, the character class in
Regex("[^a-zA-Z0-9 -_]") contains an unescaped hyphen which creates an
unintended range; update the regex used in sanitizedTitle (in
EpicConstants.getGameInstallPath) to either escape the hyphen or place it at the
start/end of the class (for example use "[^a-zA-Z0-9 \\-_]" or "[^a-zA-Z0-9
_-]") so only the intended characters (letters, digits, space, underscore,
hyphen) are allowed.
In @app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.kt:
- Around line 229-232: Decide whether the explicit User-Agent is required and
remove the TODO accordingly: if Epic's CDN requires the UE Launcher User-Agent,
keep the .header("User-Agent", "UELauncher/...") call on Request.Builder() and
delete the TODO; if it is not required, remove the entire .header(...) call and
delete the TODO. Update the request construction around Request.Builder() / the
local variable request in EpicDownloadManager (or its download method) to
reflect your choice and ensure no leftover TODOs remain.
🧹 Nitpick comments (9)
app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.kt (2)
351-379: Remove deprecated function.The deprecated
decompressChunkfunction is kept "for reference" but adds maintenance overhead. Reference implementations belong in git history or documentation, not in production code. Consider removing it to keep the codebase clean.🔎 Proposed fix
- /** - * Decompress a chunk file using zlib inflation (deprecated - keeping for reference) - * Epic chunks use zlib compression (deflate algorithm) - */ - @Deprecated("Use readEpicChunk instead") - private fun decompressChunk(compressedFile: File, outputFile: File, expectedSize: Long) { - val inflater = Inflater() - try { - compressedFile.inputStream().use { input -> - outputFile.outputStream().use { output -> - val compressedData = input.readBytes() - inflater.setInput(compressedData) - - val buffer = ByteArray(CHUNK_BUFFER_SIZE) - var totalDecompressed = 0L - - while (!inflater.finished()) { - val decompressedCount = inflater.inflate(buffer) - if (decompressedCount > 0) { - output.write(buffer, 0, decompressedCount) - totalDecompressed += decompressedCount - } - } - - if (totalDecompressed != expectedSize) { - throw Exception("Decompressed size mismatch: expected $expectedSize, got $totalDecompressed") - } - } - } - } finally { - inflater.end() - } - }
407-429: Remove unused function.The
verifyChunkHashfunction is not referenced anywhere in the codebase. OnlyverifyChunkHashBytesis used (line 264). Consider removing this unused code to reduce maintenance burden.🔎 Proposed fix
- /** - * Verify chunk SHA-1 hash from file - */ - private fun verifyChunkHash(file: File, expectedHash: String): Boolean { - return try { - val digest = MessageDigest.getInstance("SHA-1") - file.inputStream().use { input -> - val buffer = ByteArray(8192) - var bytesRead: Int - while (input.read(buffer).also { bytesRead = it } != -1) { - digest.update(buffer, 0, bytesRead) - } - } - val actualHash = digest.digest().joinToString("") { "%02x".format(it) } - val matches = actualHash.equals(expectedHash, ignoreCase = true) - - if (!matches) { - Timber.tag("Epic").e("Hash mismatch: expected $expectedHash, got $actualHash") - } - - matches - } catch (e: Exception) { - Timber.tag("Epic").e(e, "Hash verification failed") - false - } - }app/src/main/java/app/gamenative/service/epic/EpicAuthManager.kt (2)
50-59: Duplicate code:extractCodeFromInputalso exists inEpicLoginDialog.kt.This function is duplicated in
EpicLoginDialog.kt(lines 25-34). Consider consolidating into a single location to avoid divergence and maintenance overhead.
173-190:saveCredentialssilently swallows exceptions.If credential saving fails, the user will believe they're logged in, but subsequent app restarts will require re-authentication. Consider propagating this error to the caller.
🔎 Proposed fix: Return success/failure status
- private fun saveCredentials(context: Context, credentials: EpicCredentials) { + private fun saveCredentials(context: Context, credentials: EpicCredentials): Boolean { + return try { - try { val json = JSONObject().apply { put("access_token", credentials.accessToken) put("refresh_token", credentials.refreshToken) put("account_id", credentials.accountId) put("display_name", credentials.displayName) put("expires_at", credentials.expiresAt) } val file = File(getCredentialsFilePath(context)) file.writeText(json.toString()) Timber.d("Credentials saved to ${file.absolutePath}") + true } catch (e: Exception) { Timber.e(e, "Failed to save credentials") + false } }app/src/main/java/app/gamenative/service/epic/EpicService.kt (2)
285-296: Unbounded directory walk could be slow for large game installations.
File.walk()recursively traverses the entire install directory. For large games with many files, this could cause performance issues. Consider limiting depth or using a more targeted search.🔎 Proposed fix: Limit search depth
// Try to find the main executable // Common patterns: Game.exe, GameName.exe, or in Binaries/Win64/ - val exeFiles = installDir.walk() + val exeFiles = installDir.walk().maxDepth(4) // Limit to reasonable depth .filter { it.extension.equals("exe", ignoreCase = true) } .filter { !it.name.contains("UnityCrashHandler", ignoreCase = true) } .filter { !it.name.contains("UnrealCEFSubProcess", ignoreCase = true) } .sortedBy { it.absolutePath.length } // Prefer shorter paths (usually main exe) .toList()
497-525: ConsiderSTART_NOT_STICKYfor download service.
START_STICKYcauses the system to recreate the service after it's killed, even without pending work. For a download service,START_NOT_STICKYorSTART_REDELIVER_INTENTmay be more appropriate to avoid unnecessary restarts when there are no active downloads.app/src/main/java/app/gamenative/service/epic/EpicManager.kt (1)
538-542: Simplify boolean expression.The
if-elsereturningtrue/falsecan be simplified.🔎 Proposed fix
- val isEaManaged = if(parsedAttributes.thirdPartyManagedApp != null && parsedAttributes.thirdPartyManagedApp.lowercase() in listOf("origin", "the ea app")){ - true - } else { - false - } + val isEaManaged = parsedAttributes.thirdPartyManagedApp?.lowercase() in listOf("origin", "the ea app")app/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.kt (2)
315-365: OrphanedCoroutineScopewon't be cancelled on navigation.
CoroutineScope(Dispatchers.IO).launchcreates an unmanaged coroutine that continues even when the user navigates away. Consider using a lifecycle-aware scope. The same issue exists inperformUninstall(line 419) andgetSourceSpecificMenuOptions(line 515).Note:
rememberCoroutineScope()is imported but unused.
54-95: Static dialog state may cause issues with multiple instances.
uninstallDialogAppIdsandinstallDialogAppIdsare staticmutableStateListOfin the companion object. This works if only oneEpicAppScreenexists, but could cause unexpected behavior if the architecture changes to support multiple instances.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
app/src/main/java/app/gamenative/service/epic/EpicAuthClient.ktapp/src/main/java/app/gamenative/service/epic/EpicAuthManager.ktapp/src/main/java/app/gamenative/service/epic/EpicCloudSavesManager.ktapp/src/main/java/app/gamenative/service/epic/EpicConstants.ktapp/src/main/java/app/gamenative/service/epic/EpicDownloadManager.ktapp/src/main/java/app/gamenative/service/epic/EpicManager.ktapp/src/main/java/app/gamenative/service/epic/EpicService.ktapp/src/main/java/app/gamenative/ui/PluviaMain.ktapp/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.ktapp/src/main/java/app/gamenative/ui/screen/library/components/LibraryListPane.ktapp/src/main/res/values/strings.xmlapp/src/test/java/app/gamenative/service/epic/EpicManagerTest.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- app/src/main/java/app/gamenative/ui/PluviaMain.kt
- app/src/main/res/values/strings.xml
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-12-17T05:14:05.133Z
Learnt from: utkarshdalal
Repo: utkarshdalal/GameNative PR: 344
File: app/src/main/java/app/gamenative/service/SteamAutoCloud.kt:415-419
Timestamp: 2025-12-17T05:14:05.133Z
Learning: In SteamAutoCloud.kt, when uploading files to Steam Cloud API (beginFileUpload and commitFileUpload), the filename parameter intentionally uses different formats: `file.path + file.filename` (relative path without placeholder) when `appInfo.ufs.saveFilePatterns.isEmpty()` is true (fallback case), and `file.prefixPath` (includes placeholder like %SteamUserData%) when patterns are configured. This difference is by design.
Applied to files:
app/src/main/java/app/gamenative/service/epic/EpicCloudSavesManager.kt
📚 Learning: 2025-09-19T17:07:27.941Z
Learnt from: utkarshdalal
Repo: utkarshdalal/GameNative PR: 159
File: app/src/main/java/app/gamenative/service/DownloadService.kt:6-6
Timestamp: 2025-09-19T17:07:27.941Z
Learning: In app/src/main/java/app/gamenative/service/DownloadService.kt, the getSizeFromStoreDisplay() function uses runBlocking to call SteamService.getDownloadableDepots() because the latter was converted from synchronous to suspend in this PR. The user confirmed the previous synchronous implementation worked fine, and if performance is acceptable, keeping runBlocking is a valid approach.
Applied to files:
app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.ktapp/src/main/java/app/gamenative/service/epic/EpicService.ktapp/src/main/java/app/gamenative/service/epic/EpicManager.ktapp/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.kt
📚 Learning: 2025-09-19T17:13:01.017Z
Learnt from: utkarshdalal
Repo: utkarshdalal/GameNative PR: 159
File: app/src/main/java/app/gamenative/ui/screen/library/LibraryAppScreen.kt:136-136
Timestamp: 2025-09-19T17:13:01.017Z
Learning: In LibraryAppScreen.kt, the user prefers to use runBlocking to maintain synchronous APIs when underlying methods have been converted to suspend functions, rather than refactoring all calling code to be async. This approach prevents UI breakage and maintains API compatibility. The user confirmed this is acceptable when performance is not a concern.
Applied to files:
app/src/main/java/app/gamenative/service/epic/EpicManager.ktapp/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.kt
🧬 Code graph analysis (2)
app/src/main/java/app/gamenative/service/epic/EpicService.kt (2)
app/src/main/java/app/gamenative/service/SteamService.kt (1)
getInstalledExe(715-799)app/src/main/java/app/gamenative/service/gog/GOGManager.kt (1)
getInstalledExe(787-814)
app/src/main/java/app/gamenative/service/epic/EpicAuthManager.kt (2)
app/src/main/java/app/gamenative/ui/component/dialog/EpicLoginDialog.kt (1)
extractCodeFromInput(26-35)app/src/main/java/app/gamenative/service/epic/EpicService.kt (1)
hasStoredCredentials(76-78)
🪛 detekt (1.23.8)
app/src/main/java/app/gamenative/service/epic/EpicAuthClient.kt
[warning] 146-146: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
[warning] 152-152: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
app/src/main/java/app/gamenative/service/epic/EpicCloudSavesManager.kt
[warning] 219-219: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (10)
app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.kt (1)
86-87: Verify Cloudflare CDN exclusion rationale.The code filters out all Cloudflare CDN URLs without explanation. If Cloudflare is the only available CDN or all other CDNs fail, downloads will fail. Document why Cloudflare is excluded (e.g., authentication issues, performance problems) or verify this is the intended behavior.
app/src/main/java/app/gamenative/service/epic/EpicCloudSavesManager.kt (1)
677-688: LGTM: Good defensive coding for empty files.The filtering of empty packaged files before upload is excellent defensive coding. This prevents upload/download failures and provides clear logging of what was skipped.
app/src/main/java/app/gamenative/ui/screen/library/components/LibraryListPane.kt (1)
113-121: LGTM: Consistent Epic Games integration.The Epic Games integration follows the established pattern for Steam, GOG, and Custom Games:
- Installed count calculation includes Epic games when visible
- Remember dependencies updated to track Epic visibility
- Skeleton loading calculation includes Epic installed count
- Bottom sheet receives Epic visibility flag
The debug logging at line 343 is helpful for troubleshooting counts.
Also applies to: 151-151, 337-346, 459-459
app/src/test/java/app/gamenative/service/epic/EpicManagerTest.kt (1)
1-143: LGTM: Comprehensive test coverage.The test suite is well-structured with:
- Clear, descriptive test names
- Good coverage of game types (regular, Ubisoft, EA)
- Verification of third-party priority ordering
- Appropriate assertions on key fields (metadata, images, third-party flags, DLC attributes)
- Proper isolation with mocked DAO
The tests validate the critical parsing logic without over-testing implementation details.
app/src/main/java/app/gamenative/service/epic/EpicAuthClient.kt (1)
1-159: LGTM: Robust OAuth implementation.The Epic OAuth client is well-implemented with:
- Clean separation of authentication and token refresh flows
- Comprehensive error handling using
Resultwrappers- Good logging for debugging authentication issues
- Defensive
parseExpiresAtwith multi-layered fallback strategies (long → ISO8601 → calculated from expires_in)The static analysis warnings at lines 146 and 152 about swallowed exceptions are false positives—the exception handling is intentional fallback behavior that gracefully handles different API response formats.
app/src/main/java/app/gamenative/service/epic/EpicConstants.kt (1)
23-24: Two different User-Agent strings defined.
USER_AGENT(line 24) andEPIC_USER_AGENT(line 44) define different values. Ensure the correct one is used consistently across API calls to avoid authentication issues.Also applies to: 43-44
app/src/main/java/app/gamenative/service/epic/EpicManager.kt (2)
254-344: LGTM!The pagination logic correctly handles cursor-based iteration with proper termination condition. HTTP response handling is appropriate.
696-851: LGTM!The manifest fetching logic properly handles CDN URL extraction, query parameter parsing, and error cases. Good use of separate HTTP clients for API vs CDN requests.
app/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.kt (2)
124-135: LGTM!The
DisposableEffectproperly registers and unregisters the event listener, addressing the memory leak concern from the previous review.
579-658: LGTM!The observer pattern with disposable tracking is well-implemented. Cleanup function properly unregisters all listeners.
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: 7
🤖 Fix all issues with AI Agents
In @app/src/main/java/app/gamenative/service/epic/EpicAuthManager.kt:
- Around line 93-101: The call to saveCredentials in
EpicAuthManager.authenticateWithCode can fail silently and leave the app in an
inconsistent state; update the implementation so persistence failures are
surfaced and handled: either change saveCredentials to throw on failure (so
authenticateWithCode's existing try/catch will catch it) or make saveCredentials
return a boolean/result and check its success in authenticateWithCode before
returning Result.success; reference the saveCredentials function and
EpicAuthManager.authenticateWithCode to implement one of these two approaches
and ensure failures lead to a clear error return/path instead of silently
succeeding.
- Around line 173-190: saveCredentials currently writes EpicCredentials as
plaintext JSON to disk; instead, serialize the JSONObject, encrypt it using the
existing Crypto utilities (use the KeyStore-backed encrypt method in Crypto.kt)
and write the encrypted payload (e.g., base64 or bytes) to the credentials file,
and update the counterpart reader (wherever loadCredentials/parsing occurs) to
decrypt using the corresponding Crypto.decrypt method before parsing; preserve
the existing try/catch and Timber logging, and apply the same change to the GOG
credential save/load functions to avoid storing tokens in plaintext.
In @app/src/main/java/app/gamenative/service/epic/EpicCloudSavesManager.kt:
- Around line 649-661: In EpicCloudSavesManager.kt the code uses
chunkPart.offset.toInt() and (offset+size).toInt() with copyOfRange which will
overflow for files >2GB; validate that chunkPart.offset and chunkPart.size are
within 0..Int.MAX_VALUE and that (offset+size) does not exceed chunkData.size
before converting to Int, and if they exceed ranges either handle copying using
a Long-safe streaming approach (e.g., write in looped smaller Int-sized blocks
from chunkData to output) or log and throw a clear error; update the block that
builds partData and calls output.write to use the safe path and bounds checks
referencing fileManifest.chunkParts, chunkPart.offset, chunkPart.size,
chunks[...] and output.write.
In @app/src/main/java/app/gamenative/service/epic/EpicConstants.kt:
- Around line 127-129: The function getGameInstallPathByAppName does not
sanitize its appName parameter like getGameInstallPath does; update
getGameInstallPathByAppName to apply the same filesystem-safe sanitization
(restrict to letters, digits, space, hyphen, underscore, period) used by
getGameInstallPath before joining with defaultEpicGamesPath(context), so invalid
characters are stripped/replaced and the resulting path string is safe and
consistent with the other helper.
In @app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.kt:
- Around line 292-334: The code currently hardcodes a 1 MiB decompression buffer
in readEpicChunk, but the chunk header contains the true uncompressed size which
is skipped; read the uncompressed size from the header (the 4-byte uncompressed
size after the SHA hash and hash-type bytes currently skipped), use that value
to allocate the result ByteArray(uncompressedSize) instead of 1 MiB, and
validate bounds when slicing dataBytes (ensure dataStart + compressedSize <=
chunkBytes.size). Update the Inflater usage to inflate into the buffer sized by
uncompressedSize (or loop until finished) and keep the existing inflater.end()
in the finally block.
- Around line 207-212: The cached-chunk check only compares file size; instead,
after confirming decompressedFile.exists() and its length equals
chunk.windowSize, compute the file's SHA-1 and compare it to the expected hash
on the chunk (e.g., chunk.sha1); if the hash matches, log and call
downloadInfo.updateBytesDownloaded(chunk.fileSize) and return
Result.success(decompressedFile); if the hash does not match, delete or move the
corrupted decompressedFile, log a warning with chunk.guidStr, do NOT update
bytesDownloaded, and allow the normal download flow to proceed so the chunk is
re-fetched.
- Around line 441-442: The code uses InputStream.skip() inside
EpicDownloadManager when reading chunkFile.inputStream() and passing
chunkPart.offset, but skip() can return fewer bytes than requested; update the
read logic in the method that processes chunkFile.inputStream() (the block
referencing chunkPart.offset) to implement a reliable skipFully loop that
repeatedly calls skip and/or reads and discards bytes until the total skipped
equals chunkPart.offset, and handle unexpected EOF by throwing/logging an error;
ensure the existing input.use { ... } block is retained and that subsequent
reads start at the correct offset after the reliable skip.
🧹 Nitpick comments (13)
app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.kt (2)
86-86: Clarify the Cloudflare CDN filtering logic.The code filters out Cloudflare CDN URLs without explanation. If Cloudflare CDNs are known to be problematic or incompatible with the download logic, please add a comment explaining why. If this is a temporary workaround, consider logging a warning or tracking it as a known limitation.
🔎 Suggested improvement
-val cdnUrls = manifestData.cdnUrls.filter { !it.baseUrl.startsWith("https://cloudflare.epicgamescdn.com") } +// Note: Cloudflare CDN URLs are excluded due to [reason: auth requirements/rate limiting/etc.] +val cdnUrls = manifestData.cdnUrls.filter { !it.baseUrl.startsWith("https://cloudflare.epicgamescdn.com") }
341-368: Consider removing deprecated code.The
decompressChunkmethod is marked deprecated and is not referenced elsewhere. While the comment says "keeping for reference", deprecated code increases maintenance burden and can confuse future developers.Consider removing this method entirely, or if the reference value is significant, move the implementation details to a code comment above
readEpicChunkinstead.app/src/main/java/app/gamenative/service/epic/EpicAuthManager.kt (3)
50-59: Consider extracting regex to a companion object constant.The regex pattern on line 54 is compiled on every call to
extractCodeFromInput. For better performance, consider defining it as a constant.🔎 Proposed refactor
Add to the top of the object:
object EpicAuthManager { + private val CODE_PARAM_REGEX = Regex("[?&]code=([^&]+)") + private fun getCredentialsFilePath(context: Context): String {Then update the method:
private fun extractCodeFromInput(input: String): String { val trimmed = input.trim() // Check if it's a URL with code parameter if (trimmed.startsWith("http")) { - val codeMatch = Regex("[?&]code=([^&]+)").find(trimmed) + val codeMatch = CODE_PARAM_REGEX.find(trimmed) return codeMatch?.groupValues?.get(1) ?: "" }
159-171: Code duplication: logout and clearStoredCredentials are nearly identical.Both
logoutandclearStoredCredentials(lines 31-43) perform the same operation—deleting the credentials file. The only difference is the return type (Result vs Boolean).Consider consolidating by having
logoutcallclearStoredCredentialsinternally.🔎 Proposed refactor
suspend fun logout(context: Context): Result<Unit> { return try { - val credentialsFile = File(getCredentialsFilePath(context)) - if (credentialsFile.exists()) { - credentialsFile.delete() - Timber.i("Epic credentials cleared") + if (clearStoredCredentials(context)) { + Timber.i("Epic credentials cleared") + Result.success(Unit) + } else { + Result.failure(Exception("Failed to clear credentials")) } - Result.success(Unit) } catch (e: Exception) { Timber.e(e, "Failed to clear Epic credentials") Result.failure(e) } }
173-190: Consider using Dispatchers.IO for file operations.The file I/O operations in
saveCredentials(and similarly inloadCredentials) are synchronous and could block the coroutine dispatcher if called from a non-IO context. While the JSON files are small and the impact is minimal, wrapping these operations inwithContext(Dispatchers.IO)follows Android best practices.🔎 Proposed refactor
+import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext + object EpicAuthManager { // ... - private fun saveCredentials(context: Context, credentials: EpicCredentials) { + private suspend fun saveCredentials(context: Context, credentials: EpicCredentials) { + withContext(Dispatchers.IO) { // ... existing file write code + } } - private fun loadCredentials(context: Context): EpicCredentials? { + private suspend fun loadCredentials(context: Context): EpicCredentials? { + return withContext(Dispatchers.IO) { // ... existing file read code + } }app/src/main/java/app/gamenative/service/epic/EpicCloudSavesManager.kt (5)
51-54: SingletonOkHttpClientlackswriteTimeout.The HTTP client configures
connectTimeoutandreadTimeoutbut omitswriteTimeout. For upload operations (likeuploadFile), this could cause indefinite hangs on slow networks.🔎 Proposed fix
private val httpClient = OkHttpClient.Builder() .connectTimeout(30, TimeUnit.SECONDS) .readTimeout(30, TimeUnit.SECONDS) + .writeTimeout(30, TimeUnit.SECONDS) .build()
220-225: Fallback logic is confusing but functionally correct.When
lastSyncis null, the comparisoncloudTimestamp >= (lastSync ?: "")always evaluates totruesince any timestamp string is lexicographically greater than an empty string. This results inSyncAction.DOWNLOAD, which is the intended behavior for first-time syncs, but the logic is non-obvious.🔎 Proposed clarification
// No sync timestamp - just compare cloud vs local - if (cloudTimestamp >= (lastSync ?: "")) { - SyncAction.DOWNLOAD - } else { - SyncAction.NONE - } + // No previous sync - download cloud saves if they exist + SyncAction.DOWNLOAD
817-834: Response handling pattern is error-prone.The response body is read, then the response is manually closed, then
response.isSuccessfulis checked. While this works becauseisSuccessfulreads cached state, this pattern is fragile. Useresponse.use {}for consistent resource management.🔎 Proposed 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.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") + "" + } + + if (!response.isSuccessful) { + Timber.tag("Epic").e("[Cloud Saves] Failed to request write links: ${response.code}") + Timber.tag("Epic").e("[Cloud Saves] Response body: $responseBody") + return@withContext emptyMap() + } + // ... rest of parsing logic } - - response.close() - - if (!response.isSuccessful) { - Timber.tag("Epic").e("[Cloud Saves] Failed to request write links: ${response.code}") - Timber.tag("Epic").e("[Cloud Saves] Response body: $responseBody") - return@withContext emptyMap() - }
1224-1234: Subdirectory selection is non-deterministic when multiple exist.
firstOrNullpicks an arbitrary subdirectory when multiple contain files. This could lead to inconsistent sync behavior if a game has saves in multiple subdirectories (e.g., different profiles or slots).Consider using a more deterministic approach, such as selecting the most recently modified subdirectory, or syncing all subdirectories:
- val dirWithFiles = subDirs.firstOrNull { subDir -> + val dirWithFiles = subDirs.filter { subDir -> subDir.listFiles()?.any { it.isFile } == true - } + }.maxByOrNull { subDir -> + subDir.listFiles()?.maxOfOrNull { it.lastModified() } ?: 0L + }
1008-1013: Padding all chunks to 1MB wastes storage and bandwidth.Every chunk is padded to exactly 1MB with zeros, even if the actual data is much smaller. For cloud saves which are typically small files, this significantly increases upload size and storage costs.
Consider only padding if required by the Epic format, or using the actual data size:
- // Pad to 1 MB if needed - val paddedData = if (data.size < 1024 * 1024) { - data + ByteArray(1024 * 1024 - data.size) - } else { - data - } + // Use actual data size - padding may not be required for cloud saves + val paddedData = dataAlternatively, verify whether Epic's cloud save format actually requires 1MB padding by testing with smaller chunks.
app/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.kt (2)
514-546: Consider usingrememberCoroutineScope()for Composable click handlers.The cloud sync click handler creates an orphaned
CoroutineScope. Within a Composable,rememberCoroutineScope()provides a lifecycle-aware scope that's automatically cancelled when the Composable leaves composition.🔎 Proposed fix
@Composable override fun getSourceSpecificMenuOptions( context: Context, libraryItem: LibraryItem, onEditContainer: () -> Unit, onBack: () -> Unit, onClickPlay: (Boolean) -> Unit, isInstalled: Boolean ): List<AppMenuOption> { val options = mutableListOf<AppMenuOption>() + val scope = rememberCoroutineScope() // Add cloud sync option if game supports cloud saves val epicGame = EpicService.getEpicGameOf(libraryItem.appId.removePrefix("EPIC_")) if (epicGame?.cloudSaveEnabled == true) { options.add( AppMenuOption( optionType = AppOptionMenuType.ForceCloudSync, onClick = { - val scope = CoroutineScope(Dispatchers.Main + SupervisorJob()) scope.launch {
671-671: Remove redundant context declaration that shadows outer variable.Line 697 declares a new
contextthat shadows the one from line 671, but they're identical (LocalContext.current). This is confusing and unnecessary.🔎 Proposed fix
// Show install confirmation dialog if (showInstallDialog) { - val context = androidx.compose.ui.platform.LocalContext.current val appId = libraryItem.appIdAlso applies to: 697-697
app/src/main/java/app/gamenative/service/epic/EpicConstants.kt (1)
134-136: Address TODO: Pull version from BuildConfig.The hardcoded version string should be replaced with
BuildConfig.VERSION_NAMEto ensure the user-agent reflects the actual app version.🔎 Suggested implementation
private fun getBuildVersion(): String { - return "0.1.0" // TODO: Pull from BuildConfig + return try { + BuildConfig.VERSION_NAME + } catch (e: Exception) { + "0.1.0" // Fallback version + } }Note: You may need to import
app.gamenative.BuildConfigat the top of the file.Would you like me to open an issue to track this improvement?
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/src/main/java/app/gamenative/service/epic/EpicAuthManager.ktapp/src/main/java/app/gamenative/service/epic/EpicCloudSavesManager.ktapp/src/main/java/app/gamenative/service/epic/EpicConstants.ktapp/src/main/java/app/gamenative/service/epic/EpicDownloadManager.ktapp/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.kt
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-09-19T17:07:27.941Z
Learnt from: utkarshdalal
Repo: utkarshdalal/GameNative PR: 159
File: app/src/main/java/app/gamenative/service/DownloadService.kt:6-6
Timestamp: 2025-09-19T17:07:27.941Z
Learning: In app/src/main/java/app/gamenative/service/DownloadService.kt, the getSizeFromStoreDisplay() function uses runBlocking to call SteamService.getDownloadableDepots() because the latter was converted from synchronous to suspend in this PR. The user confirmed the previous synchronous implementation worked fine, and if performance is acceptable, keeping runBlocking is a valid approach.
Applied to files:
app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.ktapp/src/main/java/app/gamenative/service/epic/EpicCloudSavesManager.ktapp/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.kt
📚 Learning: 2025-12-17T05:14:05.133Z
Learnt from: utkarshdalal
Repo: utkarshdalal/GameNative PR: 344
File: app/src/main/java/app/gamenative/service/SteamAutoCloud.kt:415-419
Timestamp: 2025-12-17T05:14:05.133Z
Learning: In SteamAutoCloud.kt, when uploading files to Steam Cloud API (beginFileUpload and commitFileUpload), the filename parameter intentionally uses different formats: `file.path + file.filename` (relative path without placeholder) when `appInfo.ufs.saveFilePatterns.isEmpty()` is true (fallback case), and `file.prefixPath` (includes placeholder like %SteamUserData%) when patterns are configured. This difference is by design.
Applied to files:
app/src/main/java/app/gamenative/service/epic/EpicConstants.ktapp/src/main/java/app/gamenative/service/epic/EpicCloudSavesManager.kt
📚 Learning: 2025-09-19T17:13:01.017Z
Learnt from: utkarshdalal
Repo: utkarshdalal/GameNative PR: 159
File: app/src/main/java/app/gamenative/ui/screen/library/LibraryAppScreen.kt:136-136
Timestamp: 2025-09-19T17:13:01.017Z
Learning: In LibraryAppScreen.kt, the user prefers to use runBlocking to maintain synchronous APIs when underlying methods have been converted to suspend functions, rather than refactoring all calling code to be async. This approach prevents UI breakage and maintains API compatibility. The user confirmed this is acceptable when performance is not a concern.
Applied to files:
app/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.kt
🧬 Code graph analysis (1)
app/src/main/java/app/gamenative/service/epic/EpicAuthManager.kt (1)
app/src/main/java/app/gamenative/service/epic/EpicService.kt (1)
hasStoredCredentials(76-78)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (10)
app/src/main/java/app/gamenative/service/epic/EpicCloudSavesManager.kt (3)
244-291: LGTM!The
listCloudSavesfunction properly handles HTTP response lifecycle withresponse.useand executes onDispatchers.IO.
1282-1327: LGTM!The chunk decompression handles both compressed and uncompressed data correctly, with graceful fallback to direct decompression on parse failures.
985-992: Manifest serialization still missing — upload will fail.
createManifestpopulates the manifest's components (meta, chunks, files) but does not serialize them tomanifest.data. TheBinaryManifestclass only has deserialization logic (read()), somanifest.dataremains an emptyByteArray(0). This causes the manifest to upload as 0 bytes, breaking cloud save uploads.This was flagged in a previous review. The
BinaryManifestclass needs awrite()orserialize()method that encodes the manifest components back to binary format, and it must be called before accessingmanifest.dataon line 988.Likely an incorrect or invalid review comment.
app/src/main/java/app/gamenative/ui/screen/library/appscreen/EpicAppScreen.kt (3)
54-76: LGTM!The dialog state management using
mutableStateListOfin the companion object is acceptable since all access appears to be from the Main thread (UI callbacks and Composable functions).
124-135: LGTM!The event listener is properly registered and unregistered using
DisposableEffectwithonDispose, preventing the memory leak identified in prior reviews.
579-658: LGTM!The
observeGameStatefunction properly manages event listeners with a cleanup pattern. All registered listeners are tracked indisposablesand removed when the returned cleanup function is invoked.app/src/main/java/app/gamenative/service/epic/EpicConstants.kt (4)
12-14: LGTM - Follows Legendary's public OAuth pattern.The hardcoded credentials follow Legendary CLI's established pattern for public OAuth clients. The comment appropriately documents that these are non-sensitive public client credentials.
154-154: Verify zero timeout for downloads is intentional.
LEGENDARY_DOWNLOAD_TIMEOUT = 0Lmeans no timeout, which could cause the app to hang indefinitely on stalled network connections. While this may be necessary for large game downloads, consider whether a very large timeout (e.g., several hours) or separate connection/read timeout configuration would be more robust.
16-71: LGTM - Well-organized constants with appropriate values.The API endpoints, OAuth URLs, launch parameters, cache TTLs, and error messages are all well-defined and follow Epic Games' documented integration patterns. The organization with comments makes the constants easy to understand and maintain.
Also applies to: 139-169
79-112: Path resolution logic is sound - no further action needed.The code correctly handles path resolution with proper fallback logic.
PrefManager.externalStoragePathis guaranteed to be non-null (defaults to empty string), and theFile().exists()check safely handles both empty and valid paths by falling back to internal storage when external storage is unavailable or not configured.
| val credentials = EpicCredentials( | ||
| accessToken = authResponse.accessToken, | ||
| refreshToken = authResponse.refreshToken, | ||
| accountId = authResponse.accountId, | ||
| displayName = authResponse.displayName, | ||
| expiresAt = authResponse.expiresAt | ||
| ) | ||
|
|
||
| saveCredentials(context, credentials) |
There was a problem hiding this comment.
saveCredentials can fail silently, leaving user in inconsistent state.
The call to saveCredentials on line 101 can fail (throws/catches internally) without propagating the error. If credential persistence fails, the authentication appears successful but the user won't remain logged in on next app launch. This creates a confusing user experience.
Consider making saveCredentials throw exceptions so they can be caught here, or check for save success before returning Result.success.
🔎 Proposed fix: Make saveCredentials throw
Update saveCredentials to throw instead of catching:
- private fun saveCredentials(context: Context, credentials: EpicCredentials) {
+ private fun saveCredentials(context: Context, credentials: EpicCredentials) {
- try {
val json = JSONObject().apply {
put("access_token", credentials.accessToken)
put("refresh_token", credentials.refreshToken)
put("account_id", credentials.accountId)
put("display_name", credentials.displayName)
put("expires_at", credentials.expiresAt)
}
val file = File(getCredentialsFilePath(context))
file.writeText(json.toString())
Timber.d("Credentials saved to ${file.absolutePath}")
- } catch (e: Exception) {
- Timber.e(e, "Failed to save credentials")
- }
}Then the try-catch in authenticateWithCode will naturally catch any save failures.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @app/src/main/java/app/gamenative/service/epic/EpicAuthManager.kt around
lines 93 - 101, The call to saveCredentials in
EpicAuthManager.authenticateWithCode can fail silently and leave the app in an
inconsistent state; update the implementation so persistence failures are
surfaced and handled: either change saveCredentials to throw on failure (so
authenticateWithCode's existing try/catch will catch it) or make saveCredentials
return a boolean/result and check its success in authenticateWithCode before
returning Result.success; reference the saveCredentials function and
EpicAuthManager.authenticateWithCode to implement one of these two approaches
and ensure failures lead to a clear error return/path instead of silently
succeeding.
…pload. Also removed EpicConverter since GOGConverter does the same thing.
|
Converting back to Draft for now as I've found a nasty crash that I need to address before going forward. |
app/build.gradle.kts
Outdated
| } | ||
| } | ||
| dynamicFeatures += setOf(":ubuntufs") | ||
| buildToolsVersion = "35.0.0" |
There was a problem hiding this comment.
This was automatically put in by my Android Studio. Happy to remove if unneeded.
… epic. And fixed crash with install
…g of Ids. Will use an int ID that is exclusive to GN, and just use the correct fields for the API-related work.
…to a proper Int id).
…erly test everything and fix any bugs.
Now its time to start getting the data to be checked correctly before moving into the functionality .
We don't get release date from Epic which is wild.
|
Currently doing a refactor of this work as I want to ensure that it has a much cleaner interaction with the rest of the app and doesn't need so many interface-level adjustments. |
…ownloading another time.
…65/GameNative into epic-games-integration
TODO: Fix up the GameManager to give the correct information to the downloader.
2 participants
This is the Epic Games Integration PR
This brings the Epic Games Store to the GameNative app so that everyone can enjoy their lovely backlog of games they've accumulated!
A huge shoutout to Legendary & the Heroic Launcher developers for their great work on Legendary, this integration wouldn't be possible without them paving the way and assisting me in understanding the Epic APIs.
Features
Thing to Follow-up On
Architecture
This is based on the architecture I designed for GOG:
Some things to keep in mind
Testing
I have created tests regarding parsing manifests for download. This is really important as there is JSON parsing and Binary parsing. Then the URL generation can have 2 different version (v3 and v4) still being used which have their own way of working.
Due to this, I've created some integration tests with resource files to assert and to input into the tests.
For Reviewers
The big things that would be very helpful is:
Summary by CodeRabbit
Release Notes
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.