[RFC] BaseContainer now has a FileItemList #2818

Closed
wants to merge 1 commit into from

4 participants

@Fice

BaseContainer now has a FileItemList instead of a vector.
Reason behind this is, that you want to let skinner add lists everywhere (e.g. a Favourites list in library view). In order to do that we need to move content specific logic out of the CGUIWindows*.

Right now my Drag&Drop stuff uses this, to determine if a FileItemList is dropable and reorderable.
I also started to work on a context menu rfc where i wanted to move the decision of the visible items out of CGUIWindow*. There I need this one too.

@jmarshallnz
Team Kodi member

Damn, I didn't know you were working on this else I would have commented sooner - please accept my apologies.

I think we need to be a little careful about moving stuff inside of guilib. Separation of concerns suggests that the ownership of the items, and perhaps how to act on them should be outside of guilib. e.g. I'm currently working on allowing lists to auto-fill with content from a directory (the example of favourites could use this mechanism for instance) - in this case we want the provider to be external to guilib as directory fetching will take a while and feed items into the container after they've been fetched and made pretty (info loaded etc.) My plan is to create an IListProvider interface with implementations to wrap the current static item stuff and provide a directory service as well. I propose that this external source would be the one to handle reordering and also handle any actions that should occur on the items etc.

I can probably get a basic implementation of the interface + an implementation for the existing static content up and running later today that you could build off, assuming you agree that this is the way forward.

Also, I think this breaks static items. A CFileItemList holds CFileItemPtr's (i.e. shared_ptr's) whereas static content is shared_ptr. Both are derived from CGUIListItem but they are not the same thing. I suspect it will "work", but as soon as someone tries to do a GetPath() on one of those pointers all hell will break out.

@Fice

Apologies accepted ;) But to be fair, when i started this, I excepted all hell to break loose but it went surprisingly smooth, so no worries. Just had to change function names mostly.

All I need this for right now is to know if a list is reorderable/dropable from a BaseContainer and I'm sure your IListProvide approach can achieve that.

Just out of curiosity, what does m_staticContent store exactly?

@jmarshallnz
Team Kodi member

m_staticContent is for when the skinner specifies content for a list themselves via the <content> tag. Most skin's home screens use that.

You can checkout my list_provider branch if you like:

https://github.com/jmarshallnz/xbmc/branches/list_provider

@Fice

Finally had time to check it out. Are there plans to use IListProvider for "non-static" content as well, or will m_items stay as it is?

I could definitely use you IListProver to find out if static content is reorderable/dropable...
Question now is, how can I find that our for m_items.

@jmarshallnz
Team Kodi member

For reorderable the basic idea is to instead of using GUI_MSG_LABEL_BIND to send the items across, instead provide the IListProvider to the control.

I have a class underway intended for skinner-defined directory listings which may well be a case where some of the provided lists would be reorderable (e.g. favourites).

@jenkins4xbmc

Is this PR ment to be tested by jenkins?

@jmarshallnz jmarshallnz was assigned Oct 17, 2013
@MartijnKaijser
Team Kodi member

Closing this. Please resubmit for comments when it's rebased

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