GNOME Bugzilla – Bug 743310
Updates to Smart Playlist functionality in 3.15.4
Last modified: 2015-02-10 10:21:57 UTC
Here are a handful of patches for the latest update to GNOME Music (3.15.4, merged 1/19/15), accomplishing the following: - adding 'recently added' playlist; modifying 'recently_played' query so that it doesn't pick up never-played songs (i.e. songs with playcount=null). Also, minor comment fixes for consistency. - updating all static playlists whenever media added to library (via view._on_changes_pending) - updating static playlists from player.py at time of scrobble rather than from playlists.update_playcount--there was a problem here where, depending on the order that playlists.update_playcount and playlists.update_last_played got called in, changes to a song's last_played value might not be reflected in the playlist update. - updating static playlists on program start--i.e. at the time they are fetched/created by playlists.fetch_or_create_static_playlists.
Created attachment 295123 [details] [review] added 'recently added' playlist, filtered out never-played songs from 'recently played'
Created attachment 295124 [details] [review] update all static playlists whenever media added to library
Created attachment 295125 [details] [review] static playlists now update from player.py at time of scrobble rather than from playlists.update_playcount
Created attachment 295126 [details] [review] static playlists updated on program start (i.e. at time of fetch/create)
Review of attachment 295123 [details] [review]: Looks good to me
Review of attachment 295124 [details] [review]: ::: gnomemusic/view.py @@ +141,3 @@ @log def _on_changes_pending(self, data=None): + playlists.update_all_static_playlists() Since its a base class for others (Albums, Views etc.) we should make sure base function is also called from classes inheriting it: ViewContainer._on_changes_pending(self, None) However I'm not sure if its worth to update playlists from a base class - this clearly belongs to Playlists class in view.py - this class is also loaded while another view is active.
Review of attachment 295125 [details] [review]: Looks fine
Review of attachment 295126 [details] [review]: +1
Pushed those to master: https://git.gnome.org/browse/gnome-music/commit/?id=34f1362 https://git.gnome.org/browse/gnome-music/commit/?id=95f81e4 https://git.gnome.org/browse/gnome-music/commit/?id=267299c Keeping bug open for patch from comment #2
Created attachment 295307 [details] [review] update all static playlists whenever media added to library (revision of earlier patch -- this one now updates static playlists from Playlists container (view.Playlists._on_changes_pending) rather than from ViewContainer.)
(In reply to comment #6) Okay, revised that patch so that playlist updating is called from the Playlists container rather than the ViewContainer superclass. See comment #10. > Review of attachment 295124 [details] [review]: > > ::: gnomemusic/view.py > @@ +141,3 @@ > @log > def _on_changes_pending(self, data=None): > + playlists.update_all_static_playlists() > > Since its a base class for others (Albums, Views etc.) we should make sure base > function is also called from classes inheriting it: > > ViewContainer._on_changes_pending(self, None) > > > However I'm not sure if its worth to update playlists from a base class - this > clearly belongs to Playlists class in view.py - this class is also loaded while > another view is active.
(In reply to Maia from comment #10) > Created attachment 295307 [details] [review] [review] > update all static playlists whenever media added to library Pushed it as https://git.gnome.org/browse/gnome-music/commit/?id=1cdaa3a, thanks