Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

inboxPlaylist WIP. Works, but incorrectly follows the User.starredPlaylist pattern. Should be on PlaylistContainer instead? #66

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nzoschke
Copy link
Contributor

I will need some help landing this one. I'm not a C++ pro, yet.

I would like to gain access to the session user inbox playlist via sp_session_inbox_create. Copying the way that sp_session_starred_for_user_create turns into the starredPlaylist accessor was the easiest for me.

I know that casting it to a StarredPlaylist isn't correct, nor is having it on User, when it only makes sense for a session user.

@FrontierPsychiatrist
Copy link
Owner

I guess since this is only tied to the session the best way to put it would be just `spotify.inboxPlaylist'. User does not make sense since it would be the same for all users.

Instead of StarredPlaylist a completely new Playlist subtype should be created

class InboxPlaylist : public Playlist {
public:
  InboxPlaylist(sp_playlist* playlist) : Playlist(playlist) {}
  std::string name() { return "Inbox"; }
}

Furthermore, I guess you can't add tracks to an InboxPlaylist, so maybe it would be good to overwrite playlist.addTracks() to just throw an exception. But maybe this even works with the existing error handling.

@nzoschke
Copy link
Contributor Author

nzoschke commented Jul 2, 2014

It looks like you can add tracks to the inbox playlist just like any other playlist! But they don't show up in the UI, so you need to delete them programmatically. You can argue that you shouldn't but I'm not sure it needs to throw an exception...

@nzoschke
Copy link
Contributor Author

nzoschke commented Jul 2, 2014

Updated the patch. Now correctly implements InboxPlaylist class accessible through the Spotify object.

@FrontierPsychiatrist
Copy link
Owner

Hi!
I hope next week I will be able to look at this (and finish 0.7.0...).

@FrontierPsychiatrist FrontierPsychiatrist added this to the 0.8.0 milestone Dec 23, 2014
@FrontierPsychiatrist FrontierPsychiatrist modified the milestones: 1.0.0, 0.8.0 Jun 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants