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

[filesystem] add videodb://inprogresstvshows #8261

Merged
merged 2 commits into from Jan 10, 2016
Merged

[filesystem] add videodb://inprogresstvshows #8261

merged 2 commits into from Jan 10, 2016

Conversation

InuSasha
Copy link
Contributor

add the videodb://inprogresstvshows to the filesystem-tree.

@un1versal: after merge you may make a second try on #4469

@un1versal
Copy link
Contributor

Thanks!! this is fantastic, will test asap.

related forum request thread http://forum.kodi.tv/showthread.php?tid=233374

@un1versal: after merge you may make a second try on #4469

What do you mean?

This will need skin implementation like in #4469 no?

@ronie
Copy link
Member

ronie commented Oct 19, 2015

+1 from my end

never understood there's a videodb:// path for everything except inprogress tv shows

@@ -0,0 +1,56 @@
/*
* Copyright (C) 2005-2013 Team XBMC

This comment was marked as spam.

@InuSasha
Copy link
Contributor Author

@razzeee done, but i have found 66 different kinds of this header!!!!

* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with XBMC; see the file COPYING. If not, see

This comment was marked as spam.

@InuSasha
Copy link
Contributor Author

@razzeee changed, too

un1versal added a commit to un1versal/kodi that referenced this pull request Oct 20, 2015
@InuSasha
Copy link
Contributor Author

added the json-rpc method

@MartijnKaijser
Copy link
Member

Needs jsonrpc version bump. @Montellese ping

@InuSasha
Copy link
Contributor Author

have something todo for the version bump?

@hudokkow
Copy link
Member

See https://github.com/xbmc/xbmc/pull/7788/commits for an example of jsonrpc bump

@un1versal
Copy link
Contributor

@InuSasha
Copy link
Contributor Author

bumped to 6.30.1

@un1versal
Copy link
Contributor

bumped to 6.30.1

I always thought minor version bumps are bug fixes and the others features like hudo done hudokkow@138f720

@hudokkow
Copy link
Member

Thanks @un1versal
@InuSasha, please bump to 6.31.0. Sorry for not being more specific.

@@ -134,6 +134,7 @@ JsonRpcMethodMap CJSONServiceDescription::m_methodMaps[] = {
{ "VideoLibrary.GetRecentlyAddedMovies", CVideoLibrary::GetRecentlyAddedMovies },
{ "VideoLibrary.GetRecentlyAddedEpisodes", CVideoLibrary::GetRecentlyAddedEpisodes },
{ "VideoLibrary.GetRecentlyAddedMusicVideos", CVideoLibrary::GetRecentlyAddedMusicVideos },
{ "VideoLibrary.GetInProgressTvShows", CVideoLibrary::GetInProgressTvShows },

This comment was marked as spam.

@Montellese
Copy link
Member

Not a huge fan of introducing yet another hardcoded videodb://foo URL as we have been trying to get rid of them by introducing the library nodes feature. But I know that the library nodes aren't perfect yet either and not fully reliable for usage in skins etc.

@MartijnKaijser
Copy link
Member

@Montellese since it's too late anyways to get this in v16 maybe some suggestion on how to proceed?

@InuSasha
Copy link
Contributor Author

bump to 6.31.0.
And rename TvShow to TVShow. But the camell-case of TVShow is different between JSON (TVShow) and rest (TvShow).

@razzeee
Copy link
Member

razzeee commented Oct 21, 2015

Just a comment and I know that the other vfs are also like this one, but I think it would make much more sense to go after something likevideodb://tvshows/inprogress in the long run.

@InuSasha
Copy link
Contributor Author

great idea! when the other agree, i will change it.

@Montellese
Copy link
Member

Please keep it in line with the existing naming schema for now. IMO there shouldn't be any of these shortcuts as they can all be achieved using videodb://tvshows/?xsp={....} with the proper JSON representation of a smartplaylist in JSON doing the proper filtering.

@un1versal
Copy link
Contributor

Ive tested this and works 100% ontop of master.

@Montellese since it's too late anyways to get this in v16 maybe some suggestion on how to proceed?

:( thats really a shame and bad news with this in - #8279 is also a possibility and is up to help more testing to happen even if a few users have already tested and report this working.

un1versal added a commit to un1versal/kodi that referenced this pull request Oct 23, 2015
un1versal added a commit to un1versal/kodi that referenced this pull request Nov 27, 2015
@@ -1 +1 @@
6.30.0
6.31.0

This comment was marked as spam.

@anaconda
Copy link
Contributor

anaconda commented Jan 9, 2016

jenkins build this please

@razzeee
Copy link
Member

razzeee commented Jan 9, 2016

Btw does this recommend specials or are they excluded?

@InuSasha
Copy link
Contributor Author

InuSasha commented Jan 9, 2016

what do mean exactly?

@razzeee
Copy link
Member

razzeee commented Jan 10, 2016

@InuSasha
Never mind, I just compiled it myself and checked. Everything is fine.

@InuSasha
Copy link
Contributor Author

@anaconda updated the license header (to 2016 and kodi). do not change license header in untouched files (like requested)

@razzeee
Copy link
Member

razzeee commented Jan 10, 2016

Let's build this one more time

jenkins build this please

@razzeee
Copy link
Member

razzeee commented Jan 10, 2016

Please rebase one more time, I'll merge this then.
Sorry for the conflicts :(

@InuSasha
Copy link
Contributor Author

done, with no change

@razzeee
Copy link
Member

razzeee commented Jan 10, 2016

jenkins build and merge

@jenkins4kodi jenkins4kodi merged commit e0744f0 into xbmc:master Jan 10, 2016
@InuSasha InuSasha deleted the feature/master/inprogesstvshows_node branch January 10, 2016 19:21
ronie added a commit that referenced this pull request Jan 10, 2016
[confluence] add "In progress" submenu entry to TV SHOWS - Depends on #8261
razzeee added a commit that referenced this pull request Jan 10, 2016
ronie added a commit that referenced this pull request Jan 11, 2016
@razzeee
Copy link
Member

razzeee commented Mar 9, 2016

As a headsup, @InuSasha
I will do a cleanup here, as the name "in progress" is misleading or wrongly used. On movies, it referes to movies you started and not finished. What we're doing here will need another name, to keep it clear.

@InuSasha
Copy link
Contributor Author

InuSasha commented Mar 9, 2016

do you really mean is misleading?
It refers to started but not finished tvshows (view some not all episodes).
But the wording is not important for me. Actual, i do not know a better wording.

@razzeee
Copy link
Member

razzeee commented Mar 9, 2016

I think it's misleading, as we can already filter episodes/movies by in progress. But that's a different thing.

Movies "In progress" are stuff you haven't finished watching.
image

TV Show "In progress" is what's next to watch...
image

@razzeee
Copy link
Member

razzeee commented Mar 9, 2016

So if we add the same row that movies has for episodes, it becomes weird ;)
image

@Montellese
Copy link
Member

Personally I don't find it misleading. It's not called "In progress episodes" which would/should be identicall to "In progress movies". A tv show is a container and it's in progress if any of its children has been watched and the progress is the latest/newest child that has been watched.

@ronie
Copy link
Member

ronie commented Mar 9, 2016

i'm not really seeing a problem with it either...

@razzeee
Copy link
Member

razzeee commented Mar 9, 2016

@Montellese
I believe that's a very technical way of thinking about it. Not sure if users would get that/think about that in that way from the start.
http://forum.kodi.tv/showthread.php?tid=263585&pid=2275356#pid2275356
But that would mean we can leave it as it is in core.

@InuSasha
Copy link
Contributor Author

InuSasha commented Mar 9, 2016

@razzeee
you mean something like "next episode on in progress tvshows".
have you a proposal?

@Hitcher
Copy link
Contributor

Hitcher commented Mar 10, 2016

Many skins have used 'In Progress TV Shows' for some years now.

@MartijnKaijser
Copy link
Member

@razzeee if we would follow the genera users opinion or reasoning this program would be totally FUBAR. There's nothing wrong with in progress tvshow

@razzeee
Copy link
Member

razzeee commented Mar 10, 2016

Well then ignore me :)
Sorry for the noise

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

Successfully merging this pull request may close these issues.

None yet