Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Changed

- [playback] Changed type of `SpotifyId` fields in `PlayerEvent` members to `SpotifyUri` (breaking)
- [metadata] Changed arguments for `Metadata` trait from `&SpotifyId` to `&SpotifyUri` (breaking)
- [player] `load` function changed from accepting a `SpotifyId` to accepting a `SpotifyUri` (breaking)
- [player] `preload` function changed from accepting a `SpotifyId` to accepting a `SpotifyUri` (breaking)
- [spclient] `get_radio_for_track` function changed from accepting a `SpotifyId` to accepting a `SpotifyUri` (breaking)
- [playback] Changed type of `SpotifyId` fields in `PlayerEvent` members to `SpotifyUri` (breaking)
- [playback] `load` function changed from accepting a `SpotifyId` to accepting a `SpotifyUri` (breaking)
- [playback] `preload` function changed from accepting a `SpotifyId` to accepting a `SpotifyUri` (breaking)
- [core] `get_radio_for_track` function changed from accepting a `SpotifyId` to accepting a `SpotifyUri` (breaking)

### Fixed

- [connect] Fixed failed transferring with transfer data that had an empty context uri and no tracks

### Removed

Expand Down
46 changes: 29 additions & 17 deletions connect/src/spirc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1056,7 +1056,7 @@ impl SpircTask {
fn handle_transfer(&mut self, mut transfer: TransferState) -> Result<(), Error> {
let mut ctx_uri = match transfer.current_session.context.uri {
None => Err(SpircError::NoUri("transfer context"))?,
// can apparently happen when a state is transferred stared with "uris" via the api
// can apparently happen when a state is transferred and was started with "uris" via the api
Some(ref uri) if uri == "-" => String::new(),
Some(ref uri) => uri.clone(),
};
Expand Down Expand Up @@ -1088,16 +1088,21 @@ impl SpircTask {
ContextAction::Replace,
));
} else {
self.load_context_from_tracks(
transfer
.current_session
.context
.pages
.iter()
.cloned()
.flat_map(|p| p.tracks)
.collect::<Vec<_>>(),
)?
let all_tracks = transfer
.current_session
.context
.pages
.iter()
.cloned()
.flat_map(|p| p.tracks)
.collect::<Vec<_>>();

if !all_tracks.is_empty() {
self.load_context_from_tracks(all_tracks)?
} else {
warn!("tried to transfer with an invalid state, using fallback ({fallback})");
ctx_uri = fallback.clone();
}
}

self.context_resolver.add(ResolveContext::from_uri(
Expand All @@ -1111,7 +1116,7 @@ impl SpircTask {

let timestamp = self.now_ms();
let state = &mut self.connect_state;
state.handle_initial_transfer(&mut transfer);
state.handle_initial_transfer(&mut transfer, ctx_uri.clone());

// adjust active context, so resolve knows for which context it should set up the state
state.active_context = if autoplay {
Expand Down Expand Up @@ -1148,11 +1153,18 @@ impl SpircTask {
if load_from_context_uri {
self.transfer_state = Some(transfer);
} else {
let ctx = self.connect_state.get_context(ContextType::Default)?;
let idx = ConnectState::find_index_in_context(ctx, |pt| {
self.connect_state.current_track(|t| pt.uri == t.uri)
})?;
self.connect_state.reset_playback_to_position(Some(idx))?;
match self.connect_state.get_context(ContextType::Default) {
Err(_) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be helpful to capture the error?

Err(e) => {
    warn!("no context to reset with: {e}, continuing transfer after context resolved"),
    // ..
}

self.transfer_state = Some(transfer);
warn!("no context to reset with, continuing transfer after context resolved");
}
Ok(ctx) => {
let idx = ConnectState::find_index_in_context(ctx, |pt| {
self.connect_state.current_track(|t| pt.uri == t.uri)
})?;
self.connect_state.reset_playback_to_position(Some(idx))?;
}
}
}

self.load_track(is_playing, position.try_into()?)
Expand Down
8 changes: 5 additions & 3 deletions connect/src/state/transfer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@ impl ConnectState {
transfer: &TransferState,
) -> Result<ProvidedTrack, Error> {
let track = if transfer.queue.is_playing_queue.unwrap_or_default() {
debug!("transfer track was used from the queue");
transfer.queue.tracks.first()
} else {
debug!("transfer track was the current track");
transfer.playback.current_track.as_ref()
}
.ok_or(StateError::CouldNotResolveTrackFromTransfer)?;
Expand All @@ -37,7 +39,7 @@ impl ConnectState {
}

/// handles the initially transferable data
pub fn handle_initial_transfer(&mut self, transfer: &mut TransferState) {
pub fn handle_initial_transfer(&mut self, transfer: &mut TransferState, ctx_uri: String) {
let current_context_metadata = self.context.as_ref().map(|c| c.metadata.clone());
let player = self.player_mut();

Expand Down Expand Up @@ -84,8 +86,8 @@ impl ConnectState {
}
}

player.context_url.clear();
player.context_uri.clear();
player.context_url = format!("context://{ctx_uri}");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to check for an empty or otherwise malformed ctx_uri here?

player.context_uri = ctx_uri;

if let Some(metadata) = current_context_metadata {
for (key, value) in metadata {
Expand Down
Loading