Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

JSON-RPC: cleanup #1174

Merged
merged 12 commits into from Aug 6, 2012

Conversation

Projects
None yet
2 participants
Owner

Montellese commented Jul 17, 2012

These commits are basically cleanup commits which remove hacks that are in the code to be backwards compatible with Eden. There are more and more of these hacks and some stuff (see e.g. #1161) just can't be done without breaking backwards compatibility. So let's make the break and bring all the benefits with it.

b229237 changes the type of some properties provided by xbmc from a concatenated string to an array of strings (e.g. "Rock / Pop" becomes [ "Rock", "Pop" ]).

174c560 properly integrates tagging support into JSON-RPC (not really cleanup).

2fe12ff removes two properties from movies which were never meant to be there.

All those "filter" related commits move the filtering-parameters like "genreid" etc in the FooLibrary.GetFoo() methods into a "filter" parameter. The main reason behind this is that the database logic is not able to apply multiple of those filters (e.g. "genreid" and "actorid") at the same time but nothing prevents a JSON-RPC client from specifying both. With the new "filter" parameter we can restrict JSON-RPC clients to only be able to provide one of those filters per request. Furthermore I have added additional filters like "genre" (in addition to "genreid") and "actor", "studio", "director", "year" etc where possible. So this will allow JSON-RPC clients to provide users with a similar GUI hierarchy as xbmc provides. Furthermore this provides a better solution to #1117 which is currently broken in master.

Member

jmarshallnz commented Jul 17, 2012

Looks good. Possibility to do CVariant -> CVideoDbUrl in the future do you think. It would be nice to have a unified way of handling the filter structure (we currently have at least 3 such things).

@ghost ghost assigned Montellese Jul 18, 2012

Owner

Montellese commented Jul 18, 2012

What do you mean with CVariant -> CVideoDbUrl? That "genreid" is automatically stored in the CVideoDbUrl? Well in this PR there's no CVideoDbUrl yet because that will be introduced with another PR. We'll probably have to discuss where/how we want to handle injecting those options into CVideoDbUrl. Doing in CVideoDatabase::GetFooNav() has the advantage that the rest of the code doesn't have to know how the filters have to be named.

Montellese added a commit that referenced this pull request Aug 6, 2012

Merge pull request #1174 from Montellese/jsonrpc_cleanup
json-rpc: cleanup of hacks and addition of new "filter" parameter to FooLibrary.GetFoos

@Montellese Montellese merged commit 28a085f into xbmc:master Aug 6, 2012

tru added a commit to plexinc/plex-home-theater-public that referenced this pull request May 19, 2014

tru added a commit to plexinc/plex-home-theater-public that referenced this pull request May 19, 2014

Refactor PQM a bit to handle resume offsets.
Turns out that sending resume offsets via the playMedia command and
“everything is a play queue” stuff is was a pretty deep rabbit hole.

I had to make the PlayQueue methods take a new option to allow
prompting, this lead to the introduction of the PlexPlayQueueOptions
class instead of just adding more options to the commands.

Fixes #1174

tru added a commit to RasPlex/plex-home-theatre that referenced this pull request Aug 21, 2014

tru added a commit to RasPlex/plex-home-theatre that referenced this pull request Aug 21, 2014

Refactor PQM a bit to handle resume offsets.
Turns out that sending resume offsets via the playMedia command and
“everything is a play queue” stuff is was a pretty deep rabbit hole.

I had to make the PlayQueue methods take a new option to allow
prompting, this lead to the introduction of the PlexPlayQueueOptions
class instead of just adding more options to the commands.

Fixes #1174
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment