Rockbox

This is the bug/patch tracker for Rockbox. Click here for more information.

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#12469 - add a few playlist wrappers for Lua

Attached to Project: Rockbox
Opened by Albert Bloomfield (abloomfield) - Tuesday, 20 December 2011, 15:43 GMT+2
Last edited by Maurus Cuelenaere (mcuelenaere) - Friday, 17 February 2012, 12:07 GMT+2
Task Type Patches
Category Plugins
Status Closed
Assigned To No-one
Player Type All players
Severity Low
Priority Normal
Reported Version Daily build (which?)
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Private No

Details

adds wrappers for playlist_sync, playlist_remove_all_tracks, and playlist_insert_track, playlist_insert_directory

also constants for:
PLAYLIST_PREPEND,
PLAYLIST_INSERT,
PLAYLIST_INSERT_LAST,
PLAYLIST_INSERT_FIRST,
PLAYLIST_INSERT_SHUFFLED,
PLAYLIST_REPLACE, and
PLAYLIST_INSERT_LAST_SHUFFLED

I made all the functions take the same parameters, but right now it just ignores any struct playlist_info* parameter and just passes NULL to work with the current playlist.
   lua_playlist_wrappers.diff (2.4 KiB) 
 apps/plugins/lua/rocklib.c |   65 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

This task depends upon

Closed by  Maurus Cuelenaere (mcuelenaere)
Friday, 17 February 2012, 12:07 GMT+2
Reason for closing:  Fixed
Additional comments about closing:  Merged.
Comment by Thomas Martitz (kugel.) - Wednesday, 21 December 2011, 08:13 GMT+2
What about the rest of the playlist functions?
Comment by Albert Bloomfield (abloomfield) - Wednesday, 21 December 2011, 14:43 GMT+2
I thought all the other ones were already done automatically. When I rant the example lua to create a rb.txt with all the members of rb, these were the only ones I noticed that were missing.
Comment by Thomas Martitz (kugel.) - Wednesday, 21 December 2011, 14:44 GMT+2
Ah that's probably true. Sorry
Comment by Maurus Cuelenaere (mcuelenaere) - Wednesday, 21 December 2011, 19:20 GMT+2
Perhaps instead of ignoring the first parameter in playlist_insert_{track,directory}, why not just skip it and shift all parameters one to the left? e.g. rb.playlist_insert_track("bla", rb.PLAYLIST_INSERT, false, false)

Also, I'm not sure the luaL_optint() will work as it's not the last parameter/it's followed by non-optional parameters.

Besides these, the patch looks OK.
Comment by Albert Bloomfield (abloomfield) - Wednesday, 21 December 2011, 19:41 GMT+2
Yeah, I can certainly make that change to shift the parameters. Is there any place that it should be documented?

Regarding the luaL_optint(), my idea was that passing in nil would be the same as passing in PLAYLIST_INSERT. But I guess that doesn't really make sense and I didn't do that on playlist_insert_directory(). So I guess I should just change those to luaL_checknumber() then?
Comment by Albert Bloomfield (abloomfield) - Wednesday, 21 December 2011, 19:50 GMT+2
I mean luaL_checkint()
Comment by Albert Bloomfield (abloomfield) - Wednesday, 21 December 2011, 20:01 GMT+2
Although now I look at it, maybe the boolean parameters should be optional? in playlist_insert_track() queue could default to false and sync could default to true (or should it be false?) and in playlist_insert_directory() queue and recurse would both default to false. Right now it seems like the booleans will always default to false if that argument is missing.
Comment by Maurus Cuelenaere (mcuelenaere) - Wednesday, 21 December 2011, 23:15 GMT+2
RE documentation, the only place I can think of, is rocklib.c :/
Currently, Lua is underdocumented in Rockbox (most of the plugin API is too).

Having all the boolean parameters as optional sounds good. Later on, if people are interested, one could port struct playlist_info over to Lua.
Comment by Albert Bloomfield (abloomfield) - Thursday, 22 December 2011, 18:22 GMT+2
OK, here's a new version that changes the playlist_insert_{track,directory} to only have one required argument. The boolean arguments default to false and the position argument defaults to PLAYLIST_INSERT.
   lua_playlist_wrappers.diff (2.6 KiB) 
 apps/plugins/lua/rocklib.c |   65 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

Comment by Albert Bloomfield (abloomfield) - Wednesday, 15 February 2012, 22:27 GMT+2
is there anything more I should do to get this accepted?
Comment by Maurus Cuelenaere (mcuelenaere) - Wednesday, 15 February 2012, 22:28 GMT+2
Now that we have moved to gerrit (gerrit.rockbox.org) for patches, you should probably submit it there and remove this one.
Comment by Albert Bloomfield (abloomfield) - Thursday, 16 February 2012, 14:47 GMT+2
I put this on gerrit now, so this one can be removed.

Loading...