Skip to content
This repository has been archived by the owner on Sep 20, 2024. It is now read-only.

NewPublisher: Prepared implementation of optional pyblish plugin #2943

Merged
merged 4 commits into from
Mar 25, 2022

Conversation

iLLiCiTiT
Copy link
Member

Brief description

Optional plugins must be set to active/inactive in new publisher UI somehow. The way is to implement get_attribute_definitions in OpenPypePyblishPluginMixin.

Description

Implemented OptionalPyblishPluginMixin which will check optional attribute and will care about attributes for the plugin. Publish plugin must inherit from the mixin to be able use it. Should be backwards compatible with current publisher.

Testing notes:

  1. Add plugin which inherit from OptionalPyblishPluginMixin
  2. Make the validator optional
  3. Open new publisher in context where the plugin should be visible and check publish attributes

@iLLiCiTiT iLLiCiTiT added the type: enhancement Enhancements to existing functionality label Mar 24, 2022
@iLLiCiTiT iLLiCiTiT self-assigned this Mar 24, 2022
@iLLiCiTiT iLLiCiTiT requested a review from kalisp March 24, 2022 12:27
@BigRoy
Copy link
Collaborator

BigRoy commented Mar 24, 2022

Am I understanding the logic here that we're also deviating from Pyblish behavior itself? It seems like this logic doesn't flow with how Pyblish actually works? E.g. Pyblish won't deal with these classes correctly out of the box?

I hate to do this again - but could we identify why we need this?

I'm a bit worried that somehow I can't do e.g. a command line publish using pyblish.util.publish()?

Co-authored-by: Roy Nieterau <roy_nieterau@hotmail.com>
@iLLiCiTiT
Copy link
Member Author

Am I understanding the logic here that we're also deviating from Pyblish behavior itself? It seems like this logic doesn't flow with how Pyblish actually works? E.g. Pyblish won't deal with these classes correctly out of the box?

This is extending the behavior of publish plugins. Pyblish plugin can tell which attributes can be shown in UI and artist can change. In case of optional plugins you can enable/disable them (and store that information).

Some plugins may need more information, from top of my head it could be deadline pools. That is now hacked, uncomfortable to change and hard to explain to artists.

I'm a bit worried that somehow I can't do e.g. a command line publish using pyblish.util.publish()?

I'm afraid that you're right and it would not work identically.

@BigRoy
Copy link
Collaborator

BigRoy commented Mar 24, 2022

Some plugins may need more information, from top of my head it could be deadline pools. That is now hacked, uncomfortable to change and hard to explain to artists.

The changes made would still be made on the actual instance to persist with the scene, no? So in essence that would only be UX to manage the instance? And not customize the data going through Pyblish itself?

I'm a bit worried we'll now also be absorbing Pyblish completely to work around cases like this but I feel the feature set we're looking for doesn't need that complexity.

@iLLiCiTiT iLLiCiTiT added this to the next milestone Mar 25, 2022
@iLLiCiTiT iLLiCiTiT merged commit e591bf5 into develop Mar 25, 2022
@iLLiCiTiT iLLiCiTiT deleted the enhancement/default_optional_plugin_mixin branch March 25, 2022 16:38
@BigRoy
Copy link
Collaborator

BigRoy commented Mar 25, 2022

It's a bit sad to see this merged with my questions left unanswered. It seems we might still be expecting a cleanup on the new Publisher as we go to still resolve things that could be simplified? It might however mean quite some more work to clean up, simplify and streamline things.

@iLLiCiTiT
Copy link
Member Author

The changes made would still be made on the actual instance to persist with the scene, no? So in essence that would only be UX to manage the instance? And not customize the data going through Pyblish itself?

The new publisher also pass create context into pyblish context from which are created pyblish instances. Without create context they're not collected as would they be. And until host is completely converted to new publisher we can't add collector which would do that because we want to have ability have new publisher as experimental feature until is tested (host by host).

I'm a bit worried we'll now also be absorbing Pyblish completely to work around cases like this but I feel the feature set we're looking for doesn't need that complexity.

Since new publisher does not show plugins to artists at all, but artists must be able turn on/off optional plugins, and even make them optional per instance there is not much other way then extend logic of pyblish plugins themselves this way. This way it is also added option to set more data per instance that are based on module's (or addon's) logic. Currently it's hacked into creators that are in hosts and make sense to have them only when you have some module e.g. deadline in maya creators which require to have deadline logic in host and more creators for the same family.

It's a bit sad to see this merged with my questions left unanswered.

I've merged the PR when I woke up during my illness. This PR does not change anything, the code extending plugins was part of code base for a long time. This just adds preimplemented helper logic of optional plugins.

It seems we might still be expecting a cleanup on the new Publisher as we go to still resolve things that could be simplified?

This was created as helper for aftereffects PR which is in progress. New publisher is changing every day, but I don't think this exact logic will changed much. I'm opened to discussion but it can't be described in one PR comment and also I'm not the one who came out with requirements (causing limitations) of new publisher.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: enhancement Enhancements to existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants