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

AY-6253_Convert Flame creators to new publisher #33

Merged
merged 55 commits into from
Dec 5, 2024

Conversation

robin-ynput
Copy link
Contributor

@robin-ynput robin-ynput commented Nov 5, 2024

Changelog Description

resolve #10
Make flame creators use the Creator API and new widget. Publishable products are now listed as individual instances following a similar logic than Hiero and Resolve.

flame_ayon

Done:

Testing notes:

In Flame:

  • Create a timeline with clips associated to movie and img-sequence media
  • From AYON Menu (right click)
  • Create and publish product(s) of type shot, audio, plate and review from there

Tested locally on Flame 2024.2

resolves #25

AY-6253

@robin-ynput robin-ynput added type: enhancement Improvement of existing functionality or minor addition sponsored This is directly sponsored by a client or community member bump minor labels Nov 5, 2024
@robin-ynput robin-ynput self-assigned this Nov 5, 2024
@robin-ynput robin-ynput force-pushed the 10_creator_new_publisher branch from 005ed37 to 86f3c66 Compare November 11, 2024 19:07
@robin-ynput robin-ynput marked this pull request as ready for review November 11, 2024 23:57
@jakubjezek001
Copy link
Member

The PR looks solid to me. After a short testing session, I have the following notes:

  • We need to ensure the Reviewable source distribution matches the Hiero workflow 100%. Currently, it sets all vertically aligned instances as review ON, but it shouldn't. Also, we're missing an option for Clip media as a reviewable source.
  • In Hiero, if an instance is removed from the Publisher's panel, it also removes the related clip marker. In Flame, it only removes nested data from the marker. The question is whether this might confuse users, and if we should also remove the markers.

Copy link
Member

@jakubjezek001 jakubjezek001 left a comment

Choose a reason for hiding this comment

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

The PR look already great but I have to request some changes regarding the Reviewable source distribution enhancements.

client/ayon_flame/plugins/create/create_shot_clip.py Outdated Show resolved Hide resolved
client/ayon_flame/plugins/create/create_shot_clip.py Outdated Show resolved Hide resolved
client/ayon_flame/plugins/create/create_shot_clip.py Outdated Show resolved Hide resolved
@robin-ynput
Copy link
Contributor Author

robin-ynput commented Dec 2, 2024

@jakubjezek001 I have started a draft PR aside for the review work:
https://github.com/ynput/ayon-flame/pull/36/files this is still WIP ready for review.

For the marker comment, I couldn't find any API to delete markers from within Flame hence the empty marker for now.
I looked at the API docs, the forums and the community pages... do you have any idea how this is done ?

@jakubjezek001
Copy link
Member

@jakubjezek001 I have started a draft PR aside for the review work: #36 (files) this is still WIP.

For the marker comment, I couldn't find any API to delete markers from within Flame hence the empty marker for now. I looked at the API docs, the forums and the community pages... do you have any idea how this is done ?

Not aware of any solution for it. Lets keep them there for now. Users should do it manualy. We can add warning into documentation.

@jakubjezek001 jakubjezek001 changed the title Convert Flame creators to new publisher AY-6253_Convert Flame creators to new publisher Dec 5, 2024
AY-6253_Report review changes from Hiero to Flame
@jakubjezek001 jakubjezek001 self-requested a review December 5, 2024 13:29
Copy link
Member

@jakubjezek001 jakubjezek001 left a comment

Choose a reason for hiding this comment

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

Great work! it works well now and it is good to be merged! Thank you!

@jakubjezek001 jakubjezek001 merged commit c8297f2 into develop Dec 5, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bump minor type: enhancement Improvement of existing functionality or minor addition
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AY-6972_Failing audio publishing AY-6253_Conversion creators in new publisher
4 participants