fix: Prevent overwriting valid package IDs during PICS updates#1230
fix: Prevent overwriting valid package IDs during PICS updates#1230muazkadan wants to merge 2 commits into
Conversation
📝 WalkthroughWalkthroughWhen processing PICS package results, SteamService no longer always overwrites an existing SteamApp.packageId with the current package. It preserves the existing packageId if a matching SteamLicense row exists for that packageId, the license's ELicenseFlags passes the bitmask test (licenseFlags & 8 == 0), and that license’s appIds includes the appid; otherwise it updates to the current pkg.id. Changes
Sequence Diagram(s)sequenceDiagram
participant Proc as PICS Processor
participant Svc as SteamService
participant DB as DB (SteamLicense / SteamApp)
Proc->>Svc: receive package result (appid, pkg)
Svc->>DB: query existing SteamApp by appid
alt steamApp exists
Svc->>DB: query SteamLicense where packageId = steamApp.packageId
DB-->>Svc: license row (flags, appIds) or none
alt license exists and (flags & 8 == 0) and appIds includes appid
Svc->>DB: preserve steamApp.packageId (no change)
else
Svc->>DB: update steamApp.packageId = pkg.id
end
else no steamApp
Svc->>DB: insert new SteamApp row with packageId = pkg.id
end
Svc-->>Proc: done
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/app/gamenative/service/SteamService.kt`:
- Around line 3907-3915: The existing logic in steamApp creation preserves
existingApp.packageId based only on license validity; update the conditional in
the steamApp assignment (the appDao.findApp(...)?.let { existingApp -> ... }
block) to also verify that the retrieved currentLicense actually contains this
appid before keeping existingApp.packageId: fetch currentLicense via
licenseDao.findLicense(existingApp.packageId), then require currentLicense !=
null && ELicenseFlags.code(currentLicense.licenseFlags) and 8 == 0 &&
currentLicense.appIds (or the field that lists app IDs) contains appid; if any
check fails, set packageId to pkg.id. Ensure you reference
existingApp.packageId, currentLicense, ELicenseFlags, and pkg.id in the updated
conditional.
🪄 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: 63aa650a-b427-4e11-b6d3-e27e1cf4cd15
📒 Files selected for processing (1)
app/src/main/java/app/gamenative/service/SteamService.kt
|
Interesting - we have a related issue where if a game is family shared, it ONLY shows as family shared and not as owned. Is this addressing that issue as well? We need to take it into account here. |
|
The family-sharing issue is different and probably related to From what I can see, the code already prefers the user-owned license when both owned and shared licenses exist for the same package. val preferred = licensesEntry.value.firstOrNull {
it.ownerAccountID == userSteamId?.accountID?.toInt()
} ?: licensesEntry.value.first() |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/main/java/app/gamenative/service/SteamService.kt (1)
3907-3920: PreferEnumSet.contains(ELicenseFlags.Expired)over theand 8 == 0bitmask for readability.The fix correctly handles both the original bug (preserving valid existing package) and the follow-up concern (verifying the preserved license still grants
appid). However, sincelicenseFlagsis already anEnumSet<ELicenseFlags>, a named-enum check avoids the magic number and is more idiomatic Kotlin.♻️ Proposed refactor
- val steamApp = appDao.findApp(appid)?.let { existingApp -> - val currentLicense = licenseDao.findLicense(existingApp.packageId) - existingApp.copy( - packageId = if ( - currentLicense != null && - ELicenseFlags.code(currentLicense.licenseFlags) and 8 == 0 && - currentLicense.appIds.contains(appid) - ) { - existingApp.packageId - } else { - pkg.id - }, - ) - } + val steamApp = appDao.findApp(appid)?.let { existingApp -> + val currentLicense = licenseDao.findLicense(existingApp.packageId) + val keepExisting = currentLicense != null && + !currentLicense.licenseFlags.contains(ELicenseFlags.Expired) && + currentLicense.appIds.contains(appid) + existingApp.copy( + packageId = if (keepExisting) existingApp.packageId else pkg.id, + ) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/service/SteamService.kt` around lines 3907 - 3920, The code in SteamService.kt that builds steamApp (starting at val steamApp = appDao.findApp(...)) uses a magic bitmask check "ELicenseFlags.code(currentLicense.licenseFlags) and 8 == 0" to test expiration; replace that with an idiomatic EnumSet check: use the license returned by licenseDao.findLicense(existingApp.packageId) and ensure you test that the EnumSet does NOT contain ELicenseFlags.Expired (e.g., !currentLicense.licenseFlags.contains(ELicenseFlags.Expired)) and that currentLicense.appIds contains appid before preserving existingApp.packageId in existingApp.copy; otherwise fall back to pkg.id. Ensure you only change the bitmask expression and keep the surrounding logic (appDao.findApp, licenseDao.findLicense, existingApp.copy) intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/src/main/java/app/gamenative/service/SteamService.kt`:
- Around line 3907-3920: The code in SteamService.kt that builds steamApp
(starting at val steamApp = appDao.findApp(...)) uses a magic bitmask check
"ELicenseFlags.code(currentLicense.licenseFlags) and 8 == 0" to test expiration;
replace that with an idiomatic EnumSet check: use the license returned by
licenseDao.findLicense(existingApp.packageId) and ensure you test that the
EnumSet does NOT contain ELicenseFlags.Expired (e.g.,
!currentLicense.licenseFlags.contains(ELicenseFlags.Expired)) and that
currentLicense.appIds contains appid before preserving existingApp.packageId in
existingApp.copy; otherwise fall back to pkg.id. Ensure you only change the
bitmask expression and keep the surrounding logic (appDao.findApp,
licenseDao.findLicense, existingApp.copy) intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 264a1f49-898e-4ef6-b4c4-c857707d6fd2
📒 Files selected for processing (1)
app/src/main/java/app/gamenative/service/SteamService.kt
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
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:3913">
P2: Package retention still depends on possibly stale `currentLicense.appIds`, so valid package IDs can still be overwritten during package PICS iteration.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| packageId = if ( | ||
| currentLicense != null && | ||
| ELicenseFlags.code(currentLicense.licenseFlags) and 8 == 0 && | ||
| currentLicense.appIds.contains(appid) |
There was a problem hiding this comment.
P2: Package retention still depends on possibly stale currentLicense.appIds, so valid package IDs can still be overwritten during package PICS iteration.
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 3913:
<comment>Package retention still depends on possibly stale `currentLicense.appIds`, so valid package IDs can still be overwritten during package PICS iteration.</comment>
<file context>
@@ -3907,7 +3907,11 @@ class SteamService : Service(), IChallengeUrlChanged {
+ packageId = if (
+ currentLicense != null &&
+ ELicenseFlags.code(currentLicense.licenseFlags) and 8 == 0 &&
+ currentLicense.appIds.contains(appid)
+ ) {
existingApp.packageId
</file context>
There was a problem hiding this comment.
If currentLicense.appIds is stale, we only fall back to pkg.id, and pkg.id is the package currently being processed for this exact appid inside appIds.forEach { appid -> ... }.
|
@muazkadan, is this still an issue? I fixed this in 0.9.2. Please let me know if so and resolve the conflict accordingly, can get this in. |
3 participants
Description
Fixes a Steam library ownership issue caused by
steam_app.package_idbeing overwritten by the last package processed during PICS sync.Some apps belong to multiple Steam packages. In cases like Unrailed! (
appid 1016920), the app could flip between a valid owned package and an expired/free-weekend package. When the invalid package won, the app disappeared from the library.Recording
Screen_recording_20260415_210611.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
Prevent overwriting a game's existing package ID during PICS sync when the current package has a valid, non‑expired license that covers the app. If the current license is missing, expired, or doesn’t include the app, we now switch to the new package to avoid stale IDs and stop multi‑package titles disappearing from the library.
Written for commit 721b509. Summary will update on new commits.
Summary by CodeRabbit