Conversation
Co-authored-by: Copilot <[email protected]>
Other expensive events might be added to `OptInPlayerEvents` in the future.
9dd062a to
5bf1c5b
Compare
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
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.
What does this do?
This emits a new universal
Player::SetQueueevent.Background info: How is the Spotify Connect queue organised?
The Spotify connect queue basically has the following data structure:
Each Track has at least a track URI and a provider which can be
autoplay,context,queueorunavailable.autoplay: Added by Spotify to keep playingcontext: part of the current context identified bycontext_uriqueue: manually queued while a context was loadedunavailable: self explanatoryWhy is this event "universal"?
There are multiple actions that manipulate the queue in different ways. Examples:
context_uri,current_track,prev_tracks,next_tracksnext_tracksnext_tracksnext_tracksIf a GUI application wants to adjust its state when any of these actions are performed remote or local, it just needs to listen to this one universal event. The queue displayed in the GUI is always correct and always up to date.
Why does this remove AddToQueue that was just added in 87d37c3 ?
AddToQueueis a special case ofSetQueue, andSetQueuecovers all cases ofAddToQueueusage. Also, it's less error prone for app developers. Say a GUI developer wants to display the current queue from Spirc, and the queue looks like this:When another track is queued, it will be placed after the currently playing track:
In this case,
AddToQueuewill be emitted for Track 1, and the GUI developer will be responsible to add it in the right position of the app's state.When yet another track (or collection of tracks, e.g. an album with 2 tracks) is queued, those will be placed after the currently playing track AND after all the other
queuetracks:In this case,
AddToQueuewill be emitted with Track 2, then with Track 3. Again, the developer of the consuming app is responsible for placing the new tracks in the right position.In contrast to this,
SetQueuealways emits the whole queue with the tracks in the right order, so the developer of the consuming app cannot place the tracks in the wrong position.Also,
AddToQueuewas never in a release, so removing it is ok imho.Are there downsides?
As we're emitting ALL THE QUEUE FIELDS \o/ with every change, we do sometimes emit more than necessary. But we have to emit
next_tracksmost of the time, and the remaining queue fields are not that large compared tonext_tracks.current_trackandcontext_uriis negligible, andprev_tracksis capped at 20 tracks.How to go from here?
I'd love to hear your input on this. Do you think a universal
SetQueueevent is a good thing? Or would you rather have more fine grained events emitting less data, e.g. one forprev_tracks, one fornext_tracksone forcurrent_trackandcurrent_track? Or should we keep the universalSetQueuefor actions that manipulate all ofnext_tracks, but keepAddToTrackwhen adding tracks to the queue, manipulating only small parts ofnext_tracks?The current approach works well for me and my reactive SwiftUI app, and makes updating the queue incredibly easy. But I'm still pretty new to this and there might be use cases that I didn't think of.