Separate "watched" flag for TV and Movies
#1
I am a longtime user of XBMC, remembering fondly (and nervously) modding my first Xbox and installing XMP. Years later and I have a Revo 3610 with XBMC Live in the lounge, XBMC for Windows on my laptop and XBMC for Windows on another PC.

I have long wanted to help with development of XBMC; finally I have checked out the code, got a development environment up and going, and have been reading through the code trying to understand it. I wanted something small that I could try my hand at initially, something that I would personally find useful but also something that would hopefully be accepted eventually by the dev team. I looked through Trac and then spotted #9103. Perfect!

I have been looking through the code, trying to find out how the watched state is currently handled and how the setting is read/updated. I have an understanding of this now, and a basic concept in mind of how it could work. But I have a few questions first.

1. Would a patch for this be something likely to be accepted by the devs? The only reason I hesitate with this is that it is the first time I can see that the video settings would be different for TV Shows vs Movies, and I am not sure what other changes might be in the works for this.

2. As far as I can see, any change to support this would require support in the skin as well as xbmc code. What is the teams attitude to this? To be avoided by a newbie?

3. Broadly speaking I would propose replacing watchmode in the settings file, and introducing two new Xml elements to replace it - something like watchmodetv and watchmodemovies. watchmode is actually an integer, so in theory it could be extended to support all the permutations of watch mode for movies and tvshows, however most skins (including Confluence) just treat it like a bool which would no longer work.

Obviously some additional code would be required to then filter the item list based on whether they are episodes or movies, and a new button control id for skins.

I like this feature request because I would find it really useful myself. After reading it, I realised the reason I never use the "Hide Watched" feature is because I want all my movies to be displayed all the time, but only show unwatched TV shows just as the creator of the Trac item suggests.

Any comments appreciated.
Reply
#2
Something like that should be transparent to the skin. There shouldn't be a need to have two different buttons, or labels, because you can't have both content types displayed at the same time in the library.

If you have three unique filter settings, one for each content type:

g_settings.m_iMyVideoWatchModeMovies
g_settings.m_iMyVideoWatchModeTVShows
g_settings.m_iMyVideoWatchModeMusicVideos

Then the code in GUIWindowVideoNav.cpp should then be able to determine which of these to use for the Watched button based off the current NODE_TYPE.

Likewise, if you want to take it even further, there could be independent filters for the additional nodes where watched status is available separating tvshows, seasons, episodes, and all the recently added listings.
Always read the XBMC online-manual, FAQ and search the forum before posting.
Do not e-mail XBMC-Team members directly asking for support. Read/follow the forum rules.
For troubleshooting and bug reporting please make sure you read this first.
Reply
#3
I agree that there should only be one button, but I hadn't thought of seperating the filter settings from the button. Great idea thanks.

So to make this work, it would have to be possible to have some context of which screen you're in when you return the radio button value. So the "Hide Watched" radio button showed hte correct state in Movies, TV Shows, etc. Do you know this when you return this value?

Thanks for the tip.
Reply
#4
Well, I can look at the content type of the items in the file items list. m_vecItems->GetContent() is set to "movies", "tvshows", "seasons", "episodes" or "musicvideos".

Is this the best way to figure out which watchmode property to get or set?

If this is the case, then this should be easily doable.
Reply
#5
Your plan seems reasonable to me, but please don't have 5 different settings variables - use a vector instead.

Cheers,
Jonathan
Always read the XBMC online-manual, FAQ and search the forum before posting.
Do not e-mail XBMC-Team members directly asking for support. Read/follow the forum rules.
For troubleshooting and bug reporting please make sure you read this first.


Image
Reply
#6
I have a working patch for this now that just needs to be tidied up before being submitted.

However, there are two places in the code that m_iMyVideoWatchMode was used that I am not sure about.

CGUIInfoManager::GetMultiInfoBool contains the following code section.

Code:
case SYSTEM_SETTING:
        if ( m_stringParameters[info.GetData1()].Equals("hidewatched") )
          bReturn = g_settings.m_iMyVideoWatchMode == VIDEO_SHOW_UNWATCHED;
        break;

Also CXbmcHttp::xbmcSTSetting in XBMChttp.cpp has the following.
Code:
if (paras[i]=="myvideowatchmode")
  tmp.Format("%i",g_settings.m_iMyVideoWatchMode);

In both of these sections I don't understand the purpose of the code and where it is being triggered from. The first one, I couldn't trigger a breakpoint by going into system settings - could the code be old? The second one I am assuming is related to the HTTP interface - what should I return for this, or should I introduce new parameters? I'm assuming if I do that, something on the other side of the interface will need changing?
Reply
#7
Awesome! im glad somebody saw the ticket I put in and agreed with me! I will be excited to try this out when its finished
Reply
#8
I had forgotten about GetContent(). That's easier than using the Node Types as there's fewer of them. The Content is set based off the Node Type anyway.

I'm honestly not sure if it makes any sense to have separate options for tvshows, seasons, and episodes. Think about the usability. What would be user expectation when setting hidewatched at the tv show title listing. Should it continue downward through the navigation or not?

The CGUIInfoManager::GetMultiInfoBool() code returns a boolean true or false for various test conditions. This one is returning true for a "hidewatched" passed parameter if the Xbmc is only showing unwatched items.

A skin could use something like <visible>system.setting(hidewatched)</visible> to make something only visible when hiding watching things.

CXbmcHttp::xbmcSTSetting() just returns internal setting values. That code returns the watched status.

Hmm... how to handle these... Two ways:

These functions could (should?) be updated to account for your separate settings. For instance, "hidewatched" would be replaced with "hidewatchedmovies" and "hidewatchedtvshows", etc, to get direct access to that parameter.

But to retain some backward compatibility, accept the current parameters could be done. They could just return the status based off the current content type. And when the content type is not one of those five (the root node would set it blank, the music video section as artist, album, etc), it should just return False or SHOW_VIDEO_ALL respectively for those functions.
Always read the XBMC online-manual, FAQ and search the forum before posting.
Do not e-mail XBMC-Team members directly asking for support. Read/follow the forum rules.
For troubleshooting and bug reporting please make sure you read this first.
Reply
#9
kraqh3d Wrote:I'm honestly not sure if it makes any sense to have separate options for tvshows, seasons, and episodes. Think about the usability. What would be user expectation when setting hidewatched at the tv show title listing. Should it continue downward through the navigation or not?
I initially thought this, but after seeing it work I think it works quite well. I didn't find it at all cumbersome, but I am open to feedback. It would be easy to change.

kraqh3d Wrote:A skin could use something like <visible>system.setting(hidewatched)</visible> to make something only visible when hiding watching things.
Yep, I have been working through that portion of the code all afternoon and just came to this realisation. I'm downloading Alaska skin on my dev PC, because I suspect this uses that mechanism. It displays text instead of a radio button like Confluence.

kraqh3d Wrote:But to retain some backward compatibility, accept the current parameters could be done. They could just return the status based off the current content type. And when the content type is not one of those five (the root node would set it blank, the music video section as artist, album, etc), it should just return False or SHOW_VIDEO_ALL respectively for those functions.
I would prefer to make it completely transparent to the skin, and am hunting for a way to look up the correct value in my vector. But how do you tell the current content type when you are in GUIInfoManager? You don't have a reference to the vector items or anything else that I can find that gives me some context for the current location. I will keep hunting...
Reply
#10
hermy65 Wrote:Awesome! im glad somebody saw the ticket I put in and agreed with me! I will be excited to try this out when its finished

You're welcome. Thanks for the idea Big Grin
Reply
#11
Look at the "containers" in the Info Manager. This is how info is displayed based off the currently highlighted item in a list, so they should give you examples to follow. But its kind of like this.

1) Make a pointer to the Video Library window. (The container code likely uses GetCurrentWindow() or something, but its unnecessary here. You want this specific window.)

CGUIWindow *pWindow = g_windowManager.GetWindow(WINDOW_VIDEO_NAV);

2) Make a pointer to the items in that window. (You need to find the public Window function that returns a CFileItemListPtr. I forget what it is, but I'm guessing its GetItems.)

CFileItemListPtr& pItems = pWindow->GetItems();

3) Once you have a CFileItemListPtr, you can directly access the content

pItems->GetContent();

(THIS IS FROM MEMORY! Please don't expect it to be exact. But its probably pretty close.)
Always read the XBMC online-manual, FAQ and search the forum before posting.
Do not e-mail XBMC-Team members directly asking for support. Read/follow the forum rules.
For troubleshooting and bug reporting please make sure you read this first.
Reply
#12
Just a small question as it fits in this thread:

Which file do I have to backup, if I want to save the watched/unwatched-flag of my database ?

Thanks in Advance.
Reply
#13
kraqh3d Wrote:2) Make a pointer to the items in that window. (You need to find the public Window function that returns a CFileItemListPtr. I forget what it is, but I'm guessing its GetItems.)

CFileItemListPtr& pItems = pWindow->GetItems();

Thanks again for the tip. Your memory is uncannily accurate.

However, once I have a Video Nav window I can't find a property to access the items. The nearest I found was GetCurrentDirectory() on CGUIMediaWindow which returns a CFileItemList&, but when I test the list (after hacking up Confluence) the list is always empty and GetContent is an empty string.

Any thoughts here?
Reply
#14
Check Container.Content(foo) to see how that works.

I'd suggest not having the ability for the skinner to ask for a particular watched type. Instead, just leave it general, and return true if hide is enabled for the current content type.

Reason is, I don't want to have to support it, as I suspect this will be eliminated once we go to a more generic library.

Cheers,
Jonathan
Always read the XBMC online-manual, FAQ and search the forum before posting.
Do not e-mail XBMC-Team members directly asking for support. Read/follow the forum rules.
For troubleshooting and bug reporting please make sure you read this first.


Image
Reply
#15
I just browsed through the code via trac, and GetCurrentDirectory() should be the correct function. However, looking the container code, you need to cast your window pointer to a GCUIMediaWindow. I would think this would work. (With some liberal assumptions around what you're doing.)

Code:
CGUIWindow *pWindow = g_windowManager.GetWindow(WINDOW_VIDEO_NAV);
const CFileItemList& pItems = ((CGUIMediaWindow *)pWindow)->CurrentDirectory();
const CStdString& strContent = pItems->GetContent();
bReturn = false;
int iType = -1;
if (strContent.Equals("movies")) { iType = 0; }
else if (strContent.Equals("tvshows")) { iType = 1; }
else if (strContent.Equals("seasons")) { iType = 2; }
else if (strContent.Equals("episodes")) { iType = 3; }
else if (strContent.Equals("musicvideos")) { iType = 4; }
if (iType > -1) { bReturn = (g_settings.vecWatched.at(iType) == VIDEO_SHOW_UNWATCHED); }

But while typing this out, I thought of something... why not use position 0 in the vector to store the current content type as an integer which references the correct index to use, with -1 as not valid. Then you can easily do something like this:

Code:
int iType = g_settings.vecWatched.at(0);
if (iType > -1) { bReturn = (g_settings.vecWatched.at(iType) == VIDEO_SHOW_UNWATCHED); }
Always read the XBMC online-manual, FAQ and search the forum before posting.
Do not e-mail XBMC-Team members directly asking for support. Read/follow the forum rules.
For troubleshooting and bug reporting please make sure you read this first.
Reply

Logout Mark Read Team Forum Stats Members Help
Separate "watched" flag for TV and Movies0