diff --git a/CHANGELOG.md b/CHANGELOG.md index 02cd9a35..f1ecc37f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## [Unreleased] + +### Fixed + +- **Volume display glitch on rapid changes**: Fixed the volume percentage briefly reverting to an old value after the user changed it, especially noticeable when spamming volume up/down. The UI now always shows the user's intended volume until Spotify's API confirms it matches. + ## [v0.38.0] - 2026-03-23 ### Added diff --git a/src/core/app.rs b/src/core/app.rs index 8f5d522d..3beb8873 100644 --- a/src/core/app.rs +++ b/src/core/app.rs @@ -592,6 +592,13 @@ pub struct SettingItem { } pub struct App { + /// What the user actually wants the volume to be. We keep this around until + /// Spotify's API comes back with the same value — otherwise a slow poll + /// response can flash the old volume back on screen. + pub pending_volume: Option, + /// The last value we actually sent to the API. Lets us skip redundant + /// dispatches while we're just waiting for confirmation. + pub last_dispatched_volume: Option, pub instant_since_last_current_playback_poll: Instant, navigation_stack: Vec, pub spectrum_data: Option, @@ -779,6 +786,10 @@ pub struct App { pub saved_tracks_prefetch_generation: u64, /// Incremented every time the playlist track table is reloaded to guard stale prefetch tasks pub playlist_tracks_prefetch_generation: u64, + /// Tracks whether a ChangeVolume request is on its way to Spotify. + /// When true, we hold off on sending another one — rapid key presses + /// just update `pending_volume` and the latest value wins. + pub is_volume_change_in_flight: bool, /// Reference to the native streaming player for direct control (bypasses event channel) #[cfg(feature = "streaming")] pub streaming_player: Option>, @@ -945,6 +956,9 @@ impl Default for App { _playlist_refresh_generation: 0, saved_tracks_prefetch_generation: 0, playlist_tracks_prefetch_generation: 0, + is_volume_change_in_flight: false, + pending_volume: None, + last_dispatched_volume: None, #[cfg(feature = "streaming")] streaming_player: None, #[cfg(all(feature = "mpris", target_os = "linux"))] @@ -1567,6 +1581,27 @@ impl App { } } + /// Picks up pending volume changes from the tick loop and sends them to Spotify. + /// + /// Skips dispatching if the previous request is still in flight, or if we + /// already sent this exact value and are just waiting for the API to confirm. + /// + /// We intentionally don't clear `pending_volume` here — it sticks around until + /// `get_current_playback` sees the matching value come back from the API. + pub fn flush_pending_volume(&mut self) { + if self.is_volume_change_in_flight { + return; // previous request still processing + } + if let Some(volume) = self.pending_volume { + if self.last_dispatched_volume == Some(volume) { + return; // already dispatched this value, waiting for API to confirm + } + self.is_volume_change_in_flight = true; + self.last_dispatched_volume = Some(volume); + self.dispatch(IoEvent::ChangeVolume(volume)); + } + } + pub fn get_recommendations_for_seed( &mut self, seed_artists: Option>, @@ -1604,70 +1639,99 @@ impl App { } } + /// Returns the volume the UI should show and volume-up/down should use as a base. + /// + /// If the user just pressed a volume key, we show their input (not what the API + /// says) because Spotify can be slow to reflect the change. Without this, you'd + /// see the percentage jump back to the old value for a split second before + /// correcting — especially noticeable when spamming volume up/down. + pub fn desired_volume(&self) -> u32 { + if let Some(pending) = self.pending_volume { + return pending as u32; + } + self + .current_playback_context + .as_ref() + .and_then(|c| c.device.volume_percent) + .unwrap_or(0) + } + + /// Bump volume up. Uses `desired_volume()` as the base so rapid presses + /// don't accidentally calculate from a stale API value. pub fn increase_volume(&mut self) { - if let Some(context) = self.current_playback_context.clone() { - let current_volume = context.device.volume_percent.unwrap_or(0) as u8; - let next_volume = min( - current_volume + self.user_config.behavior.volume_increment, - 100, - ); + let current_volume = self.desired_volume() as u8; + let next_volume = min( + current_volume + self.user_config.behavior.volume_increment, + 100, + ); - if next_volume != current_volume { - info!("increasing volume: {} -> {}", current_volume, next_volume); - // Use native streaming player for instant control (bypasses event channel latency) - #[cfg(feature = "streaming")] - if self.is_native_streaming_active_for_playback() { - if let Some(ref player) = self.streaming_player { - player.set_volume(next_volume); - - // Update UI state immediately - if let Some(ctx) = &mut self.current_playback_context { - ctx.device.volume_percent = Some(next_volume.into()); - } - self.user_config.behavior.volume_percent = next_volume; - let _ = self.user_config.save_config(); - return; + if next_volume != current_volume { + info!("increasing volume: {} -> {}", current_volume, next_volume); + // Use native streaming player for instant control (bypasses event channel latency) + #[cfg(feature = "streaming")] + if self.is_native_streaming_active_for_playback() { + if let Some(ref player) = self.streaming_player { + player.set_volume(next_volume); + + // Update UI state immediately + if let Some(ctx) = &mut self.current_playback_context { + ctx.device.volume_percent = Some(next_volume.into()); } + self.user_config.behavior.volume_percent = next_volume; + let _ = self.user_config.save_config(); + self.pending_volume = Some(next_volume); + return; } + } - // Fallback to API-based volume control for external devices + // Fallback to API-based volume control for external devices + // Coalesce: only dispatch if no request is already in flight + self.pending_volume = Some(next_volume); + if !self.is_volume_change_in_flight { + self.is_volume_change_in_flight = true; self.dispatch(IoEvent::ChangeVolume(next_volume)); } } } + /// Bump volume down. Uses `desired_volume()` as the base so rapid presses + /// don't accidentally calculate from a stale API value. pub fn decrease_volume(&mut self) { - if let Some(context) = self.current_playback_context.clone() { - let current_volume = context.device.volume_percent.unwrap_or(0) as i8; - let next_volume = max( - current_volume - self.user_config.behavior.volume_increment as i8, - 0, - ); + let current_volume = self.desired_volume() as i8; + let next_volume = max( + current_volume - self.user_config.behavior.volume_increment as i8, + 0, + ); - if next_volume != current_volume { - let next_volume_u8 = next_volume as u8; - info!( - "decreasing volume: {} -> {}", - current_volume, next_volume_u8 - ); + if next_volume != current_volume { + let next_volume_u8 = next_volume as u8; + info!( + "decreasing volume: {} -> {}", + current_volume, next_volume_u8 + ); - // Use native streaming player for instant control (bypasses event channel latency) - #[cfg(feature = "streaming")] - if self.is_native_streaming_active_for_playback() { - if let Some(ref player) = self.streaming_player { - player.set_volume(next_volume_u8); + // Use native streaming player for instant control (bypasses event channel latency) + #[cfg(feature = "streaming")] + if self.is_native_streaming_active_for_playback() { + if let Some(ref player) = self.streaming_player { + player.set_volume(next_volume_u8); - // Update UI state immediately - if let Some(ctx) = &mut self.current_playback_context { - ctx.device.volume_percent = Some(next_volume_u8.into()); - } - self.user_config.behavior.volume_percent = next_volume_u8; - let _ = self.user_config.save_config(); - return; + // Update UI state immediately + if let Some(ctx) = &mut self.current_playback_context { + ctx.device.volume_percent = Some(next_volume_u8.into()); } + self.user_config.behavior.volume_percent = next_volume_u8; + let _ = self.user_config.save_config(); + self.pending_volume = Some(next_volume_u8); + return; } + } - // Fallback to API-based volume control for external devices + // Fallback to API-based volume control for external devices + // Coalesce: only dispatch if no request is already in flight + self.pending_volume = Some(next_volume_u8); + if !self.is_volume_change_in_flight { + self.is_volume_change_in_flight = true; self.dispatch(IoEvent::ChangeVolume(next_volume_u8)); } } diff --git a/src/infra/network/playback.rs b/src/infra/network/playback.rs index d73ed611..d54624c4 100644 --- a/src/infra/network/playback.rs +++ b/src/infra/network/playback.rs @@ -246,6 +246,22 @@ impl PlaybackNetwork for Network { } } + // Check if Spotify finally caught up to the user's volume change. + // If the API now returns what the user asked for, we can clear pending_volume + // and let the API take over again. If not, this response is stale — ignore it. + if let Some(pending) = app.pending_volume { + let api_vol = c.device.volume_percent.unwrap_or(0) as u8; + if api_vol == pending { + app.pending_volume = None; + app.last_dispatched_volume = None; + } else { + // API hasn't caught up yet — keep showing the user's intended value + if let Some(ctx) = app.current_playback_context.as_ref() { + c.device.volume_percent = ctx.device.volume_percent; + } + } + } + // On first load with native streaming AND native device is active, // override API shuffle with saved preference. #[cfg(feature = "streaming")] @@ -708,6 +724,15 @@ impl PlaybackNetwork for Network { } } + /// Sends the volume change to Spotify, either through the native streaming + /// player or the Web API depending on which device is active. + /// + /// On success we clear the in-flight flag but keep `pending_volume` around. + /// It only gets cleared when `get_current_playback` comes back with a matching + /// volume — that's our signal that Spotify actually caught up. + /// + /// On error we bail and clear everything so the UI falls back to whatever + /// the API last reported. async fn change_volume(&mut self, volume: u8) { #[cfg(feature = "streaming")] if is_native_streaming_active_for_playback(self).await { @@ -717,6 +742,9 @@ impl PlaybackNetwork for Network { if let Some(ctx) = &mut app.current_playback_context { ctx.device.volume_percent = Some(volume.into()); } + app.is_volume_change_in_flight = false; + app.last_dispatched_volume = Some(volume); + // Keep pending_volume set — cleared when API confirms the value matches return; } } @@ -727,9 +755,15 @@ impl PlaybackNetwork for Network { if let Some(ctx) = &mut app.current_playback_context { ctx.device.volume_percent = Some(volume.into()); } + app.is_volume_change_in_flight = false; + app.last_dispatched_volume = Some(volume); + // Keep pending_volume set — cleared when get_current_playback confirms } Err(e) => { let mut app = self.app.lock().await; + app.is_volume_change_in_flight = false; + app.pending_volume = None; + app.last_dispatched_volume = None; app.handle_error(anyhow!(e)); } } diff --git a/src/main.rs b/src/main.rs index f706ab5c..e6aec694 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1950,12 +1950,21 @@ async fn handle_player_events( } if let Ok(mut app) = app.try_lock() { - if let Some(ref mut ctx) = app.current_playback_context { - ctx.device.volume_percent = Some(volume_percent as u32); + if let Some(pending) = app.pending_volume { + if volume_percent == pending { + // Native player caught up — safe to clear pending + app.pending_volume = None; + app.last_dispatched_volume = None; + } + // If it doesn't match, the event is stale or from an external + // change — leave pending_volume alone so the UI stays correct. + } else { + if let Some(ref mut ctx) = app.current_playback_context { + ctx.device.volume_percent = Some(volume_percent as u32); + } + app.user_config.behavior.volume_percent = volume_percent.min(100); + let _ = app.user_config.save_config(); } - // Persist the latest volume so it is restored on next launch - app.user_config.behavior.volume_percent = volume_percent.min(100); - let _ = app.user_config.save_config(); } } PlayerEvent::PositionChanged { @@ -2646,6 +2655,7 @@ async fn start_ui( #[cfg(feature = "streaming")] app.flush_pending_native_seek(); app.flush_pending_api_seek(); + app.flush_pending_volume(); #[cfg(feature = "discord-rpc")] if let Some(ref manager) = discord_rpc_manager { @@ -2955,6 +2965,7 @@ async fn start_ui( #[cfg(feature = "streaming")] app.flush_pending_native_seek(); app.flush_pending_api_seek(); + app.flush_pending_volume(); #[cfg(feature = "discord-rpc")] if let Some(ref manager) = discord_rpc_manager { diff --git a/src/tui/ui/player.rs b/src/tui/ui/player.rs index 9ebacb9d..35615b68 100644 --- a/src/tui/ui/player.rs +++ b/src/tui/ui/player.rs @@ -536,7 +536,7 @@ pub fn draw_playbar(f: &mut Frame<'_>, app: &App, layout_chunk: Rect) { current_playback_context.device.name, shuffle_text, repeat_text, - current_playback_context.device.volume_percent.unwrap_or(0) + app.desired_volume() ); if let Some(session) = &app.party_session {