Navigation Menu

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

Enable editing of userrating from the frontend #8049

Merged
merged 3 commits into from Oct 16, 2015

Conversation

razzeee
Copy link
Member

@razzeee razzeee commented Sep 14, 2015

This adds the logic to increase and decrease userrating from the videoinfo. The videoinfo dialog can now have an up and down control and editing this will edit the userrating. It will also refresh the listing, if the userrating got changed.

Still needs some more testing, as I've only tested movies so far.

I have added a modified DialogVideoInfo.xml that needs to be removed before this can be merged.
Maybe one of our skinners can do a real replacement for it, otherwise I would like it to go in without that.

@razzeee razzeee added the WIP PR that is still being worked on label Sep 14, 2015
@razzeee
Copy link
Member Author

razzeee commented Sep 14, 2015

Maybe @phil65 @vonH @Hitcher?

<onclick>DecreaseRating</onclick>
<texturenofocus>scroll-down-2.png</texturenofocus>
<texturefocus>scroll-down-focus-2.png</texturefocus>
<onleft>8</onleft>

This comment was marked as spam.

@phil65
Copy link
Contributor

phil65 commented Sep 14, 2015

Only looked briefly through code, can runtime-test next days if needed.

@razzeee
Copy link
Member Author

razzeee commented Sep 15, 2015

@phil65
I was more thining along the lines of, can somebody with real skinning skills check this out, revert my skin changes and do it right? The current one is just for testing purpose and has to be replaced. You will know when you see it in kodi ;)

@phil65
Copy link
Contributor

phil65 commented Sep 15, 2015

Hmm what do you think about changing it to a builtin which opens dialogselect to select desired rating ("set rating") instead of adding 2 builtins for increasing/decreasing? That way it would fit much better into the typical settings lists in infodialogs.

@Hitcher
Copy link
Contributor

Hitcher commented Sep 15, 2015

And change song info rating at the same time ;)

@phil65
Copy link
Contributor

phil65 commented Sep 15, 2015

yup, that would have been my next request after this is done :)

@razzeee
Copy link
Member Author

razzeee commented Sep 15, 2015

So a button with a new pop up essentially?

@zag2me
Copy link
Contributor

zag2me commented Sep 15, 2015

Just wanted to say please make this as easy and quick as possible to update the rating. The latest Aeon 5 nox does it very nicely for music ratings, it might be worth taking a look at that.

@razzeee
Copy link
Member Author

razzeee commented Sep 15, 2015

@zag2me
Aeon Nox is just using two buildins as in confluence and has them focused directly?

@phil65
Copy link
Contributor

phil65 commented Sep 15, 2015

No idea how Nox does it, in general I hate mapping actual "actions" to navigation keys (like onup/ondown). In ExtendedInfo script i use DialogSelect to set rating and that works very well and is easy to integrate into any menu.

@zag2me
Copy link
Contributor

zag2me commented Sep 15, 2015

Heres a video of the Nox process

https://youtu.be/F8PFHMxMZIg

@phil65
Copy link
Contributor

phil65 commented Sep 15, 2015

...well that's the "classic" way with two builtins. While this might still work well with music which has 5 rating values it becomes cumbersome for movies with 10 rating values.
Perhaps allowing both? Keepin Increase/decreaserating + also adding SetRating()?

@zag2me
Copy link
Contributor

zag2me commented Sep 15, 2015

Indeed, that's why I prefer votes out of 5 for user ratings. Much quicker to set. I think we had that discussion already though.

@razzeee
Copy link
Member Author

razzeee commented Sep 15, 2015

I'm not a skinner but I think the way the video info window is done, it might be very hard to add the two up/down buttons, without rewriting it.

@phil65
Copy link
Contributor

phil65 commented Sep 15, 2015

yep, it doesnt fit into most layouts, that's why I would prefer to have one "Set rating" button which opens dialogselect.

@MartijnKaijser
Copy link
Member

we should go with same rating system for music/video. No way that we should do 5 stars for music and 10 for videos.

@phil65
Copy link
Contributor

phil65 commented Sep 15, 2015

"No way" comes a bit too late since everything is already merged. ;)

@MartijnKaijser
Copy link
Member

@phil65 i am responding to @zag2me his remark about 5star.

edit:
and if it's not the same it must be changed. this stupid split of handling it in different ways has to stop

@phil65
Copy link
Contributor

phil65 commented Sep 15, 2015

I still dont understand. You suggest to also change song rating to a 10-step system?

@MartijnKaijser
Copy link
Member

yes

edit:
not suggest, i insist

@phil65
Copy link
Contributor

phil65 commented Sep 15, 2015

Ah ok. Up to the music guys, I never used song ratings up to now.

@MartijnKaijser
Copy link
Member

if video uses a 10 step system for personal rating then so should music.

@razzeee
Copy link
Member Author

razzeee commented Sep 15, 2015

will get on that as soon as we have this sorted. I also want to change the datatype for music ratings, as they are chars right now, which is pretty stupid to me.

@Hitcher
Copy link
Contributor

Hitcher commented Sep 15, 2015

@MartijnKaijser All albums are currently rated 0-5 though.

@razzeee
Copy link
Member Author

razzeee commented Oct 11, 2015

Yes, we should change each appearance to userrating.

Two types of ratings for music may also be nice. But as far as I know there is no source for those, yet. @zag2me will know. (but it's out of scope for this pr)

I like your ideas for better names, but it's a shame that it comes up after working on it for two month. Anyway I will change everything to "Personal rating" as soon as possible.

@un1versal
Copy link
Contributor

music rating (songs/track) is already userratting (only, theres no external rating from online databases being pulled or at least displayed), you cant rate albums, and you cant rate songs in the view screenshot in #8049 (comment) shows. Only via pressing (i) when song is playing using (im sure you know) http://kodi.wiki/view/Action_IDs IncreaseRating and DecreaseRating (would be nice if video userrating responded to those action IDs though no matter how you do it.)

You have quite a challenge here to make video/music rating consistent good luck.

Music rating is outside the scope of this PR though

@razzeee
Copy link
Member Author

razzeee commented Oct 11, 2015

You can rate via that button in the screenshot.

Increase and decrease rating should already work for video.

@ronie
Copy link
Member

ronie commented Oct 13, 2015

thank you for your code contribution on behalf of the entire team.

i've reviewed the skin related changes and there's a few things that could be improved:

song info dialog:

  • please center the buttons at the bottom of the dialog

video info dialog:

  • you forgot to add the userrating label to the list for musicvideos
  • position and size of the lists need to be adjusted, the number of available items now exceeds the height of the lists (user will have to scroll to see all available info).

@razzeee
Copy link
Member Author

razzeee commented Oct 14, 2015

Dear ronie,

I think I'm not skinner enough for that. I might be able to figure the center stuff out, but that will take some time.
As you may have seen by now I added the userrating label for musicvideos.
The third change is probably also to much of a skinning thing for me. Maybe we just delete the userrating from that list, as it is sortable and viewable via that?

@ronie
Copy link
Member

ronie commented Oct 14, 2015

no problem, here's the needed changes: ronie@f73353d

@razzeee
Copy link
Member Author

razzeee commented Oct 15, 2015

Thanks ronie!
I'll squash and merge this tomorrow, if nobody speaks up against this.

@razzeee
Copy link
Member Author

razzeee commented Oct 15, 2015

jenkins build this please

@razzeee
Copy link
Member Author

razzeee commented Oct 15, 2015

Build errors are unrelated

@razzeee
Copy link
Member Author

razzeee commented Oct 16, 2015

jenkins build and merge

@jenkins4kodi jenkins4kodi merged commit a0e11f1 into xbmc:master Oct 16, 2015
@razzeee razzeee deleted the userrating-frontend branch October 16, 2015 10:51
@un1versal
Copy link
Contributor

@razzeee do you plan on making the My rating available in the various list views of skin?

I had some TV shows with no rating scraped at time of scanning, so after setting My rating for them and was slightly disappointed that the rating doesn't show up on that the list views of skin and that there was still no entry there.

@razzeee
Copy link
Member Author

razzeee commented Oct 18, 2015

Well if you order by my rating it should show up. Otherwise I may not understand what you're saying.

@un1versal
Copy link
Contributor

Ah, I dindt realize thers a special rating sorting, feel odd having to activate a view to see 3 or 4 ratings and then dont show any rating for the items that have scraped rating...

Problem is I dont want to rate 1000 items or more especially to make this view useful.
Any existing libraries this a job for the ages.

I thought this was going to integrate in a different manner, never mind, Ill keep using WIMM.

@razzeee
Copy link
Member Author

razzeee commented Oct 18, 2015

Addons will be able to give you a good start. I will add this to trakt soon.

@un1versal
Copy link
Contributor

Trakt is not an option here nor will it ever be.

Thats fine with WIMM Im able to add rating that integrates perfectly with stock Confluence in those times where simply need a fast and simple manner to rate the odd episode or movie that doesnt have scrapable rating at the scan time, without having to fiddle about too much and or having to sign into tvdb or tmdb to add a rating that I can scrape later.

I want to spend the little time I have enjoying Kodi, not spend it rating everything we have.

@razzeee
Copy link
Member Author

razzeee commented Oct 18, 2015

Not sure what WIMM is, but it sounds like it may write nfos? If thats the case, userratings should also work from nfos.

@un1versal
Copy link
Contributor

No nfos and no user rating http://forum.kodi.tv/showthread.php?tid=188839 simple and works like I need it to. In this cases I can just add a rating and save, done, as if it was scraped from online databases.

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