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

Make sure implicit thumbnail in higher priority over the thumbnail source #26

Merged

Conversation

moonyuet
Copy link
Member

@moonyuet moonyuet commented Jul 17, 2024

Changelog Description

When users trying to create screengrab thumbnails and publish along with the review product type, it will error out the intgerate.py due to the duplicate of the thumbnail representation data. This PR is to make sure the extract thumbnail is always at the higher priority than the extract thumbnail sources

Additional info

please test on some other hosts with thumbnail extractors in review family, maybe similar bug is hit when you publish both screengrab and review family

ported from ynput/ayon-core#695

Testing notes:

  1. Launch Maya/Max
  2. Create Review Instance
  3. Create Screengrab in publisher tab
  4. Publish
  • No error should occur.
  • The thumbnail set in the publisher UI will be ignored/discarded; instead the thumbnail from Maya's Extract Review will be forcefully used. As such, setting a thumbnail in the publisher UI does nothing for review instances.

Copy link
Member

@LiborBatek LiborBatek left a comment

Choose a reason for hiding this comment

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

Works fine and without any issues...

Tested in maya 2024

LGTM

@LiborBatek
Copy link
Member

when testing in blender it works too and no errrors shown and as there is no any sort of nested product inside "review" product all user captured thumbnails stays intact too (besides review which been taken over by implicit one)

All good.

@BigRoy
Copy link
Contributor

BigRoy commented Jul 30, 2024

@moonyuet could you add to the PR description which of the two thumbnails now take higher priority? Which of the two will be used and which will be discarded? That was a topic for discussion here so it would be great if the PR would clearly state that (and have it confirmed/tested also via the testing notes)

Also tagging @dee-ynput because there was still (as far as I know unanswered 'to be discussed' point here raised here)

@moonyuet
Copy link
Member Author

moonyuet commented Jul 30, 2024

@moonyuet could you add to the PR description which of the two thumbnails now take higher priority? Which of the two will be used and which will be discarded? That was a topic for discussion here so it would be great if the PR would clearly state that (and have it confirmed/tested also via the testing notes)

Also tagging @dee-ynput because there was still (as far as I know unanswered 'to be discussed' point here raised here)

The extract thumbnail from the host takes higher priority for this PR, not the thumbnail source which takes the screengrab as thumbnail.

@BigRoy BigRoy added the type: bug Something isn't working label Jul 30, 2024
@BigRoy BigRoy requested a review from dee-ynput July 30, 2024 14:44
@BigRoy BigRoy assigned dee-ynput and unassigned LiborBatek Jul 30, 2024
@dee-ynput dee-ynput merged commit ee6c0aa into develop Jul 30, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants