It's great to see that you had some use for the Dynamic Playlist plugin
when doing this. 

I had a quick look at the code and here are some bugs/improvements you
might want to consider:


1. 
In the end of getNextDynamicPlayListTracks you have this code:

Code:
--------------------
    splice(@tracks, $offset, $limit);
--------------------

If you instead change that to this, I think it will work correctly:

Code:
--------------------
    @tracks = splice(@tracks, $offset, $limit);
--------------------

As the code looks now I think the result will be that it will return
all tracks besides those that you actually want to return. I think you
can see the problem if you just mark a single artist which have less
than 10 songs in total. In this situation the playlist will never be
started when one tries to play it.



2.
In the begining of the getNextDynamicPlayListTrack you have the
following code:

Code:
--------------------
    
  my $thisPlaylistID = $playlist->{'dynamicplaylistid'};
  
--------------------

Even though this actually work, it could break if I change the
internals in Dynamic Playlist plugin. I would suggest that you instead
do this:

Code:
--------------------
    
  my $thisPlaylistID = $playlist->{'id'};
  
--------------------

But to make that work you will also have to do a change in the
getDynamicPlayLists function, so instead of this:

Code:
--------------------
    
                my %playlist = (
                        'name' => $playlistName,
                        'url' => 
'plugins/ArtistPlaylist/artistplaylist.html?playlistID=' . $playlistID,
                );
  
--------------------

You should do this:

Code:
--------------------
    
                my %playlist = (
                        'id' => $playlistID,
                        'name' => $playlistName,
                        'url' => 
'plugins/ArtistPlaylist/artistplaylist.html?playlistID=' . $playlistID,
                );
  
--------------------

The reason this is better is because this way you are using your own
keys in the hash received in $playlist. The hash you return from the
getDynamicPlayLists function is the same as the one you receive in the
getNextDynamicPlayListTracks. The 'dynamicplaylistid' exists there just
because the Dynamic Playlist adds this key to the hash, but this might
change in the future so I would suggest that you change the code as
describes above.



3.
In the getDynamicPlayList function you have this code:

Code:
--------------------
    
  $result{$playlistID} = \%playlist;
  
--------------------

I would suggest that you change that to this:

Code:
--------------------
    
  $result{'artistplaylist_'.$playlistID} = \%playlist;
  
--------------------

The reason that this is better is because this key will actually be the
id used when starting to play the playlist. At the moment your key is
for example '1' and there is a risk that this will collide with other
playlists in Dynamic Playlist plugin. If you change the code like
described above, the id will instead be 'artistplaylist_1' which should
minimize the risk of a collision.



4.
If you want to get rid of the "Artist Playlists" entry in the
SqueezeBox menu under "Plugins", you can just remove the setMode
function.



5.
I think you really should consider to store the artist names in the
settings instead of the artist id. People will probably not like the
fact that they have to remake their playlists after each full rescan.
If you store the artist name instead of the id it should work good
enough with rescans.


-- 
erland

Erland Isaksson
'My homepage' (http://erland.homeip.net) 'My download page'
(http://erland.homeip.net/download)
(Developer of 'TrackStat, SQLPlayList, DynamicPlayList, Custom Browse,
Custom Scan,  Custom Skip, Multi Library and RandomPlayList plugins'
(http://wiki.erland.homeip.net/index.php/Category:SlimServer))
------------------------------------------------------------------------
erland's Profile: http://forums.slimdevices.com/member.php?userid=3124
View this thread: http://forums.slimdevices.com/showthread.php?t=37867

_______________________________________________
plugins mailing list
[email protected]
http://lists.slimdevices.com/lists/listinfo/plugins

Reply via email to