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

[guilib] attach collection artwork to movie items as "set.[arttype]" #13564

Merged
merged 2 commits into from Oct 21, 2018

Conversation

@rmrector
Copy link
Contributor

commented Feb 20, 2018

Description

Movie collection artwork is available from movie ListItems/Player in skins and JSON-RPC as "set.[arttype]" ("set.poster", "set.fanart", "set.clearlogo", etc), like TV show and artist artwork is available to episodes and songs.

Motivation and Context

Collection artwork can sometimes be useful from here in skins.

How Has This Been Tested?

Manually, with a full library, set artwork was available where it should be, and other items were unaffected.

Screenshots (if appropriate):

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the Code guidelines of this project
  • My change requires a change to the documentation, either Doxygen or wiki
  • I have updated the documentation accordingly
  • I have read the CONTRIBUTING document
  • I have added tests to cover my change
  • All new and existing tests passed

@rmrector rmrector changed the title [guilib] attach collection artwork to movie items as "set.[arttype]" [guilib] attach collection artwork to movie items as "set.[arttype]" on lookup Feb 21, 2018

@rmrector rmrector changed the title [guilib] attach collection artwork to movie items as "set.[arttype]" on lookup [guilib] attach collection artwork to movie items as "set.[arttype]" Feb 22, 2018

if (i != m_showArt.end())
{
item.AppendArt(i->second, "tvshow");
item.AppendArt(artmap, "tvshow");

This comment has been minimized.

Copy link
@Razzeee

Razzeee Mar 26, 2018

Member

"tvshow" should be MediaTypeTvShow

@rmrector rmrector force-pushed the rmrector:artwork-set-to-movie branch from ba32c10 to 3ecc149 Mar 27, 2018

@Rechi Rechi force-pushed the rmrector:artwork-set-to-movie branch from 3ecc149 to 70837b9 Sep 25, 2018

@rmrector

This comment has been minimized.

Copy link
Contributor Author

commented Oct 8, 2018

Anyone willing to review this for merging?

I'm not too keen on how this peppers a ton of tiny SQL requests to gather artwork for each item individually, but one more round shouldn't make this horribly worse (famous last words 😄).

@Razzeee I appreciate the code review, any thoughts on merging?

typedef std::map<int, std::map<std::string, std::string> > ArtCache;
ArtCache m_showArt;
ArtCache m_seasonArt;
typedef std::map<std::pair<MediaType, int>, std::map<std::string, std::string> > ArtCache;

This comment has been minimized.

Copy link
@garbear

garbear Oct 9, 2018

Member

Use using and can you break this up into multiple lines for small screens?

@@ -683,3 +674,20 @@ void CVideoThumbLoader::DetectAndAddMissingItemData(CFileItem &item)
if (!stereoMode.empty())
item.SetProperty("stereomode", CStereoscopicsManager::NormalizeStereoMode(stereoMode));
}

bool CVideoThumbLoader::GetArtFromCache(const std::string &mediaType, const int id, std::map<std::string, std::string> &art) {

This comment has been minimized.

Copy link
@garbear

garbear Oct 9, 2018

Member

{ on separate line, and when you address breaking the using statements above into multiple lines, you can use one of those types here to shorten the line

@garbear

This comment has been minimized.

Copy link
Member

commented Oct 9, 2018

I suggested three code improvements, but otherwise this looks OK

@rmrector rmrector force-pushed the rmrector:artwork-set-to-movie branch from 70837b9 to 6572589 Oct 16, 2018

@DaveTBlake

This comment has been minimized.

Copy link
Member

commented Oct 21, 2018

jenkins build this please

@DaveTBlake DaveTBlake added this to the Leia 18.0-beta4 milestone Oct 21, 2018

@DaveTBlake DaveTBlake merged commit 62b7be4 into xbmc:master Oct 21, 2018

1 check passed

default You're awesome. Have a cookie
Details

@rmrector rmrector deleted the rmrector:artwork-set-to-movie branch Oct 21, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.