Theme Engine #392
Theme Engine #392
Conversation
📝 WalkthroughWalkthroughThis PR introduces a comprehensive Android theme system enabling runtime discovery, loading, validation, and rendering of custom game UI layouts. It includes theme asset management, XML parsing with nested includes, variable resolution with orientation breakpoints, media asset resolution, and integration into the library screen with persistence, validation, and focus navigation support. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant MainApp as PluviaApp
participant ThemeManager as ThemeManager
participant ThemeLoader as ThemeLoader
participant ThemeValidator as ThemeValidator
participant ThemeXmlMapper as ThemeXmlMapper
participant PrefManager as PrefManager
User->>MainApp: App Launch
MainApp->>ThemeManager: init(context)
ThemeManager->>ThemeLoader: scanAllThemes()
ThemeLoader->>ThemeLoader: scanBuiltIn + scanUser + scanExternal
ThemeLoader-->>ThemeManager: themes: List[ThemeEntry]
ThemeManager->>PrefManager: activeThemeId?
ThemeManager->>ThemeLoader: load(selectedThemeDir)
ThemeLoader->>ThemeLoader: manifest.xml + variables.xml + theme.xml
ThemeLoader->>ThemeValidator: validate(tree)
ThemeValidator-->>ThemeLoader: ValidationResult
alt Valid
ThemeLoader->>ThemeXmlMapper: map(tree)
ThemeXmlMapper-->>ThemeLoader: ThemeDefinition
ThemeLoader-->>ThemeManager: activeTheme
ThemeManager->>PrefManager: persist(activeThemeId)
else Invalid
ThemeManager->>ThemeManager: fallback to default
end
MainApp-->>User: App Ready
sequenceDiagram
participant LibraryScreen as LibraryScreen
participant BreakpointResolver as BreakpointResolver
participant ThemeManager as ThemeManager
participant ThemedGameGrid as ThemedGameGrid
participant AssetResolver as AssetResolver
participant MediaSourceManager as MediaSourceManager
LibraryScreen->>BreakpointResolver: rememberResolvedVariables(baseVars, breakpoints)
BreakpointResolver->>BreakpointResolver: getOrientation + getScreenWidth
BreakpointResolver-->>LibraryScreen: resolvedVars: Map[String, String]
LibraryScreen->>ThemedGameGrid: render(items, layout, vars)
ThemedGameGrid->>AssetResolver: resolveImage(logical, fallbacks, themeRoot)
AssetResolver->>AssetResolver: cache lookup → resolve path → normalize URI
AssetResolver-->>ThemedGameGrid: image URI + errors
ThemedGameGrid->>MediaSourceManager: resolve(mediaSource, allowVideo, bindingResolver)
MediaSourceManager->>AssetResolver: resolveImage/Video
MediaSourceManager-->>ThemedGameGrid: ResolvedMedia (Image | Video)
ThemedGameGrid->>ThemedGameGrid: renderLayers(card) per item
ThemedGameGrid-->>LibraryScreen: Themed Grid UI
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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)
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: 18
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Fix all issues with AI Agents 🤖
In @app/build.gradle.kts:
- Around line 236-243: The project is using outdated dependencies for Coil and
Media3 referenced as libs.coil.svg and libs.bundles.media3; update
gradle/libs.versions.toml to set Coil to 3.3.0 and Media3 to 1.9.0 (leave
profileinstaller at 1.4.1), then sync the project so the
implementation(libs.coil.svg) and implementation(libs.bundles.media3) entries
pick up the new versions; after updating, run a Gradle sync/build (or ./gradlew
dependencies) to verify there are no dependency conflicts and adjust any
code/API changes if compilation errors appear.
In @app/src/main/assets/Themes/SwitchinItUp/theme.xml:
- Around line 142-152: The clock element with navigationId "clock" creates a
focus trap because it has no outgoing navigation; either remove navigationId
"clock" if it shouldn't be focusable, or add explicit escape routes by setting
navigateLeft="searchBtn" and navigateDown="carousel" on the systemTime element
so focus can move back to searchBtn or down to carousel.
In @app/src/main/baseline-prof.txt:
- Around line 1-136: Remove the invalid baseline entries referencing
PluviaActivity and PluviaMain** and instead add baseline rules for the missing
theme engine classes used on hot paths: include HSPL entries for
LayoutEngineKt;->**(**)** and the BindingContext interface
(BindingContext;->**(**)**), plus BreakpointResolverKt, VariableResolverKt,
SelectionEngineKt, StateEngineKt, SpatialFocusManagerKt, BindingKt,
BreakpointKt, EngineKt and other core model classes (e.g., Binding, Breakpoint,
Engine) under the app/gamenative/theme package using the existing HSPL
app/gamenative/theme/**;->**(**)** pattern or explicit
HSPLapp/gamenative/theme/<ClassName>Kt;->**(**)** entries so these symbols are
precompiled at install time.
In @app/src/main/java/app/gamenative/theme/io/ThemeLoader.kt:
- Line 150: In ThemeLoader.kt update the element name comparison that currently
uses node.name.equals("variables", ignoreCase = false) to perform a
case-insensitive match (ignoreCase = true) so it is consistent with other
validators (e.g., ThemeValidator) and will correctly recognize "variables"
regardless of casing; locate the check in the ThemeLoader class and change the
equals call to use ignoreCase = true.
In @app/src/main/java/app/gamenative/theme/media/AssetResolver.kt:
- Around line 170-177: The catch block for exceptions when resolving URIs
swallows the exception and loses context; update the catch (e: Exception) branch
in AssetResolver.kt to include the exception details in the fallback error path:
when File(uriOrPath) doesn't exist return AssetResult with an error that
includes e.message (or e.toString()) and a descriptive code (e.g.,
"RESOLVE_FAILED") and ensure existing successful fallback still returns
AssetResult(uri = toFileUri(f)); reference the catch (e: Exception) block,
uriOrPath, File, toFileUri, AssetResult and MediaError when making the change.
- Around line 161-163: Update the KDoc for AssetResolver's constructor parameter
remoteFetcher to clearly state that by default remote fetching is disabled (no
network), that callers may inject a custom RemoteFetcher implementation to
enable actual HTTP requests, and that leaving the default will cause remote URLs
to return AssetResult errors with code "REMOTE_UNAVAILABLE" (e.g., as produced
in the http/https branch of resolve where RemoteFetcher.exists is checked).
Include mention of the default behavior, how to provide a custom RemoteFetcher,
and the resulting error behavior so callers aren't surprised.
In @app/src/main/java/app/gamenative/theme/media/VideoController.kt:
- Around line 168-171: The companion object field touchCounter is shared across
all VideoController instances and ++touchCounter is not thread-safe; replace the
Long with a thread-safe AtomicLong in the VideoController companion (e.g.,
private val touchCounter = AtomicLong(0)) and update all uses of ++touchCounter
to touchCounter.incrementAndGet() when setting lastTouched so each instance gets
a monotonic, thread-safe recency value; leave lastTouched as before but ensure
it receives the AtomicLong increment result.
In @app/src/main/java/app/gamenative/theme/perf/PerfConfig.kt:
- Around line 110-117: getOrBuildLayerTree has the same race where multiple
threads can call build() for the same key; fix it with double-checked locking:
first read layerTreeCache[key] and return if present, otherwise enter a
synchronized block (use the same monitor as used elsewhere, e.g., the instance
lock or the lock used by getOrPutTemplateMeasure), re-check layerTreeCache[key]
inside the synchronized block, and only if still null call build(), call
putLayerTree(key, built) and return the stored value; reference
getOrBuildLayerTree, layerTreeCache, putLayerTree, and enabled when making the
change.
- Around line 91-98: getOrPutTemplateMeasure currently has a race where two
threads can see templateMeasureCache[key] missing and both call compute(); wrap
the check/compute/put with a synchronization (double-checked locking) so only
one thread computes and others read the cached value: first read
templateMeasureCache[key], if null enter a synchronized block (use the same lock
object used elsewhere in PerfConfig), re-check templateMeasureCache[key] inside
the lock, call compute() only if still null, then call putTemplateMeasure(key,
computed) and return the cached value; apply the identical double-checked
locking pattern to getOrBuildLayerTree to prevent duplicate builds.
In @app/src/main/java/app/gamenative/theme/runtime/layers/LayerRenderers.kt:
- Line 101: Move the MediaSourceManager out of per-layer creation so all layers
share one cache: instead of calling remember { MediaSourceManager() } inside the
per-layer scope in LayerRenderers.kt, instantiate and remember a single
MediaSourceManager in the parent composable (where layers are
collected/rendered) and pass that instance as a parameter into ImageLayerView
and VideoLayerView. This ensures the AssetResolver LRU cache (capacity 128)
inside MediaSourceManager is shared across layers and avoids cache
fragmentation; update the ImageLayerView and VideoLayerView function signatures
to accept a MediaSourceManager parameter and wire the new parent-scoped instance
through the layer-rendering call chain.
In @app/src/main/java/app/gamenative/theme/runtime/LayoutEngine.kt:
- Around line 256-262: The contentPadding currently set to 0.dp prevents
centering the first page; use the computed scaledItemWidth to set horizontal
contentPadding ≈ (parentWidth - scaledItemWidth) / 2 so pages snap to center.
Wrap the HorizontalPager in a BoxWithConstraints (or use
LocalConfiguration/LocalDensity to read parent width) to get the available width
at composition time, compute padding using scaledItemWidth (the value calculated
around line 225), then pass PaddingValues(horizontal = calculatedPadding) into
contentPadding; keep pageSpacing for inter-item gaps only and remove the inline
0.dp placeholder.
In @app/src/main/java/app/gamenative/theme/runtime/SelectionEngine.kt:
- Around line 209-226: The initial selectedIndex from readLastSelection in
rememberSelectionState can be out-of-bounds until LaunchedEffect runs; fix it by
coercing initialIndex when creating the initial State: compute val safeInitial =
(readLastSelection(containerKey) ?: 0).coerceIn(0, max(0, totalItems - 1)) and
use selectedIndex = safeInitial (and adjust firstVisibleIndex if needed) so the
initial State (constructed in remember(...){ mutableStateOf(State(...)) })
always has a valid selectedIndex.
In @app/src/main/java/app/gamenative/theme/runtime/ThemedGameGrid.kt:
- Around line 279-286: In ThemedGameGrid's itemsIndexed block, replace the
remember { itemFocusRequesters.getOrPut(index) { FocusRequester() } } pattern
because remember can create a new FocusRequester while the map still holds a
stale entry; instead retrieve/create the requester directly from the map (use
itemFocusRequesters.getOrPut(index) { FocusRequester() }) and assign it to
itemFocusRequester without wrapping in remember, keeping FocusRequester
lifecycle tied to the map (adjust keying if needed to use item.appId instead of
index to avoid stale entries when item order changes).
In @app/src/main/java/app/gamenative/theme/ThemeManager.kt:
- Around line 649-660: In copyAssetFile, calling
outFile.parentFile?.exists()/mkdirs() can NPE when parentFile is null; guard the
parent directory access by checking if outFile.parentFile is non-null before
calling exists() or mkdirs() (e.g., val parent = outFile.parentFile; if (parent
!= null && !parent.exists()) parent.mkdirs()), then proceed with
appCtx.assets.open(assetPath) and copy; keep the existing catch/Timbler logging
intact.
In @app/src/main/java/app/gamenative/theme/validate/ThemeValidator.kt:
- Around line 438-444: The conditional that sets severity for ValidationIssue
uses the same value for both branches causing all missing-asset reports to be
WARNING; update the ternary so that when warnMissing is true it uses
Severity.WARNING and when false it uses Severity.ERROR (i.e., change the else
branch from Severity.WARNING to Severity.ERROR) at the ValidationIssue
construction that references ValidationCode.ASSET_NOT_FOUND, warnMissing,
node.source and out.
In
@app/src/main/java/app/gamenative/ui/screen/library/appscreen/CustomGameAppScreen.kt:
- Around line 162-173: The lastPlayedText computation currently constructs
ContainerManager on every recomposition causing blocking I/O; change it to a
remembered mutable state (e.g., val lastPlayedTextState = remember {
mutableStateOf(context.getString(R.string.library_never_played)) }) and load the
real value inside a LaunchedEffect tied to libraryItem.appId using
withContext(Dispatchers.IO) to create ContainerManager, find the container and
update lastPlayedTextState.value; ensure you reference ContainerManager,
lastPlayedText (replace with lastPlayedTextState.value), and use the same
pattern as releaseDate and sizeOnDisk; also avoid the Int cast overflow by using
toLong() (or adapt SteamUtils.fromSteamTime to accept Long) when converting
container.lastPlayedTimestamp.
In @app/src/main/java/app/gamenative/ui/screen/library/LibraryScreen.kt:
- Around line 322-329: The remember block that builds containerTimestamps
swallows exceptions and returns an empty map; update the catch in the
ContainerManager(context).containers associate block to log the caught exception
(including the exception message/stack) before returning emptyMap() so failures
to load container.lastPlayedTimestamp are visible; reference the
ContainerManager, containers, container.id, container.lastPlayedTimestamp, and
containerTimestamps symbols and use the app's existing logging facility (e.g.,
Log or Timber) with a clear message identifying the LibraryScreen timestamp
load.
In @app/src/main/java/app/gamenative/ui/screen/settings/ThemePickerScreen.kt:
- Around line 37-40: Replace the empty catch in the LaunchedEffect that calls
ThemeManager.init(context) so initialization failures are not swallowed: change
the catch(_: Throwable) to catch(e: Throwable) and log the exception (e.g.,
Log.e or your project's logger) with a clear message like "ThemeManager.init
failed" and include the exception object; keep the call inside the same
LaunchedEffect to preserve idempotent init behavior and, if desired, forward
critical errors to your telemetry/analytics.
In @app/src/main/java/com/winlator/container/Container.java:
- Around line 913-919: When a container is launched, update its
lastPlayedTimestamp by calling
Container.setLastPlayedTimestamp(System.currentTimeMillis()) in the launch/play
code path (e.g., inside the method that starts or launches a container
instance), and then ensure the updated timestamp is persisted by invoking the
existing container save/serialize routine so the change is stored. This uses the
existing Container.getLastPlayedTimestamp/lastPlayedTimestamp field and the
setLastPlayedTimestamp setter to keep "sort by last played" accurate.
In @app/src/main/res/values-da/strings.xml:
- Line 324: Line 324 currently has multiple <string> elements merged together;
split them into separate XML <string name="..."> entries so each resource
(max_device_memory, frame_synchronization, use_adrenotools_turnip) is on its own
element and line, ensuring proper XML escaping and formatting; if the
frame_synchronization translation is missing, add the correct Danish translation
for the string named "frame_synchronization" (or verify and insert it), and
confirm encoding/closing tags are valid so the resources file parses cleanly.
In @app/src/main/res/values-de/strings.xml:
- Around line 869-907: The German strings file contains English placeholder
values for many keys (e.g., add_game, container_config_custom_resolution_title,
last_played, settings_theme_picker_title, theme_add_dialog_message,
theme_remove_message, theme_removed_toast, theme_validation_failed, etc.);
replace each English string with proper German translations for those keys or
remove/omit shipping this locale file for the experimental feature so the app
falls back to default English, and add a tracking TODO/issue referencing these
specific keys to ensure translations are completed before the feature leaves
experimental status.
In @app/src/main/res/values-fr/strings.xml:
- Around line 932-970: The added theme-related string resources (e.g.,
theme_activate, theme_active, theme_add_dialog_message, theme_add_external,
theme_added_toast, theme_applied_toast, theme_by_author, theme_fallback_toast,
theme_menu_button, theme_no_description, theme_remove, theme_remove_confirm,
theme_remove_message, theme_remove_title, theme_removed_toast,
theme_select_folder, theme_selector_title, theme_source_external,
theme_validation_failed, and the others like
settings_theme_picker_title/settings_theme_reload_dev/settings_theme_source_builtin/settings_theme_source_user/settings_use_theme_engine_ui_title/settings_use_theme_engine_ui_subtitle)
are still English placeholders; replace each value with the correct French
translation so the theme UI and related dialogs are localized consistently
(update the string values in strings.xml to French for all 38 entries,
preserving placeholders like %1$s and escape sequences like \n and \' where
present).
In @app/src/main/res/values-uk/strings.xml:
- Around line 575-949: Ukrainian locale contains ~375 English placeholder
strings (e.g. theme_add_dialog_message, custom_games_title,
wine_proton_install_success) that need translation; open a tracking issue that
lists all missing string resource names, links to this PR, and outlines the
localization workflow (preferred translator/contact, target branch, and
deadline), then add a short note in this PR referencing that issue; optionally
attach an exported list (CSV/JSON) of the resource keys from
values-uk/strings.xml to the issue to make translation work actionable.
In @app/src/main/res/values-zh-rTW/strings.xml:
- Line 472: The two string resources "max_device_memory" and
"use_adrenotools_turnip" are on the same line; split them so each <string>
element is on its own line (preserving surrounding indentation and XML
formatting) so that <string name="max_device_memory">最大內存容量</string> and <string
name="use_adrenotools_turnip">Use Adrenotools Turnip</string> appear as separate
lines for readability and cleaner diffs.
In @app/src/test/java/app/gamenative/theme/media/MediaResolverTest.kt:
- Around line 21-23: Replace hard-coded Windows backslashes in test file
MediaResolverTest.kt (e.g., the File constructor call creating img with
"images\\card.png") with a platform-neutral path: use File(theme, "images" +
File.separator + "card.png") or use forward slashes ("images/card.png") which
Java normalizes; apply the same change to the other occurrences referenced in
the review (lines creating files at lines 42, 61, 84, 106, and 107) so tests run
on Unix CI as well as Windows.
In @app/src/test/java/app/gamenative/theme/ThemeLoaderSmokeTest.kt:
- Around line 29-38: The test creates a File using a Windows-style path
("sections\\grid.xml"), which breaks on Unix CI; update the File construction in
ThemeLoaderSmokeTest (the variable split/File(themeDir, ...)) to use a
platform-independent separator (e.g., build the path with File.separator between
"sections" and "grid.xml" or simply use "sections/grid.xml") so
parentFile.mkdirs() and split.writeText(...) work cross-platform.
In @gradle/libs.versions.toml:
- Line 22: Update dependency versions and artifact IDs: change the coil entry
(currently coil = "2.7.0") to use Coil 3.3.0 and the new artifact name
(io.coil-kt:coil3) wherever referenced; bump all Media3 artifacts from 1.5.1 to
1.9.0 (look for entries like media3-*, media3-exoplayer, media3-ui) and update
their version key in libs.versions.toml and any alias entries; ensure
constraintlayoutComposeAndroid is set to 1.1.1 and profileinstaller to 1.4.1 per
the comment; apply the same version/artifact name changes in other occurrences
mentioned (lines referenced 42-43, 81, 112-118) so all aliases and module
coordinates are consistent.
♻️ Duplicate comments (1)
app/src/main/assets/Themes/Showcase/manifest.xml (1)
6-6: Same verification needed as SquareGrid manifest.This manifest follows the same pattern as SquareGrid with
minAppVersion="0.0.0"and leading-slash path formats. The verification requested for SquareGrid (lines 6, 12-13) applies here as well.Also applies to: 12-13
🟡 Minor comments (17)
app/src/main/res/values-fr/strings.xml-932-970 (1)
932-970: French translations missing for theme-related strings.All 38 newly added theme-related string resources are English placeholders rather than French translations. French-speaking users will see English text in the theme selection UI, settings, and related dialogs, creating an inconsistent localization experience.
Consider adding proper French translations for these strings to maintain localization quality across the application.
app/src/main/res/values-de/strings.xml-869-907 (1)
869-907: English placeholders in German locale file should be translated before release.These 39 new string resources are in English rather than German, which will result in German-speaking users seeing English text for theme-related UI elements. While acknowledged as placeholders and the feature is experimental, this impacts the user experience for the German locale.
Consider:
- Adding a tracking issue or TODO to ensure these get translated before the feature exits experimental status
- Alternatively, omitting the German strings file for this feature until translations are ready (the app will fall back to default English strings)
- Verifying translation coverage for other locale files as well
app/src/main/res/values-uk/strings.xml-575-949 (1)
575-949: Track Ukrainian translations for new theme engine and feature strings.The Ukrainian locale file contains 375 English placeholder strings for the new theme engine, controller bindings, custom games, and Wine/Proton manager features. While this is acceptable for an experimental feature rollout, Ukrainian-locale users will encounter untranslated UI text.
Do you want me to open a tracking issue for Ukrainian localization of these strings, or is there already a localization workflow in place for this PR?
app/src/main/res/values-zh-rCN/strings.xml-923-966 (1)
923-966: Track translation work for English placeholders.The 44 new string resources added here are English placeholders in the Chinese Simplified locale file. While this is acceptable for an experimental feature during active development, Chinese users who enable the Theme Engine will see English text for theme-related UI elements.
Consider tracking a follow-up task to translate these strings before the Theme Engine feature exits experimental status or becomes more widely adopted.
Would you like me to generate a checklist of the untranslated string keys to track this translation work?
app/src/main/java/app/gamenative/theme/runtime/SelectionEngine.kt-209-226 (1)
209-226: Initial state may briefly have out-of-boundsselectedIndex.The
initialIndexread fromreadLastSelection(line 215) could exceedtotalItemsif the persisted value is stale (e.g., after items were removed). This creates a brief window between initial composition andLaunchedEffectexecution wherestate.value.selectedIndex >= totalItems.While
visibleRange()handles this gracefully viacoerceIn, direct access toselectedIndexbefore the effect runs could cause issues in consuming code.🔎 Proposed fix: coerce initialIndex at creation
@Composable fun rememberSelectionState( containerKey: String, totalItems: Int, config: Config, ): MutableState<State> { - val initialIndex = readLastSelection(containerKey) ?: 0 + val initialIndex = (readLastSelection(containerKey) ?: 0).coerceIn(0, max(0, totalItems - 1)) val state = remember(containerKey, totalItems, config) { mutableStateOf(State(totalItems, config, containerKey, selectedIndex = initialIndex, firstVisibleIndex = 0)) }app/src/main/java/app/gamenative/theme/media/VideoController.kt-168-171 (1)
168-171: StatictouchCountermay cause ordering issues with multiple instances.
touchCounteris a companion object variable shared across allVideoControllerinstances. If multiple controllers exist,lastTouchedvalues could interleave unexpectedly. Also,++touchCounteris not thread-safe.Potential fix using AtomicLong
companion object { - // Monotonic counter for recency ordering (no time source dependency). - private var touchCounter: Long = 0 + // Monotonic counter for recency ordering (no time source dependency). + private val touchCounter = java.util.concurrent.atomic.AtomicLong(0) } private class SlotInfo(val key: String) { // ... - fun touch() { lastTouched = ++touchCounter } + fun touch() { lastTouched = touchCounter.incrementAndGet() } }Committable suggestion skipped: line range outside the PR's diff.
app/src/main/java/app/gamenative/ui/screen/settings/ThemePickerScreen.kt-37-40 (1)
37-40: Empty catch block silently swallows initialization errors.While
ThemeManager.initis documented as idempotent, catching allThrowableand ignoring it could hide legitimate initialization failures (e.g., IO errors, corrupt theme files). Consider at minimum logging the exception.🔎 Proposed fix
LaunchedEffect(Unit) { - try { ThemeManager.init(context) } catch (_: Throwable) { } + try { + ThemeManager.init(context) + } catch (e: IllegalStateException) { + // Already initialized - expected, ignore + } catch (e: Throwable) { + timber.log.Timber.w(e, "ThemeManager init failed in dialog") + } }app/src/main/java/app/gamenative/theme/runtime/layers/LayerRenderers.kt-101-101 (1)
101-101: Move MediaSourceManager to parent scope to avoid cache fragmentation.Creating a new
MediaSourceManager()instance per layer defeats caching.AssetResolvermaintains an LRU image cache (capacity 128), but each layer gets an isolated cache instance. If multiple layers resolve the same images, the cache won't help and resolution work is duplicated. MoveMediaSourceManagerto the parent composable scope and pass it down toImageLayerViewandVideoLayerViewso all layers share a single cache.app/src/main/java/app/gamenative/theme/validate/ThemeValidator.kt-438-444 (1)
438-444: Possible copy-paste error: both branches useSeverity.WARNING.The ternary expression
if (warnMissing) Severity.WARNING else Severity.WARNINGalways evaluates toWARNING. Based on the method's logic, missing assets for required attributes (likesrc) should likely beERRORwhile optional attributes (likefallback) should beWARNING.🔎 Proposed fix
out += ValidationIssue( ValidationCode.ASSET_NOT_FOUND, - if (warnMissing) Severity.WARNING else Severity.WARNING, + if (warnMissing) Severity.WARNING else Severity.ERROR, "Asset not found: '${value}'.", node.source )app/src/main/java/app/gamenative/ui/screen/library/LibraryScreen.kt-322-329 (1)
322-329: Log swallowed exception for debugging.The exception is caught and silently returns an empty map. While this graceful fallback is appropriate, logging the exception would help with debugging issues related to container timestamp loading.
🔎 Proposed fix
val containerTimestamps: Map<String, Long> = remember { try { ContainerManager(context).containers .associate { container -> container.id to container.lastPlayedTimestamp } } catch (e: Exception) { + android.util.Log.w("LibraryScreen", "Failed to load container timestamps", e) emptyMap() } }app/src/main/java/app/gamenative/theme/io/ThemeLoader.kt-150-150 (1)
150-150: Inconsistent case sensitivity for element name matching.The check for
"variables"usesignoreCase = false, but other element checks in this codebase (like in ThemeValidator) useignoreCase = true. This could cause issues if theme authors use different casing.🔎 Proposed fix
- if (node.name.equals("variables", ignoreCase = false)) { + if (node.name.equals("variables", ignoreCase = true)) {app/src/main/java/app/gamenative/theme/io/ThemeXmlMapper.kt-781-781 (1)
781-781: Layout structure validation is missing fromThemeValidator; error() calls are necessary defensive checks, not optional.
ThemeValidator.validate()is indeed called beforeThemeXmlMapper.map()in the initial load (line 561 before 585), and blocking errors prevent mapping. The remapping flow (line 696) safely reuses an already-validatedactiveThemeTree.However,
ThemeValidatordoes not validate layout structure—it doesn't check for missing<layout>elements or missing/unknown layout nodes (canvas/grid/carousel). Theerror()calls at lines 781, 859, 868, 872, and 877 are therefore necessary defensive checks, not redundant assertions. A theme with a malformed layout will pass validation but crash during mapping withIllegalStateException.Consider:
- Adding layout structure validation to
ThemeValidator(preferred for consistency), or- Improving error handling in
ThemeXmlMapperto return aResult<ThemeDefinition>instead of throwingapp/src/main/java/app/gamenative/theme/media/AssetResolver.kt-170-177 (1)
170-177: Log or include exception details for debugging.The exception is caught and swallowed without preserving any context. While the fallback behavior (treating it as a file path) is reasonable, losing the exception information can make debugging difficult when path resolution fails unexpectedly.
🔎 Proposed fix to preserve exception context
- } catch (e: Exception) { - // Fallback: assume it's a file path + } catch (_: Exception) { + // URI parsing failed; fallback to treating as file path val f = File(uriOrPath) if (f.exists()) AssetResult(uri = toFileUri(f)) - else AssetResult(uri = null, errors = listOf(MediaError("FILE_NOT_FOUND", "File not found: ${f.absolutePath}"))) + else AssetResult(uri = null, errors = listOf(MediaError("FILE_NOT_FOUND", "Could not resolve path: $uriOrPath"))) }Alternatively, if you want to preserve the exception message:
} catch (e: Exception) { val f = File(uriOrPath) if (f.exists()) AssetResult(uri = toFileUri(f)) else AssetResult(uri = null, errors = listOf( MediaError("RESOLVE_FAILED", "Failed to resolve '$uriOrPath': ${e.message}") )) }Committable suggestion skipped: line range outside the PR's diff.
app/src/main/java/app/gamenative/theme/media/AssetResolver.kt-161-163 (1)
161-163: DocumentremoteFetcherconstructor parameter and its implications.The
RemoteFetcherinterface is documented with "Default: false (no network)", but this behavior isn't explained in theAssetResolverconstructor's KDoc. Developers instantiatingAssetResolverwithout understanding that remote URLs will fail silently could be surprised. Add documentation to the constructor parameter explaining: (1) the default disables remote fetching, (2) a customRemoteFetchercan be injected for actual HTTP requests, and (3) this causesREMOTE_UNAVAILABLEerrors if no custom fetcher is provided.app/src/main/java/app/gamenative/theme/runtime/LayoutEngine.kt-256-262 (1)
256-262: The contentPadding = 0.dp implementation for center-focused carousel appears incomplete.The code sets
contentPaddingto0.dpwith a comment indicating it "will be handled by pageSpacing and item alignment," but neitherpageSpacing(which only affects inter-item gaps) nor theColumn's horizontal alignment handle centering the first item in the viewport. Note thatscaledItemWidthis calculated on line 225 but never used in the layout or padding calculations.For proper center-focus carousel behavior where items snap to center,
contentPaddingtypically needs to be calculated as approximately(screenWidth - itemWidth) / 2. This would require wrapping theHorizontalPagerin aBoxWithConstraintsto access the parent width at composition time.app/src/main/java/app/gamenative/theme/ThemeManager.kt-649-660 (1)
649-660: Potential NPE ifoutFile.parentFileis null.
outFile.parentFilecan returnnullfor root paths. Callingexists()ormkdirs()on null will throw an NPE.🔎 Proposed fix
private fun copyAssetFile(assetPath: String, outFile: File) { try { - if (!outFile.parentFile.exists()) outFile.parentFile.mkdirs() + outFile.parentFile?.let { parent -> + if (!parent.exists()) parent.mkdirs() + } appCtx.assets.open(assetPath).use { input ->app/src/main/java/app/gamenative/theme/runtime/ThemedGameGrid.kt-279-286 (1)
279-286: Focus requester creation in itemsIndexed could cause issues.Using
remember { itemFocusRequesters.getOrPut(index) { FocusRequester() } }insideitemsIndexedis problematic. When items are recomposed (e.g., after scrolling and returning), therememberblock may create a newFocusRequesterwhile the old one still exists in the map, or the map entry may be stale.Consider moving the focus requester retrieval outside
remembersince the map already provides persistence:- val itemFocusRequester = remember { - itemFocusRequesters.getOrPut(index) { FocusRequester() } - } + val itemFocusRequester = itemFocusRequesters.getOrPut(index) { FocusRequester() }
| implementation(libs.coil.svg) | ||
| debugImplementation(libs.androidx.ui.tooling) | ||
|
|
||
| // Media3 (ExoPlayer) for video playback | ||
| implementation(libs.bundles.media3) | ||
|
|
||
| // Baseline Profiles - enables precompilation of critical code paths | ||
| implementation(libs.profileinstaller) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find and display the version catalog
find . -name "libs.versions.toml" -type f | head -5Repository: utkarshdalal/GameNative
Length of output: 94
🌐 Web query:
What are the latest stable versions of Coil SVG, AndroidX Media3, and AndroidX ProfileInstaller as of January 2026?
💡 Result:
- Coil SVG: 3.3.0 (stable; documented usage shows implementation("io.coil-kt.coil3:coil-svg:3.3.0") — May 2025 release metadata). [1][2]
- AndroidX Media3: 1.9.0 (stable; released December 17, 2025). [3]
- AndroidX ProfileInstaller: 1.4.1 (stable; released October 2, 2024). [4]
Sources:
[1] Coil SVG docs (usage).
[2] Maven listing for coil-svg.
[3] AndroidX Media3 release notes (android.com).
[4] AndroidX ProfileInstaller release notes (android.com).
🏁 Script executed:
cat -n ./gradle/libs.versions.toml | grep -A 5 -B 2 -E "(coil|media3|profileinstaller)"Repository: utkarshdalal/GameNative
Length of output: 3607
Update Coil and Media3 to current versions.
The version catalog shows outdated dependency versions:
- Coil is at 2.7.0; latest stable is 3.3.0
- Media3 is at 1.5.1; latest stable is 1.9.0 (released December 2025)
- ProfileInstaller is at 1.4.1 (current)
Update gradle/libs.versions.toml to use the latest stable releases for Coil (3.3.0) and Media3 (1.9.0) to ensure current security patches and feature availability.
🤖 Prompt for AI Agents
In @app/build.gradle.kts around lines 236-243, The project is using outdated
dependencies for Coil and Media3 referenced as libs.coil.svg and
libs.bundles.media3; update gradle/libs.versions.toml to set Coil to 3.3.0 and
Media3 to 1.9.0 (leave profileinstaller at 1.4.1), then sync the project so the
implementation(libs.coil.svg) and implementation(libs.bundles.media3) entries
pick up the new versions; after updating, run a Gradle sync/build (or ./gradlew
dependencies) to verify there are no dependency conflicts and adjust any
code/API changes if compilation errors appear.
| # Baseline Profile Rules for GameNative | ||
| # These rules tell the Android Runtime (ART) which code paths should be | ||
| # precompiled at install time for better startup and runtime performance. | ||
| # | ||
| # Format: HSPLclass_name;method_name(parameters)return_type | ||
| # H = Hot (frequently called), S = Startup, P = Post-startup, L = Class | ||
| # | ||
| # Reference: https://developer.android.com/topic/performance/baselineprofiles | ||
|
|
||
| # ============================================================================= | ||
| # Theme Runtime - Carousel Rendering (Critical for smooth scrolling) | ||
| # ============================================================================= | ||
|
|
||
| # ThemedGameGrid - Main carousel/grid rendering | ||
| HSPLapp/gamenative/theme/runtime/ThemedGameGridKt;->**(**)** | ||
| HSPLapp/gamenative/theme/runtime/ThemedGameGrid_**;->**(**)** | ||
|
|
||
| # Theme utilities used during rendering | ||
| HSPLapp/gamenative/theme/runtime/ThemeUtils;->**(**)** | ||
| HSPLapp/gamenative/theme/runtime/ThemeUtilsKt;->**(**)** | ||
|
|
||
| # Layer renderers for card content | ||
| HSPLapp/gamenative/theme/runtime/layers/LayerRenderersKt;->**(**)** | ||
| HSPLapp/gamenative/theme/runtime/SharedElementRenderers;->**(**)** | ||
|
|
||
| # Fixed element rendering | ||
| HSPLapp/gamenative/theme/runtime/FixedElementRendererKt;->**(**)** | ||
|
|
||
| # Binding context for data resolution | ||
| HSPLapp/gamenative/theme/runtime/BindingContext;->**(**)** | ||
| HSPLapp/gamenative/theme/runtime/BindingContextKt;->**(**)** | ||
|
|
||
| # ============================================================================= | ||
| # Theme Model Classes | ||
| # ============================================================================= | ||
|
|
||
| # Layout models | ||
| HSPLapp/gamenative/theme/model/Layout**;->**(**)** | ||
| HSPLapp/gamenative/theme/model/Layers**;->**(**)** | ||
| HSPLapp/gamenative/theme/model/Fixed**;->**(**)** | ||
| HSPLapp/gamenative/theme/model/Card**;->**(**)** | ||
| HSPLapp/gamenative/theme/model/Dimension**;->**(**)** | ||
|
|
||
| # ============================================================================= | ||
| # Library Screen Components | ||
| # ============================================================================= | ||
|
|
||
| # Main library screen | ||
| HSPLapp/gamenative/ui/screen/library/LibraryScreenKt;->**(**)** | ||
| HSPLapp/gamenative/ui/screen/library/LibraryListPaneKt;->**(**)** | ||
|
|
||
| # Library components | ||
| HSPLapp/gamenative/ui/screen/library/components/**;->**(**)** | ||
|
|
||
| # ============================================================================= | ||
| # Compose Foundation - Pager (HorizontalPager for carousels) | ||
| # ============================================================================= | ||
|
|
||
| # Pager core | ||
| HSPLandroidx/compose/foundation/pager/**;->**(**)** | ||
|
|
||
| # Lazy layout infrastructure (used by pager) | ||
| HSPLandroidx/compose/foundation/lazy/layout/**;->**(**)** | ||
|
|
||
| # Gestures for scrolling | ||
| HSPLandroidx/compose/foundation/gestures/**;->**(**)** | ||
|
|
||
| # ============================================================================= | ||
| # Compose Animation (Smooth transitions) | ||
| # ============================================================================= | ||
|
|
||
| HSPLandroidx/compose/animation/core/**;->**(**)** | ||
| HSPLandroidx/compose/animation/**;->**(**)** | ||
|
|
||
| # ============================================================================= | ||
| # Compose UI Graphics Layer (Scale, alpha, offset transforms) | ||
| # ============================================================================= | ||
|
|
||
| HSPLandroidx/compose/ui/graphics/**;->**(**)** | ||
| HSPLandroidx/compose/ui/draw/**;->**(**)** | ||
|
|
||
| # Layout and measurement | ||
| HSPLandroidx/compose/ui/layout/**;->**(**)** | ||
|
|
||
| # Modifier chains | ||
| HSPLandroidx/compose/ui/Modifier**;->**(**)** | ||
|
|
||
| # ============================================================================= | ||
| # Image Loading (Coil - for game covers) | ||
| # ============================================================================= | ||
|
|
||
| HSPLcoil/**;->**(**)** | ||
| HSPLcom/skydoves/landscapist/**;->**(**)** | ||
|
|
||
| # ============================================================================= | ||
| # Compose Material3 Components | ||
| # ============================================================================= | ||
|
|
||
| HSPLandroidx/compose/material3/**;->**(**)** | ||
|
|
||
| # ============================================================================= | ||
| # Kotlin Coroutines (Async operations during scrolling) | ||
| # ============================================================================= | ||
|
|
||
| HSPLkotlinx/coroutines/**;->**(**)** | ||
|
|
||
| # ============================================================================= | ||
| # Data Classes and ViewModels | ||
| # ============================================================================= | ||
|
|
||
| HSPLapp/gamenative/data/LibraryItem;->**(**)** | ||
| HSPLapp/gamenative/data/LibraryItem**;->**(**)** | ||
| HSPLapp/gamenative/ui/data/LibraryState;->**(**)** | ||
| HSPLapp/gamenative/ui/model/LibraryViewModel;->**(**)** | ||
|
|
||
| # ============================================================================= | ||
| # Theme Manager and Parser | ||
| # ============================================================================= | ||
|
|
||
| HSPLapp/gamenative/theme/ThemeManager;->**(**)** | ||
| HSPLapp/gamenative/theme/io/ThemeXmlMapper;->**(**)** | ||
| HSPLapp/gamenative/theme/io/ThemeXmlMapperKt;->**(**)** | ||
|
|
||
| # ============================================================================= | ||
| # Startup Classes | ||
| # ============================================================================= | ||
|
|
||
| # Application class | ||
| HSPLapp/gamenative/PluviaApp;->**(**)** | ||
|
|
||
| # Main Activity | ||
| HSPLapp/gamenative/PluviaActivity;->**(**)** | ||
| HSPLapp/gamenative/MainActivity;->**(**)** | ||
|
|
||
| # Navigation | ||
| HSPLapp/gamenative/ui/PluviaMain**;->**(**)** |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find actual kotlin source files for theme classes
find app/src/main/kotlin -type f -name "*.kt" | grep -E "(Theme|Library|Binding|Layer|Renderer)" | head -20Repository: utkarshdalal/GameNative
Length of output: 121
🏁 Script executed:
# Check the actual package structure for theme
find app/src/main/kotlin -type d -path "*theme*" | head -20Repository: utkarshdalal/GameNative
Length of output: 121
🏁 Script executed:
# Search for ThemedGameGrid class definition
rg "class ThemedGameGrid" -t kotlin --max-count 5Repository: utkarshdalal/GameNative
Length of output: 49
🏁 Script executed:
# Search for BindingContext class
rg "class BindingContext|object BindingContext" -t kotlin --max-count 5Repository: utkarshdalal/GameNative
Length of output: 49
🏁 Script executed:
# Check if there are any inner classes matching ThemedGameGrid_ pattern
rg "class ThemedGameGrid_" -t kotlin --max-count 5Repository: utkarshdalal/GameNative
Length of output: 49
🏁 Script executed:
# List the repository structure at the root
ls -la | head -30Repository: utkarshdalal/GameNative
Length of output: 1729
🏁 Script executed:
# Find all .kt files in the repository
find . -name "*.kt" -type f | head -20Repository: utkarshdalal/GameNative
Length of output: 1205
🏁 Script executed:
# Locate the baseline-prof.txt file and check its path
find . -name "baseline-prof.txt" -type fRepository: utkarshdalal/GameNative
Length of output: 99
🏁 Script executed:
# Check the structure of app directory
find ./app -type d | head -20Repository: utkarshdalal/GameNative
Length of output: 727
🏁 Script executed:
# Search for theme runtime classes
find app/src/main/java -type f -name "*.kt" | xargs grep -l "class ThemedGameGrid\|fun ThemedGameGrid" 2>/dev/null | head -5Repository: utkarshdalal/GameNative
Length of output: 131
🏁 Script executed:
# Search for BindingContext class
find app/src/main/java -type f -name "*.kt" | xargs grep -l "class BindingContext\|object BindingContext" 2>/dev/nullRepository: utkarshdalal/GameNative
Length of output: 51
🏁 Script executed:
# Search for ThemeUtils class
find app/src/main/java -type f -name "*.kt" | xargs grep -l "class ThemeUtils\|object ThemeUtils" 2>/dev/nullRepository: utkarshdalal/GameNative
Length of output: 127
🏁 Script executed:
# Check what theme-related files actually exist
find app/src/main/java -type d -name "*theme*"Repository: utkarshdalal/GameNative
Length of output: 147
🏁 Script executed:
# List theme directory contents if it exists
find app/src/main/java/app/gamenative -type f -name "*.kt" 2>/dev/null | head -30Repository: utkarshdalal/GameNative
Length of output: 1656
🏁 Script executed:
# Verify the package path format in the baseline-prof.txt matches the actual package structure
grep "app/gamenative" app/src/main/baseline-prof.txt | head -5Repository: utkarshdalal/GameNative
Length of output: 369
🏁 Script executed:
# List all files in theme/runtime directory to see what actually exists
find app/src/main/java/app/gamenative/theme/runtime -type f -name "*.kt"Repository: utkarshdalal/GameNative
Length of output: 805
🏁 Script executed:
# List all files in theme/model directory
find app/src/main/java/app/gamenative/theme/model -type f -name "*.kt" 2>/dev/nullRepository: utkarshdalal/GameNative
Length of output: 791
🏁 Script executed:
# Search for BindingContext more broadly
rg "BindingContext" app/src/main/java --type kotlin --max-count 10Repository: utkarshdalal/GameNative
Length of output: 3109
🏁 Script executed:
# Check for library screen components
find app/src/main/java -type f -name "*Library*.kt" | grep -i screenRepository: utkarshdalal/GameNative
Length of output: 692
🏁 Script executed:
# Verify PluviaApp, PluviaActivity, MainActivity exist
find app/src/main/java -type f \( -name "PluviaApp.kt" -o -name "PluviaActivity.kt" -o -name "MainActivity.kt" \)Repository: utkarshdalal/GameNative
Length of output: 161
🏁 Script executed:
# Search for Dimension class or references
rg "class Dimension|object Dimension" app/src/main/java --type kotlinRepository: utkarshdalal/GameNative
Length of output: 145
🏁 Script executed:
# Check for inner classes in ThemedGameGrid
rg "class.*Dimension\|data class.*Dimension" app/src/main/java --type kotlin --max-count 10Repository: utkarshdalal/GameNative
Length of output: 49
🏁 Script executed:
# Verify if PluviaActivity exists (mentioned in baseline-prof.txt)
find app/src/main/java -name "PluviaActivity.kt"Repository: utkarshdalal/GameNative
Length of output: 49
🏁 Script executed:
# Check PluviaMain referenced in baseline-prof.txt
rg "class PluviaMain|object PluviaMain" app/src/main/java --type kotlinRepository: utkarshdalal/GameNative
Length of output: 49
Remove references to non-existent classes and add critical missing theme engine classes.
The baseline profile contains references to classes that don't exist in the codebase (PluviaActivity and PluviaMain**). More critically, it's missing several hot-path classes essential for theme rendering performance:
LayoutEngine.kt(defines theBindingContextinterface used throughout rendering)BreakpointResolver.ktVariableResolver.ktSelectionEngine.ktStateEngine.ktSpatialFocusManager.ktBinding.kt,Breakpoint.kt,Engine.kt, and other model classes from the theme package
Remove the incorrect class references and add the missing theme engine classes to ensure critical rendering paths are precompiled at install time.
🤖 Prompt for AI Agents
In @app/src/main/baseline-prof.txt around lines 1-136, Remove the invalid
baseline entries referencing PluviaActivity and PluviaMain** and instead add
baseline rules for the missing theme engine classes used on hot paths: include
HSPL entries for LayoutEngineKt;->**(**)** and the BindingContext interface
(BindingContext;->**(**)**), plus BreakpointResolverKt, VariableResolverKt,
SelectionEngineKt, StateEngineKt, SpatialFocusManagerKt, BindingKt,
BreakpointKt, EngineKt and other core model classes (e.g., Binding, Breakpoint,
Engine) under the app/gamenative/theme package using the existing HSPL
app/gamenative/theme/**;->**(**)** pattern or explicit
HSPLapp/gamenative/theme/<ClassName>Kt;->**(**)** entries so these symbols are
precompiled at install time.
| fun getOrPutTemplateMeasure(key: TemplateKey, compute: () -> TemplateMeasure): TemplateMeasure { | ||
| if (!enabled || !premeasureEnabled) return compute() | ||
| val existing = templateMeasureCache[key] | ||
| if (existing != null) return existing | ||
| val computed = compute() | ||
| putTemplateMeasure(key, computed) | ||
| return computed | ||
| } |
There was a problem hiding this comment.
Race condition in getOrPutTemplateMeasure allows duplicate computation.
Multiple threads can concurrently check templateMeasureCache[key] at line 93, both find it missing, and then both call compute(). This wastes resources by computing the same expensive result multiple times, and the last write wins.
🔎 Proposed fix using double-checked locking
fun getOrPutTemplateMeasure(key: TemplateKey, compute: () -> TemplateMeasure): TemplateMeasure {
if (!enabled || !premeasureEnabled) return compute()
- val existing = templateMeasureCache[key]
- if (existing != null) return existing
- val computed = compute()
- putTemplateMeasure(key, computed)
- return computed
+ // Fast path: check without lock
+ templateMeasureCache[key]?.let { return it }
+
+ // Slow path: compute under lock to prevent duplicate work
+ synchronized(tmCacheLock) {
+ // Double-check after acquiring lock
+ templateMeasureCache[key]?.let { return it }
+ val computed = compute()
+ templateMeasureCache[key] = computed
+ return computed
+ }
}Apply the same pattern to getOrBuildLayerTree (lines 110-117).
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @app/src/main/java/app/gamenative/theme/perf/PerfConfig.kt around lines
91-98, getOrPutTemplateMeasure currently has a race where two threads can see
templateMeasureCache[key] missing and both call compute(); wrap the
check/compute/put with a synchronization (double-checked locking) so only one
thread computes and others read the cached value: first read
templateMeasureCache[key], if null enter a synchronized block (use the same lock
object used elsewhere in PerfConfig), re-check templateMeasureCache[key] inside
the lock, call compute() only if still null, then call putTemplateMeasure(key,
computed) and return the cached value; apply the identical double-checked
locking pattern to getOrBuildLayerTree to prevent duplicate builds.
| fun getOrBuildLayerTree(key: TemplateKey, build: () -> LayerTree): LayerTree { | ||
| if (!enabled) return build() | ||
| val existing = layerTreeCache[key] | ||
| if (existing != null) return existing | ||
| val built = build() | ||
| putLayerTree(key, built) | ||
| return built | ||
| } |
There was a problem hiding this comment.
Same race condition in getOrBuildLayerTree.
This method has the identical race condition as getOrPutTemplateMeasure: multiple threads can concurrently call build() for the same key, wasting computation.
Apply the same double-checked locking fix as suggested for getOrPutTemplateMeasure.
🤖 Prompt for AI Agents
In @app/src/main/java/app/gamenative/theme/perf/PerfConfig.kt around lines
110-117, getOrBuildLayerTree has the same race where multiple threads can call
build() for the same key; fix it with double-checked locking: first read
layerTreeCache[key] and return if present, otherwise enter a synchronized block
(use the same monitor as used elsewhere, e.g., the instance lock or the lock
used by getOrPutTemplateMeasure), re-check layerTreeCache[key] inside the
synchronized block, and only if still null call build(), call putLayerTree(key,
built) and return the stored value; reference getOrBuildLayerTree,
layerTreeCache, putLayerTree, and enabled when making the change.
| <!-- Missing translations (English placeholders) --> | ||
| <string name="add_game">Add Game</string> | ||
| <string name="back">Back</string> | ||
| <string name="base_app_reset_container_confirm">Reset</string> | ||
| <string name="base_app_reset_container_title">Reset Container?</string> | ||
| <string name="bcn_emulation">BCn Emulation</string> | ||
| <string name="bcn_emulation_cache">BCn Emulation Cache</string> | ||
| <string name="bcn_emulation_type">BCn Emulation Type</string> | ||
| <string name="box86_64_env_var_help__avx"><![CDATA[ | ||
| AVX and AVX2 implemented, along with BMI1, BMI2, ADX, FMA, F16C and RDANDR extension!<br /><br /> | ||
| <b>0</b> : Disable AVX extension<br /> | ||
| <b>1</b> : enable AVX, BMI1, F16C and VAES extension.<br /> | ||
| <b>2</b> : All 1 plus enable AVX2, BMI2, FMA, ADX,VPCLMULQDQ and RDRAND.<br />\ | ||
| ]]></string> | ||
| <string name="box86_64_env_var_help__dynarec_aligned_atomics"><![CDATA[ | ||
| Generate aligned atomics only (only available on Arm64 for now).<br /><br /> | ||
| <b>0</b> : Generate unaligned atomics handling code.<br /> | ||
| <b>1</b> : Generate aligned atomics only, which is faster and smaller code size, but will cause SIGBUS for LOCK prefixed opcodes operating on aligned data addresses.]]></string> | ||
| <string name="box86_64_env_var_help__dynarec_df"><![CDATA[ | ||
| Enable or disable the use of deferred flags.<br /><br /> | ||
| <b>0</b> : Disable the use of deferred flags.<br /> | ||
| <b>1</b> : Enable the use of deferred flags.]]></string> | ||
| <string name="box86_64_env_var_help__dynarec_dirty"><![CDATA[ | ||
| Allow continue running a block that is unprotected and potentially dirty.<br /><br /> | ||
| <b>0</b> : Do not allow continue running a block that is unprotected and potentially dirty.<br /> | ||
| <b>1</b> : Allow continue to run a dynablock that write data in the same page as code. It can gets faster in loading time of some game but can also get unexpected crashes.<br /> | ||
| <b>2</b> : Will also, when it detect an HotPage, flag that page as NEVERCLEAN, and so it will not be write protected but Block build from that page will always be tested. It can be faster that way (but some SMC case might not be trapped).]]></string> | ||
| <string name="box86_64_env_var_help__dynarec_nativeflags"><![CDATA[ | ||
| Enable or disable the use of native flags.<br /><br /> | ||
| <b>0</b> : Do not use native flags.<br /> | ||
| <b>1</b> : Use native flags when possible.]]></string> | ||
| <string name="box86_64_env_var_help__dynarec_pause"><![CDATA[ | ||
| Enable x86 PAUSE emulation, may help the performance of spinlocks.<br /><br /> | ||
| <b>0</b> : Ignore x86 PAUSE instruction.<br /> | ||
| <b>1</b> : Use YIELD to emulate x86 PAUSE instruction.<br /> | ||
| <b>2</b> : Use WFI to emulate x86 PAUSE instruction.<br /> | ||
| <b>3</b> : Use SEVL+WFE to emulate x86 PAUSE instruction. | ||
| ]]></string> | ||
| <string name="box86_64_env_var_help__dynarec_weakbarrier"><![CDATA[ | ||
| Tweak memory barriers to reduce performance impact of strong memory emulation<br /><br /> | ||
| <b>0</b> : Use regular safe barriers<br /> | ||
| <b>1</b> : Use weak barriers for mild performance boost<br /> | ||
| <b>2</b> : Use weak barriers, additionally disable last write barriers]]></string> | ||
| <string name="box86_64_env_var_help__maxcpu">Maximum number of CPUs presented to programs by box64</string> | ||
| <string name="box86_64_env_var_help__mmap32">Force every memory allocation in 32bit address spaces</string> | ||
| <string name="box86_64_env_var_help__unityplayer">Detect UnityPlayer.dll and apply strongmem settings</string> | ||
| <string name="cancel_import_confirm">Yes, Cancel Import</string> | ||
| <string name="cancel_import_keep">No, Keep Import</string> | ||
| <string name="cancel_import_message">An import is currently in progress. Cancelling will discard all extracted files and you will need to start the import again.\n\nAre you sure you want to cancel?</string> | ||
| <string name="cancel_import_title">Cancel Import?</string> | ||
| <string name="container_config_custom_resolution_error_aspect">Width must be greater than height</string> | ||
| <string name="container_config_custom_resolution_error_nonzero">Width and height must be greater than 0</string> | ||
| <string name="container_config_custom_resolution_separator">x</string> | ||
| <string name="container_config_custom_resolution_title">Set Custom Resolution</string> | ||
| <string name="custom_game_unknown_developer">Unknown</string> | ||
| <string name="disable_present_wait">Disable KHR_present_wait</string> | ||
| <string name="fexcore_env_var_help__disablel2cache">Disables FEXCore JIT L2 cache lookup, saving memory but introducing stutters.</string> | ||
| <string name="fexcore_env_var_help__dynamicl1cache">Switches FEXCore JIT L1 cache to be dynamically sized, saving memory but introducing stutters.</string> | ||
| <string name="fexcore_env_var_help__halfbarriertsoenabled">Uses half-barrier atomics for unaligned loads/stores under TSO.</string> | ||
| <string name="fexcore_env_var_help__hidehypervisorbit">Hides CPUID hypervisor bit (useful for apps that crash with it).</string> | ||
| <string name="fexcore_env_var_help__hostfeatures">Controls CPU feature forcing.</string> | ||
| <string name="fexcore_env_var_help__maxinst">Maximum instruction budget per translation block. Higher values can improve performance but may reduce stability.</string> | ||
| <string name="fexcore_env_var_help__memcpysettsoenabled">Makes REP MOVS / REP STOS atomic under TSO.</string> | ||
| <string name="fexcore_env_var_help__monohacks">Special SMC + JIT block hacks for Mono detection.</string> | ||
| <string name="fexcore_env_var_help__multiblock">Enables multiblock code compilation. Can cause longer JIT compilation and stutter.</string> | ||
| <string name="fexcore_env_var_help__smalltscscale">Scales TSC on low-frequency systems.</string> | ||
| <string name="fexcore_env_var_help__smc_checks">Self-Modifying Code checks.</string> | ||
| <string name="fexcore_env_var_help__tsoenabled">Enables TSO IR ops (required for multithreaded apps).</string> | ||
| <string name="fexcore_env_var_help__vectortsoenabled">Makes vector load/stores atomic when TSO is enabled.</string> | ||
| <string name="fexcore_env_var_help__volatilemetadata">Uses volatile metadata from PE files for TSO when available.</string> | ||
| <string name="fexcore_env_var_help__x87reducedprecision">Emulates X87 floating point using 64-bit precision. This reduces emulation accuracy and may result in rendering bugs.</string> | ||
| <string name="fexcore_preset">FEXCore Preset</string> | ||
| <string name="fexcore_presets">FEXCore Presets</string> | ||
| <string name="fexcore_presets_description">View, modify, and create FEXCore presets</string> | ||
| <string name="last_played">Last Played</string> | ||
| <string name="library_app_source">App Source</string> | ||
| <string name="library_layout_title">Layout</string> | ||
| <string name="library_never_played">Never played</string> | ||
| <string name="library_source_custom">Custom Games</string> | ||
| <string name="library_source_steam">Steam</string> | ||
| <string name="present_modes">Present Modes</string> | ||
| <string name="resource_type">Memory Resource Type</string> | ||
| <string name="settings_emulation_wine_proton_manager_subtitle">Import custom Wine/Proton versions (Bionic only)</string> | ||
| <string name="settings_emulation_wine_proton_manager_title">Wine/Proton Manager</string> | ||
| <string name="settings_interface_sort_last_played_subtitle">Show recently played games first in the library</string> | ||
| <string name="settings_interface_sort_last_played_title">Sort installed by last played</string> | ||
| <string name="settings_theme_picker_title">Theme</string> | ||
| <string name="settings_theme_reload_dev">Reload theme (dev)</string> | ||
| <string name="settings_theme_source_builtin">Built-in</string> | ||
| <string name="settings_theme_source_user">User</string> | ||
| <string name="settings_use_theme_engine_ui_subtitle">Enable the new themed Library screen. May reduce interactivity.</string> | ||
| <string name="settings_use_theme_engine_ui_title">Use Theme Engine UI (experimental)</string> | ||
| <string name="sharpness_denoise">Sharpness Denoise</string> | ||
| <string name="sharpness_effect">Sharpness Boost</string> | ||
| <string name="sharpness_level">Sharpness Level</string> | ||
| <string name="steam_continue">Continue</string> | ||
| <string name="steam_never">Never</string> | ||
| <string name="steam_not_logged_in">You must be logged into Steam to use this feature</string> | ||
| <string name="steam_uninstall_confirmation_message">Are you sure you want to uninstall %1$s? This action cannot be undone.</string> | ||
| <string name="steam_uninstall_failed">Failed to uninstall game</string> | ||
| <string name="steam_uninstall_success">%1$s has been uninstalled</string> | ||
| <string name="sync_frame">Sync Every Frame</string> | ||
| <string name="theme_activate">Activate</string> | ||
| <string name="theme_active">Active</string> | ||
| <string name="theme_add_dialog_message">Please select a folder containing a valid theme.\n\nA valid theme folder must include a manifest.xml file with the required fields (id, version, engineVersion, minAppVersion).\n\nNote: The app may restart after selecting a folder.</string> | ||
| <string name="theme_add_dialog_title">Add External Theme</string> | ||
| <string name="theme_add_dont_show_again">Don\'t show this dialog again</string> | ||
| <string name="theme_add_external">Add Theme</string> | ||
| <string name="theme_added_toast">Theme added: %1$s</string> | ||
| <string name="theme_applied_toast">Theme applied: %1$s</string> | ||
| <string name="theme_by_author">By %s</string> | ||
| <string name="theme_fallback_toast">Theme load failed. Falling back to %1$s</string> | ||
| <string name="theme_menu_button">Themes</string> | ||
| <string name="theme_no_description">No description available</string> | ||
| <string name="theme_remove">Remove</string> | ||
| <string name="theme_remove_confirm">Remove</string> | ||
| <string name="theme_remove_message">Remove \"%1$s\" from the theme list?\n\nThis will not delete the theme files from your device.</string> | ||
| <string name="theme_remove_title">Remove Theme</string> | ||
| <string name="theme_removed_toast">Theme removed: %1$s</string> | ||
| <string name="theme_select_folder">Select a folder containing a theme</string> | ||
| <string name="theme_selector_title">Select Theme</string> | ||
| <string name="theme_source_external">External</string> | ||
| <string name="theme_validation_failed">Invalid theme: %1$s</string> | ||
| <string name="win_proton_example">For example: "proton-10.0-ARM64ec.wcp"</string> | ||
| <string name="wine_proton_all_files_trusted">✓ All files are trusted. Ready to install.</string> | ||
| <string name="wine_proton_bin_path">Bin Path</string> | ||
| <string name="wine_proton_bionic_notice_header">Bionic Images Only</string> | ||
| <string name="wine_proton_cannot_open">Cannot open file</string> | ||
| <string name="wine_proton_containers_in_use">Containers using this version:</string> | ||
| <string name="wine_proton_containers_will_break">These containers will no longer work if you proceed:</string> | ||
| <string name="wine_proton_delete_content_desc">Delete</string> | ||
| <string name="wine_proton_description">Description</string> | ||
| <string name="wine_proton_error_badprofile">profile.json is invalid</string> | ||
| <string name="wine_proton_error_badtar">File cannot be recognized as valid archive</string> | ||
| <string name="wine_proton_error_exist">This Wine/Proton version already exists</string> | ||
| <string name="wine_proton_error_missingfiles">Package is missing required files (bin/, lib/, or prefixPack.txz)</string> | ||
| <string name="wine_proton_error_noprofile">profile.json not found in package</string> | ||
| <string name="wine_proton_error_nospace">Not enough storage space</string> | ||
| <string name="wine_proton_error_unable_install">Unable to install Wine/Proton package</string> | ||
| <string name="wine_proton_error_unknown">Unknown error occurred</string> | ||
| <string name="wine_proton_error_untrustprofile">Package cannot be trusted</string> | ||
| <string name="wine_proton_extracting">Extracting and validating package (this may take 2-3 minutes for large files)...</string> | ||
| <string name="wine_proton_failed_file_picker">Failed to open file picker: %s</string> | ||
| <string name="wine_proton_file_empty">File is empty or cannot be read</string> | ||
| <string name="wine_proton_filename_error">Filename must begin with \'wine\' or \'proton\' (case-insensitive)</string> | ||
| <string name="wine_proton_glibc_incompatible">This Wine/Proton build requires GLIBC containers and is not compatible with GameNative. Please use ARM64/bionic builds only.</string> | ||
| <string name="wine_proton_import_package">Import Wine/Proton Package</string> | ||
| <string name="wine_proton_import_wcp_button">Import .wcp Package</string> | ||
| <string name="wine_proton_info_description">Import custom Wine or Proton versions for Bionic containers. Filename must begin with \'wine\' or \'proton\' (case-insensitive). Packages must include bin/, lib/, and prefixPack.txz. All imports are bionic-compatible only.</string> | ||
| <string name="wine_proton_install_error">Installation error: %s</string> | ||
| <string name="wine_proton_install_failed">Failed to install: %s</string> | ||
| <string name="wine_proton_install_package">Install Package</string> | ||
| <string name="wine_proton_install_success">%1$s %2$s installed successfully</string> | ||
| <string name="wine_proton_installed_versions">Installed Wine/Proton Versions</string> | ||
| <string name="wine_proton_lib_path">Lib Path</string> | ||
| <string name="wine_proton_manager">Wine/Proton Manager</string> | ||
| <string name="wine_proton_no_containers_warning">No containers are currently using this version.</string> | ||
| <string name="wine_proton_no_versions_found">No installed Wine or Proton versions found.</string> | ||
| <string name="wine_proton_not_wine_or_proton">Package is not Wine or Proton (type: %s)</string> | ||
| <string name="wine_proton_package_details">Package Details</string> | ||
| <string name="wine_proton_processing">Processing...</string> | ||
| <string name="wine_proton_remove_failed">Failed to remove: %s</string> | ||
| <string name="wine_proton_remove_message">Are you sure you want to remove %1$s %2$s (%3$d)? Containers using this version will no longer work.</string> | ||
| <string name="wine_proton_remove_title">Remove Wine/Proton Version</string> | ||
| <string name="wine_proton_removed_toast">Removed %s</string> | ||
| <string name="wine_proton_select_file_description">Select a .wcp file (with filename starting with \'wine\' or \'proton\')</string> | ||
| <string name="wine_proton_type">Type</string> | ||
| <string name="wine_proton_type_mismatch">Filename indicates %1$s but package contains %2$s</string> | ||
| <string name="wine_proton_untrusted_files_detected">This package includes files outside the trusted set.</string> | ||
| <string name="wine_proton_untrusted_files_label">Untrusted files:</string> | ||
| <string name="wine_proton_untrusted_files_message">This package includes files outside the trusted set. Review and confirm to proceed with installation.</string> | ||
| <string name="wine_proton_version">Version</string> | ||
| <string name="wine_proton_version_already_exists">Wine/Proton version already exists</string> | ||
| <string name="wine_proton_version_code">Version Code</string> |
There was a problem hiding this comment.
Track Portuguese translations for new feature strings.
173 English placeholder strings have been added to the Portuguese Brazilian (pt-rBR) localization file. Portuguese-speaking users will see English text for the new Theme Engine, Wine/Proton Manager, and related features, creating an inconsistent bilingual experience.
While the comment indicates these are intentional placeholders, consider tracking translation completion to ensure a consistent user experience for Portuguese speakers.
Do you want me to open an issue to track the Portuguese translation work for these strings?
| val img = File(theme, "images\\card.png").apply { | ||
| parentFile.mkdirs(); writeText("fake") | ||
| } |
There was a problem hiding this comment.
Windows path separators will break on Unix systems.
Using backslashes (\\) for path construction will fail on Linux/macOS CI. Use File.separator or forward slashes (which Java normalizes on all platforms).
🔎 Proposed fix
- val img = File(theme, "images\\card.png").apply {
+ val img = File(theme, "images/card.png").apply {
parentFile.mkdirs(); writeText("fake")
}Apply the same fix to lines 42, 61, 84, 106, and 107.
📝 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.
| val img = File(theme, "images\\card.png").apply { | |
| parentFile.mkdirs(); writeText("fake") | |
| } | |
| val img = File(theme, "images/card.png").apply { | |
| parentFile.mkdirs(); writeText("fake") | |
| } |
🤖 Prompt for AI Agents
In @app/src/test/java/app/gamenative/theme/media/MediaResolverTest.kt around
lines 21-23, Replace hard-coded Windows backslashes in test file
MediaResolverTest.kt (e.g., the File constructor call creating img with
"images\\card.png") with a platform-neutral path: use File(theme, "images" +
File.separator + "card.png") or use forward slashes ("images/card.png") which
Java normalizes; apply the same change to the other occurrences referenced in
the review (lines creating files at lines 42, 61, 84, 106, and 107) so tests run
on Unix CI as well as Windows.
| // split file to be included | ||
| val split = File(themeDir, "sections\\grid.xml") | ||
| split.parentFile.mkdirs() | ||
| split.writeText( | ||
| """ | ||
| <layouts> | ||
| <grid id="mainGrid" columns="3" rows="2"/> | ||
| </layouts> | ||
| """.trimIndent() | ||
| ) |
There was a problem hiding this comment.
Use platform-independent path separator.
The hardcoded backslash \\ on line 30 will fail on Unix-based CI/build systems. Use File.separator or forward slashes (Java/Kotlin normalizes them).
🔎 Proposed fix
- val split = File(themeDir, "sections\\grid.xml")
+ val split = File(themeDir, "sections${File.separator}grid.xml")Or simpler, just use forward slashes which work cross-platform:
- val split = File(themeDir, "sections\\grid.xml")
+ val split = File(themeDir, "sections/grid.xml")📝 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.
| // split file to be included | |
| val split = File(themeDir, "sections\\grid.xml") | |
| split.parentFile.mkdirs() | |
| split.writeText( | |
| """ | |
| <layouts> | |
| <grid id="mainGrid" columns="3" rows="2"/> | |
| </layouts> | |
| """.trimIndent() | |
| ) | |
| // split file to be included | |
| val split = File(themeDir, "sections/grid.xml") | |
| split.parentFile.mkdirs() | |
| split.writeText( | |
| """ | |
| <layouts> | |
| <grid id="mainGrid" columns="3" rows="2"/> | |
| </layouts> | |
| """.trimIndent() | |
| ) |
🤖 Prompt for AI Agents
In @app/src/test/java/app/gamenative/theme/ThemeLoaderSmokeTest.kt around lines
29-38, The test creates a File using a Windows-style path
("sections\\grid.xml"), which breaks on Unix CI; update the File construction in
ThemeLoaderSmokeTest (the variable split/File(themeDir, ...)) to use a
platform-independent separator (e.g., build the path with File.separator between
"sections" and "grid.xml" or simply use "sections/grid.xml") so
parentFile.mkdirs() and split.writeText(...) work cross-platform.
| kotlinter = "5.0.1" # https://plugins.gradle.org/plugin/org.jmailen.kotlinter | ||
| ksp = "2.1.21-2.0.2" # https://mvnrepository.com/artifact/com.google.devtools.ksp/symbol-processing-api | ||
| landscapistCoil = "2.4.6" # https://mvnrepository.com/artifact/com.github.skydoves/landscapist-coil | ||
| coil = "2.7.0" # https://mvnrepository.com/artifact/io.coil-kt/coil |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n gradle/libs.versions.tomlRepository: utkarshdalal/GameNative
Length of output: 14067
🌐 Web query:
latest stable version of androidx.media3 exoplayer 2024 2025
💡 Result:
Latest stable: androidx.media3 (ExoPlayer) 1.9.0 — released December 17, 2025. Use:
implementation "androidx.media3:media3-exoplayer:1.9.0". [1][2]
Sources:
[1] Android Developers — Media3 release notes (shows 1.9.0 stable, Dec 17, 2025).
[2] CommonsBlog summary listing Media3 1.9.0 artifacts.
🌐 Web query:
latest stable version of io.coil-kt coil 2024 2025
💡 Result:
As of Jan 5, 2026:
- Coil 3 (new package name io.coil-kt:coil3) — latest stable: 3.3.0 (released July 22, 2025). [1][2]
- Coil 2 (legacy 2.x coordinates io.coil-kt:coil / io.coil-kt:coil-compose) — latest 2.x stable: 2.7.0 (published July 17, 2024). [3][4]
Sources: Coil changelog / GitHub release notes for 3.3.0. [1][2]; Maven Central / artifact index for 2.7.0. [3][4]
References
[1] Coil changelog (3.3.0 release notes).
[2] coil-kt GitHub README / releases (examples using 3.3.0).
[3] Maven Central — io.coil-kt:coil-compose:2.7.0.
[4] mvnrepository / io.coil-kt:coil (artifact listing noting 2.7.0).
🌐 Web query:
latest stable version of androidx.constraintlayout compose 2024 2025
💡 Result:
Latest stable version (artifact: androidx.constraintlayout:constraintlayout-compose) is 1.1.1 — released Feb 26, 2025. Use:
implementation("androidx.constraintlayout:constraintlayout-compose:1.1.1"). [1][2]
Sources:
[1] Android Developers — ConstraintLayout release notes (shows 1.1.1 released Feb 26, 2025).
[2] Maven / artifact listing for constraintlayout-compose (shows 1.1.1 as newest).
🌐 Web query:
latest stable version of androidx.profileinstaller 2024 2025
💡 Result:
The latest stable release is androidx.profileinstaller:profileinstaller:1.4.1 (released October 2, 2024). [1][2]
Sources:
[1] Android Developers — ProfileInstaller release notes.
[2] Maven Repository — profileinstaller versions.
Update Media3 and Coil to latest stable versions.
- Media3: Currently 1.5.1, but 1.9.0 is available (released Dec 17, 2025)
- Coil: Currently 2.7.0; consider upgrading to Coil 3.3.0 (package name changes to io.coil-kt:coil3)
- constraintlayoutComposeAndroid 1.1.1 and profileinstaller 1.4.1 are current
Also applies to: 42-43, 81, 112-118
🤖 Prompt for AI Agents
In @gradle/libs.versions.toml around line 22, Update dependency versions and
artifact IDs: change the coil entry (currently coil = "2.7.0") to use Coil 3.3.0
and the new artifact name (io.coil-kt:coil3) wherever referenced; bump all
Media3 artifacts from 1.5.1 to 1.9.0 (look for entries like media3-*,
media3-exoplayer, media3-ui) and update their version key in libs.versions.toml
and any alias entries; ensure constraintlayoutComposeAndroid is set to 1.1.1 and
profileinstaller to 1.4.1 per the comment; apply the same version/artifact name
changes in other occurrences mentioned (lines referenced 42-43, 81, 112-118) so
all aliases and module coordinates are consistent.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI Agents
In @app/src/main/java/app/gamenative/ui/model/LibraryViewModel.kt:
- Around line 381-396: lastPlayedMap is keyed by container.id (container.id) but
later looked up by entry.item.appId, causing all lookups to fail; update the
construction of lastPlayedMap in LibraryViewModel so it keys timestamps by the
same appId used in lookups (entry.item.appId) instead of container.id, or build
an intermediate containerId->appId mapping via ContainerManager (e.g., query
container metadata or use a helper like ContainerManager.getAppIdForContainer)
and then produce lastPlayedMap: Map<String, Long> keyed by appId; ensure the
rest of the code still reads from lastPlayedMap using entry.item.appId.
In @app/src/main/java/app/gamenative/ui/screen/library/LibraryScreen.kt:
- Around line 322-331: The remember block that computes containerTimestamps
swallows exceptions; update the catch in the remember { ... } for
containerTimestamps to log the caught Exception (e.g., using Android Log.e or
your project's logger) with context mentioning ContainerManager or "loading
container timestamps" before returning emptyMap(); reference the
ContainerManager(context).containers/associate call and the containerTimestamps
variable so the exception is recorded for debugging while preserving the
emptyMap() fallback.
In @app/src/main/res/values-da/strings.xml:
- Around line 909-911: Add the missing Danish translation for the string
resource named library_app_source by inserting a <string> resource with
name="library_app_source" and the Danish text "App-kilde" into the Danish
strings file (alongside existing entries like library_layout_title and
library_source_custom) so the localization matches the English file.
In @app/src/main/res/values-de/strings.xml:
- Around line 906-945: The German localization file contains English placeholder
text for many Theme Engine, settings and library strings (e.g., add_game, back,
last_played, library_app_source, library_never_played,
container_config_custom_resolution_* and theme_*/settings_* keys); replace each
English string with an accurate German translation while preserving XML escapes
and formatting (keep %1$s and %s placeholders intact, preserve \n in
theme_add_dialog_message and any escaped quotes like theme_add_dont_show_again),
then run a quick lint/validate of values-de/strings.xml to ensure well-formed
XML and no missing translations remain for these keys.
🧹 Nitpick comments (5)
app/src/main/res/values-uk/strings.xml (1)
614-987: English placeholders in Ukrainian locale file should be tracked for translation.The comment correctly documents these as "Missing translations." This is acceptable for initial development to prevent missing resource crashes, but these ~370 new strings will display in English for Ukrainian users.
Consider opening an issue to track translation efforts for the new Theme Engine and related features to ensure proper i18n coverage.
app/src/main/res/values-zh-rTW/strings.xml (1)
964-1009: Track English placeholder strings for translation.The block of English placeholder strings is acceptable for an experimental feature, but Traditional Chinese users will see untranslated English text for theme-related UI elements. Consider creating a tracking issue or adding these to your translation management system before the Theme Engine feature moves from experimental to general availability.
Additionally, if any of these strings are not intended for translation (e.g., developer-only strings), consider adding the
translatable="false"attribute to explicitly mark them:<string name="settings_theme_reload_dev" translatable="false">Reload theme (dev)</string>app/src/main/java/app/gamenative/ui/screen/library/components/LibraryBottomSheet.kt (1)
162-180: Preview function doesn't demonstrate both theme engine states.The preview function doesn't control or demonstrate the
PrefManager.useThemeEngineUiflag behavior. Consider adding a preview variant that shows both states (with/without the legacy layout selector) to visualize the conditional rendering during development.💡 Example preview variant
You could create a preview-only parameter or separate preview functions to demonstrate both UI states:
@Preview(name = "Legacy UI", uiMode = Configuration.UI_MODE_NIGHT_YES) @Composable private fun Preview_LibraryBottomSheet_Legacy() { // Shows UI with legacy layout selector visible PluviaTheme { Surface { LibraryBottomSheet(/* ... */) } } } @Preview(name = "Theme Engine UI", uiMode = Configuration.UI_MODE_NIGHT_YES) @Composable private fun Preview_LibraryBottomSheet_ThemeEngine() { // Would need to mock PrefManager or refactor to accept flag as parameter PluviaTheme { Surface { LibraryBottomSheet(/* ... */) } } }Note: This would require refactoring to make the flag testable/previewable, possibly by accepting it as a parameter with a default value from PrefManager.
app/src/main/java/app/gamenative/ui/screen/library/components/LibraryListPane.kt (1)
156-158: Consider removing unused theme observation or clarifying its purpose.The
selectedThemeIdis collected but never used in the composable body. While the comment indicates this is "to provide a visible cue when themes change," the observation triggers recomposition without any actual visual change based on the value. If the intent is to force recomposition when the theme changes, this works but could be clarified or optimized.app/src/main/java/app/gamenative/ui/screen/library/LibraryScreen.kt (1)
290-320: Consider extracting helper functions for better organization.The
findSteamGridDBImageandfindSteamGridDBHeroImagefunctions work correctly but are defined inside the composable body. Extracting them as top-level or extension functions would improve readability and testability.Example refactoring
// At file or companion object level private fun findSteamGridDBImage(item: LibraryItem, imageType: String): String? { if (item.gameSource != GameSource.CUSTOM_GAME) return null val gameFolderPath = CustomGameScanner.getFolderPathFromAppId(item.appId) ?: return null // ... rest of implementation }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
app/src/main/assets/Themes/SwitchinItUp/assets/gog.pngis excluded by!**/*.png
📒 Files selected for processing (23)
app/build.gradle.ktsapp/src/main/assets/Themes/SwitchinItUp/theme.xmlapp/src/main/java/app/gamenative/MainActivity.ktapp/src/main/java/app/gamenative/PluviaApp.ktapp/src/main/java/app/gamenative/PrefManager.ktapp/src/main/java/app/gamenative/ui/PluviaMain.ktapp/src/main/java/app/gamenative/ui/model/LibraryViewModel.ktapp/src/main/java/app/gamenative/ui/model/MainViewModel.ktapp/src/main/java/app/gamenative/ui/screen/library/LibraryAppScreen.ktapp/src/main/java/app/gamenative/ui/screen/library/LibraryScreen.ktapp/src/main/java/app/gamenative/ui/screen/library/components/LibraryBottomSheet.ktapp/src/main/java/app/gamenative/ui/screen/library/components/LibraryListPane.ktapp/src/main/java/app/gamenative/ui/screen/settings/SettingsGroupInterface.ktapp/src/main/res/values-da/strings.xmlapp/src/main/res/values-de/strings.xmlapp/src/main/res/values-fr/strings.xmlapp/src/main/res/values-pt-rBR/strings.xmlapp/src/main/res/values-uk/strings.xmlapp/src/main/res/values-zh-rCN/strings.xmlapp/src/main/res/values-zh-rTW/strings.xmlapp/src/main/res/values/strings.xmlapp/src/test/java/app/gamenative/theme/ThemeValidatorTest.ktdocs/theme-documentation.html
🚧 Files skipped from review as they are similar to previous changes (8)
- app/src/main/java/app/gamenative/ui/model/MainViewModel.kt
- app/src/main/assets/Themes/SwitchinItUp/theme.xml
- app/src/test/java/app/gamenative/theme/ThemeValidatorTest.kt
- app/src/main/java/app/gamenative/ui/screen/library/LibraryAppScreen.kt
- app/src/main/java/app/gamenative/ui/PluviaMain.kt
- app/build.gradle.kts
- app/src/main/java/app/gamenative/PrefManager.kt
- app/src/main/res/values-zh-rCN/strings.xml
🧰 Additional context used
🧠 Learnings (2)
📚 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/LibraryScreen.ktapp/src/main/java/app/gamenative/ui/screen/library/components/LibraryBottomSheet.kt
📚 Learning: 2025-10-15T08:07:14.359Z
Learnt from: RadicalDog
Repo: utkarshdalal/GameNative PR: 195
File: app/src/main/java/app/gamenative/ui/screen/library/components/LibraryAppItem.kt:106-116
Timestamp: 2025-10-15T08:07:14.359Z
Learning: In Jetpack Compose, the `.clickable()` modifier makes a component focusable by default, so explicitly adding `.focusable()` is not necessary when `.clickable()` is already present in the modifier chain.
Applied to files:
app/src/main/java/app/gamenative/ui/screen/library/components/LibraryListPane.kt
🧬 Code graph analysis (2)
app/src/main/java/app/gamenative/ui/screen/settings/SettingsGroupInterface.kt (1)
app/src/main/java/app/gamenative/ui/theme/Color.kt (1)
settingsTileColorsAlt(34-38)
app/src/main/java/app/gamenative/ui/screen/library/components/LibraryBottomSheet.kt (1)
app/src/main/java/app/gamenative/ui/component/FlowFilterChip.kt (1)
FlowFilterChip(27-69)
🪛 detekt (1.23.8)
app/src/main/java/app/gamenative/ui/screen/library/LibraryScreen.kt
[warning] 327-327: 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 (22)
app/src/main/res/values-fr/strings.xml (1)
968-1006: LGTM! English placeholders are well-structured.The new theme-related string resources are properly formatted with consistent naming conventions and correct XML syntax. Using English placeholders in locale-specific files during development is a standard practice.
Consider requesting French translations for these strings in a follow-up to provide a complete localized experience for French-speaking users.
One minor note: Line 971's constraint message "Width must be greater than height" suggests custom resolutions only allow landscape orientation. If portrait orientations should also be supported, consider revising this validation message and the underlying logic.
app/src/main/java/app/gamenative/ui/model/LibraryViewModel.kt (2)
59-60: LGTM! Carousel page tracking added.The
carouselPageIndexproperty follows the established pattern for view state management and provides necessary state persistence for carousel-based theme navigation.
404-417: Sorting comparator logic is well-structured, but depends on fixing the key mismatch.The multi-level sorting comparator is correctly structured:
- Installed items first
- Most recently played first (for installed items)
- Alphabetically as final tiebreaker
However, this feature won't function until the
lastPlayedMapkey mismatch is resolved (see comment on lines 381-396). Once fixed, this comparator will properly sort games by last played timestamp.app/src/main/java/app/gamenative/ui/screen/library/components/LibraryBottomSheet.kt (1)
92-92: Both string resources exist across all localization files. No issues found.app/src/main/java/app/gamenative/PluviaApp.kt (1)
70-72: No changes needed. TheThemeManager.init()implementation already properly dispatches heavy I/O operations toDispatchers.IOand includes comprehensive error handling. The lightweight migration operation runs synchronously on the main thread, which is appropriate for preference operations.Likely an incorrect or invalid review comment.
app/src/main/res/values/strings.xml (1)
975-1014: LGTM! Theme Engine strings are well-organized.The new Theme Engine strings follow consistent naming conventions with appropriate prefixes (
theme_,settings_theme_). Format specifiers are correctly used (%1$s), and special characters like escaped quotes intheme_remove_messageare properly handled.app/src/main/res/values-da/strings.xml (1)
934-954: LGTM! Theme Engine translations are complete and consistent.All Theme Engine strings have been properly translated to Danish with correct format specifiers matching the English base. The translations appear linguistically appropriate.
app/src/main/java/app/gamenative/ui/screen/settings/SettingsGroupInterface.kt (2)
259-270: Theme Engine toggle implementation is correct and follows established patterns.The code properly:
- Manages state with
rememberSaveablefor persistence across configuration changes- Initializes from
PrefManager.useThemeEngineUi- Updates both local state and PrefManager on change
- Uses consistent styling with
settingsTileColorsAlt()- References existing string resources across all supported locales
272-283: Implementation is correct. The sort by last played toggle follows the exact same pattern as the Theme Engine toggle above it, with proper state management usingrememberSaveableand persistence throughPrefManager. String resources and the preference property are confirmed to exist.app/src/main/java/app/gamenative/MainActivity.kt (4)
64-69: LGTM! Clean configuration state management for theme engine.The companion object state pattern effectively exposes device configuration to the theming system while working around
android:configChangeslimitations. The counter-based recomposition trigger is a solid approach, and the comments clearly document the rationale.
139-143: Configuration state initialization looks correct.The state values are properly initialized from the current configuration during
onCreate, ensuring the theme system has accurate data from the first composition.
366-372: Configuration change handling is implemented correctly.The state updates properly propagate device configuration changes to the theme system, and the counter increment ensures Compose recomposition triggers reliably. The explanatory comments are helpful for maintainability.
197-197: SVG decoder integration looks good.The
SvgDecoder.Factory()addition properly enables SVG asset support for the theme system. The Coil SVG dependency (io.coil-kt:coil-svg) is correctly declared in your Gradle dependencies at version 2.7.0.app/src/main/java/app/gamenative/ui/screen/library/components/LibraryListPane.kt (3)
167-200: LGTM! Transient paneType with responsive initialization.The refactoring to make
paneTypetransient (starting asUNDECIDEDand initialized based on view width/orientation) is well-implemented. The LaunchedEffect correctly sets the initial layout choice without persisting it, which aligns with the PR objectives.
218-222: LGTM! Focus clearing implemented correctly.The tap gesture handler to clear focus is a standard UX pattern and is correctly implemented with
pointerInput(Unit)anddetectTapGestures.
456-458: LGTM! Bottom sheet correctly updates transient paneType.The bottom sheet now updates
paneTypeas local state only, without persisting toPrefManager. This is consistent with the transient layout choice refactoring.app/src/main/java/app/gamenative/ui/screen/library/LibraryScreen.kt (6)
118-148: Configuration tracking implementation looks correct but adds complexity.The approach of reading configuration state from
MainActivity(rather thanLocalConfiguration) and keying onorientationTriggerto force recomposition is a valid workaround forandroid:configChangespreventing automatic recomposition. The comment clearly explains the rationale.
156-157: LGTM! Carousel parameters properly added.The new
carouselPageIndexandonCarouselPageChangedparameters extend the API appropriately for themed carousel support. They're correctly forwarded fromHomeLibraryScreenand the preview is updated with default values.
249-272: LGTM! Theme state observation and focus management properly implemented.The theme state collection (
activeTheme,reloadTick,activeThemeRootDir) and focus clearing gesture are correctly implemented. The orientation-aware theme effects ensure responsive breakpoint handling.
452-544: LGTM! Layout element rendering is well-structured.The z-order sorting and type-safe rendering of layout elements (Fixed, Grid, Carousel, Canvas) is correctly implemented. The null checks for cards and the conditional rendering based on
useThemeUiprovide proper fallback behavior.
587-688: LGTM! Conditional FAB and bottom sheet rendering properly implemented.The conditional logic correctly prevents duplicate UI elements when using the theme engine. The hardcoded
GRID_CAPSULEand no-oponViewChangedfor themed bottom sheets are intentional since theme definitions control the layout. The separation between legacy and themed rendering paths is clear.
334-418: gameId is properly initialized for custom games; the Steam fallback URLs are unreachable code.The binding provider correctly extracts
gameIdfromappIdfor custom games (LibraryItem.kt lines 64-65). However, the Steam fallback URLs at lines 340, 347, and 356 will never execute for custom games becauseiconHashis always set to an empty string when creating custom game LibraryItems (CustomGameScanner.kt line 487). Theif (item.iconHash.isNotEmpty())condition prevents these branches from running, making them dead code for custom games rather than a safety concern.Likely an incorrect or invalid review comment.
| // Check if we should sort by last played | ||
| val sortByLastPlayed = PrefManager.sortInstalledByLastPlayed | ||
|
|
||
| // Build map of appId -> lastPlayedTimestamp for installed games (if sorting by last played) | ||
| val lastPlayedMap: Map<String, Long> = if (sortByLastPlayed) { | ||
| try { | ||
| val containerManager = ContainerManager(context) | ||
| containerManager.containers | ||
| .associate { container -> container.id to container.lastPlayedTimestamp } | ||
| } catch (e: Exception) { | ||
| Timber.tag("LibraryViewModel").e(e, "Error loading container timestamps") | ||
| emptyMap() | ||
| } | ||
| } else { | ||
| emptyMap() | ||
| } |
There was a problem hiding this comment.
Critical: Key mismatch remains unresolved—container.id incompatible with appId lookup.
The previous review identified that lastPlayedMap is keyed by container.id (line 389), which are directory-based identifiers like "0" or "1". However, the lookup at line 409 attempts to retrieve values using entry.item.appId, which has a formatted structure like "STEAM_123", "GOG_456", or "CUSTOM_xyz" (see lines 312, 355).
These key formats are fundamentally incompatible, causing all lookups to fail and return the default value 0L. This means the sort-by-last-played feature is non-functional—all games will appear equally recently played.
To fix this, you need to either:
- Establish a mapping between container IDs and game appIds, or
- Store
lastPlayedTimestampindexed by the appId format directly
Based on past review investigation confirming the format mismatch.
🤖 Prompt for AI Agents
In @app/src/main/java/app/gamenative/ui/model/LibraryViewModel.kt around lines
381 - 396, lastPlayedMap is keyed by container.id (container.id) but later
looked up by entry.item.appId, causing all lookups to fail; update the
construction of lastPlayedMap in LibraryViewModel so it keys timestamps by the
same appId used in lookups (entry.item.appId) instead of container.id, or build
an intermediate containerId->appId mapping via ContainerManager (e.g., query
container metadata or use a helper like ContainerManager.getAppIdForContainer)
and then produce lastPlayedMap: Map<String, Long> keyed by appId; ensure the
rest of the code still reads from lastPlayedMap using entry.item.appId.
| // Load container timestamps for last played info | ||
| val containerTimestamps: Map<String, Long> = remember { | ||
| try { | ||
| ContainerManager(context).containers | ||
| .associate { container -> container.id to container.lastPlayedTimestamp } | ||
| } catch (e: Exception) { | ||
| emptyMap() | ||
| } | ||
| } | ||
| val neverPlayedText = stringResource(R.string.library_never_played) |
There was a problem hiding this comment.
Log the swallowed exception for better debugging.
The try-catch block silently swallows exceptions when loading container timestamps, which could hide initialization failures or data corruption issues. While the fallback to emptyMap() ensures graceful degradation, logging the exception would aid debugging.
Proposed fix
val containerTimestamps: Map<String, Long> = remember {
try {
ContainerManager(context).containers
.associate { container -> container.id to container.lastPlayedTimestamp }
} catch (e: Exception) {
+ Timber.tag("LibraryScreen").w(e, "Failed to load container timestamps")
emptyMap()
}
}📝 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.
| // Load container timestamps for last played info | |
| val containerTimestamps: Map<String, Long> = remember { | |
| try { | |
| ContainerManager(context).containers | |
| .associate { container -> container.id to container.lastPlayedTimestamp } | |
| } catch (e: Exception) { | |
| emptyMap() | |
| } | |
| } | |
| val neverPlayedText = stringResource(R.string.library_never_played) | |
| // Load container timestamps for last played info | |
| val containerTimestamps: Map<String, Long> = remember { | |
| try { | |
| ContainerManager(context).containers | |
| .associate { container -> container.id to container.lastPlayedTimestamp } | |
| } catch (e: Exception) { | |
| Timber.tag("LibraryScreen").w(e, "Failed to load container timestamps") | |
| emptyMap() | |
| } | |
| } | |
| val neverPlayedText = stringResource(R.string.library_never_played) |
🧰 Tools
🪛 detekt (1.23.8)
[warning] 327-327: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
🤖 Prompt for AI Agents
In @app/src/main/java/app/gamenative/ui/screen/library/LibraryScreen.kt around
lines 322 - 331, The remember block that computes containerTimestamps swallows
exceptions; update the catch in the remember { ... } for containerTimestamps to
log the caught Exception (e.g., using Android Log.e or your project's logger)
with context mentioning ContainerManager or "loading container timestamps"
before returning emptyMap(); reference the
ContainerManager(context).containers/associate call and the containerTimestamps
variable so the exception is recorded for debugging while preserving the
emptyMap() fallback.
| <string name="library_layout_title">Layout</string> | ||
| <string name="library_source_custom">Brugerdefinerede spil</string> | ||
| <string name="library_source_steam">Steam</string> |
There was a problem hiding this comment.
Missing Danish translation for library_app_source.
The English file adds library_app_source at line 815, but this string is not present in the Danish translations. While Android will fall back to the English value, adding the translation would complete the localization.
Consider adding:
<string name="library_app_source">App-kilde</string>🤖 Prompt for AI Agents
In @app/src/main/res/values-da/strings.xml around lines 909 - 911, Add the
missing Danish translation for the string resource named library_app_source by
inserting a <string> resource with name="library_app_source" and the Danish text
"App-kilde" into the Danish strings file (alongside existing entries like
library_layout_title and library_source_custom) so the localization matches the
English file.
|
|
||
| <!-- Missing translations (English placeholders) --> | ||
| <string name="add_game">Add Game</string> | ||
| <string name="back">Back</string> | ||
| <string name="container_config_custom_resolution_error_aspect">Width must be greater than height</string> | ||
| <string name="container_config_custom_resolution_error_nonzero">Width and height must be greater than 0</string> | ||
| <string name="container_config_custom_resolution_separator">x</string> | ||
| <string name="container_config_custom_resolution_title">Set Custom Resolution</string> | ||
| <string name="last_played">Last Played</string> | ||
| <string name="library_app_source">App Source</string> | ||
| <string name="library_never_played">Never played</string> | ||
| <string name="settings_interface_sort_last_played_subtitle">Show recently played games first in the library</string> | ||
| <string name="settings_interface_sort_last_played_title">Sort installed by last played</string> | ||
| <string name="settings_theme_picker_title">Theme</string> | ||
| <string name="settings_theme_reload_dev">Reload theme (dev)</string> | ||
| <string name="settings_theme_source_builtin">Built-in</string> | ||
| <string name="settings_theme_source_user">User</string> | ||
| <string name="settings_use_theme_engine_ui_subtitle">Enable the new themed Library screen. May reduce interactivity.</string> | ||
| <string name="settings_use_theme_engine_ui_title">Use Theme Engine UI (experimental)</string> | ||
| <string name="theme_activate">Activate</string> | ||
| <string name="theme_active">Active</string> | ||
| <string name="theme_add_dialog_message">Please select a folder containing a valid theme.\n\nA valid theme folder must include a manifest.xml file with the required fields (id, version, engineVersion, minAppVersion).\n\nNote: The app may restart after selecting a folder.</string> | ||
| <string name="theme_add_dialog_title">Add External Theme</string> | ||
| <string name="theme_add_dont_show_again">Don\'t show this dialog again</string> | ||
| <string name="theme_add_external">Add Theme</string> | ||
| <string name="theme_added_toast">Theme added: %1$s</string> | ||
| <string name="theme_applied_toast">Theme applied: %1$s</string> | ||
| <string name="theme_by_author">By %s</string> | ||
| <string name="theme_fallback_toast">Theme load failed. Falling back to %1$s</string> | ||
| <string name="theme_menu_button">Themes</string> | ||
| <string name="theme_no_description">No description available</string> | ||
| <string name="theme_remove">Remove</string> | ||
| <string name="theme_remove_confirm">Remove</string> | ||
| <string name="theme_remove_message">Remove \"%1$s\" from the theme list?\n\nThis will not delete the theme files from your device.</string> | ||
| <string name="theme_remove_title">Remove Theme</string> | ||
| <string name="theme_removed_toast">Theme removed: %1$s</string> | ||
| <string name="theme_select_folder">Select a folder containing a theme</string> | ||
| <string name="theme_selector_title">Select Theme</string> | ||
| <string name="theme_source_external">External</string> | ||
| <string name="theme_validation_failed">Invalid theme: %1$s</string> |
There was a problem hiding this comment.
German translations missing for Theme Engine strings.
These 39 strings are English placeholders in the German localization file (values-de). German-speaking users will see English text throughout the Theme Engine UI, including settings, dialogs, theme management, and library features like "Last Played" sorting.
This significantly degrades the user experience for German locale users when using the new Theme Engine feature.
Affected strings requiring German translation
Theme-related strings (26):
- theme_activate, theme_active, theme_add_dialog_message, theme_add_dialog_title, theme_add_dont_show_again, theme_add_external, theme_added_toast, theme_applied_toast, theme_by_author, theme_fallback_toast, theme_menu_button, theme_no_description, theme_remove, theme_remove_confirm, theme_remove_message, theme_remove_title, theme_removed_toast, theme_select_folder, theme_selector_title, theme_source_external, theme_validation_failed
Settings strings (6):
- settings_theme_picker_title, settings_theme_reload_dev, settings_theme_source_builtin, settings_theme_source_user, settings_use_theme_engine_ui_subtitle, settings_use_theme_engine_ui_title, settings_interface_sort_last_played_subtitle, settings_interface_sort_last_played_title
Library/Container strings (7):
- add_game, back, last_played, library_app_source, library_never_played, container_config_custom_resolution_* (4 strings)
🤖 Prompt for AI Agents
In @app/src/main/res/values-de/strings.xml around lines 906 - 945, The German
localization file contains English placeholder text for many Theme Engine,
settings and library strings (e.g., add_game, back, last_played,
library_app_source, library_never_played, container_config_custom_resolution_*
and theme_*/settings_* keys); replace each English string with an accurate
German translation while preserving XML escapes and formatting (keep %1$s and %s
placeholders intact, preserve \n in theme_add_dialog_message and any escaped
quotes like theme_add_dont_show_again), then run a quick lint/validate of
values-de/strings.xml to ensure well-formed XML and no missing translations
remain for these keys.
|
Let me know when you've addressed the coderabbitai comments and I'll give this a proper review |
2 participants
Theme Engine System
Summary
This PR introduces a comprehensive Theme Engine for GameNative, allowing theme creators to completely customize the appearance of the game library through XML-based theme definitions. The system supports multiple layout types, responsive breakpoints, custom fixed UI elements, and a library of 8 built-in themes - 5 new, and the 3 existing.
This functionality needs to be toggled on via the settings for now, and can thus be treated as an experimental feature, and the user won't experience any changes unless they manually choose to enable the theme engine.
It supports external themes, and it would be possible to have a theme "store" in the future, or have the creators share the themes through Discord.
I have created a detailed standalone HTML documentation on how to create themes, with examples, quick start guide and examples. - This could be hosted as github page in the repo, or seperately.
Features
🎨 Theme Engine Core
@{...}syntax for dynamic values (game title, cover art, play time, etc.)📐 Layout System
🃏 Card Layers
Highlights:
🎯 Fixed UI Elements
🧭 Navigation System
navigateUp,navigateDown, etc.) for more precise control if needed📦 Built-in Themes
8 ready-to-use themes showcasing different styles and capabilities:
📖 Documentation
🔧 Theme Management
Technical Details
New Components
ThemeManager- Theme discovery, loading, and state managementThemeXmlMapper- XML parsing into runtime data modelsThemedGameGrid- Composable renderer for themed layoutsFixedElementRenderer- Renderer for fixed UI overlaysThemeUtils- Positioning, color, and binding utilitiesData Models
ThemeDefinition,Manifest,Variable,BreakpointCard,Layer(Image, Text, Rect, Border, Shadow, Video, Backdrop, Button)LayoutNode(Grid, Carousel),LayoutElement,FixedContainerFixedElement(Header, SearchBar, ProfileButton, etc.)Screenshots
Video Demo
VID_20260105140221.1.mp4
Testing
Breaking Changes
None - the theme engine is an optional feature that can be enabled in settings.
Related Documentation: Theme Documentation
Summary by CodeRabbit
Release Notes
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.