Hide platform-hidden Steam and GOG games from the library - #1796
Hide platform-hidden Steam and GOG games from the library#1796whatobiplays wants to merge 11 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 persisted hidden-game state for GOG, synchronizes hidden products from GOG, filters Steam and GOG libraries, adds a visibility setting, and persists filtered library counts. Room schema version 26 includes the migration and updated schema metadata. ChangesHidden game visibility
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant SettingsGroupInterface
participant PrefManager
participant LibraryViewModel
participant GOGManager
participant GOGApiClient
participant GOGGameDao
SettingsGroupInterface->>PrefManager: persist showHiddenGamesByDefault
SettingsGroupInterface->>LibraryViewModel: emit HiddenGamesSettingChanged
LibraryViewModel->>GOGManager: refreshHiddenIds()
GOGManager->>GOGApiClient: getHiddenGameIds(context)
GOGManager->>GOGGameDao: applyHiddenFlags(hiddenIds)
LibraryViewModel->>LibraryViewModel: filter Steam and GOG games
LibraryViewModel->>PrefManager: persist filtered library counts
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.
Wouldn't it just be easier to add a new column to our gog db entries rather than hold an entirely new table? Seems excessive
There was a problem hiding this comment.
That’s fair. I can roll it into the GOG table, wasn’t sure how y’all felt about migrations, but I guess it’s a very lightweight one
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
app/src/main/java/app/gamenative/service/gog/GOGApiClient.kt (2)
205-205: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMove the fallback host into
GOGConstants.The primary host uses
GOGConstants.GOG_EMBED_URL, but the fallback host is a string literal. Declare the www host next to the other GOG hosts so both endpoints stay configurable in one place.🤖 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/gog/GOGApiClient.kt` at line 205, Move the fallback host into GOGConstants by declaring a named www-host constant alongside the existing GOG host constants, then update fetchHiddenGameIdsFrom to reference that constant instead of the hardcoded URL.
241-271: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winBound the pagination loop.
totalPagesis re-read from every response, and the loop continues whilepage <= totalPages. A wrong or hostile response value drives a long sequence of blocking HTTP calls during library sync. Each call also holds a 30-second read timeout, so the worst case is a very long sync. Add a page cap and stop when it is reached.♻️ Proposed guard
+ val maxPages = 100 return try { var page = 1 var totalPages = 1 val hiddenIds = mutableSetOf<String>() - while (page <= totalPages) { + while (page <= totalPages && page <= maxPages) {Log a warning when the cap truncates the result, so a silent partial hidden set is visible in diagnostics.
🤖 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/gog/GOGApiClient.kt` around lines 241 - 271, Bound the pagination in the hidden-game retrieval loop around totalPages/page by enforcing a fixed maximum page count and stopping once that cap is reached, regardless of response-provided totalPages values. When the cap truncates pagination, emit a Timber warning indicating that hidden-game results may be incomplete, while preserving normal failure handling and page progression below the cap.
🤖 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/GOGGameDao.kt`:
- Around line 75-87: Update applyHiddenFlags in GOGGameDao to call markHidden
only when hiddenIds.isNotEmpty(), while always clearing existing hidden flags.
Add a GOGGameDaoTest case covering applyHiddenFlags(emptyList()) and verifying
it completes without executing invalid empty-IN SQL.
In `@app/src/main/java/app/gamenative/service/gog/GOGManager.kt`:
- Around line 155-178: Update refreshHiddenIds so failures from
gogGameDao.applyHiddenFlags are caught and logged without propagating,
preserving the method’s null-on-failure behavior and leaving existing flags
unchanged when persistence fails. Keep API fetch handling and successful ID
return behavior intact.
In `@app/src/main/java/app/gamenative/ui/model/LibraryViewModel.kt`:
- Around line 655-660: Update the collection-count calculation in the
LibraryViewModel flow around steamCollectionCounts to use the default-visible
Steam IDs for normal collections, while retaining the pre-hidden count only for
SteamCollection.ID_HIDDEN. Ensure collection badges match the games remaining
after hidden filtering and preserve the Hidden collection’s full count.
---
Nitpick comments:
In `@app/src/main/java/app/gamenative/service/gog/GOGApiClient.kt`:
- Line 205: Move the fallback host into GOGConstants by declaring a named
www-host constant alongside the existing GOG host constants, then update
fetchHiddenGameIdsFrom to reference that constant instead of the hardcoded URL.
- Around line 241-271: Bound the pagination in the hidden-game retrieval loop
around totalPages/page by enforcing a fixed maximum page count and stopping once
that cap is reached, regardless of response-provided totalPages values. When the
cap truncates pagination, emit a Timber warning indicating that hidden-game
results may be incomplete, while preserving normal failure handling and page
progression below the cap.
🪄 Autofix
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 Plus
Run ID: d2bb3d72-2800-4327-bce9-e3a429460330
📒 Files selected for processing (23)
app/schemas/app.gamenative.db.PluviaDatabase/26.jsonapp/src/main/java/app/gamenative/PrefManager.ktapp/src/main/java/app/gamenative/data/GOGGame.ktapp/src/main/java/app/gamenative/data/HiddenGameFilter.ktapp/src/main/java/app/gamenative/db/PluviaDatabase.ktapp/src/main/java/app/gamenative/db/dao/GOGGameDao.ktapp/src/main/java/app/gamenative/events/AndroidEvent.ktapp/src/main/java/app/gamenative/service/gog/GOGApiClient.ktapp/src/main/java/app/gamenative/service/gog/GOGManager.ktapp/src/main/java/app/gamenative/service/gog/GOGService.ktapp/src/main/java/app/gamenative/service/gog/GogFilteredProductsParser.ktapp/src/main/java/app/gamenative/steam/SteamCollectionFilter.ktapp/src/main/java/app/gamenative/ui/data/LibraryCounts.ktapp/src/main/java/app/gamenative/ui/model/LibraryViewModel.ktapp/src/main/java/app/gamenative/ui/screen/settings/SettingsGroupInterface.ktapp/src/main/res/values/strings.xmlapp/src/test/java/app/gamenative/PrefManagerHiddenGamesDefaultsTest.ktapp/src/test/java/app/gamenative/data/HiddenGameFilterTest.ktapp/src/test/java/app/gamenative/db/dao/GOGGameDaoTest.ktapp/src/test/java/app/gamenative/service/gog/GogFilteredProductsParserTest.ktapp/src/test/java/app/gamenative/steam/SteamCollectionFilterTest.ktapp/src/test/java/app/gamenative/ui/data/LibraryCountsTest.ktapp/src/test/java/app/gamenative/utils/TestPrefManager.kt
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 23 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/gog/GOGService.kt">
<violation number="1" location="app/src/main/java/app/gamenative/service/gog/GOGService.kt:155">
P2: Logging out during an in-flight hidden-ID refresh can leave the prior account’s hidden flags on installed rows, because this clear races with `applyHiddenFlags`. Serialize/cancel-and-join hidden refreshes before clearing flags (or guard their DB write against logout) so logged-out metadata cannot be restored.</violation>
</file>
<file name="app/src/test/java/app/gamenative/utils/TestPrefManager.kt">
<violation number="1" location="app/src/test/java/app/gamenative/utils/TestPrefManager.kt:59">
P2: installFakePrefManager replaces PrefManager's private backing store via reflection and never restores it. Since PrefManager is a singleton `object`, the fake persists across the whole shared JVM, so any other test class that later calls PrefManager (same-module local unit tests) will silently read/write this test's fake store instead of a real one, producing order-dependent results. Restore the previous field value afterward (or in an @After), or scope the fake per-test.</violation>
</file>
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Tip: cubic used a learning from your PR history. Let your coding agent read cubic learnings directly with the cubic MCP.
Re-trigger cubic
| Timber.i("[GOGService] All non-installed GOG games removed from database") | ||
|
|
||
| // Hidden-game metadata belongs to the logged-out account. | ||
| instance.gogManager.clearHiddenFlags() |
There was a problem hiding this comment.
P2: Logging out during an in-flight hidden-ID refresh can leave the prior account’s hidden flags on installed rows, because this clear races with applyHiddenFlags. Serialize/cancel-and-join hidden refreshes before clearing flags (or guard their DB write against logout) so logged-out metadata cannot be restored.
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/gog/GOGService.kt, line 155:
<comment>Logging out during an in-flight hidden-ID refresh can leave the prior account’s hidden flags on installed rows, because this clear races with `applyHiddenFlags`. Serialize/cancel-and-join hidden refreshes before clearing flags (or guard their DB write against logout) so logged-out metadata cannot be restored.</comment>
<file context>
@@ -151,6 +151,9 @@ class GOGService : Service() {
Timber.i("[GOGService] All non-installed GOG games removed from database")
+ // Hidden-game metadata belongs to the logged-out account.
+ instance.gogManager.clearHiddenFlags()
+
// Stop the service
</file context>
|
|
||
| val dataStoreField = PrefManager::class.java.getDeclaredField("dataStore") | ||
| dataStoreField.isAccessible = true | ||
| dataStoreField.set(PrefManager, fake) |
There was a problem hiding this comment.
P2: installFakePrefManager replaces PrefManager's private backing store via reflection and never restores it. Since PrefManager is a singleton object, the fake persists across the whole shared JVM, so any other test class that later calls PrefManager (same-module local unit tests) will silently read/write this test's fake store instead of a real one, producing order-dependent results. Restore the previous field value afterward (or in an @after), or scope the fake per-test.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/test/java/app/gamenative/utils/TestPrefManager.kt, line 59:
<comment>installFakePrefManager replaces PrefManager's private backing store via reflection and never restores it. Since PrefManager is a singleton `object`, the fake persists across the whole shared JVM, so any other test class that later calls PrefManager (same-module local unit tests) will silently read/write this test's fake store instead of a real one, producing order-dependent results. Restore the previous field value afterward (or in an @After), or scope the fake per-test.</comment>
<file context>
@@ -0,0 +1,60 @@
+
+ val dataStoreField = PrefManager::class.java.getDeclaredField("dataStore")
+ dataStoreField.isAccessible = true
+ dataStoreField.set(PrefManager, fake)
+}
</file context>
There was a problem hiding this comment.
1 issue found across 10 files (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/steam/SteamCollectionFilter.kt">
<violation number="1" location="app/src/main/java/app/gamenative/steam/SteamCollectionFilter.kt:41">
P3: This change removes the last production caller of `SteamCollectionFilter.collectionCounts(...)`, leaving it used only by its own unit tests. Since the new `visibleCollectionCounts` supersedes it (and `libraryCounts` in LibraryViewModel now uses that instead), the now-production-unused `collectionCounts` is redundant maintenance surface. Consider removing it along with its two tests, or consolidating it into `visibleCollectionCounts`, to avoid keeping two overlapping count helpers.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| * it keeps its full count (and stays discoverable), while every other collection counts only | ||
| * [visibleAppIds] so badges match the games actually rendered. | ||
| */ | ||
| fun visibleCollectionCounts( |
There was a problem hiding this comment.
P3: This change removes the last production caller of SteamCollectionFilter.collectionCounts(...), leaving it used only by its own unit tests. Since the new visibleCollectionCounts supersedes it (and libraryCounts in LibraryViewModel now uses that instead), the now-production-unused collectionCounts is redundant maintenance surface. Consider removing it along with its two tests, or consolidating it into visibleCollectionCounts, to avoid keeping two overlapping count helpers.
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/steam/SteamCollectionFilter.kt, line 41:
<comment>This change removes the last production caller of `SteamCollectionFilter.collectionCounts(...)`, leaving it used only by its own unit tests. Since the new `visibleCollectionCounts` supersedes it (and `libraryCounts` in LibraryViewModel now uses that instead), the now-production-unused `collectionCounts` is redundant maintenance surface. Consider removing it along with its two tests, or consolidating it into `visibleCollectionCounts`, to avoid keeping two overlapping count helpers.</comment>
<file context>
@@ -33,6 +33,21 @@ object SteamCollectionFilter {
+ * it keeps its full count (and stays discoverable), while every other collection counts only
+ * [visibleAppIds] so badges match the games actually rendered.
+ */
+ fun visibleCollectionCounts(
+ collections: List<SteamCollection>?,
+ visibleAppIds: Collection<Int>,
</file context>
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 (3)
app/src/test/java/app/gamenative/ui/data/LibraryCountsTest.kt (3)
41-49: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse non-zero fixtures for every persisted count.
PrefManagerdefaults these properties to0. The test writes and asserts0for custom, Epic, Epic-installed, and Amazon counts. It will still pass ifLibraryCounts.persiststops writing those properties. Use distinct non-zero values to verify the complete persistence mapping.Also applies to: 59-65
🤖 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/test/java/app/gamenative/ui/data/LibraryCountsTest.kt` around lines 41 - 49, Update the LibraryCounts.persist test fixtures and corresponding assertions to use distinct non-zero values for every persisted count, including customGames, epicGames, epicInstalledGames, and amazonInstalledGames. Preserve the existing visible Steam, visible GOG, and installed GOG coverage while ensuring each property’s persisted value is independently verified.
26-39: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse independent expected counts.
The test calculates
visibleGogCountandvisibleSteamCountwithHiddenGameFilter, the production code that defines visibility. If either filter regresses, the test derives the incorrect expected value and can still pass. Use explicit expected counts, such as2for GOG and1for Steam, and test the filter predicates separately.🤖 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/test/java/app/gamenative/ui/data/LibraryCountsTest.kt` around lines 26 - 39, Replace the derived visibleGogCount and visibleSteamCount values in LibraryCountsTest with explicit expected counts (2 for GOG and 1 for Steam). Add or retain separate assertions that directly test the HiddenGameFilter.passesGog and HiddenGameFilter.passesSteam predicates, without using those production predicates to calculate the expected library counts.
51-65: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winWait for every persisted value before asserting.
The test treats preference writes as asynchronous but waits only for Steam, GOG, and GOG-installed counts. The remaining assertions can run before their writes complete. Include all seven persisted properties in the
awaitUntilcondition.🤖 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/test/java/app/gamenative/ui/data/LibraryCountsTest.kt` around lines 51 - 65, Update the awaitUntil condition in LibraryCountsTest to wait for all seven persisted count properties asserted below: customGamesCount, steamGamesCount, gogGamesCount, gogInstalledGamesCount, epicGamesCount, epicInstalledGamesCount, and amazonInstalledGamesCount. Keep each expected value aligned with its corresponding assertion before executing the assertions.
🤖 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/test/java/app/gamenative/ui/data/LibraryCountsTest.kt`:
- Around line 41-49: Update the LibraryCounts.persist test fixtures and
corresponding assertions to use distinct non-zero values for every persisted
count, including customGames, epicGames, epicInstalledGames, and
amazonInstalledGames. Preserve the existing visible Steam, visible GOG, and
installed GOG coverage while ensuring each property’s persisted value is
independently verified.
- Around line 26-39: Replace the derived visibleGogCount and visibleSteamCount
values in LibraryCountsTest with explicit expected counts (2 for GOG and 1 for
Steam). Add or retain separate assertions that directly test the
HiddenGameFilter.passesGog and HiddenGameFilter.passesSteam predicates, without
using those production predicates to calculate the expected library counts.
- Around line 51-65: Update the awaitUntil condition in LibraryCountsTest to
wait for all seven persisted count properties asserted below: customGamesCount,
steamGamesCount, gogGamesCount, gogInstalledGamesCount, epicGamesCount,
epicInstalledGamesCount, and amazonInstalledGamesCount. Keep each expected value
aligned with its corresponding assertion before executing the assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1e316843-f807-4bfc-90d5-c15f4fa5d743
📒 Files selected for processing (6)
app/src/main/java/app/gamenative/service/gog/GOGManager.ktapp/src/main/java/app/gamenative/steam/SteamCollectionFilter.ktapp/src/test/java/app/gamenative/PrefManagerHiddenGamesDefaultsTest.ktapp/src/test/java/app/gamenative/steam/SteamCollectionFilterTest.ktapp/src/test/java/app/gamenative/ui/data/LibraryCountsTest.ktapp/src/test/java/app/gamenative/utils/TestPrefManager.kt
💤 Files with no reviewable changes (2)
- app/src/main/java/app/gamenative/steam/SteamCollectionFilter.kt
- app/src/test/java/app/gamenative/steam/SteamCollectionFilterTest.kt
🚧 Files skipped from review as they are similar to previous changes (3)
- app/src/test/java/app/gamenative/PrefManagerHiddenGamesDefaultsTest.kt
- app/src/test/java/app/gamenative/utils/TestPrefManager.kt
- app/src/main/java/app/gamenative/service/gog/GOGManager.kt
There was a problem hiding this comment.
All reported issues were addressed across 6 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/src/main/java/app/gamenative/service/gog/GOGManager.kt (2)
308-317: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPreserve the hidden flag in single-game refreshes.
refreshSingleGame()insertsGOGGamewith the defaulthidden = false, whilerefreshLibrary()applieshiddenIdsbefore insertion. If the user hides a game after the last library refresh,refreshSingleGame()can overwrite it and expose the previously hidden game. Fetch the existinghiddenstate before inserting, and preserve it inGOGGameDao.insert()/insertAll()on primary-key conflict, or use upserts that keep existinghidden.🤖 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/gog/GOGManager.kt` around lines 308 - 317, The single-game refresh path must preserve an existing game's hidden state instead of resetting it to false. Update refreshSingleGame and the GOGGameDao insert/insertAll conflict handling so the current hidden value is retained when the primary key already exists, while keeping refreshLibrary's hiddenIds behavior unchanged.
218-224: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winPropagate coroutine cancellation.
CancellationExceptionis aRuntimeException; the broadcatch (e: Exception)converts a canceledrefreshHiddenIds()into a normalnull. Re-throwCancellationExceptionbefore handling database failures.🤖 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/gog/GOGManager.kt` around lines 218 - 224, Update the exception handling around gogGameDao.applyHiddenFlags in refreshHiddenIds to rethrow CancellationException before the broad database-failure handling; only non-cancellation exceptions should be logged and converted to null, preserving coroutine cancellation propagation.
🤖 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/gog/GOGManager.kt`:
- Around line 45-60: Make HiddenRefreshCoordinator.begin() atomically assign and
publish each refresh generation so counter and latestGeneration cannot be
observed out of order; serialize the increment-and-publication sequence or
atomically retain the maximum generation. Preserve isLatest() behavior while
ensuring refreshHiddenIds() rejects stale generations.
---
Outside diff comments:
In `@app/src/main/java/app/gamenative/service/gog/GOGManager.kt`:
- Around line 308-317: The single-game refresh path must preserve an existing
game's hidden state instead of resetting it to false. Update refreshSingleGame
and the GOGGameDao insert/insertAll conflict handling so the current hidden
value is retained when the primary key already exists, while keeping
refreshLibrary's hiddenIds behavior unchanged.
- Around line 218-224: Update the exception handling around
gogGameDao.applyHiddenFlags in refreshHiddenIds to rethrow CancellationException
before the broad database-failure handling; only non-cancellation exceptions
should be logged and converted to null, preserving coroutine cancellation
propagation.
🪄 Autofix
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 Plus
Run ID: 8224c401-f2fe-4fc6-8c1b-88868cce4e2a
📒 Files selected for processing (5)
app/src/main/java/app/gamenative/service/gog/GOGManager.ktapp/src/test/java/app/gamenative/PrefManagerHiddenGamesDefaultsTest.ktapp/src/test/java/app/gamenative/service/gog/HiddenRefreshCoordinatorTest.ktapp/src/test/java/app/gamenative/ui/data/LibraryCountsTest.ktapp/src/test/java/app/gamenative/utils/TestPrefManager.kt
💤 Files with no reviewable changes (3)
- app/src/test/java/app/gamenative/PrefManagerHiddenGamesDefaultsTest.kt
- app/src/test/java/app/gamenative/ui/data/LibraryCountsTest.kt
- app/src/test/java/app/gamenative/utils/TestPrefManager.kt
There was a problem hiding this comment.
Actionable comments posted: 2
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/gog/GOGManager.kt (1)
266-269: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDo not insert new games after a superseded hidden-flag refresh.
refreshHiddenIds()returnsnullwhen its generation is superseded, andrefreshLibrary()uses that to sethidden = falsefor new games. If a newer refresh persists before the older batch inserts its fresh games, later inserts can leave visible hidden games. Return a distinctisSupersededresult or apply the latest hidden set after the library insert.🤖 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/gog/GOGManager.kt` around lines 266 - 269, Update refreshLibrary() and refreshHiddenIds() so a superseded hidden-flag refresh cannot cause newly inserted games to be assigned hidden = false; return a distinct isSuperseded result or reapply the latest hidden set after insertion, while preserving existing flags on refresh failure.
🤖 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/gog/GOGManager.kt`:
- Around line 126-128: Update refreshSingleGame() to apply the current hidden-ID
set to the parsed game before persistence, including newly inserted and existing
hidden products. Persist it through upsertPreservingHidden(), then return the
record read from the DAO so the result reflects the stored hidden state rather
than the unadjusted parsed object.
- Around line 221-225: Update refreshLibrary() and startBackgroundSync() to
catch and rethrow CancellationException before their generic Exception handlers,
preserving cancellation propagation through both sync wrappers while retaining
existing failure handling for other exceptions.
---
Outside diff comments:
In `@app/src/main/java/app/gamenative/service/gog/GOGManager.kt`:
- Around line 266-269: Update refreshLibrary() and refreshHiddenIds() so a
superseded hidden-flag refresh cannot cause newly inserted games to be assigned
hidden = false; return a distinct isSuperseded result or reapply the latest
hidden set after insertion, while preserving existing flags on refresh failure.
🪄 Autofix
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 Plus
Run ID: e3fa53f1-e03a-4444-b155-1e2f2929a042
📒 Files selected for processing (4)
app/src/main/java/app/gamenative/db/dao/GOGGameDao.ktapp/src/main/java/app/gamenative/service/gog/GOGApiClient.ktapp/src/main/java/app/gamenative/service/gog/GOGManager.ktapp/src/test/java/app/gamenative/db/dao/GOGGameDaoTest.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- app/src/main/java/app/gamenative/service/gog/GOGApiClient.kt
- app/src/main/java/app/gamenative/db/dao/GOGGameDao.kt
There was a problem hiding this comment.
1 issue found across 8 files (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/db/dao/GOGGameDao.kt">
<violation number="1" location="app/src/main/java/app/gamenative/db/dao/GOGGameDao.kt:36">
P2: This new method duplicates the hidden-preservation logic that `upsertPreservingInstallStatus` already covers, but only preserves the `hidden` flag and not install state. A single-game refresh via GOGManager.insertGame (refreshSingleGame path) inserts a game parsed with isInstalled=false/installPath="" (see parseGameObject), so an already-installed game's install status and path are still wiped on refresh. Reusing `upsertPreservingInstallStatus(listOf(game))` would preserve hidden and install state in one code path and avoid the duplication.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| * single-game refresh cannot reset hidden state. | ||
| */ | ||
| @Transaction | ||
| suspend fun upsertPreservingHidden(game: GOGGame) { |
There was a problem hiding this comment.
P2: This new method duplicates the hidden-preservation logic that upsertPreservingInstallStatus already covers, but only preserves the hidden flag and not install state. A single-game refresh via GOGManager.insertGame (refreshSingleGame path) inserts a game parsed with isInstalled=false/installPath="" (see parseGameObject), so an already-installed game's install status and path are still wiped on refresh. Reusing upsertPreservingInstallStatus(listOf(game)) would preserve hidden and install state in one code path and avoid the duplication.
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/db/dao/GOGGameDao.kt, line 36:
<comment>This new method duplicates the hidden-preservation logic that `upsertPreservingInstallStatus` already covers, but only preserves the `hidden` flag and not install state. A single-game refresh via GOGManager.insertGame (refreshSingleGame path) inserts a game parsed with isInstalled=false/installPath="" (see parseGameObject), so an already-installed game's install status and path are still wiped on refresh. Reusing `upsertPreservingInstallStatus(listOf(game))` would preserve hidden and install state in one code path and avoid the duplication.</comment>
<file context>
@@ -28,6 +28,16 @@ interface GOGGameDao {
+ * single-game refresh cannot reset hidden state.
+ */
+ @Transaction
+ suspend fun upsertPreservingHidden(game: GOGGame) {
+ val existing = getById(game.id)
+ insert(if (existing != null) game.copy(hidden = existing.hidden) else game)
</file context>
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
2 participants
Description
Adds support for hiding platform-hidden games from the main library tabs.
account/getFilteredProducts?hiddenFlag=1, embed host with www fallback) and cached locally. Hidden GOG games are excluded from library tabs by default.Tests added for the hidden-game filter, GOG hidden-page parser, hidden-ID repository/cache, preference defaults, and persisted count behavior. Legacy and modern unit-test suites compile; targeted feature tests pass.
Known limitation: GOG hidden state comes from the gog.com account ("Hidden" folder). Games hidden only inside the Galaxy desktop client's local library are not exposed by GOG's API and cannot be filtered.
Recording
IMG_3209_480p-2.mov
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
Hides platform‑hidden Steam and GOG games from library tabs by default, with a toggle to show them. Preserves Steam’s Hidden collection behavior and keeps GOG hidden state consistent without flicker or stale writes.
New Features
account/getFilteredProducts?hiddenFlag=1(embed host withwwwfallback), stored ingog_games.hidden. Hidden state is applied to new inserts and refreshed rows using only the latest committed hidden set.HiddenGamesSettingChangedand refreshes the library immediately.HiddenGameFilter(missing metadata fails open). Persisted library counts store post‑hidden totals viaLibraryCounts.persist.Migration
hiddencolumn togog_gameswith auto‑migration 25→26. No manual steps.Written for commit 909b3b0. Summary will update on new commits.
Summary by CodeRabbit