Fix recently played library sort#1509
Open
xXJSONDeruloXx wants to merge 3 commits into
Open
Conversation
Contributor
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds persistent per-app Steam play stats (last-played and play-time minutes) with a DB migration to v22, DAO support to read/update stats and trigger library reloads, service-side collection/persistence, sorting utilities and ViewModel wiring for "Recently Played", plus unit tests for the sorting utilities. ChangesSteam Play Stats Tracking and Recently Played Sorting
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related PRs
Suggested Reviewers
🚥 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 |
![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 across 8 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
![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: 3
🤖 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/db/dao/SteamAppDao.kt`:
- Around line 158-159: The call site in
SteamService.refreshOwnedGamesFromServer() should chunk existingAppIds before
calling SteamAppDao.findAccountPlayStats() to avoid SQLite bind-variable limits:
split existingAppIds.toList() into batches (e.g., ~900 ids per batch, similar to
MAX_PICS_BUFFER but larger), call findAccountPlayStats(batch) for each chunk,
collect and merge all returned SteamAppPlayStatsUpdate results into a single
list used downstream; ensure the chunking logic mirrors the existing
missingAppIds batching so large libraries won’t generate “too many SQL
variables” errors.
In `@app/src/main/java/app/gamenative/service/SteamService.kt`:
- Around line 813-815: The query call to SteamAppDao.findAccountPlayStats is
passed a potentially huge existingAppIds.toList() from
refreshOwnedGamesFromServer which can exceed SQLite bind-variable limits; fix by
chunking the app ID list (e.g., using Kotlin List.chunked with a safe size like
900) and calling findAccountPlayStats for each chunk, then merge the results
into a single map (associateBy { it.id }) so currentStatsByAppId is built from
the concatenated results; update the code around existingAppIds,
findAccountPlayStats, and currentStatsByAppId to perform chunked queries and
merge the responses before proceeding.
In `@app/src/main/java/app/gamenative/ui/model/LibraryViewModel.kt`:
- Around line 526-529: The current merge strategy uses max(...) so directory
mtime can override Steam's real lastPlayed; change LibraryViewModel to treat
install-directory mtime as a fallback: call
LibrarySortUtils.normalizeLastPlayedMillis(item.lastPlayed) into a local (e.g.,
normalizedLastPlayed) and set lastPlayed to normalizedLastPlayed unless it
equals 0L, in which case (and only if isInstalled) use
steamInstalledLastPlayedMillis(item.id) else 0L; update the assignment that
currently uses max(...) to this conditional fallback logic so Steam timestamps
always take precedence.
🪄 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: 92aa9de2-ad90-4f36-bffa-4ff6e6b89f09
📒 Files selected for processing (8)
app/schemas/app.gamenative.db.PluviaDatabase/22.jsonapp/src/main/java/app/gamenative/data/SteamApp.ktapp/src/main/java/app/gamenative/db/PluviaDatabase.ktapp/src/main/java/app/gamenative/db/dao/SteamAppDao.ktapp/src/main/java/app/gamenative/service/SteamService.ktapp/src/main/java/app/gamenative/ui/model/LibrarySortUtils.ktapp/src/main/java/app/gamenative/ui/model/LibraryViewModel.ktapp/src/test/java/app/gamenative/ui/model/LibrarySortUtilsTest.kt
xXJSONDeruloXx
force-pushed
the
fix/recently-played-library-sort
branch
from
e019858 to
9f85720
Compare
…ed-library-sort # Conflicts: # app/src/main/java/app/gamenative/service/SteamService.kt # app/src/main/java/app/gamenative/ui/model/LibraryViewModel.kt
Owner
|
@xXJSONDeruloXx the tests are failing |
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
2 participants
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
Fixes incorrect Recently Played sorting in the Library view. Previously, non-installed games in that sort mode fell back to alphabetical ordering because Steam account last-played data was not persisted and refreshed for library ordering.
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
Fixes Recently Played sorting in the Library by persisting Steam play stats and using true last-played times across stores. Sorting now favors installed games, then most recently played, and keeps stats intact during PICS updates.
Written for commit 2206d63. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
Tests