fix(platform): dispatch BigBox PlayGame calls to WPF UI thread#527
Open
wizzomafizzo wants to merge 8 commits into
Open
fix(platform): dispatch BigBox PlayGame calls to WPF UI thread#527wizzomafizzo wants to merge 8 commits into
wizzomafizzo wants to merge 8 commits into
Conversation
LaunchGameById is called from a background thread (named pipe reader), but BigBoxMainViewModel.PlayGame requires the WPF UI thread. Capture the Dispatcher during plugin init and use Invoke() to marshal PlayGame calls onto it. Closes #509
ShowGame navigates BigBox UI to the NFC-scanned game, firing LEDBlinky Event 9 (game selection). Without this, LEDBlinky shows controls for whichever game the user had highlighted in the BigBox menu rather than the game actually being launched.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
|
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 due to trivial changes (1)
📝 WalkthroughWalkthroughAssembly/file version metadata bumped to 1.2.0. The LaunchBox plugin rewrites IPC command handling and launch lifecycle (case-normalized dispatch, pending-launch state with 30s stale timeout, UI-dispatcher PlayGame, Win32 focus hook, navigation restore, error reporting, and logging). Dockerfile publish no longer uses --no-restore. Changes
Sequence DiagramsequenceDiagram
participant External as External Trigger (NFC / IPC)
participant Plugin as Zaparoo Plugin
participant Dispatcher as UI Dispatcher
participant BigBox as BigBox / LaunchBox UI
participant Game as Launched Game Process
participant WinEvent as Win32 Foreground Hook
External->>Plugin: Send command (case-normalized)
Plugin->>Plugin: Validate command / resolve id -> IGame or IAdditionalApplication
alt launch already pending
Plugin-->>External: ErrorEvent (concurrent launch rejected)
else
Plugin->>Plugin: Set pending-launch id (30s timeout)
Plugin->>Dispatcher: Invoke PlayGame(...) on UI thread
Dispatcher->>BigBox: Optionally show target (FilterType.None) before launch
Plugin->>Game: PlayGame starts
WinEvent->>Plugin: Foreground change detected
Plugin->>Dispatcher: Re-show game in BigBox if needed
Game-->>Plugin: GameExitedEvent (prefers additional app info)
Plugin->>Dispatcher: If configured, emulate BigBox back key or ShowAllGames
Plugin->>Plugin: Clear pending-launch state
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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 docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@scripts/windows/launchbox-plugin/ZaparooPlugin.cs`:
- Around line 116-124: The branch that handles matching pending BigBox restores
leaves the field _shouldRestoreBigBoxView unchanged on the non-match path,
allowing a previously-set true value to persist; update the logic around
MatchesPendingBigBoxRestore(game, app) so ClearPendingBigBoxRestore() is always
called and explicitly set _shouldRestoreBigBoxView = false when there is no
match (or invert the flow: call ClearPendingBigBoxRestore() first and then set
_shouldRestoreBigBoxView = MatchesPendingBigBoxRestore(game, app)) so
OnGameExited() cannot restore BigBox for unrelated launches; refer to the
MatchesPendingBigBoxRestore, ClearPendingBigBoxRestore, and
_shouldRestoreBigBoxView symbols when making the change.
- Around line 531-543: The InvokeOnUiThread method currently executes action()
directly when _uiDispatcher is null, which can run UI work on the pipe
background thread; change this to gate or enqueue the work until a real UI
dispatcher exists: when _uiDispatcher (or Application.Current?.Dispatcher) is
null, do not call action() — instead store the Action in a short-lived queue or
await obtaining the dispatcher (e.g., subscribe to an initialization event or
poll Application.Current.Dispatcher) and then Invoke/BeginInvoke queued actions
on that dispatcher; update InvokeOnUiThread and the _uiDispatcher initialization
path to flush the queue once a valid dispatcher is captured so no UI work runs
on the pipe thread.
🪄 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: 703b965f-ead1-451d-a022-5f041525ad1a
📒 Files selected for processing (2)
scripts/windows/launchbox-plugin/ZaparooLaunchBoxPlugin.csprojscripts/windows/launchbox-plugin/ZaparooPlugin.cs
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 `@scripts/windows/launchbox-plugin/ZaparooPlugin.cs`:
- Around line 680-689: Before calling
PluginHelper.BigBoxMainViewModel.PlayGame(...) in the branch where BigBox is
running, ensure the NFC-targeted title is selected first; insert a call to the
BigBox selection API on PluginHelper.BigBoxMainViewModel (e.g. SelectGame(game)
or set SelectedGame = game) immediately before Log(...) / PlayGame(...) so the
UI highlight matches the scanned game and LEDBlinky receives the correct
selection; keep EnsureForegroundHook() and BeginPendingLaunch(...) as-is and
then invoke the selection prior to
PluginHelper.BigBoxMainViewModel.PlayGame(game, app, null, null).
- Around line 568-620: For the listed switch branches ("launch", "showplatform",
"showplaylist", "search", "getgamesforplatform") add explicit error reporting
when required fields are missing instead of silently no-op: where you currently
check string.IsNullOrEmpty(command.Id/Platform/Playlist/Query) and skip calling
LaunchGameById, ShowPlatform, ShowPlaylist, Search, or SendGamesForPlatform,
call the IPC error-reporting path to emit an ErrorEvent with a clear message
(e.g., "missing required field 'Id'/'Platform'/'Playlist'/'Query' for <action>")
so callers can distinguish bad input from success; implement this in the
branches referencing LaunchGameById, ShowPlatform, ShowPlaylist, Search, and
SendGamesForPlatform.
🪄 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: 611c4719-a384-4d5f-ac85-c4d478e4d913
📒 Files selected for processing (3)
scripts/windows/launchbox-plugin/Dockerfilescripts/windows/launchbox-plugin/ZaparooLaunchBoxPlugin.csprojscripts/windows/launchbox-plugin/ZaparooPlugin.cs
✅ Files skipped from review due to trivial changes (1)
- scripts/windows/launchbox-plugin/ZaparooLaunchBoxPlugin.csproj
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
1 participant
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.
Summary
PlayGamecalls to the WPF UI thread viaDispatcher.Invoke()— the named pipe reader runs on a background thread which causes BigBox to silently failDispatcherduring plugin initialization (PluginInitialized,LaunchBoxStartupCompleted,BigBoxStartupCompleted) for reliable access from background threadsNullReferenceExceptionShowGame()beforePlayGame()in BigBox mode so LEDBlinky receives Event 9 (game selection) for the NFC-scanned game — without this, LEDBlinky shows controls for whichever game was highlighted in the BigBox menuCloses #509
Closes #545
Summary by CodeRabbit
Chores
Bug Fixes
New Features