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

[PVR] Fix empty videoplayer.plot infolabel when playing recorded TV and add … #7742

Closed
wants to merge 6 commits into from
Closed

[PVR] Fix empty videoplayer.plot infolabel when playing recorded TV and add … #7742

wants to merge 6 commits into from

Conversation

joethepartylion
Copy link
Contributor

…new videoplayer.recordeddate label.

This something that has been asked about on the forums (http://forum.kodi.tv/archive/index.php?thread-219946.html) and by me so I have tried to see if I could code it myself. I have no prior C++ experience so it may be a load of codswallop but here goes (hopefully I've done it right this time not like #7737)

@joethepartylion joethepartylion changed the title Populate videoplayer.plot infolabel when playing recorded TV and add … Fix empty videoplayer.plot infolabel when playing recorded TV and add … Sep 1, 2015
@mkortstiege
Copy link
Member

@xhaggi mind checking?

@xhaggi
Copy link
Member

xhaggi commented Sep 2, 2015

makes sense but we need to take care of other pvr related info like genre etc.

@xhaggi
Copy link
Member

xhaggi commented Sep 2, 2015

I did only a quick look at this. I'm off for vacation until 14th of September. @ksoo mind taking a look too?

@joethepartylion
Copy link
Contributor Author

I'm happy to update the commit with the additional fields if someone can point me to what fields are available from GetPVRRecordingInfoTag as I don't know how to work this out myself. I just took those used in the mypvrrecordings.xml window.

@ksooo
Copy link
Member

ksooo commented Sep 14, 2015

May I suggest not to mix up different things in one PR. The original request in the forum was to fix a bug with "Videoplayer.Plot". The other stuff in this PR is new functionality. I'd like to see two different PRs here - one for the fix, one for the functional enhancements.

Rational: The fix for "VideoPlayer.Plot" is already implemented right imo, straight forward and can be merged in almost no time. The enhancements need a bit more 'love'. I'm not sure why you picked just some of the CPVRRecording data fields and not all, for instance. Escpecially I would be glad to see a concrete PR that actually uses the enhancemants. Otherwise I fear to extend guiinfo more and more with stuff that nobody uses at the end.

@ksooo ksooo changed the title Fix empty videoplayer.plot infolabel when playing recorded TV and add … [PVR] Fix empty videoplayer.plot infolabel when playing recorded TV and add … Sep 14, 2015
@joethepartylion
Copy link
Contributor Author

Yes I thought that after i submitted it, sorry. @ksooo, I can submit just the "VideoPlayer.Plot" separately, Do you want me to remove all the other data fields i.e. plotoutline, title etc or just the new recordeddate one?

The reason I haven't covered all of the data fields is that I don't know where to find what is available as I don't have a lot of programming experience, I was just copying the ones referenced in the PVRRecording window list to get started. If you can give me any points in the right direction then i will have a go at it.

@ksooo
Copy link
Member

ksooo commented Sep 14, 2015

I can submit just the "VideoPlayer.Plot" separately,

Cool.

Do you want me to remove all the other data fields i.e. plotoutline, title etc or just the new recordeddate one?

Yes, please leave just the fix for VideoPlayer.Plot in this PR, create a new PR for all the rest (the functional enhancements), please.

@joethepartylion
Copy link
Contributor Author

Hi @ksooo, I've updated the commits but can't squash them until i get home, once I've done that I will let you know.

@joethepartylion
Copy link
Contributor Author

OK, looks like I've completely messed up the commits and not sure how to get it back, will close this pull request and try again. 3rd time luck, at least I'm understanding git a bit more now :-)

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

Successfully merging this pull request may close these issues.

None yet

5 participants