phil65 / script.extendedinfo

script.extendedinfo
http://forum.xbmc.org/showthread.php?tid=160558
GNU General Public License v2.0
60 stars 79 forks source link

TV-Ratings and browse local TV Shows #17

Closed Nessus85100 closed 9 years ago

Nessus85100 commented 9 years ago
phil65 commented 9 years ago

When clicking on a TV Show from trakt.tv or TMDB, and that show is in local DB, browse it instead of opening the video info dialog.

should work now.

Nessus85100 commented 9 years ago

Nope. I've try with TMDB (Popular Shows) and Trakt (Trending Shows). Still opens script's extended videoinfo dialog.

Let me know if you need a full debug log.

Thanks Nessus

phil65 commented 9 years ago

Online name of TV show exactly (!!!) the same as local tvshow name?

Nessus85100 commented 9 years ago

Yes, "Game of Thrones" and "Flash" for example.

phil65 commented 9 years ago

then debug log, please. You adjusted script settings to not show infodialog by default, right?

Nessus85100 commented 9 years ago

No, i didn't touch addon's settings. I thought will work like the movies.

I mean... Movies: If the item is in local library then play it, if not open the infodialog TVShows: If the item is in local library then browse it, if not open the infodialog

IMHO that's the proper default state.

phil65 commented 9 years ago

that's exactly the way it works atm except that you have to change the default behaviour to not open the info dialog by default in settings.

Nessus85100 commented 9 years ago

No it's not. If you change the default behavior to not open the info dialog in settings, then in Movies it plays the Trailer if the item is not in local library. IMHO the default state/behaviour must be like the one i describe above. Without any user interaction or settings change. After all, currently, it works like that in Movies. Why in TV Shows must be different ?

phil65 commented 9 years ago

Again: It is not different. depending on the setting I mentioned, for movies it plays the trailer (I will probably change that to play the whole movie if available) and for TV shows it goes to the corresponding library node if tvshow is available locally (thats what my comment was about). Havin "open info dialog" as default instead of the behaviour just described is a reasonable decision based on the fact that there is no other way to get to the info dialog. Pretty much everyone on forums agreed on this.

Nessus85100 commented 9 years ago

I didn't say that the default state should be the browse option in TV Shows. I am just saying that the browse option should be the default behavior IF THE SHOW is in local library. Just like the Movies.

"Havin "open info dialog" as default instead of the behaviour just described is a reasonable decision based on the fact that there is no other way to get to the info dialog."

Currently in Movies, the default state is that you don't get the infodialog at all for the items that are in local library. So, where is the problem if the same happens for the TV Shows ?

Sorry that i insist on this but if you decide to leave it like this, can you please tell me a way to add a "Browse" button inside the infodialog to be visible only when the Show is in local library ?. Something like Ronie has in script-globalsearch-infodialog.xml for TV Shows.

phil65 commented 9 years ago

with default settings, the default action should always be opening the infodialog, no matter if it is for movies, tvshows or whatever. if that is not the case atm I would consider that a bug. If you decide to change the corresponding script setting, then default behaviour should be either "play movie/trailer" for movies or "browse" for tvshows if the media is available locally, and "open infoscreen" if not available. The browse button already gets hidden in case there is no local tvshow available.

Nessus85100 commented 9 years ago

Ok, fair enough. I haven't saw that button in the dialogs code (Play for Movies and Browse for Shows). Can you tell me the ID's or point in the code lines here please ?

By the way the default behavior in Movies is like that. So i guess according to your intention... is a bug.

Thanks and sorry for all this. Nessus

phil65 commented 9 years ago

https://github.com/phil65/script.extendedinfo/commit/e91a59d498744e2170f337724992a724f833d21b contains the new "browse" button. the button to play movies is right above, id 8.

phil65 commented 9 years ago

By the way the default behavior in Movies is like that. So i guess according to your intention... is a bug.

I still dont see where it doesnt behave correctly after a quick test. If behaviour is not explained as above, please give detailed steps how to reproduce.

phil65 commented 9 years ago

Also, fyi: http://forum.kodi.tv/showthread.php?tid=160558&pid=2029844#pid2029844 I think you asked for that behaviour some time ago.

Nessus85100 commented 9 years ago

Also, fyi: http://forum.kodi.tv/showthread.php?tid=160558&pid=2029844#pid2029844 I think you asked for that behaviour some time ago.

Yes, i remember. That was before i decide to fully support the addon's infodialogs.

I still dont see where it doesnt behave correctly after a quick test. If behaviour is not explained as above, please give detailed steps how to reproduce.

For some weird reason sometimes the addon settings changes are not apply until you reload the skin. Also i notice that when you change an addon setting the list's are different after you reload the skin. I have no idea if that happens only in my setup but it started a couple weeks a go. I have the latest commit installed except of your last new "browse" button.

You can reproduce this like this...

Here is the debug log of the above procedure... http://xbmclogs.com/ppaeand5x

I hope this helps you find the problem.

Thanks Nessus

EDIT: Is it possible to add the (infodialogs.active) property to the script-ExtendedInfo Script-VideoList.xml window ?

phil65 commented 9 years ago

Do you use the plugin:// method or the window property method?

Nessus85100 commented 9 years ago

The plugin:// method

phil65 commented 9 years ago

ok, thx. and what do you mean with "the lists are little different" ?

Nessus85100 commented 9 years ago

I think they might be the same but the items are in different order. For example in "Popular Movies" the "Tomorrowland" is first but after the settings change and the skin reload, the "Tomorrowland" is 3th or 4th.

phil65 commented 9 years ago

There should be no need to reload the skin, but you have to make sure that the list containing the items gets populated again (= the script has to execute again). Needs reworkin some parts to get that workin without re-populating list, will see when / if I decide to tackle that. Concerning different order: please try https://github.com/phil65/script.extendedinfo/commit/931e0c75f72d1c9da867b640f0502ba2d16c9aaa

Nessus85100 commented 9 years ago

I use skin.shortcuts for select and set the widgets. Is there any other way to re-populate the lists ?

Besides of the extended info widgets i also use library data provider which re-populates the lists with a "reload" parameter without the need of run the script again. Is that possible here ?

phil65 commented 9 years ago

library data provider also always executes the script again, it only manages things differently in the background by usin a separate script thread to update data (it´s a much worse approach than the one in ExtendedInfo if you ask me). I would have to solve that differently, but that´s out of scope for 3.0 I think. Could you check the ordering issue?

Nessus85100 commented 9 years ago

Ok. Thanks for the info. Ordering is working fine after the latest commit.

Moreover did you had a look of adding the (infodialogs.active) property to the script-ExtendedInfo Script-VideoList.xml dialog and the missing TV-Ratings from TV Shows (the second request of this thread).?

Thanks for all Nessus

phil65 commented 9 years ago

What should that be useful for? Semantically it doesnt make too much sense since it is not really an infodialog. With ratings you mean certification? That´s not part of the TheMovieDB API it seems. http://docs.themoviedb.apiary.io/#reference/tv/tvid/get

Nessus85100 commented 9 years ago

It will be useful when you open the infodialogs from inside that VideoList. I don't mind the naming just need the property so the visible/hidden to work properly. Just like they do with the infodialogs.

With ratings you mean certification? That´s not part of the TheMovieDB API it seems. http://docs.themoviedb.apiary.io/#reference/tv/tvid/get

That sucks !. I don't get it !. The info is there... https://www.themoviedb.org/tv/1399-game-of-thrones/edit?active_nav_item=content_ratings. Why is not included in their API ???!!!

phil65 commented 9 years ago

hehe thx for the hint. Seems as if I overlooked something. It is there indeed, but naming is different from movies, that´s why I was confused.

phil65 commented 9 years ago

https://github.com/phil65/script.extendedinfo/commit/1219c900fa7fcb69dd2213759c5b6e1190a806ec Window.Property(mpaa) should do the trick.

phil65 commented 9 years ago

...here the other one: https://github.com/phil65/script.extendedinfo/commit/3a9f19b7c2d5cf588e7bdb471fcd7d96b82dc603

Nessus85100 commented 9 years ago

Cool !... although the property is empty and i think it cause some other issues.

Let me know if you need a debug log.

The (infodialogs.active) property is working fine.

Thanks Nessus

phil65 commented 9 years ago

Window.Property(movie.number_of_seasons) --> Window.Property(movie.TotalSeasons) Window.Property(movie.number_of_episodes) --> Window.Property(movie.TotalEpisodes) genre should be fixed now, too.

phil65 commented 9 years ago

...and before I meant movie.mpaa of course.

Nessus85100 commented 9 years ago

The seasons/episodes totals and genres are working fine. The Window.Property(movie.Writer) property for both local and non-local Movies is still empty.

..and before I meant movie.mpaa of course.

Yes, i know. I find out why the movie.mpaa was empty. I takes the first certification in the list which usually is not the US rating. Can we "catch" only the US rating because it will be difficult to have a set of flags for every country ?.

phil65 commented 9 years ago

writer/director prop should be fixed with https://github.com/phil65/script.extendedinfo/commit/3264248142edeb6077f4089b3bb701340750cd42

Nessus85100 commented 9 years ago

Yes, it's fixed now. What about the US ratings thing ?

phil65 commented 9 years ago

https://github.com/phil65/script.extendedinfo/commit/564f06bdb0838b969425ef24b78aa6205961e5bc will probably change it to depend on a script setting somewhen though.

Nessus85100 commented 9 years ago

I understand that, but the default state it will be the US Ratings... right ?

phil65 commented 9 years ago

Probably, I have no idea yet. will figure something out when I´m in the mood. Perhaps I will make the default certification property based on language setting + an alternate property for us cert, so it depends on skin what to show. This can get closed now, right?

Nessus85100 commented 9 years ago

Yes, thank again for all.

Cheers Nessus