Allow for choosing which steam family library copy to use (DLCs etc) - #1707
Allow for choosing which steam family library copy to use (DLCs etc)#17075TXdYWK wants to merge 7 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Steam Families preferred-copy persistence, lender selection and synchronization, package-ranking support, change notifications, and library UI for viewing and changing the preferred copy. ChangesPreferred copy selection
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant LibraryUser
participant SteamAppScreen
participant SteamService
participant Steam
participant PrefManager
participant LibraryViewModel
LibraryUser->>SteamAppScreen: Select preferred copy
SteamAppScreen->>SteamService: setPreferredCopy(appId, lenderSteamId)
SteamService->>Steam: Set preferred family lender
SteamService->>PrefManager: Persist preferred family lender
SteamService->>LibraryViewModel: Emit PreferredCopyChanged
LibraryViewModel->>LibraryViewModel: Reload affected app and filters
SteamAppScreen-->>LibraryUser: Show updated preferred-copy status
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: 4
🧹 Nitpick comments (1)
app/src/main/java/app/gamenative/ui/screen/library/LibraryAppScreen.kt (1)
988-1009: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd
role = Role.Buttonto the clickable "Change preferred copy" text.The clickable
Textat line 1006 is functionally a button but lacks an accessibility role. Screen readers won't announce it as an actionable element. Addingrole = Role.Buttonimproves this with minimal effort.♿ Suggested accessibility improvement
Text( text = stringResource(R.string.change_preferred_copy), style = MaterialTheme.typography.labelLarge, color = MaterialTheme.colorScheme.primary, - modifier = Modifier.clickable(onClick = onChangePreferredCopy), + modifier = Modifier.clickable( + role = Role.Button, + onClick = onChangePreferredCopy, + ), )🤖 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 988 - 1009, Update the clickable “Change preferred copy” Text in the displayInfo.showChangePreferredCopy block to provide an accessibility button role through its Modifier.clickable configuration. Preserve the existing onChangePreferredCopy callback and styling.
🤖 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/PrefManager.kt`:
- Around line 1410-1443: Update setPreferredFamilyLender to perform the read,
mutation, and persistence within a single dataStore.edit transaction instead of
using the synchronous preferredFamilyLenders getter followed by an asynchronous
write. Decode the current PREFERRED_FAMILY_LENDERS_JSON value inside that block,
apply the add or removal for appId, and persist the resulting map atomically,
removing the preference when it becomes empty.
In `@app/src/main/java/app/gamenative/service/SteamService.kt`:
- Around line 553-555: In the surrounding service method, update the userSteamId
handling to read the computed getter once into a local variable, return
emptyList() when that captured value is null, and derive both selfSteamId and
selfAccountId from the same non-null value. Remove the later userSteamId!!
re-read while preserving the existing empty-list behavior.
- Around line 549-620: Move the database-backed work in
SteamService.getPreferredCopyOptions and its callers
hasMultiplePreferredCopyOptions and getActivePreferredCopy out of synchronous
composition paths by exposing suspend or asynchronous APIs and loading the
results from a coroutine before rendering. Remove the runBlocking calls around
license retrieval, findLicenseForLender, and countDlcForLender, while preserving
the existing option ordering and preferred/self fallback behavior.
- Around line 552-607: Update getPreferredCopyOptions and its
findLicenseForLender/countDlcForLender flow to reuse the licenses already loaded
from licenseDao.getAllLicenses instead of re-reading steam_license per owner.
Pass the loaded license data through the lookup methods or use app/owner-scoped
DAO queries, and update the wrappers that call getPreferredCopyOptions so they
do not repeat the same computation.
---
Nitpick comments:
In `@app/src/main/java/app/gamenative/ui/screen/library/LibraryAppScreen.kt`:
- Around line 988-1009: Update the clickable “Change preferred copy” Text in the
displayInfo.showChangePreferredCopy block to provide an accessibility button
role through its Modifier.clickable configuration. Preserve the existing
onChangePreferredCopy callback and styling.
🪄 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: 9e6d7cc3-6020-41ec-a236-ef04d8296cb2
📒 Files selected for processing (12)
app/src/main/java/app/gamenative/PrefManager.ktapp/src/main/java/app/gamenative/data/PreferredCopyOption.ktapp/src/main/java/app/gamenative/events/AndroidEvent.ktapp/src/main/java/app/gamenative/service/SteamService.ktapp/src/main/java/app/gamenative/ui/data/GameDisplayInfo.ktapp/src/main/java/app/gamenative/ui/enums/AppOptionMenuType.ktapp/src/main/java/app/gamenative/ui/model/LibraryViewModel.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/ui/screen/library/appscreen/SteamAppScreen.ktapp/src/main/java/app/gamenative/ui/screen/library/components/GameOptionsPanel.ktapp/src/main/res/values/strings.xml
There was a problem hiding this comment.
All reported issues were addressed across 12 files
Tip: cubic used a learning from your PR history. Let your coding agent read cubic learnings directly with the cubic MCP.
Re-trigger cubic
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/app/gamenative/service/SteamService.kt (1)
558-568: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse cached license owners only as a fallback.
When
GetSharedLibraryAppshas current owners, this loop also adds every historical DB-license owner. Expired or removed lenders can therefore appear as selectable copies, then fail atsetPreferredLender. Only derive owners fromlicensesForAppwhenfamilyAppOwnerSteamIds[appId]is absent or empty.Proposed fix
- svc.familyAppOwnerSteamIds[appId]?.let { ownerSteamIds.addAll(it) } - // Load licenses once; reuse for owner discovery, package lookup, and DLC counts. val allLicenses = svc.licenseDao.getAllLicenses() val licensesForApp = allLicenses.filter { appId in it.appIds } - for (license in licensesForApp) { - for (accountId in license.ownerAccountId) { - ownerSteamIds.add(SteamID(accountId.toLong(), EUniverse.Public, EAccountType.Individual).convertToUInt64()) + val sharedOwners = svc.familyAppOwnerSteamIds[appId] + if (!sharedOwners.isNullOrEmpty()) { + ownerSteamIds.addAll(sharedOwners) + } else { + for (license in licensesForApp) { + for (accountId in license.ownerAccountId) { + ownerSteamIds.add( + SteamID( + accountId.toLong(), + EUniverse.Public, + EAccountType.Individual, + ).convertToUInt64(), + ) + } } }🤖 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/service/SteamService.kt` around lines 558 - 568, Update the owner discovery logic around familyAppOwnerSteamIds and licensesForApp so database license owners are added only when svc.familyAppOwnerSteamIds[appId] is absent or empty. Preserve the current owners when available, and use licensesForApp solely as the fallback source.
🤖 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.
Outside diff comments:
In `@app/src/main/java/app/gamenative/service/SteamService.kt`:
- Around line 558-568: Update the owner discovery logic around
familyAppOwnerSteamIds and licensesForApp so database license owners are added
only when svc.familyAppOwnerSteamIds[appId] is absent or empty. Preserve the
current owners when available, and use licensesForApp solely as the fallback
source.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d827dec3-f79a-443a-b5c7-ba045d6245fb
📒 Files selected for processing (5)
app/src/main/java/app/gamenative/PrefManager.ktapp/src/main/java/app/gamenative/service/SteamService.ktapp/src/main/java/app/gamenative/ui/screen/library/LibraryAppScreen.ktapp/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.ktapp/src/main/res/values/strings.xml
🚧 Files skipped from review as they are similar to previous changes (3)
- app/src/main/java/app/gamenative/ui/screen/library/LibraryAppScreen.kt
- app/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.kt
- app/src/main/java/app/gamenative/PrefManager.kt
|
Could you please provide a recording? |
|
@utkarshdalal I will (even though I'd rather not display my/my family members' nicknames). I'm doing a few QoL improvements that I wanted to see after using this for a few days. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/screen/library/appscreen/SteamAppScreen.kt`:
- Line 1639: Update the save/rollback launch near rememberCoroutineScope() in
the dialog branch to reuse the longer-lived coroutine scope declared above,
rather than creating a dialog-scoped scope. Ensure dismissing the dialog cannot
cancel the RPC or rollback after the optimistic UI update.
🪄 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: 4b452dae-6da7-40b8-b044-d221297c5c57
📒 Files selected for processing (7)
app/src/main/java/app/gamenative/db/dao/SteamAppDao.ktapp/src/main/java/app/gamenative/service/SteamService.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/SteamAppScreen.ktapp/src/main/java/app/gamenative/utils/KeyValueUtils.ktapp/src/main/res/values/strings.xml
🚧 Files skipped from review as they are similar to previous changes (4)
- app/src/main/res/values/strings.xml
- app/src/main/java/app/gamenative/ui/screen/library/LibraryAppScreen.kt
- app/src/main/java/app/gamenative/ui/data/GameDisplayInfo.kt
- app/src/main/java/app/gamenative/service/SteamService.kt
| var options by remember(gameId) { mutableStateOf<List<PreferredCopyOption>>(emptyList()) } | ||
| var current by remember(gameId) { mutableStateOf<PreferredCopyOption?>(null) } | ||
| var isLoadingDlcCounts by remember(gameId) { mutableStateOf(true) } | ||
| val scope = rememberCoroutineScope() |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the inner scope shadows the longer-lived AdditionalDialogs scope.
rg -n -C4 '\brememberCoroutineScope\(\)|hidePreferredCopyDialog\(gameId\)|setPreferredCopy\(' \
app/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.ktRepository: utkarshdalal/GameNative
Length of output: 4114
🏁 Script executed:
sed -n '1008,1050p' app/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.kt
printf '\n---\n'
sed -n '1628,1715p' app/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.ktRepository: utkarshdalal/GameNative
Length of output: 6701
Launch the save/rollback from the longer-lived scope. This rememberCoroutineScope() is inside the dialog branch, so dismissing the dialog cancels it and can drop the RPC/rollback after the optimistic UI update. Reuse the scope declared above instead.
🤖 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/SteamAppScreen.kt`
at line 1639, Update the save/rollback launch near rememberCoroutineScope() in
the dialog branch to reuse the longer-lived coroutine scope declared above,
rather than creating a dialog-scoped scope. Ensure dismissing the dialog cannot
cancel the RPC or rollback after the optimistic UI update.
There was a problem hiding this comment.
All reported issues were addressed across 7 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Sync with utkarshdalal/GameNative while keeping fork preferred-copy QoL. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@utkarshdalal done. please let me know when you've reviewed the video so i can remove it. |
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
familyGroupId emits before owner caches fill; bump a version so Change Preferred Copy reappears after refresh. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Description
Added a way to choose which library copy to use. Allows for using someone else's copy (e.g. when they have more DLC than you). This was missing even though family libraries were already supported.
Vibecoded. Please review.
video_2026-07-16_20-46-58.mp4
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 per-game preferred copy selector for Steam Families so you can choose your copy or a family member’s (e.g., to use their DLC). The choice is saved, synced with Steam Family Groups, and applied to ownership, launch, and library UI.
New Features
listofdlc, depotdlcAppId, and local DLC rows; fills missing lender package appIds via PICS before counting.PrefManager, synced via Family Groups (Get/SetPreferredLender), reapplied on login and after license/PICS updates, and applied offline by updating local owner/package. Library ordering, PICS package ranking, andGamePlayedInfo.ownerIdnow prefer the chosen lender; emitsPreferredCopyChangedto refresh the library UI.Bug Fixes
Written for commit bfdcfb7. Summary will update on new commits.
Summary by CodeRabbit
Summary by CodeRabbit