Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Patches
  • Category Plugins
  • Assigned To No-one
  • Operating System All players
  • Severity Low
  • Priority Very Low
  • Reported Version Daily build (which?)
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by abloomfield - 2011-12-20
Last edited by mcuelenaere - 2012-02-17

FS#12469 - add a few playlist wrappers for Lua

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.

Closed by  mcuelenaere
2012-02-17 11:07
Reason for closing:  Fixed
Additional comments about closing:   Warning: Undefined array key "typography" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 371 Warning: Undefined array key "camelcase" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 407

Merged.

What about the rest of the playlist functions?

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.

Ah that's probably true. Sorry

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.

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?

I mean luaL_checkint()

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.

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.

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.

is there anything more I should do to get this accepted?

Now that we have moved to gerrit (gerrit.rockbox.org) for patches, you should probably submit it there and remove this one.

I put this on gerrit now, so this one can be removed.

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing