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

Workfile Templates tweaks: plugin discovery, event system, maya fixes #327

Conversation

BigRoy
Copy link
Collaborator

@BigRoy BigRoy commented Mar 29, 2024

Changelog Description

  • Implement registering and discovering of Placeholder Plugins dynamically
  • Implement event system in workfile templates
  • Maya: Implement fixes for outliner order of loaded placeholders (always place after placeholder)
  • Maya: Allow storing more complex data, e.g. EnumDef with multiselection=True
  • Maya: Implement Assign Look placeholder
  • Maya: Implement Script placeholder

Additional info

AYON equivalent to ynput/OpenPype#5830

However, with some things removed which I'll turn into separate PRs
But added refactoring Nuke and After Effects to also use the dynamic plug-in discovery

Testing notes:

After Effects

Make sure workfile templates in still behave as intended (basically existing templates should continue to work)

  1. Create placeholders
  2. Build workfile templates (from new ones and existing ones)

Nuke

Make sure workfile templates in still behave as intended (basically existing templates should continue to work)

  1. Create placeholders
  2. Build workfile templates (from new ones and existing ones)

Maya

Make sure workfile templates in still behave as intended (basically existing templates should continue to work)

  1. Create placeholders
  2. Build workfile templates (from new ones and existing ones)
  3. The Run Python Script Placeholder in Maya should work (including 'ordering' it with the order attribute)
  4. The Assign Look Placeholder in Maya should work - any contents inside the placeholder should get shaders assigned, even if e,g. the loaded content is itself loaded from a placeholder.
  5. Ordering in Maya outliner should be preserved for placeholders, also when "keep placeholders" is enabled.

- Implement registering and discovering of Placeholder Plugins dynamically
- Implement event system in workfile templates
- Maya: Implement fixes for outliner order of loaded placeholders (always place after placeholder)
- Maya: Allow storing more complex data, e.g. `EnumDef` with `multiselection=True`
- Maya: Implement Assign Look placeholder
- Maya: Implement Script placeholder
@BigRoy BigRoy requested review from tokejepsen and iLLiCiTiT March 29, 2024 16:09
@ynbot ynbot added size/XXL host: After Effects host: Maya host: Nuke type: enhancement Improvement of existing functionality or minor addition labels Mar 29, 2024
@BigRoy
Copy link
Collaborator Author

BigRoy commented Mar 29, 2024

I have a feeling that this might also need fixing/refactoring to AYON core.

However, on just building a template I didn't hit a direct error or crash. Any idea what to look out for @tokejepsen @iLLiCiTiT?
And maybe, what it should be refactored to?

No idea what _reduce_last_version_repre_entities is supposed to mean or what it's supposed to do.

@MustafaJafar MustafaJafar added the community Issues and PRs coming from the community members label Apr 1, 2024
@@ -51,6 +47,8 @@ def import_template(self, path):
class AEPlaceholderPlugin(PlaceholderPlugin):
"""Contains generic methods for all PlaceholderPlugins."""

item_type = PlaceholderItem
Copy link
Member

Choose a reason for hiding this comment

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

Please use method instead of attribute.

@abstractmethod
def _create_placeholder_item(self, item_data):
    pass

And then implement it in both plugins...

def _create_placeholder_item(self, item_data):
    return CreatePlaceholderItem(
        scene_identifier=item["uuid"],
        data=item["data"],
        plugin=self
    )

@iLLiCiTiT
Copy link
Member

Can it be split into smaller PRs? This now requires heavy testing of 4 people. If you would split it, then it may happen. The core should also be backwards compatible.

I can create some base PRs based on this if you would like.

@BigRoy
Copy link
Collaborator Author

BigRoy commented Apr 2, 2024

I can create some base PRs based on this if you would like.

That would of course be greatly appreciated - but don't spend too much time on it if you already have a big to do list. I can look into it - likely just not today but maybe later this week or next week. (I have a bit of a short week this week)

Copy link
Member

@tokejepsen tokejepsen left a comment

Choose a reason for hiding this comment

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

Yeah, lets split this into smaller PRs.

@BigRoy
Copy link
Collaborator Author

BigRoy commented Apr 17, 2024

Yeah, lets split this into smaller PRs.

I've started the work - PRs are opened.

I'll continue the maya part of things after some of them are merged (in particular #425) because those are easier for me to convert over as soon as other things are implemented. In the meantime, I'll close this PR in favor of the new ones and the upcoming ones.

These are the only files I still need to 'touch' to implement a fix in that upcoming PR:
image

All other changes of this PR should already be covered by one of the currently existing PRs.

@BigRoy BigRoy closed this Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Issues and PRs coming from the community members host: Maya host: Nuke size/XXL type: enhancement Improvement of existing functionality or minor addition
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants