search

sailfish-spotify / hutspot

Spotify Controller for SailfishOS. Documentation: https://sailfish-spotify.github.io/hutspot.
MIT License
18 stars 12 forks source link

Refactor: SpotifyController #36

Closed ksiazkowicz closed 5 years ago

ksiazkowicz commented 5 years ago

Moved most of the logic for managing playbackState, controlling playback and MPRIS stuff to SpotifyController component.

The idea is to unclutter hutspot.qml and make it easier to maintain only one, up to date copy of playbackState, instead of having the same code repeated in both hutspot.qml and on Playing.qml page.

@wdehoog please let me know if I didn't break any feature, I'm not familiar with your most recent changes.

wdehoog commented 5 years ago

Hi!. Look great. Some questions:

  1. You removed the trick to fix the progessbar when it has been dragged. Is that on purpose?
  2. refreshPlaybackState() tried to handle change of context. is that handled somewhere else or does it have to be readded?
  3. I painted myself into a corner while trying to get snapshots of playlists working. Any idea to get that working?

I did not test or build this. Please just merge it and we will see.

ksiazkowicz commented 5 years ago
  1. This was never an issue for me, can you confirm that it happens with my code as well?
  2. Yes, it is handled in SpotifyController.qml. Every time context changes on update, details about it should appear in app.controller.playbackState.contextDetails. app.controller.playbackState is now an instance of PlaybackState.qml, it provides the same properties as Spotify API does, but you can always assume it's there and the most basic properties are not undefined/null.
  3. What do you mean by snapshots of playlists?
wdehoog commented 5 years ago

1). I think it is a bug in the silica code. After dragging they assign something to 'value' which will lose the link that has been set in our qml. 3.) a playlists has snapshots. every modification (add/delete/reorder) will create a new snapshot of the playlist. if a playlist is modified while being played the old snapshot will keep on being played.

ksiazkowicz commented 5 years ago

1) Oh, right, I haven't noticed that bug, right. 3) I understand, that a call to api.spotify.com/v1/playlists/playlist_id would return a latest snapshot of the playlist with given ID, right? It has information about what tracks are on this playlist, images etc. and also a snapshot_id. If you make a change to a playlist, the ID is the same (meaning, context doesn't change), just snapshot_id value probably changes to something else. The only issue that I see here is that contextDetails would hold an outdated list of tracks, since it only compares IDs.

wdehoog commented 5 years ago

Suppose you added a track. It will not be played since the new track is not part of the old snapshot (which is being played). I have this issue in two situations:

  1. when replacing the tracks of the playlist used for the recommendations. the new tracks will not be played. the old ones are.
  2. when adding a track to the 'queue' (spotify does not have a queue so I am trying to use a playlist as a queue)

But lets work on this after you merged this pr. Do I have to agree/+/lgtm or whatever?