-
-
Notifications
You must be signed in to change notification settings - Fork 88
Fixed turnip 26 R8, fixed issue with windows save paths (Holo vs Robo… #468
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…), add nolrz + deck_emu to TU_DEBUG when using a gen8 driver, reduced game compatibility cache to leverage new materialised table
📝 WalkthroughWalkthroughThis PR implements platform-aware path separator normalization across file pattern handling, adjusts game compatibility cache expiration from 3 days to 6 hours, enhances graphics driver environment configuration for gen8 drivers, and adds special handling for specific DXVK wrapper versions alongside test data updates. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
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/utils/GameCompatibilityCache.kt (1)
10-15:⚠️ Potential issue | 🟡 MinorUpdate the TTL docstring to match the new 6‑hour cache.
The header still states “7‑day TTL,” which no longer matches
CACHE_TTL_MS.🛠️ Proposed fix
- * Persistent cache for game compatibility responses with 7-day TTL. + * Persistent cache for game compatibility responses with 6-hour TTL.app/src/test/java/app/gamenative/service/SteamAutoCloudTest.kt (1)
233-257:⚠️ Potential issue | 🟡 MinorAvoid swallowing cleanup exceptions silently.
Even in tests, a lightweight log helps diagnose flaky cleanup failures.
🛠️ Proposed fix
- } catch (e: Exception) { - // Ignore cleanup errors - files might be locked, but Robolectric will handle it - } + } catch (e: Exception) { + System.err.println("ImageFs cleanup failed: ${e.message}") + } @@ - } catch (e: Exception) { - // Ignore cleanup errors - } + } catch (e: Exception) { + System.err.println("Temp dir cleanup failed: ${e.message}") + }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 issues found across 6 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="app/src/main/java/app/gamenative/utils/GameCompatibilityCache.kt">
<violation number="1" location="app/src/main/java/app/gamenative/utils/GameCompatibilityCache.kt:15">
P3: Class-level KDoc still claims a 7-day TTL, but the cache TTL was changed to 6 hours. Update the KDoc so documentation matches the new expiration behavior.</violation>
</file>
<file name="app/src/test/java/app/gamenative/service/SteamAutoCloudTest.kt">
<violation number="1" location="app/src/test/java/app/gamenative/service/SteamAutoCloudTest.kt:138">
P2: This pattern now uses backslashes while the other patterns use forward slashes, so the three patterns no longer share the same prefix on Unix-based test runtimes. That undermines the test’s intent to validate grouping of multiple patterns under a single prefix. Keep the separators consistent across patterns (or normalize with `File.separator`) so they resolve to the same prefix.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| SaveFilePattern( | ||
| root = PathType.WinMyDocuments, | ||
| path = "My Games/TestGame/Steam/76561198025127569", | ||
| path = "My Games\\TestGame\\Steam\\76561198025127569", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: This pattern now uses backslashes while the other patterns use forward slashes, so the three patterns no longer share the same prefix on Unix-based test runtimes. That undermines the test’s intent to validate grouping of multiple patterns under a single prefix. Keep the separators consistent across patterns (or normalize with File.separator) so they resolve to the same prefix.
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/service/SteamAutoCloudTest.kt, line 138:
<comment>This pattern now uses backslashes while the other patterns use forward slashes, so the three patterns no longer share the same prefix on Unix-based test runtimes. That undermines the test’s intent to validate grouping of multiple patterns under a single prefix. Keep the separators consistent across patterns (or normalize with `File.separator`) so they resolve to the same prefix.</comment>
<file context>
@@ -135,7 +135,7 @@ class SteamAutoCloudTest {
SaveFilePattern(
root = PathType.WinMyDocuments,
- path = "My Games/TestGame/Steam/76561198025127569",
+ path = "My Games\\TestGame\\Steam\\76561198025127569",
pattern = "SystemData_0.sav",
),
</file context>
| path = "My Games\\TestGame\\Steam\\76561198025127569", | |
| path = "My Games/TestGame/Steam/76561198025127569", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's the point of the test bro, it should work with forward slashes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the feedback! I've saved this as a new learning to improve future reviews.
| */ | ||
| object GameCompatibilityCache { | ||
| private const val CACHE_TTL_MS = 3 * 24 * 60 * 60 * 1000L // 1 day | ||
| private const val CACHE_TTL_MS = 6 * 60 * 60 * 1000L // 6 hours |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P3: Class-level KDoc still claims a 7-day TTL, but the cache TTL was changed to 6 hours. Update the KDoc so documentation matches the new expiration behavior.
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/utils/GameCompatibilityCache.kt, line 15:
<comment>Class-level KDoc still claims a 7-day TTL, but the cache TTL was changed to 6 hours. Update the KDoc so documentation matches the new expiration behavior.</comment>
<file context>
@@ -12,7 +12,7 @@ import timber.log.Timber
*/
object GameCompatibilityCache {
- private const val CACHE_TTL_MS = 3 * 24 * 60 * 60 * 1000L // 1 day
+ private const val CACHE_TTL_MS = 6 * 60 * 60 * 1000L // 6 hours
private val inMemoryCache = mutableMapOf<String, GameCompatibilityService.GameCompatibilityResponse>()
</file context>
…), add nolrz + deck_emu to TU_DEBUG when using a gen8 driver, reduced game compatibility cache to leverage new materialised table
Summary by cubic
Fixed gen8 graphics stability and Windows save path handling; also reduced game compatibility cache refresh time for fresher data. Updated Turnip 26 R8 driver bundle.
Bug Fixes
Performance
Written for commit 37083ab. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
Performance
✏️ Tip: You can customize this high-level summary in your review settings.