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.

Sign up for GitHub

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

Jump to bottom

[guilib] Add Container.TotalWatched and Container.TotalUnwatched #7358

Merged
merged 1 commit into from
Jul 13, 2015
Merged

[guilib] Add Container.TotalWatched and Container.TotalUnwatched #7358

merged 1 commit into from
Jul 13, 2015

Conversation

BigNoid
Copy link
Member

@BigNoid BigNoid commented Jun 28, 2015

My original usecase for this was to be able to get the watched/unwatched count for the Home window widgets or playlist menu items, but since Home is not a media window, that doesn't work. Still these two infolabels can be useful in the library views.
For J****

@ronie @phil65

@BigNoid BigNoid force-pushed the add_watched_count branch 2 times, most recently from ae1c4d8 to d596cd7 Compare June 28, 2015 13:45
@BigNoid
Copy link
Member Author

BigNoid commented Jun 28, 2015

@ronie thx for review. I updated the pr and also removed setting it for music items, because unwatched makes no sense there and always is empty.
I'm seeing now that totalunwatched gets set to 15.0-RC1 in cases where there is no playcount. I'll update the pr if i find a fix for that.

Montellese
Montellese reviewed Jun 28, 2015
View reviewed changes
xbmc/GUIInfoManager.cpp
if (totalUnwatched > 0)
return StringUtils::Format("%i", totalUnwatched);
}
}

This comment was marked as spam.

Sign in to view

@BigNoid
Copy link
Member Author

BigNoid commented Jun 28, 2015

@Montellese Updated, thx.
@ronie Indeed it should return zero for cases there are watched and no unwatched items or vice versa. I was following the totaltime logic by leaving it empty, so it only shows where applicable. It now returns zero for all directories, but its up to skinners to only show this where its applicable then.

Montellese
Montellese reviewed Jun 28, 2015
View reviewed changes
xbmc/GUIInfoManager.cpp
return StringUtils::Format("%i", playCount);
}
}
break;

This comment was marked as spam.

Sign in to view

@BigNoid
Copy link
Member Author

BigNoid commented Jun 28, 2015

@Montellese Updated, thx

@phil65
Copy link
Contributor

phil65 commented Jun 28, 2015

Wouldnt it be nicer to "merge" CONTAINER_TOTALWATCHED and CONTAINER_TOTALUNWATCHED instead of duplicating code?
(either with an if clause for the counting or by doing watched = items.size() - unwatched at the end)

@mkortstiege
Copy link
Member

+1 for merging them since it's just the operand that differs here.

@BigNoid BigNoid force-pushed the add_watched_count branch 3 times, most recently from 59d2fc8 to d9fd85d Compare June 28, 2015 17:19
@BigNoid
Copy link
Member Author

BigNoid commented Jun 28, 2015

That is much cleaner indeed, updated

@BigNoid BigNoid force-pushed the add_watched_count branch 2 times, most recently from ec28940 to f6eb672 Compare June 28, 2015 17:29
@BigNoid
Copy link
Member Author

BigNoid commented Jun 28, 2015

@phil65 Had to do the if clause, as items.size()-playCount would also include the parent item (and any other item that has no playcount).

@phil65
Copy link
Contributor

phil65 commented Jun 28, 2015

You can use the same counter variable for both calls to save some code and make it more readable.
Looks bit weird atm :)

@BigNoid BigNoid force-pushed the add_watched_count branch 2 times, most recently from f9c531b to 68937e0 Compare June 28, 2015 18:07
@BigNoid
Copy link
Member Author

BigNoid commented Jun 28, 2015

Using one counter variable now and also merged with totaltime.
thx @phil65 for the pointer.

@phil65
Copy link
Contributor

phil65 commented Jun 28, 2015

No problem.
@mkortstiege or @Montellese should decide though if merging with TOTALTIME is welcome.
(I think it would be nicer to keep TOTALTIME separate)

@mkortstiege
Copy link
Member

I see no issue with merging besides it's a bit harder to follow. @Montellese objections?

@mkortstiege
Copy link
Member

Will shove this on in on monday if there are no further objections. jenkins build this please

@MartijnKaijser MartijnKaijser added this to the J**** 16.0-alpha1 milestone Jul 12, 2015
@mkortstiege
Copy link
Member

jenkins build this please

mkortstiege added a commit that referenced this pull request Jul 13, 2015
@mkortstiege
Merge pull request #7358 from BigNoid/add_watched_count
4480723 
[guilib] Add Container.TotalWatched and Container.TotalUnwatched
@mkortstiege mkortstiege merged commit 4480723 into xbmc:master Jul 13, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
v16 Jarvis
Projects
None yet
Milestone
Jarvis 16.0-alpha1
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@BigNoid @phil65 @mkortstiege @Montellese @ronie @MartijnKaijser