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

Added a favorites feature to remember and easily access favorite albums. #799

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

Conversation

tobag90
Copy link

@tobag90 tobag90 commented Feb 14, 2016

Added a feature to remember an easily access favorite albums. Especially useful for people with huge libraries.

@abarisain
Copy link
Owner

I'm for it. Does it auto clean if the user deleted the album?

@tobag90
Copy link
Author

tobag90 commented Feb 14, 2016

Good point, not yet. I will look into that.

@abarisain
Copy link
Owner

It's possible that it's not an issue anyway, and thinking about it, the
user might not want it to be cleared if he uses multiple servers.

I'llleave the rest to avuton. This feature would of course be better server
side, but I don't know how it can be done, if it can
Le 14 févr. 2016 20:51, "tobag90" [email protected] a écrit :

Good point, not yet. I will look into that.


Reply to this email directly or view it on GitHub
#799 (comment).

@avuton
Copy link
Contributor

avuton commented Feb 16, 2016

Hello!

First of all, thank you for the time you took to create this functionality.

With that said, I don't mind the goal of this patch, but right now I disagree with the implementation.

I believe this could and should be implemented using the Sticker subsystem.

Keeping in mind that the concept of an 'album' does not actually exist via the MPD protocol, it only really exists in our Album item abstraction, I might implement it something like this (via the protocol):

command_list_begin
sticker set song "first-song-from-album1.flac" albumfav {unixtime or precidence? Anything useful could go here}
sticker set song "second-song-from-album1.flac" albumfav {unixtime or precidence? Anything useful could go here}
...
command_list_end

There is also the

sticker find {TYPE} {URI} {NAME} = {VALUE}

command which is not yet implemented in the Sticker.java class, but I could whip up pretty swiftly, and will likely implement soon just for fun.

Then when it was time to query:

sticker find song / albumfav

There will likely be duplicated information sent and received, but it's a limitation of the protocol; The MPD protocol has many limitations, but efforts should be made to implement functionality server side when possible.

With all this said, not all parts of the Sticker.java class is not well tested. If you find the Sticker.java class to be buggy, feel free to shoot me a message via hangouts or an email and I'll get a fix in for it straight away.

@avuton
Copy link
Contributor

avuton commented Feb 16, 2016

A slight error above, the Sticker.java class is not compatible with command lists. Not a huge problem, but, this functionality could be added in the future.

@abarisain
Copy link
Owner

Thanks for the review and insights about the API

Le 16 févr. 2016 à 11:28, Avuton Olrich [email protected] a écrit :

A slight error above, the Sticker.java class is not compatible with command lists. Not a huge problem, but, this functionality could be added in the future.


Reply to this email directly or view it on GitHub.

@hurzl
Copy link
Contributor

hurzl commented Feb 17, 2016

As long as the album/artist combination respects album artists I see no problem with that concept and it has far less overhead than the sticker method.

@@ -20,6 +20,7 @@
import com.anpmech.mpd.subsystem.status.IdleSubsystemMonitor;
import com.anpmech.mpd.subsystem.status.StatusChangeListener;
import com.anpmech.mpd.subsystem.status.TrackPositionListener;
import com.namelessdev.mpdroid.favorites.Favorites;
import com.namelessdev.mpdroid.helpers.CachedMPD;
Copy link
Collaborator

Choose a reason for hiding this comment

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

test a modifier

@sludgefeast
Copy link

Pull request #831 extends the favorites feature to use MPD stickers.

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.

6 participants