diff --git a/connect/src/context_resolver.rs b/connect/src/context_resolver.rs index 2746d5ab..c66d2f12 100644 --- a/connect/src/context_resolver.rs +++ b/connect/src/context_resolver.rs @@ -1,3 +1,4 @@ +use crate::state::context::ContextType; use crate::{ core::{Error, Session}, protocol::{ @@ -6,6 +7,7 @@ use crate::{ }, state::{context::UpdateContext, ConnectState}, }; +use std::cmp::PartialEq; use std::{ collections::{HashMap, VecDeque}, fmt::{Display, Formatter}, @@ -75,10 +77,9 @@ impl ResolveContext { // otherwise we might not even check if we need to fallback and just use the fallback uri match self.resolve { Resolve::Uri(ref uri) => ConnectState::valid_resolve_uri(uri), - Resolve::Context(ref ctx) => { - ConnectState::get_context_uri_from_context(ctx).or(self.fallback.as_deref()) - } + Resolve::Context(ref ctx) => ConnectState::get_context_uri_from_context(ctx), } + .or(self.fallback.as_deref()) } /// the actual context uri @@ -147,6 +148,8 @@ pub struct ContextResolver { // time after which an unavailable context is retried const RETRY_UNAVAILABLE: Duration = Duration::from_secs(3600); +const CONCERNING_AMOUNT_OF_SKIPS: usize = 1_000; + impl ContextResolver { pub fn new(session: Session) -> Self { Self { @@ -208,6 +211,8 @@ impl ContextResolver { loop { let next = self.queue.front()?; match next.resolve_uri() { + // this is here to prevent an endless amount of skips + None if idx > CONCERNING_AMOUNT_OF_SKIPS => unreachable!(), None => { warn!("skipped {idx} because of no valid resolve_uri: {next}"); idx += 1; @@ -318,7 +323,16 @@ impl ContextResolver { return false; } - debug!("last item of type <{:?}> finishing state", next.update); + debug!("last item of type <{:?}>, finishing state", next.update); + + match (next.update, state.active_context) { + (UpdateContext::Default, ContextType::Default) => {} + (UpdateContext::Default, _) => { + debug!("skipped finishing default, because it isn't the active context"); + return false; + } + (UpdateContext::Autoplay, _) => {} + } if let Some(transfer_state) = transfer_state.take() { if let Err(why) = state.finish_transfer(transfer_state) { @@ -328,8 +342,17 @@ impl ContextResolver { let res = if state.shuffling_context() { state.shuffle() + } else if let Ok(ctx) = state.get_context(state.active_context) { + let idx = ConnectState::find_index_in_context(ctx, |t| { + state.current_track(|c| t.uri == c.uri) + }) + .ok(); + + state + .reset_playback_to_position(idx) + .and_then(|_| state.fill_up_next_tracks()) } else { - state.reset_playback_to_position(Some(state.player().index.track as usize)) + state.fill_up_next_tracks() }; if let Err(why) = res { diff --git a/connect/src/spirc.rs b/connect/src/spirc.rs index 958ea74f..bd8fc962 100644 --- a/connect/src/spirc.rs +++ b/connect/src/spirc.rs @@ -985,6 +985,13 @@ impl SpircTask { state.set_active(true); state.handle_initial_transfer(&mut transfer); + // adjust active context, so resolve knows for which context it should set up the state + state.active_context = if autoplay { + ContextType::Autoplay + } else { + ContextType::Default + }; + // update position if the track continued playing let position = if transfer.playback.is_paused { transfer.playback.position_as_of_timestamp.into() @@ -1110,6 +1117,8 @@ impl SpircTask { // for play commands with skip by uid, the context of the command contains // tracks with uri and uid, so we merge the new context with the resolved/existing context self.connect_state.merge_context(context); + + // load here, so that we clear the queue only after we definitely retrieved a new context self.connect_state.clear_next_tracks(false); self.connect_state.clear_restrictions(); @@ -1143,10 +1152,12 @@ impl SpircTask { self.connect_state.update_queue_revision() } else { self.connect_state.shuffle()?; + self.add_autoplay_resolving_when_required(); } } else { self.connect_state.set_current_track(index)?; self.connect_state.reset_playback_to_position(Some(index))?; + self.add_autoplay_resolving_when_required(); } if self.connect_state.current_track(MessageField::is_some) { @@ -1327,9 +1338,8 @@ impl SpircTask { }; }; - self.add_autoplay_resolving_when_required(); - if has_next_track { + self.add_autoplay_resolving_when_required(); self.load_track(continue_playing, 0) } else { info!("Not playing next track because there are no more tracks left in queue."); diff --git a/connect/src/state.rs b/connect/src/state.rs index 6366388f..a132e479 100644 --- a/connect/src/state.rs +++ b/connect/src/state.rs @@ -106,7 +106,7 @@ pub struct ConnectState { // separation is necessary because we could have already loaded // the autoplay context but are still playing from the default context /// to update the active context use [switch_active_context](ConnectState::set_active_context) - active_context: ContextType, + pub active_context: ContextType, fill_up_context: ContextType, /// the context from which we play, is used to top up prev and next tracks diff --git a/connect/src/state/context.rs b/connect/src/state/context.rs index a9881ec4..c0f95b69 100644 --- a/connect/src/state/context.rs +++ b/connect/src/state/context.rs @@ -7,6 +7,7 @@ use crate::{ }; use protobuf::MessageField; use std::collections::HashMap; +use std::ops::Deref; use uuid::Uuid; const LOCAL_FILES_IDENTIFIER: &str = "spotify:local-files"; @@ -21,7 +22,7 @@ pub struct StateContext { pub index: ContextIndex, } -#[derive(Default, Debug, Copy, Clone)] +#[derive(Default, Debug, Copy, Clone, PartialEq)] pub enum ContextType { #[default] Default, @@ -35,6 +36,17 @@ pub enum UpdateContext { Autoplay, } +impl Deref for UpdateContext { + type Target = ContextType; + + fn deref(&self) -> &Self::Target { + match self { + UpdateContext::Default => &ContextType::Default, + UpdateContext::Autoplay => &ContextType::Autoplay, + } + } +} + pub enum ResetContext<'s> { Completely, DefaultIndex, @@ -86,11 +98,16 @@ impl ConnectState { &self.player().context_uri } + fn different_context_uri(&self, uri: &str) -> bool { + // search identifier is always different + self.context_uri() != uri || uri.starts_with(SEARCH_IDENTIFIER) + } + pub fn reset_context(&mut self, mut reset_as: ResetContext) { self.set_active_context(ContextType::Default); self.fill_up_context = ContextType::Default; - if matches!(reset_as, ResetContext::WhenDifferent(ctx) if self.context_uri() != ctx) { + if matches!(reset_as, ResetContext::WhenDifferent(ctx) if self.different_context_uri(ctx)) { reset_as = ResetContext::Completely } self.shuffle_context = None; @@ -134,7 +151,7 @@ impl ConnectState { let ctx = match self.get_context(new_context) { Err(why) => { - debug!("couldn't load context info because: {why}"); + warn!("couldn't load context info because: {why}"); return; } Ok(ctx) => ctx, @@ -184,17 +201,8 @@ impl ConnectState { Some(p) => p, }; - let prev_context = match ty { - UpdateContext::Default => self.context.as_ref(), - UpdateContext::Autoplay => self.autoplay_context.as_ref(), - }; - debug!( - "updated context {ty:?} from <{}> ({} tracks) to <{}> ({} tracks)", - self.context_uri(), - prev_context - .map(|c| c.tracks.len().to_string()) - .unwrap_or_else(|| "-".to_string()), + "updated context {ty:?} to <{}> ({} tracks)", context.uri, page.tracks.len() ); @@ -210,8 +218,8 @@ impl ConnectState { ); // when we update the same context, we should try to preserve the previous position - // otherwise we might load the entire context twice - if !self.context_uri().contains(SEARCH_IDENTIFIER) + // otherwise we might load the entire context twice, unless it's the search context + if !self.context_uri().starts_with(SEARCH_IDENTIFIER) && self.context_uri() == &context.uri { match Self::find_index_in_context(&new_context, |t| { @@ -234,7 +242,7 @@ impl ConnectState { self.context = Some(new_context); - if !context.url.contains(SEARCH_IDENTIFIER) { + if !context.url.starts_with(SEARCH_IDENTIFIER) { self.player_mut().context_url = context.url; } else { self.player_mut().context_url.clear() diff --git a/connect/src/state/tracks.rs b/connect/src/state/tracks.rs index 700bfec5..81dd3878 100644 --- a/connect/src/state/tracks.rs +++ b/connect/src/state/tracks.rs @@ -128,6 +128,8 @@ impl<'ct> ConnectState { }; }; + debug!("next track is {new_track:#?}"); + let new_track = match new_track { None => return Ok(None), Some(t) => t, @@ -280,7 +282,7 @@ impl<'ct> ConnectState { } } - pub fn fill_up_next_tracks(&mut self) -> Result<(), StateError> { + pub fn fill_up_next_tracks(&mut self) -> Result<(), Error> { let ctx = self.get_context(self.fill_up_context)?; let mut new_index = ctx.index.track as usize; let mut iteration = ctx.index.page;