feat: steam achievements viewer (updated #1511)#1695
Open
VinceBT wants to merge 1 commit into
Open
Conversation
Contributor
📝 WalkthroughWalkthroughSteam achievements are fetched from Steam, mapped into localized display models, loaded for Steam library items, and shown through an overview row and animated detail dialog. Shared ChangesSteam achievements
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant BaseAppScreen
participant SteamService
participant SteamUserStats
participant AppScreenContent
participant AchievementsDialog
BaseAppScreen->>SteamService: fetchAchievementsForDisplay(appId)
SteamService->>SteamUserStats: fetch expanded achievements
SteamUserStats-->>SteamService: achievement data
SteamService-->>BaseAppScreen: List<Achievement> or null
BaseAppScreen->>AppScreenContent: pass achievements state
AppScreenContent->>AchievementsDialog: open detailed achievement list
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
app/src/main/java/app/gamenative/ui/screen/library/LibraryAppScreen.kt (2)
1250-1250: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate
grayMatrixconstruction inAchievementsRowandAchievementsDialog.Both composables independently build the same
ColorMatrix().apply { setToSaturation(0f) }. Consider hoisting it to a single top-levelprivate val(it's stateless) to avoid the duplication.Also applies to: 1380-1380
🤖 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 1250, Hoist the duplicated grayscale matrix into a single top-level private ColorMatrix value, then update both AchievementsRow and AchievementsDialog to reference it instead of constructing separate instances with remember.
1237-1506: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd
contentDescriptionto achievement icon images for accessibility.Landscapist's
ImageOptionssupports acontentDescriptionfield, but none of the threeCoilImagecalls here (row icons, "+N" stack icon, dialog list icon) set it. Screen-reader users get no label for achievement artwork.♿ Suggested addition
CoilImage( imageModel = { iconUrl ?: "" }, imageOptions = ImageOptions( contentScale = ContentScale.Crop, + contentDescription = ach.displayName ?: ach.name, colorFilter = if (ach.isUnlocked) null else ColorFilter.colorMatrix(grayMatrix), ), ... )🤖 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 1237 - 1506, Add accessibility labels to all three achievement CoilImage calls in AchievementsRow and AchievementsDialog by setting ImageOptions.contentDescription using each achievement’s display name or name, with a suitable fallback for the stacked “+N” preview icon.
🤖 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-3113: Update fetchAchievementsForDisplay to rethrow
CancellationException before the generic Exception handler, preserving coroutine
cancellation. Bound the Steam operations, including getUserStats(...).await()
and achievement expansion, with an appropriate coroutine timeout such as
withTimeout, while keeping existing error logging and null fallback behavior.
In `@app/src/main/java/app/gamenative/ui/component/InfoCard.kt`:
- Around line 61-66: Remove the unnecessary rememberCoroutineScope and nested
scope.launch from LaunchedEffect(isFocused); call the suspend
bringIntoViewRequester.bringIntoView() directly when isFocused is true so it
remains tied to the effect lifecycle. Remove the now-unused coroutine scope and
launch imports if they are not used elsewhere.
In `@app/src/main/res/values-de/strings.xml`:
- Around line 1571-1577: Translate the German localization value for the
achievements_total string from “Total” to an appropriate German term, preferably
“Gesamt” or “Insgesamt,” while leaving the other achievement strings unchanged.
In `@app/src/main/res/values-ru/strings.xml`:
- Around line 1558-1563: Translate the achievements_total string in the Russian
resources from the English “Total” to its Russian equivalent, matching the
surrounding Cyrillic translations.
In `@app/src/main/res/values-uk/strings.xml`:
- Around line 1626-1631: Translate the achievements_total string in the
Ukrainian achievements resource block from "Total" to the appropriate Ukrainian
equivalent, keeping the existing string name and resource structure unchanged.
In `@app/src/main/res/values-zh-rCN/strings.xml`:
- Around line 1650-1655: Translate the `achievements_total` string in the Steam
achievements resource block from “Total” to the appropriate Simplified Chinese
text, keeping the existing string name unchanged.
In `@app/src/main/res/values-zh-rTW/strings.xml`:
- Around line 1641-1646: Translate the achievements_total string in the
achievements resource block from “Total” to the appropriate Traditional Chinese
wording, matching the existing zh-rTW translations and the corresponding zh-rCN
resource.
---
Nitpick comments:
In `@app/src/main/java/app/gamenative/ui/screen/library/LibraryAppScreen.kt`:
- Line 1250: Hoist the duplicated grayscale matrix into a single top-level
private ColorMatrix value, then update both AchievementsRow and
AchievementsDialog to reference it instead of constructing separate instances
with remember.
- Around line 1237-1506: Add accessibility labels to all three achievement
CoilImage calls in AchievementsRow and AchievementsDialog by setting
ImageOptions.contentDescription using each achievement’s display name or name,
with a suitable fallback for the stacked “+N” preview icon.
🪄 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: dc134ec6-8031-43de-91b7-5cca0bb9a71d
📒 Files selected for processing (21)
app/src/main/java/app/gamenative/service/SteamService.ktapp/src/main/java/app/gamenative/ui/component/InfoCard.ktapp/src/main/java/app/gamenative/ui/data/Achievement.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
VinceBT
force-pushed
the
feat/steam-achievements
branch
from
d605396 to
3bce1dc
Compare
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found and verified against the latest diff
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: A fetch failure now looks identical to a game with no achievements, so the whole section disappears on transient Steam errors. Surfacing an error state or retrying would keep the details page from silently dropping the feature.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| } | ||
| } | ||
|
|
||
| suspend fun fetchAchievementsForDisplay(appId: Int): List<Achievement>? { |
Contributor
There was a problem hiding this comment.
P2: A fetch failure now looks identical to a game with no achievements, so the whole section disappears on transient Steam errors. Surfacing an error state or retrying would keep the details page from silently dropping the feature.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/app/gamenative/service/SteamService.kt, line 3074:
<comment>A fetch failure now looks identical to a game with no achievements, so the whole section disappears on transient Steam errors. Surfacing an error state or retrying would keep the details page from silently dropping the feature.</comment>
<file context>
@@ -3070,6 +3071,46 @@ class SteamService : Service(), IChallengeUrlChanged {
}
}
+ suspend fun fetchAchievementsForDisplay(appId: Int): List<Achievement>? {
+ if (!isConnected) return null
+ return try {
</file context>
3bce1dc to
092ea9c
Compare
VinceBT
force-pushed
the
feat/steam-achievements
branch
from
092ea9c to
ee695a0
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This is @phobos665's Steam achievements viewer from #1511, brought up to current
masterwith some refinements on top. Most of the credit here belongs to @phobos665 — the viewer, the data plumbing and the schema handling are all theirs; I rebased the work onto currentmasterand polished the UI and localization.Quoting the original PR (#1511):
On top of that, this PR:
x / ycount.Implementation note: this replaces the local
InfoCardinLibraryAppScreenwith the sharedui/component/InfoCardcomponent (a superset that addsonClick/content), so a few neighbouring cards move to it as well.Recording
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 achievements viewer to the Game Details page: a compact card with progress and icons, plus a fullscreen list. Text is localized with English fallbacks; icons come from the Steam CDN; fetching uses a 15s timeout and retries for reliability.
New Features
Refactors
ui/component/InfoCard(clickable/focusable with chevron) and updated game detail cards to use it.ui/data/Achievement,SteamService.fetchAchievementsForDisplay(),SteamUtils.steamLanguageForAppLocale(), andSteamUtils.getBaseAchievementIconUrl(); fetch runs for Steam games inBaseAppScreen(15s timeout, up to 3 attempts with a short delay); added localized strings across all locales.Written for commit ee695a0. Summary will update on new commits.
Summary by CodeRabbit