Add in-app community config browser - #1782
Conversation
📝 WalkthroughWalkthroughAdds community configuration discovery, compatibility filtering, browsing, preview, and application. The change introduces structured parsing with value preservation, composite run identity handling, localized UI resources, and tests for retrieval concurrency and configuration application. ChangesCommunity configuration feature
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant LibraryScreen
participant CommunityConfigsDialog
participant CommunityConfigService
participant BestConfigService
participant Container
LibraryScreen->>CommunityConfigsDialog: open community configurations
CommunityConfigsDialog->>CommunityConfigService: fetch compatible runs
CommunityConfigService-->>CommunityConfigsDialog: merged configuration results
CommunityConfigsDialog->>LibraryScreen: select configuration
LibraryScreen->>BestConfigService: parse with preserved values
BestConfigService-->>LibraryScreen: parsed config and missing components
LibraryScreen->>Container: apply configuration
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
app/src/main/java/app/gamenative/api/CommunityConfigService.kt (3)
939-955: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueTimestamp is re-parsed on every comparison.
createdAtMillis()runsOffsetDateTime.parseinside the comparator, so each run'screatedAtis parsed O(log n) times (up to ~200 runs per compatible page). Precomputing the key once is cheaper and keeps ordering identical.♻️ Suggested refactor
- val comparator = when (sort) { - CommunityConfigSort.HIGHEST_RATED -> compareByDescending<CommunityConfigRun> { it.rating } - .thenByDescending { it.createdAtMillis() } - .thenByDescending { it.id } - CommunityConfigSort.NEWEST -> compareByDescending<CommunityConfigRun> { it.createdAtMillis() } - .thenByDescending { it.id } - } - return runs.sortedWith(comparator) + val createdAt = runs.associateBy({ it.id }, { it.createdAtMillis() }) + fun key(run: CommunityConfigRun) = createdAt[run.id] ?: Long.MIN_VALUE + val comparator = when (sort) { + CommunityConfigSort.HIGHEST_RATED -> compareByDescending<CommunityConfigRun> { it.rating } + .thenByDescending { key(it) } + .thenByDescending { it.id } + CommunityConfigSort.NEWEST -> compareByDescending<CommunityConfigRun> { key(it) } + .thenByDescending { it.id } + } + return runs.sortedWith(comparator)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/app/gamenative/api/CommunityConfigService.kt` around lines 939 - 955, Update sortCommunityRuns to precompute each run’s parsed createdAtMillis value once before sorting, retaining Long.MIN_VALUE for invalid timestamps. Sort the precomputed entries using the existing HIGHEST_RATED and NEWEST ordering keys, then return the original CommunityConfigRun objects in the resulting order.
155-192: 🩺 Stability & Availability | 🔵 Trivial | 🏗️ Heavy lift
awaitPermitsleeps while holding the monitor and isn't cancellation-aware.
awaitPermit()is@Synchronizedand callssleepNanos(defaultTimeUnit.NANOSECONDS.sleep) inside the lock, so it blocks aDispatchers.IOthread for up to the full window and cannot be interrupted by coroutine cancellation. When the user dismisses the community dialog, the in-flight throttled request keeps the thread parked, andpostponefrom any other thread would block behind it too.Consider making the throttle suspending (
Mutex+delay) or at least releasing the lock around the wait, keeping the injectable clock/sleep hooks for the existing tests.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/app/gamenative/api/CommunityConfigService.kt` around lines 155 - 192, Refactor awaitPermit so throttling waits do not hold the synchronized monitor or block an IO thread, while remaining cancellation-aware. Use a suspending coordination mechanism such as Mutex with delay, or otherwise release the lock before waiting and reacquire it to recheck quota and blockedUntilNanos; preserve the injectable clock and sleep hooks required by existing tests, and ensure postpone can proceed concurrently.
296-340: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConcurrency here is nullified by the global request gate.
Semaphore(minOf(deviceIds.size, MAX_CONCURRENT_DEVICE_REQUESTS))plusasync/awaitAllsuggests up to 4 parallel device fetches, butexecute()wraps every call insynchronized(requestGate), so all requests run strictly one at a time anyway. Either drop the semaphore/asyncfan-out (simpler, same behavior) or narrow the gate to the throttle bookkeeping only so the intended parallelism actually happens.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/app/gamenative/api/CommunityConfigService.kt` around lines 296 - 340, Remove the ineffective Semaphore and async/awaitAll fan-out around fetchDeviceConfigSlice, and fetch each valid device slice sequentially while preserving the existing slice merging, sorting, pagination, and totals behavior. Keep the change scoped to the multi-device branch of the surrounding configuration fetch flow.app/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.kt (1)
1556-1584: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse dedicated state instead of reusing
containerData.
containerDataalso backsContainerConfigDialog's edit buffer, and it starts asContainerData(), so the community dialog renders one frame with emptycurrentLaunchArguments/currentEnvironmentVariablesbefore the IO load lands (and the reload mutates the config dialog's state if both are active). A separatecommunityContainerDatastate avoids both.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.kt` around lines 1556 - 1584, Introduce dedicated community dialog state, such as communityContainerData, initialized independently from containerData, and update the communityConfigsRequested LaunchedEffect to load into it. Pass this state to CommunityConfigsDialog for currentLaunchArguments and currentEnvironmentVariables, leaving containerData exclusively for ContainerConfigDialog.app/src/main/java/app/gamenative/utils/BestConfigService.kt (1)
886-896: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winForce-apply returns a populated config and a non-empty
missingComponents.Callers branch on
missingComponents.isNotEmpty()before looking atconfig(seeBaseAppScreen.applyKnownConfigForLibraryItemandContainerConfigTransfer.importConfig), so any future caller passingforceApply = truetoparseConfigResultwould re-open the missing-components dialog despite a successful parse. Today it's safe only because those paths callparseConfigToContainerData. Consider clearing (or renaming to e.g.replacedComponents) when defaults were substituted.Also applies to: 988-992
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/app/gamenative/utils/BestConfigService.kt` around lines 886 - 896, Update parseConfigResult’s forceApply path after replaceWithDefaults so the returned ParsedConfigResult does not report substituted components as missing; clear the collection or use a distinct replaced-components status before constructing the populated result. Preserve the non-force path’s non-empty missingComponents return so callers such as BaseAppScreen.applyKnownConfigForLibraryItem and ContainerConfigTransfer.importConfig still reject unavailable components.app/src/test/java/app/gamenative/api/CommunityConfigServiceTest.kt (1)
826-853: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHelper produces invalid
createdAtfor ids > 9.
"2026-01-0${runId}"yields2026-01-011T00:00:00Zfor therunId = deviceId = 11/12cases (line 662), which failsOffsetDateTime.parseand silently falls back toLong.MIN_VALUEinsortCommunityRuns. It doesn't break the current assertions, but it will mask ordering bugs if those fixtures are reused. Consider padding, e.g."2026-01-%02d".format(runId.coerceIn(1, 28)).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/test/java/app/gamenative/api/CommunityConfigServiceTest.kt` around lines 826 - 853, The configPage helper generates invalid ISO dates when runId exceeds 9; update createdAt formatting in configPage to produce a two-digit, valid day value while constraining it to the supported day range. Preserve valid timestamp parsing for all fixture IDs, including the existing 11/12 cases.app/src/main/java/app/gamenative/ui/component/dialog/CommunityConfigsDialog.kt (1)
787-804: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid magic string for match type comparison.
matchType == "fallback_match"hardcodes a literal that must stay in sync with whatevercommunityConfigMatchType()(imported fromCommunityConfigService) actually returns. A rename/typo on either side silently disables this GPU-mismatch warning without any compile-time signal.Consider exposing a shared constant (e.g.
CommunityConfigService.MATCH_TYPE_FALLBACK) alongsidecommunityConfigMatchType()and referencing it here instead of the literal.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/app/gamenative/ui/component/dialog/CommunityConfigsDialog.kt` around lines 787 - 804, Replace the hardcoded "fallback_match" comparison in the fallback warning block with a shared constant exposed by CommunityConfigService, such as MATCH_TYPE_FALLBACK, and ensure communityConfigMatchType() uses the same constant so the warning remains synchronized.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@app/src/main/java/app/gamenative/ui/component/dialog/CommunityConfigsDialog.kt`:
- Around line 262-286: Update the pagination merge in the coroutine around
sortCommunityRuns to deduplicate CommunityConfigRun entries using the same
composite identity as the LazyColumn: id, device.id, and createdAt. Replace
distinctBy { it.id } with a matching composite key while preserving the existing
sorting, totals, and pagination behavior.
In `@app/src/main/java/app/gamenative/utils/BestConfigService.kt`:
- Around line 921-928: Update the startupSelection mapping in the best-config
parser to always emit a Byte, including when preserveConfigValues is true. Align
this branch with resolveMissingManifestInstallRequests and the Int? expectations
in applyBestConfigMapToContainerData, while preserving the existing fallback and
filtering behavior.
In `@app/src/main/res/values-ru/strings.xml`:
- Line 130: Update the community_config_highest_rated Russian string to
explicitly mean “С самым высоким рейтингом,” replacing the ambiguous “По
рейтингу” translation while preserving the existing resource key.
---
Nitpick comments:
In `@app/src/main/java/app/gamenative/api/CommunityConfigService.kt`:
- Around line 939-955: Update sortCommunityRuns to precompute each run’s parsed
createdAtMillis value once before sorting, retaining Long.MIN_VALUE for invalid
timestamps. Sort the precomputed entries using the existing HIGHEST_RATED and
NEWEST ordering keys, then return the original CommunityConfigRun objects in the
resulting order.
- Around line 155-192: Refactor awaitPermit so throttling waits do not hold the
synchronized monitor or block an IO thread, while remaining cancellation-aware.
Use a suspending coordination mechanism such as Mutex with delay, or otherwise
release the lock before waiting and reacquire it to recheck quota and
blockedUntilNanos; preserve the injectable clock and sleep hooks required by
existing tests, and ensure postpone can proceed concurrently.
- Around line 296-340: Remove the ineffective Semaphore and async/awaitAll
fan-out around fetchDeviceConfigSlice, and fetch each valid device slice
sequentially while preserving the existing slice merging, sorting, pagination,
and totals behavior. Keep the change scoped to the multi-device branch of the
surrounding configuration fetch flow.
In
`@app/src/main/java/app/gamenative/ui/component/dialog/CommunityConfigsDialog.kt`:
- Around line 787-804: Replace the hardcoded "fallback_match" comparison in the
fallback warning block with a shared constant exposed by CommunityConfigService,
such as MATCH_TYPE_FALLBACK, and ensure communityConfigMatchType() uses the same
constant so the warning remains synchronized.
In
`@app/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.kt`:
- Around line 1556-1584: Introduce dedicated community dialog state, such as
communityContainerData, initialized independently from containerData, and update
the communityConfigsRequested LaunchedEffect to load into it. Pass this state to
CommunityConfigsDialog for currentLaunchArguments and
currentEnvironmentVariables, leaving containerData exclusively for
ContainerConfigDialog.
In `@app/src/main/java/app/gamenative/utils/BestConfigService.kt`:
- Around line 886-896: Update parseConfigResult’s forceApply path after
replaceWithDefaults so the returned ParsedConfigResult does not report
substituted components as missing; clear the collection or use a distinct
replaced-components status before constructing the populated result. Preserve
the non-force path’s non-empty missingComponents return so callers such as
BaseAppScreen.applyKnownConfigForLibraryItem and
ContainerConfigTransfer.importConfig still reject unavailable components.
In `@app/src/test/java/app/gamenative/api/CommunityConfigServiceTest.kt`:
- Around line 826-853: The configPage helper generates invalid ISO dates when
runId exceeds 9; update createdAt formatting in configPage to produce a
two-digit, valid day value while constraining it to the supported day range.
Preserve valid timestamp parsing for all fixture IDs, including the existing
11/12 cases.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f114fad1-5b49-468e-927c-af89bd51d108
📒 Files selected for processing (25)
app/src/main/java/app/gamenative/api/CommunityConfigService.ktapp/src/main/java/app/gamenative/ui/component/dialog/CommunityConfigsDialog.ktapp/src/main/java/app/gamenative/ui/enums/AppOptionMenuType.ktapp/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.ktapp/src/main/java/app/gamenative/ui/screen/library/components/GameOptionsPanel.ktapp/src/main/java/app/gamenative/ui/util/ContainerConfigTransfer.ktapp/src/main/java/app/gamenative/utils/BestConfigService.ktapp/src/main/res/values-da/strings.xmlapp/src/main/res/values-de/strings.xmlapp/src/main/res/values-es/strings.xmlapp/src/main/res/values-fr/strings.xmlapp/src/main/res/values-it/strings.xmlapp/src/main/res/values-ja/strings.xmlapp/src/main/res/values-ko/strings.xmlapp/src/main/res/values-pl/strings.xmlapp/src/main/res/values-pt-rBR/strings.xmlapp/src/main/res/values-ro/strings.xmlapp/src/main/res/values-ru/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/api/CommunityConfigServiceTest.ktapp/src/test/java/app/gamenative/utils/BestConfigServiceTest.ktapp/src/test/java/app/gamenative/utils/CommunityConfigApplicationTest.kt
There was a problem hiding this comment.
All reported issues were addressed across 25 files
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 6 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Description
Adds a Community configs entry to each game's settings menu, allowing users to browse GameNative compatibility reports for the current game. Results can be filtered by same device, same GPU, or compatible GPU family and sorted by highest rated or newest. Selected configs are validated and applied automatically, with missing manifest dependencies downloaded as needed. Launch arguments and environment variables can be reviewed and applied explicitly, while the existing Use known config behavior remains unchanged.
Recording
https://drive.google.com/file/d/1adYFW1s6apMWX18eIdeAwfwEm-oWhbhf/view?usp=drivesdk
Type of Change
Checklist
#code-changes, I have discussed this change there and it has been green-lighted. If I do not have access, I have still provided clear context in this PR. If I skip both, I accept that this change may face delays in review, may not be reviewed at all, or may be closed.CONTRIBUTING.md.Summary by cubic
Adds an in-app Community Configs browser in each game's settings to discover, filter, and apply shared configs from GameNative. Includes a safe apply flow with validation, dependency downloads, optional launch settings, and graceful rate limit handling.
New Features
Refactors
Written for commit a4982cc. Summary will update on new commits.
Summary by CodeRabbit