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

AfterEffects: refactored integrate doesnt work formulti frame publishes #3610

Conversation

kalisp
Copy link
Member

@kalisp kalisp commented Aug 3, 2022

Brief description

Frame pattern used in new integrate is not catching frames pattern in AE

Description

Locally rendered files are renamed into frame pattern that integrator plugin understands.

Additional info

(Added Kuba and Ondrej as reviewers to only highlight this.)
Added render.farm for AE into Settings to skip integrating. New publisher should have 'render' as a primary family (and 'render.farmin families), which would trigger integrating (legacy integrator has 'render.farm' inexclude_families` , this functionality is currently missing in refactored integrator).

I think we should keep skip_host_families not as a temporary solution to broken publishes for new integrator, but use it instead of 'exclude_families' which might be always useful.

Testing notes:

  1. Publish locally in AE
  2. Publish on farm in AE

New publisher expects frames in file names in '.0000.' format, AE by default provides ('_0000.'). Locally rendered files need to be renamed to appropriate format.
Previous test published only single frame, didn't catch issue in new integrate.
New publisher requires main family as 'render', so there will be need to skip 'render.farm' which should not be integrated during initial publish. (Currently only affecting AE.)
@kalisp kalisp added type: bug Something isn't working host: After Effects labels Aug 3, 2022
@kalisp kalisp self-assigned this Aug 3, 2022
@ynbot
Copy link
Contributor

ynbot commented Aug 3, 2022

@@ -167,7 +167,7 @@ class IntegrateAsset(pyblish.api.InstancePlugin):
skip_host_families = []

def process(self, instance):
if self._temp_skip_instance_by_settings(instance):
if self.skip_instance_by_settings(instance):
Copy link
Member

Choose a reason for hiding this comment

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

why the rename?

@iLLiCiTiT
Copy link
Member

iLLiCiTiT commented Aug 3, 2022

I think we should keep skip_host_families not as a temporary solution to broken publishes for new integrator, but use it instead of 'exclude_families' which might be always useful.

That was removed on purpose and was replaced by having instance.data["farm"] = True (you've added it...). If it should be added back then it should not be in settings but handled on purpose. It was removed to reduce responsibility of the integrator because it's hard to determine which families should not be integrated across all hosts in single plugin.

BTW: Current settings for that won't cause that instance is not integrated but will cause that legacy integrator is used instead.

@iLLiCiTiT
Copy link
Member

iLLiCiTiT commented Aug 3, 2022

With PR #3611 this PR is not needed

…farm"

This reverts commit ef60744
Not necessary, better to use `instance.data["farm"]`
No Settings necessary, instance itself should hold if it is targetted for farm (eg. not locally integrated.)
This reverts commit 80b6ef9

Made obsolete by #3611
…-3684_Refactored-integrate-doesnt-work-for-AE-multi-frame-publishes
@kalisp
Copy link
Member Author

kalisp commented Aug 3, 2022

This PR is still useful for new testing class and for farm publishes.

@64qam 64qam self-requested a review August 3, 2022 15:56
@kalisp kalisp merged commit 23c927c into develop Aug 3, 2022
@kalisp kalisp deleted the bugfix/OP-3684_Refactored-integrate-doesnt-work-for-AE-multi-frame-publishes branch August 3, 2022 16:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
host: After Effects type: bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants