Fix cold-launch intent race, add failure feedback#884
Fix cold-launch intent race, add failure feedback#884utkarshdalal merged 2 commits intoutkarshdalal:masterfrom
Conversation
There was a problem hiding this comment.
3 issues found across 16 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/MainActivity.kt">
<violation number="1" location="app/src/main/java/app/gamenative/MainActivity.kt:230">
P2: Warm-intent launch path no longer clears stale pending launch request, allowing an older queued launch to execute later.</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:273">
P1: Warm external Steam launches bypass the new login-required gate, allowing logged-out Steam launch flow to proceed and hit logged-in assumptions.</violation>
<violation number="2" location="app/src/main/java/app/gamenative/ui/PluviaMain.kt:287">
P2: Cold-start intent launch applies temporary container override but calls `preLaunchApp` without `useTemporaryOverride`, so override can be ignored.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
9801c9b to
79f929f
Compare
|
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 (16)
🚧 Files skipped from review as they are similar to previous changes (9)
📝 WalkthroughWalkthroughMainActivity now distinguishes cold-start vs new-intent launches (adds isNewIntent) and stores or consumes pending external launch requests accordingly. PluviaMain adds Steam-login gating and processes pending launches at cold start. New localized strings for launch failures and Steam-login requirements added across multiple locales. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant App as App (Cold Start)
participant MA as MainActivity
participant IL as IntentLaunchManager
participant PM as PluviaMain
participant UI as Snackbar / Dialog
App->>MA: deliver external Intent (onCreate)
MA->>MA: handleLaunchIntent(intent, isNewIntent=false)
alt parse success
MA->>IL: store pending launch
else parse fails / LAUNCH_GAME null
MA->>UI: show `intent_launch_failed` via SnackbarManager
end
PM->>PM: LaunchedEffect on composition
PM->>IL: check pending launch
alt pending exists
PM->>PM: needsSteamLogin? (container lookup / login state)
alt steam login required
PM->>UI: show `intent_launch_steam_login_required` Snackbar and persist pending
else
PM->>PM: resolve appId, apply temporary override if present
PM->>PM: trigger preLaunchApp (mark external launch)
end
end
User->>MA: onNewIntent (return / explicit new intent)
MA->>MA: handleLaunchIntent(intent, isNewIntent=true)
MA->>IL: consume pending launch
MA->>PM: emit AndroidEvent.ExternalGameLaunch (with override if any)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 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)
📝 Coding Plan
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.
Actionable comments posted: 2
🤖 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/MainActivity.kt`:
- Around line 231-249: Remove the early wasLaunchedViaExternalIntent = true
assignment and instead set wasLaunchedViaExternalIntent = true only when a
launch is actually committed: in the isNewIntent branch, set the flag after
consumePendingLaunchRequest() and immediately before emitting
AndroidEvent.ExternalGameLaunch(launchRequest.appId) inside
lifecycleScope.launch; in the cold-start branch, set the flag immediately when
calling setPendingLaunchRequest(launchRequest) so the pending state counts as a
committed launch. Update references to wasLaunchedViaExternalIntent accordingly
so it reflects only committed/active external launches.
In `@app/src/main/java/app/gamenative/ui/PluviaMain.kt`:
- Around line 333-339: The pending launch stored for Steam-login-required paths
only preserves appId (using IntentLaunchManager.LaunchRequest(appId =
event.appId)), which discards temporary overrides like
launchRequest.containerConfig; change the call to
MainActivity.setPendingLaunchRequest to pass the original full LaunchRequest
(the incoming event/launchRequest object) so containerConfig and other fields
are preserved across the login flow (i.e., stop constructing a new LaunchRequest
with only appId and store the original launchRequest/event instead).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2d7d84b5-0c28-48b8-9ca1-c1de9e4a8a93
📒 Files selected for processing (16)
app/src/main/java/app/gamenative/MainActivity.ktapp/src/main/java/app/gamenative/ui/PluviaMain.ktapp/src/main/res/values-da/strings.xmlapp/src/main/res/values-de/strings.xmlapp/src/main/res/values-es/strings.xmlapp/src/main/res/values-fr/strings.xmlapp/src/main/res/values-it/strings.xmlapp/src/main/res/values-ko/strings.xmlapp/src/main/res/values-pl/strings.xmlapp/src/main/res/values-pt-rBR/strings.xmlapp/src/main/res/values-ro/strings.xmlapp/src/main/res/values-ru/strings.xmlapp/src/main/res/values-uk/strings.xmlapp/src/main/res/values-zh-rCN/strings.xmlapp/src/main/res/values-zh-rTW/strings.xmlapp/src/main/res/values/strings.xml
On cold start, handleLaunchIntent fired before PluviaMain's ViewModel registered its event listener. The fire-and-forget event bus dropped the ExternalGameLaunch event. Now all cold-start intents use the pending request mechanism; onNewIntent still emits directly.
Show snackbar when launch shortcut has invalid extras, and when a Steam game requires login before it can launch.
79f929f to
3e44cf8
Compare
phobos665
left a comment
There was a problem hiding this comment.
Looks reasonable to me. Have you tested with all game types?
Steam, GOG, custom all work fine. I don't have Epic or Amazon accts set up, but they will use exactly the same Intents launch path. |
|
I did notice that, immediately after installing a game, if GN remains active and a launch intent is received, GN reports that the game can't be found. Definitely a different issue/PR (maybe a DB sync issue?), but capturing it here. |
| private fun needsSteamLogin(context: Context, appId: String): Boolean { | ||
| val gameSource = ContainerUtils.extractGameSourceFromContainerId(appId) | ||
| if (gameSource != GameSource.STEAM || SteamService.isLoggedIn) return false | ||
| // offline-mode games can launch without Steam | ||
| return !ContainerUtils.hasContainer(context, appId) || | ||
| !ContainerUtils.getContainer(context, appId).isSteamOfflineMode() |
There was a problem hiding this comment.
why is this required? Does it fail on cloud saves?
There was a problem hiding this comment.
If so, we might want to check if the user is signed in in offline mode too. PrefManager.isOffline I think
| LaunchedEffect(Unit) { | ||
| MainActivity.consumePendingLaunchRequest()?.let { launchRequest -> | ||
| Timber.i("[PluviaMain]: Processing pending launch request for app ${launchRequest.appId}") | ||
| // Steam games needing login will be handled by OnLogonEnded | ||
| if (needsSteamLogin(context, launchRequest.appId)) { | ||
| MainActivity.setPendingLaunchRequest(launchRequest) | ||
| SnackbarManager.show(context.getString(R.string.intent_launch_steam_login_required)) | ||
| } else { | ||
| when (val resolution = resolveGameAppId(context, launchRequest.appId)) { | ||
| is GameResolutionResult.Success -> { | ||
| if (launchRequest.containerConfig != null) { | ||
| IntentLaunchManager.applyTemporaryConfigOverride( | ||
| context, launchRequest.appId, launchRequest.containerConfig, | ||
| ) | ||
| } | ||
| MainActivity.wasLaunchedViaExternalIntent = true | ||
| trackGameLaunched(resolution.finalAppId) | ||
| viewModel.setLaunchedAppId(resolution.finalAppId) | ||
| viewModel.setBootToContainer(false) | ||
| preLaunchApp( | ||
| context = context, | ||
| appId = resolution.finalAppId, | ||
| useTemporaryOverride = launchRequest.containerConfig != null, | ||
| setLoadingDialogVisible = viewModel::setLoadingDialogVisible, | ||
| setLoadingProgress = viewModel::setLoadingDialogProgress, | ||
| setLoadingMessage = viewModel::setLoadingDialogMessage, | ||
| setMessageDialogState = setMessageDialogState, | ||
| onSuccess = viewModel::launchApp, | ||
| ) | ||
| } | ||
|
|
||
| is GameResolutionResult.NotFound -> { | ||
| val appName = ContainerUtils.resolveGameName(resolution.originalAppId) | ||
| Timber.w("[PluviaMain]: Game not installed: $appName (${launchRequest.appId})") | ||
| msgDialogState = MessageDialogState( | ||
| visible = true, | ||
| type = DialogType.SYNC_FAIL, | ||
| title = context.getString(R.string.game_not_installed_title), | ||
| message = context.getString(R.string.game_not_installed_message, appName), | ||
| dismissBtnText = context.getString(R.string.ok), | ||
| ) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Wait can you explain why this was needed? what's the cold start?
There was a problem hiding this comment.
sure -- backing up to the old state: Steam games were deferred via:
if (gameSource == GameSource.STEAM && !SteamService.isLoggedIn && !runsWithoutSteam) {
setPendingLaunchRequest(launchRequest) // defer until login
} else {
consumePendingLaunchRequest()
emit(ExternalGameLaunch) // launch now
}and all other game types were immediately consumed. The Steam pending request was consumed when the Steam-specific OnLogonEnded event fired. If GN wasn't running at the time this request arrived (cold-start), non-Steam game launches (and Steam launches for games which run without Steam login) would just fire into the void. Steam games would wait until login completes and then start.
This new consumePendingLaunchRequest makes the most sense in Pluvia because it's the earliest point where all requirements are met:
- Compose UI is composed (can show snackbar/dialog feedback)
- ViewModel is available (can call preLaunchApp)
- Login state is known (can gate Steam games on login)
Compose doesn't exist in MainActivity.onCreate, so that's not the right place, either.
Does that make more sense?
Closes #883
Summary
onNewIntent→ emit event) vs cold (store pending, PluviaMain consumes when UI ready)resolveNotInstalledGameNamewrapperTest plan
adb shell am start -a app.gamenative.LAUNCH_GAME --es app_id <id>— game launches after loginSummary by cubic
Fixes dropped LAUNCH_GAME intents on cold start by queuing them until the UI is ready; warm launches still fire immediately. Adds clear, localized feedback when an intent is invalid, the game isn’t installed, or Steam login is required.
Bug Fixes
onNewIntent; all cold launches are stored and consumed byPluviaMainwhen the UI is ready.New Features
Written for commit 3e44cf8. Summary will update on new commits.
Summary by CodeRabbit
New Features
Localization