Custom Media Support & Image handeling improvements #297
Conversation
…ons and made it work with steam and custom games
WalkthroughCentralized custom media handling and image-resolution added with new utilities, UI components, and scanner refactors: CustomMediaUtils and GameImageUtils, media-aware UI (ContainerConfigDialog Media tab, LoadingToast), SteamGridDB header support, updated image-fetch flows and media-version cache-busting. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as Library / App Screen
participant Base as BaseAppScreen
participant GIU as GameImageUtils
participant CMU as CustomMediaUtils
participant SGD as SteamGridDB
participant Scanner as CustomGameScanner
participant Toast as LoadingToast
User->>UI: Request "Refresh images"
UI->>Base: setFetchingImages(appId, true)
Base->>Toast: show loading (visible=true)
rect rgba(200,230,255,0.12)
note right of Base: For each image type
Base->>GIU: getGameImage(libraryItem, imageType, steamUrl)
GIU->>CMU: check hasCustom<Image>(gameId)
alt custom exists
CMU-->>GIU: return custom URI (bustCache(mediaVersion))
GIU-->>Base: custom URI
else
GIU->>SGD: query SteamGridDB by imageType/appId
SGD-->>GIU: grid/header/hero path or null
alt SteamGridDB found
GIU-->>Base: SteamGridDB URI
else
GIU->>Scanner: findIconFileForCustomGame / extractIconFromExecutable
Scanner-->>GIU: local icon path or null
GIU-->>Base: local path or steamUrl
end
end
end
Base->>CMU: notifyImagesRefreshed()
CMU->>CMU: increment mediaVersionFlow -> UIs recompose
Base->>Base: setFetchingImagesDone(appId, "…images fetched")
Base->>Toast: show doneMessage -> fade out
Toast->>Base: onDismiss -> clear fetching/done state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🔇 Additional comments (6)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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: 4
🧹 Nitpick comments (16)
app/src/main/java/app/gamenative/ui/util/Images.kt (1)
47-54: Centered, bounded spinner implementation looks good; consider reuse for consistencyWrapping the loader in a
BoxwithcontentAlignment = Alignment.Centerand constrainingCircularProgressIndicatorviasizeIn(maxWidth = 64.dp, maxHeight = 64.dp)cleanly fixes the oversized/uncentered spinner problem while respecting the avatar’s size and clipping.If you want consistent behavior across the app, you could extract a small helper composable (e.g.,
@Composable fun CenteredLoadingIndicator(maxSize: Dp = 64.dp)) and reuse it inSteamIconImageandEmoticonImageas well, but that’s optional polish rather than required for this PR.app/src/main/java/app/gamenative/utils/CustomGameScanner.kt (2)
26-26: Consider making this aconst valorprivate const val.The
extractedGameIconFileNameappears to be a constant value that shouldn't change at runtime. Usingvarallows accidental mutation.- var extractedGameIconFileName = "gameicon.extracted.ico" + const val extractedGameIconFileName = "gameicon.extracted.ico"
409-419: Consider deprecating the context overload if unused.The second overload explicitly ignores the
contextparameter. If this is only for backward compatibility, consider annotating with@Deprecatedto guide future callers toward the simpler signature, or remove it if no callers exist.+ @Deprecated( + message = "Context parameter is unused. Use findIconFileForCustomGame(appId) instead.", + replaceWith = ReplaceWith("findIconFileForCustomGame(appId)") + ) fun findIconFileForCustomGame(context: Context, appId: String): String? { return findIconFileForCustomGame(appId) }app/src/main/java/app/gamenative/ui/screen/library/LibraryAppScreen.kt (1)
245-273: LGTM! Header-first image approach with proper fallback chain.The logic correctly implements the priority:
headerUrl→heroImageUrl→ gradient fallback. The CoilImage still handles loading failures gracefully.Minor: The comment on line 268 says "no header image" but the condition actually means "no header OR hero image available."
} else { - // Fallback gradient background when no header image + // Fallback gradient background when no header or hero image available Surface(app/src/main/java/app/gamenative/ui/component/dialog/LoadingToast.kt (1)
137-146: Redundant null coalescing operator.On line 142,
currentMessage ?: messagehas a redundant fallback. ThetargetStateon line 138 evaluates to eitherdoneMessage(whenshowDone && doneMessage != null) ormessage- both are non-nullStringvalues. The?: messageis unnecessary.Crossfade( targetState = if (showDone && doneMessage != null) doneMessage else message, label = "messageTransition" ) { currentMessage -> Text( - text = currentMessage ?: message, + text = currentMessage, color = Color.White, style = MaterialTheme.typography.bodyMedium ) }app/src/main/java/app/gamenative/utils/GameImageUtils.kt (1)
53-74: Unnecessary icon lookup when imageType is not "icon".Lines 56-61 compute the
iconvariable (potentially callingSteamGridDB.findSteamGridDBImageByAppIdandCustomGameScanner.findIconFileForCustomGame) even when the requestedimageTypeis not"icon". This adds unnecessary overhead for non-icon requests.Consider moving the icon-specific logic inside the
whenbranch:// 2. Check SteamGridDB images if (appId != null) { - // Find icon from SteamGridDB - var icon = SteamGridDB.findSteamGridDBImageByAppId(appId, "icon") - - // If no icon found, find icon from custom game scanner (extracted from exe file) - if (libraryItem.gameSource == GameSource.CUSTOM_GAME && icon == null) { - icon = CustomGameScanner.findIconFileForCustomGame(appId) - } - - // Check if the game has a custom icon val steamGridUri = when (imageType) { "hero", "grid_hero" -> SteamGridDB.findSteamGridDBImageByAppId(appId, "grid_hero") "capsule", "grid_capsule" -> SteamGridDB.findSteamGridDBImageByAppId(appId, "grid_capsule") "header" -> SteamGridDB.findSteamGridDBImageByAppId(appId, "header") "logo" -> SteamGridDB.findSteamGridDBImageByAppId(appId, "logo") - "icon" -> icon + "icon" -> { + // Find icon from SteamGridDB, fallback to extracted icon for custom games + SteamGridDB.findSteamGridDBImageByAppId(appId, "icon") + ?: if (libraryItem.gameSource == GameSource.CUSTOM_GAME) { + CustomGameScanner.findIconFileForCustomGame(appId) + } else null + } else -> null }app/src/main/java/app/gamenative/ui/screen/library/components/LibraryAppItem.kt (2)
159-176: Icon resolution pipeline is sound; watch IO cost offindIconFileForCustomGameThe priority chain
CustomMediaUtils.getCustomIconUri(gameId) → CustomGameScanner.findIconFileForCustomGame(...) → clientIconUrlplusbustCache(iconModel, mediaVersion)correctly ensures custom icons win and that UI refreshes when mediaVersion changes. IffindIconFileForCustomGamedoes filesystem scanning, consider caching its result or moving that work off the main thread, since this runs during composition for each list item.
69-70: Grid/hero/header image resolution via GameImageUtils is correct; tighten Steam fallback handlingUsing
mediaVersionplusGameImageUtils.getGameImageforgrid_capsule,grid_hero, andheaderand passing the Steam CDN URLs assteamUrlmatches the new centralized image rules and allows cache busting viabustCache(baseModel, mediaVersion). Two follow‑ups you may want to consider:
steamAssetsUrlcan likely be aninternal const valrather than a mutablevar.- For
GameSource.CUSTOM_GAME, ensureGameImageUtils.getGameImageignores or safely handles the constructed Steam URLs; otherwise custom games may waste requests on invalid Steam asset IDs.Also applies to: 179-221
app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt (2)
141-241: MediaSection abstraction is solid and matches UX goals
MediaSectioncleanly encapsulates preview rendering, “choose image” and “remove custom image” actions, and mediaVersion-aware bustCache behavior. The success/failure toasts andhasModeldetection for bothStringandUricover the common cases well. Only minor nit: you can use the already-importedrememberLauncherForActivityResult/ActivityResultContracts.GetContentinstead of the fully qualified names, but that’s purely stylistic.
245-259: New media-related parameters and tab are wired reasonably; gate Media tab when no game/app contextExtending
ContainerConfigDialogwith optionalmedia*Url,gameId, andappIdparameters plus the “Media” tab is backwards-compatible thanks to defaults. WhengameId/appIdare null (e.g., generic/default container config), the Media tab currently shows static placeholders with no actions, which may confuse users. Consider only including"Media"intabswhen bothgameIdandappIdare non-null.Also applies to: 827-828
app/src/main/java/app/gamenative/utils/SteamGridDB.kt (1)
193-207: Orientation detection can rely on JSON dimensions to avoid extra bitmap workYou now read
width/heightfrom the grid JSON and also callisImageHorizontal(imageBytes)for every non-hero-size image. Since orientation is justwidth > heightwhen those values are present, you can avoid decoding image bounds from the full byte array in most cases. That would reduce CPU and memory pressure during large grid fetches.Consider:
- // Check dimensions from JSON (more efficient than downloading first) - val width = imageObj.optInt("width", 0) - val height = imageObj.optInt("height", 0) + val width = imageObj.optInt("width", 0) + val height = imageObj.optInt("height", 0) @@ - // Determine orientation for non-header images - val isHorizontal = isImageHorizontal(imageBytes) + // Determine orientation for non-hero-size images; prefer JSON dims + val isHorizontal = when { + width > 0 && height > 0 -> width > height + else -> isImageHorizontal(imageBytes) + }This keeps the bitmap-based fallback for edge cases but avoids redundant decoding when the API already tells you the size.
Also applies to: 236-265
app/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.kt (2)
44-68: Consider importing flow types and using atomic operations for state.The fully qualified
kotlinx.coroutines.flow.MutableStateFlowandkotlinx.coroutines.flow.StateFlowreferences could be simplified by adding imports at the top of the file for cleaner code.Additionally, while the current implementation works for this use case, note that
notifyMediaChanged()style increments or concurrent calls tosetFetchingImages/setFetchingImagesDonecould theoretically interleave. For simple fetching-state tracking this is acceptable, but if you later need stricter guarantees, consider usingupdate { }instead of direct assignment.
297-362: Unstructured coroutine won't be cancelled if user navigates away.
CoroutineScope(Dispatchers.IO).launchcreates an unmanaged coroutine. If the user navigates away from this screen mid-fetch, the coroutine continues running and may emit state updates to a stale context or show toasts after navigation.Consider using a structured scope tied to the composable lifecycle (e.g.,
rememberCoroutineScope()) or a ViewModel scope for better lifecycle management.- CoroutineScope(Dispatchers.IO).launch { + uiScope.launch(Dispatchers.IO) {Note: This would require passing
uiScopeintobuildCommonMenuOptionsor restructuring the menu option callbacks.app/src/main/java/app/gamenative/utils/CustomMediaUtils.kt (3)
144-150: Consider adding BitmapFactory.Options to limit decode size for large images.Decoding arbitrarily large images without
inSampleSizecan cause OOM on devices with limited memory. Consider calculating an appropriate sample size based on the target dimensions before decoding:private fun decodeBitmap(context: Context, uri: Uri, maxDim: Int = 2048): Bitmap? { return try { context.contentResolver.openInputStream(uri).use { ins -> if (ins == null) return null // First pass: get dimensions val opts = BitmapFactory.Options().apply { inJustDecodeBounds = true } BitmapFactory.decodeStream(ins, null, opts) } // Calculate sample size and decode context.contentResolver.openInputStream(uri).use { ins -> if (ins == null) return null val opts = BitmapFactory.Options().apply { inSampleSize = calculateInSampleSize(outWidth, outHeight, maxDim, maxDim) } BitmapFactory.decodeStream(ins, null, opts) } } catch (t: Throwable) { Timber.w(t, "decodeBitmap failed"); null } }
185-197: Null-safety:parentFilecan technically be null.While
mediaDirForshould always create a valid parent,File.parentFilecan returnnullfor root paths or malformed paths. Adding a null-safe call prevents potential NPE:private fun saveJpeg(bmp: Bitmap, file: File) { - if (!file.parentFile.exists()) file.parentFile.mkdirs() + file.parentFile?.let { if (!it.exists()) it.mkdirs() } FileOutputStream(file).use { fos -> bmp.compress(Bitmap.CompressFormat.JPEG, 90, fos) } } private fun savePng(bmp: Bitmap, file: File) { - if (!file.parentFile.exists()) file.parentFile.mkdirs() + file.parentFile?.let { if (!it.exists()) it.mkdirs() } FileOutputStream(file).use { fos -> bmp.compress(Bitmap.CompressFormat.PNG, 100, fos) } }
63-82: Silent exception swallowing - consider logging deletion failures.
runCatchingsilently swallows any exceptions during file deletion. While deletion failures are often benign, logging them could help diagnose issues when custom media doesn't reset as expected:fun resetCustomHeader(appId: Int) { - runCatching { getCustomHeaderFile(appId).delete() } + runCatching { getCustomHeaderFile(appId).delete() } + .onFailure { Timber.w(it, "Failed to delete custom header for appId=$appId") } notifyMediaChanged() }Apply similar pattern to other reset functions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt(9 hunks)app/src/main/java/app/gamenative/ui/component/dialog/LoadingDialog.kt(2 hunks)app/src/main/java/app/gamenative/ui/component/dialog/LoadingToast.kt(1 hunks)app/src/main/java/app/gamenative/ui/screen/library/LibraryAppScreen.kt(2 hunks)app/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.kt(9 hunks)app/src/main/java/app/gamenative/ui/screen/library/appscreen/CustomGameAppScreen.kt(6 hunks)app/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.kt(5 hunks)app/src/main/java/app/gamenative/ui/screen/library/components/LibraryAppItem.kt(5 hunks)app/src/main/java/app/gamenative/ui/util/Images.kt(3 hunks)app/src/main/java/app/gamenative/utils/CustomGameScanner.kt(7 hunks)app/src/main/java/app/gamenative/utils/CustomMediaUtils.kt(1 hunks)app/src/main/java/app/gamenative/utils/GameImageUtils.kt(1 hunks)app/src/main/java/app/gamenative/utils/SteamGridDB.kt(12 hunks)app/src/main/res/values/strings.xml(3 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 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/ui/screen/library/appscreen/SteamAppScreen.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/utils/SteamGridDB.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/LibraryAppScreen.kt
🧬 Code graph analysis (2)
app/src/main/java/app/gamenative/ui/screen/library/components/LibraryAppItem.kt (1)
app/src/main/java/app/gamenative/ui/util/Images.kt (1)
ListItemImage(28-62)
app/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.kt (1)
app/src/main/java/app/gamenative/ui/component/dialog/LoadingToast.kt (1)
LoadingToast(47-153)
🔇 Additional comments (18)
app/src/main/res/values/strings.xml (2)
352-373: Comprehensive media management strings well-organized.The media management strings are properly structured with consistent naming conventions, descriptive hints for developers, and appropriate fallback messages. All new strings follow Android resource conventions and support future localization.
90-91: No duplicate string definitions found—strings are properly defined.Verification confirms there are no duplicate string key definitions in the full strings.xml file. The three strings mentioned (lines 90–91 and 226) are each defined exactly once, and no other duplicate name attributes exist in the file. The AI summary's reference to duplicate entries appears to be outdated.
app/src/main/java/app/gamenative/ui/component/dialog/LoadingDialog.kt (1)
46-58: LGTM! Clean separation of indeterminate and determinate progress states.The conditional logic properly handles both progress modes:
CircularProgressIndicatorfor indeterminate (progress < 0) andLinearProgressIndicatorwith percentage for determinate progress. The layout with spacers provides consistent visual spacing.app/src/main/java/app/gamenative/utils/CustomGameScanner.kt (2)
43-59: Good improvement to filter out non-game executables.Adding
unitycrashhandlerto the exclusion list alongsideuninsprevents false positives when scanning for game executables. The lambda-basedlistFilesfiltering is idiomatic Kotlin.
337-391: Well-structured icon extraction with proper error handling.The function correctly handles multiple failure scenarios (missing folder, missing executable, extraction failure) with appropriate logging. The try-catch ensures exceptions don't crash the app.
One minor style observation: the
?: run { ... return false }pattern on line 364-368 works but is slightly unconventional. An alternative is:if (exeRelPath.isEmpty()) { exeRelPath = findUniqueExeRelativeToFolder(gameFolder) ?: run { Timber.tag("CustomGameScanner").w("Could not find executable for game launch: $appId") return false } }This is fine as-is though.
app/src/main/java/app/gamenative/ui/component/dialog/LoadingToast.kt (1)
58-75: State management looks correct for typical usage patterns.The
LaunchedEffect(doneMessage)triggers the done animation whendoneMessagebecomes non-null, andLaunchedEffect(visible)properly resets state when visibility toggles off. This ensures the toast can be reused across multiple loading operations.app/src/main/java/app/gamenative/utils/GameImageUtils.kt (2)
11-19: Good abstraction for triggering UI refresh.The
notifyImagesRefreshed()function provides a clear, semantic API that abstracts the underlyingCustomMediaUtils.notifyMediaChanged()implementation detail. This makes the calling code more readable.
40-51: LGTM! Custom media priority is correctly implemented.The function properly checks for custom media first (when
gameIdis available) and returns early if found, implementing the documented priority: Custom media → SteamGridDB → Steam URLs.app/src/main/java/app/gamenative/ui/screen/library/appscreen/CustomGameAppScreen.kt (3)
82-119: Centralized image resolution for custom games looks consistentUsing
mediaVersionplusremember(mediaVersion, libraryItem)to resolveheroImageUrl,capsuleUrl,headerUrl,logoUrl, andiconUrlviaGameImageUtils.getGameImageis coherent with the new global pipeline and ensures UI refreshes when custom media changes. Wiring these intoGameDisplayInfo(including the new fields) looks correct for custom games.Also applies to: 148-165
231-237: Simplified icon extraction hook is appropriateDelegating post-fetch icon extraction to
CustomGameScanner.extractIconFromExecutable(context, libraryItem.appId)keepsonAfterFetchImagesfocused and aligned with the centralized icon-handling strategy described in the PR. No functional regressions evident here.
361-397: Deletion toasts now correctly use string resourcesSwitching the delete-success, delete-failure, and button labels to
stringResource/context.getString(custom_game_deleted,custom_game_delete_failed,delete_app,cancel) improves localization and keeps copy centralized. Logic and formatting parameters look correct.app/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.kt (2)
182-224: Steam image resolution is correctly centralized and mediaVersion-awareObserving
CustomMediaUtils.mediaVersionFlowand feedinglibraryItemplus Steam URLs intoGameImageUtils.getGameImageforgrid_hero,icon,logo,grid_capsule, andheaderintegrates Steam games cleanly into the same media priority pipeline as custom games. WiringlogoUrl,capsuleUrl, andheaderUrlintoGameDisplayInfolooks consistent with the new data model.Also applies to: 287-303
1054-1081: Post-uninstall delay before navigating back is reasonableAdding a
delay(250)on the main dispatcher before callingonBack()after a successful uninstall gives the filesystem/UI a moment to settle so the library view can refresh accurately. Because this is a suspendingdelayin a coroutine, it shouldn’t block the main thread.app/src/main/java/app/gamenative/utils/SteamGridDB.kt (1)
413-457: New header existence checks and SteamGridDB image-finder helpers look goodIncluding
existingHeaderin the “all images already exist” shortcut and returning bothgridHeroPathandheaderPathinImageFetchResultaligns with the rest of the media pipeline. The newfindSteamGridDBImageandfindSteamGridDBImageByAppIdhelpers are straightforward and flexible (imageType+ optionalexcludePattern), and returningUri.fromFile(...).toString()is appropriate for consumers like Coil.Also applies to: 540-579
app/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.kt (2)
473-491: LGTM - LoadingToast integration is well-structured.The state observation and toast visibility logic correctly handles the fetching and done states. The
onDismisscallback properly clears both states to prevent stale UI.
652-658: LGTM - Media URL parameters for dialog.Passing the media URLs to
ContainerConfigDialogenables richer display of game images in the configuration dialog, aligning with the PR's custom media support goals.app/src/main/java/app/gamenative/utils/CustomMediaUtils.kt (2)
210-231: LGTM - Cache-busting helper is well-implemented.The function correctly handles different model types (String, Uri) and appends version parameters only to supported URL schemes. The fallback for unknown types is appropriate.
38-49: LGTM - Media versioning pattern for cache invalidation.The
StateFlow-based versioning is a clean approach to trigger UI refreshes when custom media changes. The singleton pattern is appropriate for app-wide media management.
app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt
Show resolved
Hide resolved
| Box( | ||
| modifier = Modifier | ||
| .fillMaxSize() | ||
| .zIndex(1000f) // High z-index to appear on top | ||
| ) { |
There was a problem hiding this comment.
Potential touch blocking on underlying UI.
The Box with fillMaxSize() covers the entire screen. While it's transparent, it may still intercept touch events, preventing interaction with the UI below the toast. Consider using Modifier.wrapContentSize() with alignment, or adding .clickable(enabled = false, onClick = {}) with indication = null to pass through touches.
Box(
modifier = Modifier
- .fillMaxSize()
+ .fillMaxSize()
+ .then(Modifier) // Allow touch passthrough - the toast should not block interaction
.zIndex(1000f) // High z-index to appear on top
- ) {
+ ).let { /* Consider using pointerInput to pass through touches */ }Alternative approach using wrapContentSize:
Box(
modifier = Modifier
.fillMaxSize()
.zIndex(1000f),
contentAlignment = Alignment.BottomCenter
) {
// Move AnimatedVisibility content here without the nested fillMaxSize Box
}🤖 Prompt for AI Agents
In app/src/main/java/app/gamenative/ui/component/dialog/LoadingToast.kt around
lines 82-86, the Box currently uses Modifier.fillMaxSize() which covers the
whole screen and can intercept touch events; replace it with
Modifier.wrapContentSize(alignment = Alignment.BottomCenter) and move the
AnimatedVisibility content into that Box so it only occupies the toast area, or
if you need to keep full-size use .clickable(enabled = false, onClick = {}) with
indication = null (and no interactionSource) so touches pass through; ensure
zIndex remains if needed and remove any nested full-size Box that still
intercepts input.
| private fun centerCropResize(src: Bitmap, targetW: Int, targetH: Int): Bitmap { | ||
| val srcW = src.width | ||
| val srcH = src.height | ||
| val scale = maxOf(targetW.toFloat() / srcW, targetH.toFloat() / srcH) | ||
| val scaledW = (srcW * scale).toInt() | ||
| val scaledH = (srcH * scale).toInt() | ||
| val scaled = Bitmap.createScaledBitmap(src, scaledW, scaledH, true) | ||
| val x = (scaledW - targetW) / 2 | ||
| val y = (scaledH - targetH) / 2 | ||
| return Bitmap.createBitmap( | ||
| scaled, | ||
| x.coerceAtLeast(0), | ||
| y.coerceAtLeast(0), | ||
| targetW.coerceAtMost(scaled.width), | ||
| targetH.coerceAtMost(scaled.height) | ||
| ) | ||
| } |
There was a problem hiding this comment.
Memory leak: intermediate scaled bitmap is not recycled.
Bitmap.createScaledBitmap creates a new bitmap that should be recycled after Bitmap.createBitmap extracts the cropped region. Without recycling, large images can accumulate and cause OOM.
private fun centerCropResize(src: Bitmap, targetW: Int, targetH: Int): Bitmap {
val srcW = src.width
val srcH = src.height
val scale = maxOf(targetW.toFloat() / srcW, targetH.toFloat() / srcH)
val scaledW = (srcW * scale).toInt()
val scaledH = (srcH * scale).toInt()
val scaled = Bitmap.createScaledBitmap(src, scaledW, scaledH, true)
val x = (scaledW - targetW) / 2
val y = (scaledH - targetH) / 2
- return Bitmap.createBitmap(
+ val result = Bitmap.createBitmap(
scaled,
x.coerceAtLeast(0),
y.coerceAtLeast(0),
targetW.coerceAtMost(scaled.width),
targetH.coerceAtMost(scaled.height)
)
+ if (scaled != src && scaled != result) {
+ scaled.recycle()
+ }
+ return result
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private fun centerCropResize(src: Bitmap, targetW: Int, targetH: Int): Bitmap { | |
| val srcW = src.width | |
| val srcH = src.height | |
| val scale = maxOf(targetW.toFloat() / srcW, targetH.toFloat() / srcH) | |
| val scaledW = (srcW * scale).toInt() | |
| val scaledH = (srcH * scale).toInt() | |
| val scaled = Bitmap.createScaledBitmap(src, scaledW, scaledH, true) | |
| val x = (scaledW - targetW) / 2 | |
| val y = (scaledH - targetH) / 2 | |
| return Bitmap.createBitmap( | |
| scaled, | |
| x.coerceAtLeast(0), | |
| y.coerceAtLeast(0), | |
| targetW.coerceAtMost(scaled.width), | |
| targetH.coerceAtMost(scaled.height) | |
| ) | |
| } | |
| private fun centerCropResize(src: Bitmap, targetW: Int, targetH: Int): Bitmap { | |
| val srcW = src.width | |
| val srcH = src.height | |
| val scale = maxOf(targetW.toFloat() / srcW, targetH.toFloat() / srcH) | |
| val scaledW = (srcW * scale).toInt() | |
| val scaledH = (srcH * scale).toInt() | |
| val scaled = Bitmap.createScaledBitmap(src, scaledW, scaledH, true) | |
| val x = (scaledW - targetW) / 2 | |
| val y = (scaledH - targetH) / 2 | |
| val result = Bitmap.createBitmap( | |
| scaled, | |
| x.coerceAtLeast(0), | |
| y.coerceAtLeast(0), | |
| targetW.coerceAtMost(scaled.width), | |
| targetH.coerceAtMost(scaled.height) | |
| ) | |
| if (scaled != src && scaled != result) { | |
| scaled.recycle() | |
| } | |
| return result | |
| } |
🤖 Prompt for AI Agents
In app/src/main/java/app/gamenative/utils/CustomMediaUtils.kt around lines 152
to 168, the intermediate 'scaled' bitmap created by Bitmap.createScaledBitmap is
never recycled which can cause OOM; after creating the final cropped bitmap,
check whether the result is a different instance than 'scaled' and if so call
scaled.recycle() (or otherwise recycle scaled when it is no longer needed),
ensuring you don't recycle the bitmap you return; wrap the creation and
recycling logic so the scaled bitmap is always recycled on all code paths (e.g.,
try/finally) while leaving the returned bitmap intact.
| /** | ||
| * Download and save an image from a URL | ||
| * @return Pair of (file path, isHorizontal) or null if failed | ||
| */ | ||
| private suspend fun downloadAndSaveImage( | ||
| imageUrl: String, | ||
| gameFolder: File, | ||
| fileName: String | ||
| ): Pair<String, Boolean?>? = withContext(Dispatchers.IO) { | ||
| try { | ||
| // Download the image | ||
| val imageRequest = Request.Builder() | ||
| .url(imageUrl) | ||
| .build() | ||
|
|
||
| val imageResponse = httpClient.newCall(imageRequest).execute() | ||
|
|
||
| if (!imageResponse.isSuccessful) { | ||
| Timber.tag("SteamGridDB").w("Failed to download image from $imageUrl - HTTP ${imageResponse.code}") | ||
| return@withContext null | ||
| } | ||
|
|
||
| val imageBytes = imageResponse.body?.bytes() ?: return@withContext null | ||
|
|
||
| // Determine orientation | ||
| val isHorizontal = isImageHorizontal(imageBytes) | ||
|
|
||
| // Determine file extension from URL | ||
| val extension = when { | ||
| imageUrl.contains(".png", ignoreCase = true) -> ".png" | ||
| imageUrl.contains(".jpg", ignoreCase = true) -> ".jpg" | ||
| imageUrl.contains(".jpeg", ignoreCase = true) -> ".jpg" | ||
| imageUrl.contains(".webp", ignoreCase = true) -> ".webp" | ||
| else -> ".png" // Default to PNG | ||
| } | ||
|
|
||
| val outputFile = File(gameFolder, "$fileName$extension") | ||
|
|
||
| // Save to file | ||
| FileOutputStream(outputFile).use { it.write(imageBytes) } | ||
|
|
||
| Timber.tag("SteamGridDB").i("Saved image to ${outputFile.absolutePath} (horizontal: $isHorizontal)") | ||
| return@withContext Pair(outputFile.absolutePath, isHorizontal) | ||
| } catch (e: Exception) { | ||
| Timber.tag("SteamGridDB").e(e, "Error downloading image from $imageUrl") | ||
| return@withContext null | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Fetch grids for a game, find horizontal and vertical images, and save them separately. | ||
| * Fetch grids for a game, find horizontal, vertical, and header-sized images, and save them separately. | ||
| * @param gameId The SteamGridDB game ID | ||
| * @param gameFolder The folder where images should be saved | ||
| * @return Pair of (heroPath, capsulePath) where hero is horizontal grid and capsule is vertical grid | ||
| * @return Triple of (heroPath, capsulePath, headerPath) where hero is horizontal grid, capsule is vertical grid, and header is 460x215 image | ||
| */ |
There was a problem hiding this comment.
Critical: Header fetching via fetchAndSaveImage is broken for imageType = "header"
fetchGameImages now does:
val headerPath = headerPathFromGrids ?: if (!existingHeader) {
fetchAndSaveImage(searchResult.gameId, gameFolder, "header")
} else { ... }But fetchAndSaveImage only handles "hero" and "logo":
val endpoint = when (imageType) {
"hero" -> HEROES_ENDPOINT
"logo" -> LOGOS_ENDPOINT
else -> {
Timber.w("Unknown image type: $imageType (grids should use fetchGrids)")
return@withContext null
}
}So the "header" call always hits the else branch, logs an “Unknown image type: header” warning, and returns null. As a result, header images will never be fetched from the heroes endpoint when grids don’t supply one, and existingHeader will stay false until a header is created some other way.
A minimal fix is to teach fetchAndSaveImage about "header" and name the file consistently:
- try {
- val endpoint = when (imageType) {
- "hero" -> HEROES_ENDPOINT
- "logo" -> LOGOS_ENDPOINT
- else -> {
- Timber.tag("SteamGridDB").w("Unknown image type: $imageType (grids should use fetchGrids)")
- return@withContext null
- }
- }
+ try {
+ val endpoint = when (imageType) {
+ "hero" -> HEROES_ENDPOINT
+ "header" -> HEROES_ENDPOINT
+ "logo" -> LOGOS_ENDPOINT
+ else -> {
+ Timber.tag("SteamGridDB").w("Unknown image type: $imageType (grids should use fetchGrids)")
+ return@withContext null
+ }
+ }
@@
- val fileName = "steamgriddb_${imageType}$extension"
+ val fileName = when (imageType) {
+ "header" -> "steamgriddb_header$extension"
+ "hero" -> "steamgriddb_hero$extension"
+ else -> "steamgriddb_${imageType}$extension"
+ }This preserves backwards compatibility with any existing steamgriddb_hero* files while ensuring new headers are stored under the steamgriddb_header* prefix expected by the existingHeader checks and findSteamGridDBImage.
Also applies to: 182-281, 392-526
There was a problem hiding this comment.
@Maaggel, I think there may be some confusion here. The issue CodeRabbit raised isn't about gameId or appId extraction—it's about the imageType parameter handling.
The problem is that in fetchGameImages(), you're calling:
fetchAndSaveImage(searchResult.gameId, gameFolder, "header")But inside fetchAndSaveImage(), the imageType switch statement only handles "hero" and "logo":
val endpoint = when (imageType) {
"hero" -> HEROES_ENDPOINT
"logo" -> LOGOS_ENDPOINT
else -> {
Timber.tag("SteamGridDB").w("Unknown image type: $imageType (grids should use fetchGrids)")
return@withContext null
}
}So when you pass "header" as the imageType, it will hit the else branch, log a warning "Unknown image type: header", and return null. This means header images will never be fetched via fetchAndSaveImage() when grids don't provide one.
Would you like to apply the fix CodeRabbit suggested—adding "header" to the switch statement and mapping it to HEROES_ENDPOINT?
🧠 Learnings used
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.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt (1)
2022-2034: Bug: LibraryItem still missinggameId– custom media previews will fail.This issue was flagged in the previous review but remains unresolved. The
LibraryItemis constructed with onlyappIdandgameSource, leavinggameIdat its default value. BecauseCustomMediaUtilshelpers (likesaveCustomLogo,hasCustomLogo) key custom media bygameId: Int, andGameImageUtils.getGameImagerelies onlibraryItem.gameId, the previews won't display custom media that was just saved.Apply this diff to include
gameIdand ensure previews refresh correctly:- val libraryItem = remember(appId) { - if (appId != null && appId.isNotEmpty()) { + val libraryItem = remember(appId, gameId) { + if (appId != null && appId.isNotEmpty() && gameId != null) { val gameSource = if (appId.startsWith("CUSTOM_GAME_")) { GameSource.CUSTOM_GAME } else { GameSource.STEAM } LibraryItem( appId = appId, + gameId = gameId, gameSource = gameSource ) } else null }
🧹 Nitpick comments (1)
app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt (1)
2037-2045: Consider including media URLs in remember keys for completeness.The
rememberblocks for media models currently key on(mediaVersion, libraryItem). While this covers the primary update triggers, thesteamUrlparameter (e.g.,mediaLogoUrl) is not included. If the parent passes different URLs during the dialog's lifetime, the preview won't update untilmediaVersionorlibraryItemchanges.For example, for the logo model:
- val currentLogoModel: Any? = remember(mediaVersion, libraryItem) { + val currentLogoModel: Any? = remember(mediaVersion, libraryItem, mediaLogoUrl) { if (libraryItem != null) { GameImageUtils.getGameImage( libraryItem = libraryItem, imageType = "logo", steamUrl = mediaLogoUrl ) } else mediaLogoUrl }Apply similar changes for icon, header, capsule, and hero models with their respective URL parameters.
Also applies to: 2064-2072, 2091-2099, 2118-2126, 2151-2159
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt(9 hunks)app/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.kt(8 hunks)app/src/main/java/app/gamenative/ui/screen/library/appscreen/CustomGameAppScreen.kt(6 hunks)app/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.kt(5 hunks)app/src/main/res/values/strings.xml(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- app/src/main/java/app/gamenative/ui/screen/library/appscreen/CustomGameAppScreen.kt
- app/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.kt
- app/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.kt
🔇 Additional comments (4)
app/src/main/res/values/strings.xml (2)
93-94: String additions look good; review pending on media duplicates.The new strings for custom game deletion feedback (
custom_game_deleted,custom_game_delete_failed) are appropriately named and formatted. The update toadd_custom_game_dialog_messageeffectively communicates to users that images can be fetched later. Thebase_app_images_fetchingstring aligns well with the LoadingToast feature.Once the media string duplication concern is resolved, these changes will be complete.
Also applies to: 278-278, 282-282
420-441: The repository clone failed. Let me analyze the information already provided in the review comment to determine the status of this issue.Looking at the review comment structure:
Provided code context: The snippet labeled "Snippet showing the final state of code at these lines" (lines 420-441) shows a single media management block with 20 string resources.
AI summary claim: The comment states the AI summary "indicates media management strings were added in two insertion points, resulting in duplicate entries" - but this summary text is not actually shown in the provided context.
Critical observation: The code snippet is explicitly labeled as showing the "final state" at lines 420-441. If duplicates existed in the final state, they would appear in this excerpt or immediately before/after it.
Based on the evidence available in the review comment itself:
No duplicate media string keys are visible in the provided final-state code excerpt. The media management block (lines 420-441) contains each key name exactly once. The strings are well-formed and complete.
The inconsistency between the AI summary's claim and the provided code suggests either:
- The AI summary was incorrect or mischaracterized the changes
- Duplicates were present in an earlier state but have been corrected
- The summary refers to a different aspect not captured in the code excerpt
The duplicate media management strings concern cannot be verified from the available code context, but no duplicates are visible in the final-state code excerpt provided.
The review comment's core verification request cannot be fully satisfied without access to the complete file or git diff. The provided excerpt alone shows well-formed, non-duplicate string resources. If actual duplicates exist elsewhere in the file, they would need to be identified through direct file inspection.
app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt (2)
143-243: MediaSection implementation looks solid.The composable properly handles nullable
gameId, includes cache-busting viamediaVersion, and provides appropriate user feedback with Toast messages. The null guard at line 205 correctly protects all operations requiring a validgameId.
254-260: LGTM: Backward-compatible signature extension.The new optional parameters for media URLs,
gameId, andappIdare properly defaulted to null, maintaining backward compatibility while enabling the Media tab functionality.
There was a problem hiding this comment.
Not for this PR to address, but I think it'd be really good to pull the components of this dialog out as it's getting absolutely massive
| if (appId != null && appId.isNotEmpty()) { | ||
| val gameSource = if (appId.startsWith("CUSTOM_GAME_")) { | ||
| GameSource.CUSTOM_GAME | ||
| } else { | ||
| GameSource.STEAM | ||
| } |
There was a problem hiding this comment.
We'll need to find a nicer way of doing this once we bring GOG support
|
|
||
| // Small delay to ensure file system updates are complete | ||
| // before navigating back (list will auto-refresh when displayed) | ||
| delay(250) |
There was a problem hiding this comment.
we don't want to use these delays. you should emit app events instead.
| if (progress < 0) { | ||
| // Show spinner for indeterminate progress | ||
| CircularProgressIndicator() | ||
| Spacer(modifier = Modifier.height(16.dp)) | ||
| } | ||
| Text(message) | ||
| Spacer(modifier = Modifier.height(16.dp)) | ||
| if (progress >= 0) { | ||
| Spacer(modifier = Modifier.height(16.dp)) |
3 participants
Custom Media Support & Image handeling improvements
This is an updated version of the custom media feature that is up to date with the current master branch.
Features
GameImageUtilsto handle image retrieval with priority: Custom media → SteamGridDB → Steam URLs.The extracted icon is now also named "gameicon.extracted.ico" instead of a dynamic name. This means we can skip scanning for an extracted icon.
Bug Fixes
strings.xmlfor proper internationalization.Technical Improvements
mediaVersionFlowto trigger UI refresh when custom media is updated or reset when done fetching from SteamGridDBFiles Changed
CustomMediaUtils.kt- Custom media management utilitiesGameImageUtils.kt- Centralized image loading logicImages.kt- Loading spinner size constraintsSteamAppScreen.kt- Uninstall navigation delayCustomGameAppScreen.kt- String resource migrationstrings.xml- Added new string resourcesVideo examples
Adding new custom game, showing the new text in the prompt, fetching SteamGridDB images and using custom media
Record_2025-11-25-12-47-12.1.mp4
Changing custom media on existing custom game
Record_2025-11-25-12-41-57.mp4
Using custom media on Steam game
Record_2025-11-25-13-10-00.mp4
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Localization
✏️ Tip: You can customize this high-level summary in your review settings.