Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JSONRPC.NotifyAll as a built-in function #3227

Closed
wants to merge 3 commits into from
Closed

JSONRPC.NotifyAll as a built-in function #3227

wants to merge 3 commits into from

Conversation

spudwebb
Copy link
Contributor

As explained here http://forum.xbmc.org/showthread.php?tid=172918, this is a simple addition to be able to call JSONRPC.NotifyAll from a skin without any python script.

@Montellese
Copy link
Member

First of all you need to get rid of the merge commit by rebasing your work instead of merging it.
Then I'd remove the reference to JSON-RPC from the command description as it will also notify xbmc core logic.
Last but not least you could also support the 4-parameter version of CAnnouncementManger::Announce by trying to interpret the (optional) third string parameter to the built-in method as JSON data and decoding it into a CVariant using the CJSONVariantParser from xbmc/utils/JSONVariantParser.h/cpp.

- support the 4-parameter version of CAnnouncementManger::Announce
@spudwebb
Copy link
Contributor Author

sorry I'm new to git and github
I did the changes you asked, and then issued a new pull request, so I'm closing this one.

@spudwebb spudwebb closed this Sep 10, 2013
@topfs2
Copy link
Contributor

topfs2 commented Sep 10, 2013

Hehe. Everyone is new once. Next time just force push to the same branch.
The pull request is updated automatically.

On the topic of the feature I like it a lot!
Den 10 sep 2013 21:05 skrev "spudwebb" notifications@github.com:

sorry I'm new to git and github
I did the changes you asked, and then issued a new pull request, so I'm
closing this one.


Reply to this email directly or view it on GitHubhttps://github.com//pull/3227#issuecomment-24185920
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants