-
Notifications
You must be signed in to change notification settings - Fork 83
Working Game Shortcuts for all game sources #423
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
base: master
Are you sure you want to change the base?
Conversation
…ectly. Will now look to test and fix front-end exporting.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds multi-source (Steam, GOG, Custom) launch handling: intent-parsed game_source drives appId construction, installation checks, per-source pre-launch sync (including GOG cloud-save flow), and propagates source to shortcut intents; introduces CustomGameScanner helpers for install/icon checks. Changes
Sequence DiagramsequenceDiagram
participant Intent as External Intent
participant IL as IntentLaunchManager
participant PM as PluviaMain
participant GS as GOGService
participant CS as CustomGameScanner
Intent->>IL: Launch intent (game_id, game_source?)
IL->>IL: Extract & validate game_source (default STEAM)
IL->>PM: Route launch request with source-aware appId
alt source == STEAM
PM->>PM: Check Steam install & cloud sync
else source == GOG
PM->>GS: Check credentials & start service
GS->>GS: Initialize and log state
PM->>PM: Run GOG cloud-save sync
else source == CUSTOM_GAME
PM->>CS: isGameInstalled(appId)
CS->>PM: return installed?/icon
end
alt installed
PM->>PM: Apply container overrides and launch
PM->>PM: Post-launch sync/result handling
else not installed
PM->>PM: Show installation error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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
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 |
|
|
||
| private const val EXTRA_GAME_SOURCE = "game_source" | ||
| private const val EXTRA_CONTAINER_CONFIG = "container_config" | ||
| private const val ACTION_LAUNCH_GAME = "app.gamenative.LAUNCH_GAME" | ||
| private const val MAX_CONFIG_JSON_SIZE = 50000 // 50KB limit to prevent memory exhaustion | ||
|
|
||
| data class LaunchRequest( | ||
| val appId: String, | ||
| val containerConfig: ContainerData? = null, | ||
| ) | ||
|
|
||
| fun parseLaunchIntent(intent: Intent): LaunchRequest? { | ||
| Timber.d("[IntentLaunchManager]: Parsing intent: action=${intent.action}") | ||
|
|
||
| if (intent.action != ACTION_LAUNCH_GAME) { | ||
| Timber.d("[IntentLaunchManager]: Intent action '${intent.action}' doesn't match expected action '$ACTION_LAUNCH_GAME'") | ||
| return null | ||
| } | ||
|
|
||
| val gameId = intent.getIntExtra(EXTRA_APP_ID, -1) | ||
| Timber.d("[IntentLaunchManager]: Extracted app_id: $gameId from intent extras") | ||
|
|
||
| if (gameId <= 0) { | ||
| Timber.w("[IntentLaunchManager]: Invalid or missing app_id in launch intent: $gameId") | ||
| return null | ||
| } | ||
|
|
||
| val appId = "${GameSource.STEAM.name}_$gameId" | ||
| // Get Game Source for launch intent | ||
| var gameSource = intent.getStringExtra(EXTRA_GAME_SOURCE)?.uppercase() | ||
| val isValidGameSource = GameSource.entries.any { it.name == gameSource } | ||
| if (!isValidGameSource) { | ||
| gameSource = GameSource.STEAM.name | ||
| } | ||
|
|
||
| val appId = "${gameSource}_$gameId" |
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.
Intent Manager will now correctly parse the gameSource.
|
|
||
| // Helper function to check if game is installed to match pattern of GOG & Steam Service | ||
| fun isGameInstalled(appId: Int): Boolean { | ||
| val isInstalled = findCustomGameById(appId) != null | ||
|
|
||
| return isInstalled | ||
| } | ||
|
|
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.
Added isGameInstalled helper function to match the pattern of GOG and Steam services.
| @@ -235,27 +241,13 @@ | |||
| return@let | |||
| } | |||
|
|
|||
| // If it's a custom game, update the appId to use CUSTOM_GAME format | |||
| val finalAppId = if (customGamePath != null && !isSteamInstalled) { | |||
| "${GameSource.CUSTOM_GAME.name}_$gameId" | |||
| } else { | |||
| launchRequest.appId | |||
| } | |||
|
|
|||
| // Update launchRequest with the correct appId if it was changed | |||
| val updatedLaunchRequest = if (finalAppId != launchRequest.appId) { | |||
| launchRequest.copy(appId = finalAppId) | |||
| } else { | |||
| launchRequest | |||
| } | |||
|
|
|||
| if (updatedLaunchRequest.containerConfig != null) { | |||
| if (launchRequest.containerConfig != null) { | |||
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.
Now correctly parses and identifies the game, and this logic is also cleaned up so that it's extensible for future purposes.
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 5 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/IntentLaunchManager.kt">
<violation number="1" location="app/src/main/java/app/gamenative/utils/IntentLaunchManager.kt:46">
P2: Use locale-independent uppercasing when normalizing `game_source`; `uppercase()` is locale-sensitive and can mis-normalize values (e.g., Turkish locale), causing valid sources to be rejected and defaulted to STEAM.</violation>
</file>
<file name="app/src/main/java/app/gamenative/ui/PluviaMain.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/PluviaMain.kt:215">
P2: Legacy GOG shortcuts without a `GOG_` prefix are treated as STEAM, so valid GOG installs will be rejected as “not installed” and never launched.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
| val appId = "${GameSource.STEAM.name}_$gameId" | ||
| // Get Game Source for launch intent | ||
| var gameSource = intent.getStringExtra(EXTRA_GAME_SOURCE)?.uppercase() |
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: Use locale-independent uppercasing when normalizing game_source; uppercase() is locale-sensitive and can mis-normalize values (e.g., Turkish locale), causing valid sources to be rejected and defaulted to STEAM.
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/IntentLaunchManager.kt, line 46:
<comment>Use locale-independent uppercasing when normalizing `game_source`; `uppercase()` is locale-sensitive and can mis-normalize values (e.g., Turkish locale), causing valid sources to be rejected and defaulted to STEAM.</comment>
<file context>
@@ -40,7 +42,14 @@ object IntentLaunchManager {
- val appId = "${GameSource.STEAM.name}_$gameId"
+ // Get Game Source for launch intent
+ var gameSource = intent.getStringExtra(EXTRA_GAME_SOURCE)?.uppercase()
+ val isValidGameSource = GameSource.entries.any { it.name == gameSource }
+ if (!isValidGameSource) {
</file context>
| var gameSource = intent.getStringExtra(EXTRA_GAME_SOURCE)?.uppercase() | |
| var gameSource = intent.getStringExtra(EXTRA_GAME_SOURCE)?.uppercase(java.util.Locale.ROOT) |
| val gameSource = ContainerUtils.extractGameSourceFromContainerId(launchRequest.appId) | ||
|
|
||
| // TODO: Check gameSource (which should always default thanks to the intent manager, have a gameSource. | ||
| val isInstalled = when (gameSource) { |
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: Legacy GOG shortcuts without a GOG_ prefix are treated as STEAM, so valid GOG installs will be rejected as “not installed” and never launched.
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/ui/PluviaMain.kt, line 215:
<comment>Legacy GOG shortcuts without a `GOG_` prefix are treated as STEAM, so valid GOG installs will be rejected as “not installed” and never launched.</comment>
<file context>
@@ -208,19 +209,24 @@ fun PluviaMain(
+ val gameSource = ContainerUtils.extractGameSourceFromContainerId(launchRequest.appId)
+
+ // TODO: Check gameSource (which should always default thanks to the intent manager, have a gameSource.
+ val isInstalled = when (gameSource) {
+ GameSource.STEAM -> {
+ SteamService.isAppInstalled(gameId)
</file context>
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/ui/PluviaMain.kt (2)
145-180: Missing GOG installation check in ExternalGameLaunch handler.This handler checks Steam and Custom game installation but doesn't check GOG. A GOG game launched via this path would incorrectly fall through with
isSteamInstalled = falseandcustomGamePath = null, leading to the wrongfinalAppIdor launch failure.The
OnLogonEndedhandler at lines 214-226 correctly checks all three sources. This handler should follow the same pattern.Suggested fix
is MainViewModel.MainUiEvent.ExternalGameLaunch -> { Timber.i("[PluviaMain]: Received ExternalGameLaunch UI event for app ${event.appId}") // Extract game ID from appId (format: "STEAM_<id>" or "CUSTOM_GAME_<id>") val gameId = ContainerUtils.extractGameIdFromContainerId(event.appId) + val gameSource = ContainerUtils.extractGameSourceFromContainerId(event.appId) - // First check if it's a Steam game and if it's installed - val isSteamInstalled = SteamService.isAppInstalled(gameId) - - // If not installed as Steam game, check if it's a custom game - val customGamePath = if (!isSteamInstalled) { - CustomGameScanner.findCustomGameById(gameId) - } else { - null + val isInstalled = when (gameSource) { + GameSource.STEAM -> SteamService.isAppInstalled(gameId) + GameSource.GOG -> GOGService.isGameInstalled(gameId.toString()) + GameSource.CUSTOM_GAME -> CustomGameScanner.isGameInstalled(gameId) } - // Determine the final appId to use - val finalAppId = if (customGamePath != null && !isSteamInstalled) { - "${GameSource.CUSTOM_GAME.name}_$gameId" - } else { - event.appId + if (!isInstalled) { + // Handle not installed case similar to OnLogonEnded + // Show error dialog and return + return@collect } + val finalAppId = event.appId + Timber.i("[PluviaMain]: Using appId: $finalAppId ...")
228-241: App name lookup incorrectly uses Steam service for all game sources.Line 229 uses
SteamService.getAppInfoOf(gameId)regardless ofgameSource. For GOG and Custom games, this returnsnull, falling back to the generic "App ${launchRequest.appId}" message.To improve UX, use source-appropriate lookups:
val appName = when (gameSource) { GameSource.STEAM -> SteamService.getAppInfoOf(gameId)?.name GameSource.GOG -> GOGService.getGOGGameOf(gameId)?.title GameSource.CUSTOM_GAME -> CustomGameScanner.findCustomGameById(gameId)?.let { File(it).name } } ?: "App ${launchRequest.appId}"Note: The custom game case falls back to using the folder name as the game name, since there is no dedicated game name retrieval method in
CustomGameScanner.
♻️ Duplicate comments (1)
app/src/main/java/app/gamenative/ui/PluviaMain.kt (1)
212-226: Multi-source installation check looks good, minor API inconsistency noted.The
whenexpression properly handles all three game sources. Note the type inconsistency:GOGService.isGameInstalledtakes aString(line 220) whileSteamService.isAppInstalledandCustomGameScanner.isGameInstalledtakeInt. Consider aligning the APIs for consistency.Regarding legacy GOG shortcuts defaulting to STEAM: since GOG shortcut creation is new in this PR, there shouldn't be legacy GOG shortcuts to worry about.
🧹 Nitpick comments (1)
app/src/main/java/app/gamenative/ui/PluviaMain.kt (1)
397-404: Consider using the importedGOGServiceinstead of fully qualified names.
GOGServiceis imported at line 54, but lines 397-404 use the fully qualifiedapp.gamenative.service.gog.GOGService. This is inconsistent with line 220 which uses the short form.Suggested simplification
- if (app.gamenative.service.gog.GOGService.hasStoredCredentials(context) && - !app.gamenative.service.gog.GOGService.isRunning - ) { + if (GOGService.hasStoredCredentials(context) && !GOGService.isRunning) { Timber.tag("GOG").d("[PluviaMain]: Starting GOGService for logged-in user") - app.gamenative.service.gog.GOGService.start(context) + GOGService.start(context) } else { - Timber.tag("GOG").d("GOG SERVICE Not going to start: ${app.gamenative.service.gog.GOGService.isRunning}") + Timber.tag("GOG").d("GOG SERVICE Not going to start: ${GOGService.isRunning}") }
Overview
This PR updates the Game Shortcut logic for exporting and consuming the launch intent.
This does the following:
Summary by cubic
Shortcuts and launch intents now work for Steam, GOG, and Custom games. Shortcuts embed the game source so they open the right game and show a clear message if the game isn’t installed.
Written for commit 26f2ce1. Summary will update on new commits.
Summary by CodeRabbit
Release Notes
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.