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

Use fanart to hide spoilers if available #15991

Merged
merged 1 commit into from
Apr 30, 2019

Conversation

netpok
Copy link
Contributor

@netpok netpok commented Apr 25, 2019

Description

Currently if thumbnails are hidden to prevent spoilers a generic OverlaySoiler image are shown instead of the thumbnail. In my opinion it would look way better if the fanart of the show is displayed instead of this, this pull requests implements this change. Of course maybe this only my opinion, so I'm open for suggestions, maybe it should be put behind a setting.

Motivation and Context

I'm a long time user of trakt.tv where the hiding of spoilers are achieved with the fanart, that's the source of this idea. Also it helps the visual identification of the show especially in the recently added section.

How Has This Been Tested?

Manually by checking on the GUI.

Screenshots (if appropriate):

With the old behavior:
image

With the new behavior:
image

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Clean up (non-breaking change which removes non-working, unmaintained functionality)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that will cause existing functionality to change)
  • Cosmetic change (non-breaking change that doesn't touch code)
  • None of the above (please explain below)

I marked it as improvement, but it can be also a "new feature"

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

@enen92 enen92 requested a review from DaVukovic April 25, 2019 20:50
@DaVukovic
Copy link
Member

That looks cool so a 👍 generally from my side.

Currently on holidays till Sunday. Will take a deeper look when I'm back.

Thanks for that

@Dnkhatri
Copy link

Would not mind using this for shows/episodes that do not have have thumbnails as well. I watch a lot of korean/asian dramas and many never get any thumbnails or if they do maybe weeks or months later.

@DaVukovic
Copy link
Member

Tested and it works great. Thanks much for that 👍

For me it's good to go

@DaVukovic DaVukovic added Type: Improvement non-breaking change which improves existing functionality v19 Matrix Component: Skin labels Apr 29, 2019
@enen92 enen92 merged commit 8632b8a into xbmc:master Apr 30, 2019
@ksooo ksooo added this to the M** 19.0-alpha 1 milestone Apr 30, 2019
@@ -360,7 +360,15 @@ bool CVideoThumbLoader::LoadItemCached(CFileItem* pItem)
!setting->FindIntInList(CSettings::VIDEOLIBRARY_THUMB_SHOW_UNWATCHED_EPISODE)
)
{
pItem->SetArt("thumb", "OverlaySpoiler.png");
// use fanart if available
if(pItem->HasArt("fanart"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blank between "if" and "(" missing. :-(

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/xbmc/xbmc/blob/master/docs/CODE_GUIDELINES.md#34-whitespace

=> "Control statement keywords have to be separated from opening parentheses by one space."

Copy link
Contributor Author

@netpok netpok Apr 30, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry that I missed this, I'm used to the autoformatter but my settings are not compatible with other parts of the code guidelines. I made a pull request (#16019) to fix this and an other instance of the same problem.

@netpok netpok mentioned this pull request Apr 30, 2019
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Content Type: Improvement non-breaking change which improves existing functionality v19 Matrix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants