Improve sound handling and show title in volume mixer #4447
Improve sound handling and show title in volume mixer #4447DavidGBrett merged 6 commits intoFlow-Launcher:devfrom
Conversation
The volume mixer uses this value, so this fixes the issue where no named showed for flow there
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughFlow Launcher makes the UseSound setting observable via property-change notifications and synchronizes sound resource lifecycle with this setting. Sound effects are conditionally initialized or torn down when the setting changes; playback is guarded against uninitialized instances and resources are re-synced after sleep and on disposal. Assembly title "Flow Launcher" was added. ChangesSound Lifecycle Synchronization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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: 1
🤖 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 `@Flow.Launcher/MainWindow.xaml.cs`:
- Around line 782-790: The wake/sleep handler must force reinitialization of the
MediaPlayer even if _animationSoundWMP is non-null; change the logic in the
dispatcher-invoked block so that SyncSoundEffectsState (or the code path it
calls) invokes InitSoundEffects unconditionally when sound is enabled on wake
instead of only when IsSoundEffectsInitialized() returns false. Concretely,
update SyncSoundEffectsState (or the wake handler that calls it) to detect a
resume event and call InitSoundEffects() regardless of
IsSoundEffectsInitialized(), ensuring _animationSoundWMP gets reconstructed
after resume; keep the Dispatcher.CheckAccess/Invoke pattern around the call to
preserve UI-thread affinity.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f72394b2-34dc-454b-a634-e529e7f3efb3
📒 Files selected for processing (3)
Flow.Launcher.Infrastructure/UserSettings/Settings.csFlow.Launcher/MainWindow.xaml.csSolutionAssemblyInfo.cs
There was a problem hiding this comment.
1 issue found across 3 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="Flow.Launcher/MainWindow.xaml.cs">
<violation number="1" location="Flow.Launcher/MainWindow.xaml.cs:758">
P2: After sleep/hibernate, sound players are no longer reinitialized when `UseSound` is enabled, so the previous "sound not playing after sleep" issue can reappear.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Add SyncSoundEffectsState and observe UseSound changes in MainWindow Only initialize sound effects on startup when Settings.UseSound is enabled. Listen for UseSound changes at runtime and sync the sound player state by initializing or disposing it as needed.
92269ab to
298f45b
Compare
There was a problem hiding this comment.
Pull request overview
This PR addresses Windows volume mixer behavior by ensuring Flow Launcher has a proper session title and by lazily initializing/disposing sound-effect players based on the UseSound setting, preventing an audio session from being created at startup when sounds are disabled (related to #2864).
Changes:
- Add
AssemblyTitle("Flow Launcher")so the app name appears correctly in the Windows volume mixer. - Make
Settings.UseSoundraisePropertyChangedso the app can react immediately to toggles. - Replace unconditional sound initialization with
SyncSoundEffectsState()plus centralized disposal and safer playback null-checks.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| SolutionAssemblyInfo.cs | Adds AssemblyTitle to improve the displayed app name in the Windows volume mixer. |
| Flow.Launcher/MainWindow.xaml.cs | Lazily initializes/tears down sound players based on UseSound, adds reinit-on-resume path, and null-safety in SoundPlay. |
| Flow.Launcher.Infrastructure/UserSettings/Settings.cs | Makes UseSound observable via PropertyChanged to support runtime lifecycle updates. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@Flow.Launcher/MainWindow.xaml.cs`:
- Around line 1534-1539: Swap the call order in the MainWindow disposal sequence
so the sleep/wake listener is deregistered before tearing down players: call
UnregisterSoundEffectsEvent() prior to DisposeSoundEffects() (near the dispose
block that currently calls _hwndSource?.Dispose(), _notifyIcon?.Dispose(),
DisposeSoundEffects(), _viewModel.ActualApplicationThemeChanged -=
ViewModel_ActualApplicationThemeChanged, UnregisterSoundEffectsEvent()). This
ensures the background Win32 callback cannot reinitialize sound players after
they have been disposed.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 95276c49-0e54-44fd-bfb3-711da1db76fe
📒 Files selected for processing (1)
Flow.Launcher/MainWindow.xaml.cs
Jack251970
left a comment
There was a problem hiding this comment.
LGTM! Feel free to merge it by yourself!
Mostly fixes #2864
Fixes the blank name issue in the volume mixer by setting AssemblyTitle
Only creates the sound / media players when _settings.UseSounds is set and observes changes to that, disposing and init'ing as needed.
This avoids showing flow in the volume mixer if it is set to false at startup, but unfortunately it doesn't seem possible to remove it from there in runtime.
As far as I can tell this is because the created audio session is tied to the flow launcher process itself in windows, so our only options would be trying some low level API's or using a seperate process for audio.
Summary by cubic
Summary of changes
Show “Flow Launcher” by name in the Windows volume mixer and make sound effects opt-in with lazy init. The mixer no longer shows Flow if sounds are off at startup (addresses #2864; runtime removal isn’t possible).
Settings.UseSoundis now observable; we react on startup, on setting changes, and after resume viaSyncSoundEffectsState. InDispose, we nowUnregisterSoundEffectsEventbeforeDisposeSoundEffectsfor safe teardown.SoundPlayadds null checks.[assembly: AssemblyTitle("Flow Launcher")]for the mixer label. New methodsSyncSoundEffectsState,DisposeSoundEffects, andIsSoundEffectsInitializedmanage player lifecycle and reinit after resume.UseSoundis off (no players created); resources released immediately when toggled off or on dispose.Written for commit c7447ea. Summary will update on new commits.