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

Already on GitHub? Sign in to your account

adds support for inprogress in shows #1349

Merged
merged 5 commits into from Sep 8, 2012

Conversation

Projects
None yet
3 participants
Member

jmarshallnz commented Sep 1, 2012

kicks in if watchedcount > 0 and watchedcount < totalCounts.

For Keith.

@Montellese Montellese and 1 other commented on an outdated diff Sep 2, 2012

xbmc/playlists/SmartPlayList.cpp
@@ -725,6 +726,11 @@ CStdString CSmartPlaylistRule::GetWhereClause(const CDatabase &db, const CStdStr
if (m_field == FieldInProgress)
return "episodeview.idFile " + negate + " IN (select idFile from bookmark where type = 1)";
}
+ else if (strType == "tvshows")
+ {
+ if (m_field == FieldInProgress)
+ return "tvshowview.idShow" + negate + " IN (select idShow from tvshowview where watchedcount > 0 AND watchedcount < totalCount)";
@Montellese

Montellese Sep 2, 2012

Owner

I'd prefer using GetField(m_field, strType) instead of a fixed "tvshowview.idShow" in the SQL. That makes it more in line with all the other field accesses (except all the *.idFile ones).

@jmarshallnz

jmarshallnz Sep 2, 2012

Member

Good idea. Not sure if it can be done without using the idShow at all, i.e. returning something like

negate + "(watchedcount > 0 AND watchedcount < totalCounts)";

@Montellese

Montellese Sep 2, 2012

Owner

It's only about the "tvshowview.idShow" and not about the idShow in the sub-SELECT. If we wouldn't wanna have the "select idShow" in the sub-query we'd probably have to use EXISTS and NOT EXISTS and I never really liked those.

Owner

Montellese commented Sep 2, 2012

Looks good except for the comment.

Member

jmarshallnz commented Sep 7, 2012

Updated with a couple of changes:

  1. Recently added nodes have moved to filters (now that @Montellese has taken care of the sort stuff which was the only hold up from the original merge of the video lib customisation).
  2. Removed a couple of advanced settings related to recently added as it's now customisable anyway.
  3. Implemented in progress as a default filter. @JezzX a new image DefaultInProgressShows.png if you feel like doing so.
  4. Re-specified the order attribute so that adding new default nodes is a little less effort next time around.

I suspect over the next week and a half I'll be in and out of internet coverage, so if someone would hit the button on my behalf once signed off that'd be great. @Montellese, @cptspiff either of you are fine for review.

Owner

Montellese commented Sep 7, 2012

You beat me to changing recently added from a folder into a filter (I have the same patch lying around somewhere). Any reason why we still need to keep the DirectoryNodeRecentlyAddedFoo.* files in xbmc/filesystem/VideodatabaseDirectory?

The changes look fine to me. I can give them a test run later to see if there are any obvious bugs.

Member

jmarshallnz commented Sep 7, 2012

No reason other than I couldn't be arsed - I can probably do that now (have another 15 mins till I need to head off)

Member

jmarshallnz commented Sep 7, 2012

Hmm, after more thought, there may be a reason. videodb://4/ may be being referenced directly by a skin. Or, more likely, RecentlyAddedMovies may be referenced. Thus, some skin links would stop working if we removed these.

Might be better to hold off until the home menu customisation is in and skins support it.

Owner

Montellese commented Sep 7, 2012

Ah you're right, forgot about that. In that case we better leave it in for now.

I did some work on grouping support in playlists anyway which might render most of the DirectoryNodeFooBar files unneeded anyway. But that's most likely for Frodo+1.

Owner

Montellese commented Sep 7, 2012

Ok gave it a quick spin and didn't spot anything obvious. Only thing I was wondering about is that the "In progress TVShows" node shows up in the video library node and not under tvshows. IMO it looks a bit odd there.

Member

jmarshallnz commented Sep 8, 2012

Same place as the recently added nodes. I don't mind either way but take
into account flattening.
On Sep 7, 2012 9:32 PM, "Sascha Montellese" notifications@github.com
wrote:

Ok gave it a quick spin and didn't spot anything obvious. Only thing I was
wondering about is that the "In progress TVShows" node shows up in the
video library node and not under tvshows. IMO it looks a bit odd there.


Reply to this email directly or view it on GitHubhttps://github.com/xbmc/xbmc/pull/1349#issuecomment-8376180.

Owner

Montellese commented Sep 8, 2012

I don't really care as I'll probably never use it anyway. I also never go into Videos -> Library except during debugging/testing.

Montellese added a commit that referenced this pull request Sep 8, 2012

Merge pull request #1349 from jmarshallnz/in_progress_shows
videolibrary: adds support for inprogress in shows

@Montellese Montellese merged commit d691aaa into xbmc:master Sep 8, 2012

@ghost ghost assigned Montellese Sep 8, 2012

Owner

MartijnKaijser commented on 5c57c76 Sep 19, 2012

imo this isn't entirely correct.
For me inprogress means that even if only start to watch the first episode and don't finish it completely it is still something in progress.
atm these tvshows are excluded from the list.

Member

jmarshallnz replied Sep 19, 2012

If you can come up with suitable SQL for grabbing it we could make the change. The tricky bit is you want both, and that the episode in progress query needs to be done on the bookmark/file/episode tables (to grab idShow where a resume bookmark is set).

Owner

Montellese replied Sep 19, 2012

Yeah that's gonna be one hell of an ugly SQL query. Either you have to do another sub-select query or you have to come up with an EXISTS() condition. Being able to cover both cases isn't that much of a problem, just do

... where (watchedcount > 0 AND watchedcount < totalCount) OR (watchedcount == 0 AND foobar)

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