feat: steam achievement viewer - #1511
Conversation
…ions. Just need to filter out the achievevements work to only for for Steam for now.
The Vulkan scanout fast-path presents the game buffer zero-copy via SurfaceControl, bypassing window.frag entirely. As a result FSR/scaling and color effects rendered but were never applied to fullscreen output: the displayed resolution was the container size, not the screen size. Gate scanout on whether any effect/filter/color adjustment is active (effectsRequireCompositor). When effects are needed, tear down scanout and route through the textured-quad compositor path; when they are cleared, re-establish scanout. Path switching is wired into setEffect, setNativeMode, onSurfaceCreated, and the scene filter. Java-only change; no native/.so rebuild required. FPS limiting is unaffected (it lives in PresentExtension, renderer-agnostic). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
… into feat/steam-achievement-viewer
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (18)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (10)
📝 WalkthroughWalkthroughAdds Steam achievement display to the game library detail screen. Introduces ChangesSteam achievements in library screens
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
… into feat/steam-achievement-viewer
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
app/src/main/java/app/gamenative/ui/screen/library/LibraryAppScreen.kt (3)
523-524: 💤 Low valueUnused parameter:
achievementsAppIdis declared but never referenced.The
achievementsAppIdparameter is passed toAppScreenContentbut not used anywhere in the function body. Either remove it or document its intended future use.🤖 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/LibraryAppScreen.kt` around lines 523 - 524, The parameter achievementsAppId in the AppScreenContent signature is unused; remove the achievementsAppId parameter from the AppScreenContent function declaration and any callers that pass achievementsAppId (or, if intended for future use, add a clear TODO/KDoc explaining its planned role and reference it in the body). Update the function signature that currently lists "achievements: List<Achievement>? = null, achievementsAppId: Int? = null" to drop achievementsAppId and remove its usage from the call sites (or replace with documented placeholder) so there are no unused parameters.
1314-1321: 💤 Low valueHardcoded content description should use a string resource.
The content description "Star" is hardcoded in English. For accessibility and localization consistency, use a string resource.
💬 Suggested fix
Icon( imageVector = Icons.Filled.Star, - contentDescription = "Star", + contentDescription = stringResource(R.string.achievements_complete), tint = Color(0xFFFFD700), modifier = Modifier.size(16.dp), )Add to
strings.xml:<string name="achievements_complete">All achievements unlocked</string>🤖 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/LibraryAppScreen.kt` around lines 1314 - 1321, Replace the hardcoded contentDescription "Star" used with the Icon (Icons.Filled.Star) by loading a string resource (e.g. stringResource(R.string.achievements_complete)) and add the corresponding entry in strings.xml (name="achievements_complete") so the contentDescription is localized; ensure the composable imports androidx.compose.ui.res.stringResource and use that resource in the Icon's contentDescription within LibraryAppScreen (or the composable containing the Icon).
1404-1404: 💤 Low value
Achievement.getFormattedUnlockTimeparameter is duplicative
Achievement.getFormattedUnlockTime(unlockTimestamp: Int)requires the parameter, so the callach.getFormattedUnlockTime(ach.unlockTimestamp)is correct; the duplication is that the method ignoresthis.unlockTimestampand just formats the passed value. Consider changing the API togetFormattedUnlockTime(): String?(or usingthis.unlockTimestamp) to remove the redundant argument.🤖 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/LibraryAppScreen.kt` at line 1404, The method Achievement.getFormattedUnlockTime currently takes unlockTimestamp as a parameter but callers pass ach.unlockTimestamp (e.g., ach.getFormattedUnlockTime(ach.unlockTimestamp)), creating a redundant API; fix by removing the parameter and switching the implementation to use this.unlockTimestamp (or alternatively update callers to pass the value consistently) so the signature becomes getFormattedUnlockTime(): String? and update all call sites like ach.getFormattedUnlockTime() (search for Achievement.getFormattedUnlockTime and unlockTimestamp usage to update implementation and callers).app/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.kt (1)
930-943: 💤 Low valueConsider adding error handling for achievement fetching.
If
fetchAchievementsForDisplaythrows an exception, the coroutine will fail andachievementsStateremains null. While this gracefully hides the achievements section, logging the error would help with debugging.🛡️ Suggested improvement
LaunchedEffect(libraryItem.gameId) { when(libraryItem.gameSource){ GameSource.STEAM -> { - achievementsState = withContext(Dispatchers.IO) { - app.gamenative.service.SteamService.fetchAchievementsForDisplay(libraryItem.gameId) + achievementsState = try { + withContext(Dispatchers.IO) { + app.gamenative.service.SteamService.fetchAchievementsForDisplay(libraryItem.gameId) + } + } catch (e: Exception) { + Timber.d(e, "Failed to fetch achievements for game ${libraryItem.gameId}") + null } } GameSource.EPIC -> { } // Add later with Epic achievements GameSource.GOG -> { } // Add later with GOG achievements GameSource.AMAZON -> { } GameSource.CUSTOM_GAME -> { } } }🤖 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 930 - 943, Wrap the call to app.gamenative.service.SteamService.fetchAchievementsForDisplay inside a try/catch within the LaunchedEffect(libraryItem.gameId) block so exceptions don’t crash the coroutine; on failure catch the Throwable, set achievementsState to a safe fallback (null or empty list) and log the error (use your app logger or Log/Timber) including libraryItem.gameId and the exception message so you can debug; apply the same pattern for other GameSource branches when implemented.
🤖 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/service/SteamService.kt`:
- Around line 3074-3078: The current code only checks isConnected then
dereferences steamUser.steamID with !! which can throw when the session isn't
authenticated; update the guard to explicitly verify authentication by ensuring
instance?._steamUser is non-null and that steamUser.steamID is non-null (avoid
using !!) before calling _steamUserStats.getUserStats(appId,
steamUser.steamID)?.await(); if either check fails, return null early so
getUserStats is never invoked in a non-authenticated state; adjust the block
around instance?._steamUser, steamUser.steamID, and getUserStats to use
safe-null checks and early returns.
---
Nitpick comments:
In
`@app/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.kt`:
- Around line 930-943: Wrap the call to
app.gamenative.service.SteamService.fetchAchievementsForDisplay inside a
try/catch within the LaunchedEffect(libraryItem.gameId) block so exceptions
don’t crash the coroutine; on failure catch the Throwable, set achievementsState
to a safe fallback (null or empty list) and log the error (use your app logger
or Log/Timber) including libraryItem.gameId and the exception message so you can
debug; apply the same pattern for other GameSource branches when implemented.
In `@app/src/main/java/app/gamenative/ui/screen/library/LibraryAppScreen.kt`:
- Around line 523-524: The parameter achievementsAppId in the AppScreenContent
signature is unused; remove the achievementsAppId parameter from the
AppScreenContent function declaration and any callers that pass
achievementsAppId (or, if intended for future use, add a clear TODO/KDoc
explaining its planned role and reference it in the body). Update the function
signature that currently lists "achievements: List<Achievement>? = null,
achievementsAppId: Int? = null" to drop achievementsAppId and remove its usage
from the call sites (or replace with documented placeholder) so there are no
unused parameters.
- Around line 1314-1321: Replace the hardcoded contentDescription "Star" used
with the Icon (Icons.Filled.Star) by loading a string resource (e.g.
stringResource(R.string.achievements_complete)) and add the corresponding entry
in strings.xml (name="achievements_complete") so the contentDescription is
localized; ensure the composable imports androidx.compose.ui.res.stringResource
and use that resource in the Icon's contentDescription within LibraryAppScreen
(or the composable containing the Icon).
- Line 1404: The method Achievement.getFormattedUnlockTime currently takes
unlockTimestamp as a parameter but callers pass ach.unlockTimestamp (e.g.,
ach.getFormattedUnlockTime(ach.unlockTimestamp)), creating a redundant API; fix
by removing the parameter and switching the implementation to use
this.unlockTimestamp (or alternatively update callers to pass the value
consistently) so the signature becomes getFormattedUnlockTime(): String? and
update all call sites like ach.getFormattedUnlockTime() (search for
Achievement.getFormattedUnlockTime and unlockTimestamp usage to update
implementation and callers).
🪄 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
Run ID: 90576c32-6594-426a-8ae6-80bb9fe4b161
📒 Files selected for processing (22)
app/src/main/java/app/gamenative/data/DownloadDisplayDetails.ktapp/src/main/java/app/gamenative/service/SteamService.ktapp/src/main/java/app/gamenative/ui/data/Achievements.ktapp/src/main/java/app/gamenative/ui/data/GameDisplayInfo.ktapp/src/main/java/app/gamenative/ui/screen/library/LibraryAppScreen.ktapp/src/main/java/app/gamenative/ui/screen/library/appscreen/BaseAppScreen.ktapp/src/main/java/app/gamenative/utils/SteamUtils.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.xml
There was a problem hiding this comment.
3 issues found across 22 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/main/java/app/gamenative/ui/screen/library/LibraryAppScreen.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/screen/library/LibraryAppScreen.kt:1287">
P2: Missing icon URLs are converted to empty-string image models, causing avoidable failed image requests and wasted Coil pipeline work.</violation>
</file>
<file name="app/src/main/java/app/gamenative/service/SteamService.kt">
<violation number="1" location="app/src/main/java/app/gamenative/service/SteamService.kt:3074">
P2: Guard should check `isLoggedIn` rather than `isConnected`. The `steamID` property is only assigned after a successful logon—during connected-but-not-authenticated windows, `steamUser.steamID!!` will throw a NullPointerException. While the surrounding try-catch prevents a crash, relying on exception-based control flow for a predictable state is fragile. Use `isLoggedIn` (or equivalent session guard) and avoid `!!` by using the already-resolved `userSteamId` property if available.</violation>
<violation number="2" location="app/src/main/java/app/gamenative/service/SteamService.kt:3077">
P2: Missing `EResult` validation after `getUserStats` call can cause empty/partial achievement lists to be shown as "no achievements" instead of an upstream failure.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
3 issues found across 22 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/main/java/app/gamenative/service/SteamService.kt">
<violation number="1" location="app/src/main/java/app/gamenative/service/SteamService.kt:3074">
P2: Guard should check `isLoggedIn` rather than `isConnected`. The `steamID` property is only assigned after a successful logon—during connected-but-not-authenticated windows, `steamUser.steamID!!` will throw a NullPointerException. While the surrounding try-catch prevents a crash, relying on exception-based control flow for a predictable state is fragile. Use `isLoggedIn` (or equivalent session guard) and avoid `!!` by using the already-resolved `userSteamId` property if available.</violation>
<violation number="2" location="app/src/main/java/app/gamenative/service/SteamService.kt:3077">
P2: Missing `EResult` validation after `getUserStats` call can cause empty/partial achievement lists to be shown as "no achievements" instead of an upstream failure.</violation>
</file>
<file name="app/src/main/java/app/gamenative/ui/screen/library/LibraryAppScreen.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/screen/library/LibraryAppScreen.kt:1287">
P2: Missing icon URLs are converted to empty-string image models, causing avoidable failed image requests and wasted Coil pipeline work.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
Merging planned soon ? |
Being discussed for when it's appropriate. I've also got Epic achievements working in a separate branch with full achievement UI too :) |
Speaking of, I believe you're quite versed in UI & UX. Any feedback regarding the visuals? I'll chat with Utkarsh about getting this in since it's highly requested by the community and the work is done anyway. |
|
Props to you, this looks great, and simple enough. |
|
This has been succeeded by #1695 We'll get that merged instead. |
Thank you! I'm really happy with it, had another developer add on-top of my work here: #1695 |
Description
View Steam Achievements in the Game Details Page (LibraryAppScreen).
Also shows a little gold star for those who get 100% of achievements.
Note: Had to adjust the params for AppScreenContent due to it having too many params which crashed the app. Created a DownloadDisplayDetails data class to add them. Happy to discuss.
Also, this has been made so that it's generic enough to support both Epic & GOG achievements later.
Recording
Horizontal in modal (My phone is very wide compared to height):

Horizontal game details achievements:

Vertical Modal:

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 a Steam achievement viewer to Game Details. Shows icons, progress, a “View all” dialog with details, and a gold star at 100% completion, plus hardening and translations.
New Features
SteamService.fetchAchievementsForDisplay; Steam-only for now; icon URLs built withSteamUtils.getBaseAchievementIconUrl.Refactors
AppScreenContentdownload args intoDownloadDisplayDetailsto prevent a param overflow crash and simplify calls.javasteamto1.8.0.1-19-SNAPSHOT.Written for commit bb7cdb8. Summary will update on new commits.
Summary by CodeRabbit
Release Notes
New Features
Localization