Refactors Nuke instance data collection #147
Conversation
Renames the collector plugin for better clarity. Updates settings to reflect the plugin name change. Adjusts the order of the collector in the pipeline. Clarifies the setting key to reflect its purpose.
| """ | ||
|
|
||
| order = pyblish.api.CollectorOrder - 0.49 | ||
| order = pyblish.api.CollectorOrder + 0.001 |
There was a problem hiding this comment.
Why this changed? It is on purpose to collect "core" instance data as early as possible.
There was a problem hiding this comment.
It is all linked to the Collect Scene Version, this plugin needs to be executed after it.
There was a problem hiding this comment.
Why this changed?
This one should be run after CollectSceneVersion
https://github.com/ynput/ayon-core/blob/2393c333191d966f0c472694e38c2b4bcc612be5/client/ayon_core/plugins/publish/collect_scene_version.py#L14
There was a problem hiding this comment.
I really feel like this plug-in actually is not warranted to exist at all - the behavior should not need to be "Nuke" specific (I'm talking about this scene version aspect of it). And if you really really want to... I'd separate that into a plug-in of it own. That just targets the families of those product types - so that the full plug-in just runs or not based on whether it's relevant, plus the functionality of that plug-in then becomes much clearer... but admittedly that also just shows that it can live just as well inside ayon-core.
At the least separate the actual nuke instance data into its own plug-in, as early ordered as possible.
There was a problem hiding this comment.
I highly agree with you.
There are few caveats though:
- Exposing product types: to have an easy enum so that admins can select easily. The enum can be a static dictionary or maybe we can a have an end point that returns product types per host.
- The
follow_workfile_versionlogic is interlaced. - Also, unfortunately, fixing this collector can be faster than generalizing this feature.
There was a problem hiding this comment.
So, to sum up, the sync of workfile version with instance version should be probably moved to separate plugin that can run after CollectSceneVersion. This plugin should change the order back to previous order.
There was a problem hiding this comment.
Yes.
And "sync of workfile version with instance version should be probably moved to separate plugin" I'd argue should be core functionality and likely be moved out of ayon-nuke.
There was a problem hiding this comment.
I'd argue should be core functionality and likely be moved out of ayon-nuke.
Agree, but it is not there now, or?
There was a problem hiding this comment.
Agree, but it is not there now, or?
No, we lack product type specific filtering in core to behave similar to what this tried to do.
There was a problem hiding this comment.
So not in this PR. In this PR there should be added new plugin with correct order that just adds "version" from context data to "version" in instance data.
|
I didn't tested yet but when looking at the code, I'm concerned that this plugin will be redundant if admins enabled See here and here |
|
On the topic of execution order; I gave this a spin earlier and shifting the order works just fine. Just throwing those 2cents in the pool. |
|
The point is that the collect instance plugin should not change order, we should add new plugin that adds the version (if it is necessary, we could just use |
Changelog Description
The
CollectInstanceDataplugin and its corresponding settings model have been renamed toCollectNukeInstanceDatafor better clarity and specificity within the Nuke context. This renaming ensures that the plugin's purpose is immediately apparent, reducing ambiguity and improving the overall maintainability of the codebase.The plugin's execution order has been adjusted to ensure that instance data is collected at the appropriate time. This adjustment helps prevent potential issues related to data dependencies and ensures that all necessary information is available when the plugin is executed.
Finally, the
sync_workfile_version_on_familiessetting has been renamed tosync_workfile_version_on_product_typesfor a more semantically correct representation of its function.Additional info
Settings change do not have backward compatibility check, because it was broken anyway for some time.
Testing notes:
sync_workfile_version_on_product_typessetting functions as expected.closes #111