ynput / ayon-core

Apache License 2.0
27 stars 32 forks source link

Chore: Houdini refactor to generic ExtractROP plug-in #676

Closed BigRoy closed 3 months ago

BigRoy commented 3 months ago

Changelog Description

Improve code re-use and implement a more generic ROP node extractor used by those families for which it applies.

This makes sure that they remain sync in implementation instead of being scattered and helps focusing the implementation on this single plug-in instead simplifying maintenance.

Additional info

This also implements #675 but also generalizes it into a single plug-in instead. (with some subclasses where code deviated)

Fixes https://github.com/ynput/ayon-core/issues/672

Testing notes:

  1. Publishing all the product types should work for single frames and multiple frames:
  1. [x] The above families that support it also work for farm submissions.
    • note: pointcache farm submission is broken, but that's also in develop
MustafaJafar commented 3 months ago

This is cool! I believe implementing render target with other products will help generalizing the extraction.

MustafaJafar commented 3 months ago

I gave it a shot. tested few product types and they worked fine. Also, out of curiosity, I tried to publish pointcache to farm, I got this error

Traceback (most recent call last):
  File "C:\Users\Mustafa Taher\AppData\Local\Ynput\AYON\dependency_packages\ayon_2406142259_windows.zip\dependencies\pyblish\plugin.py", line 527, in __explicit_process
    runner(*args)
  File "E:\Ynput\ayon-core\server_addon\deadline\client\ayon_deadline\plugins\publish\submit_publish_cache_job.py", line 285, in process
    instances = create_instances_for_cache(
  File "E:\Ynput\ayon-core\client\ayon_core\pipeline\farm\pyblish_functions.py", line 942, in create_instances_for_cache
    raise ValueError("Found multiple non related files "
ValueError: Found multiple non related files to render, don't know what to do with them.
BigRoy commented 3 months ago

I gave it a shot. tested few product types and they worked fine. Also, out of curiosity, I tried to publish pointcache to farm, I got this error

Traceback (most recent call last):
  File "C:\Users\Mustafa Taher\AppData\Local\Ynput\AYON\dependency_packages\ayon_2406142259_windows.zip\dependencies\pyblish\plugin.py", line 527, in __explicit_process
    runner(*args)
  File "E:\Ynput\ayon-core\server_addon\deadline\client\ayon_deadline\plugins\publish\submit_publish_cache_job.py", line 285, in process
    instances = create_instances_for_cache(
  File "E:\Ynput\ayon-core\client\ayon_core\pipeline\farm\pyblish_functions.py", line 942, in create_instances_for_cache
    raise ValueError("Found multiple non related files "
ValueError: Found multiple non related files to render, don't know what to do with them.

Lovely! I've never tried the farm submissions. Does it work outside of this PR? (I suppose not?)

BigRoy commented 3 months ago

@moonyuet any chance you can also push this through a test-run?

moonyuet commented 3 months ago

@moonyuet any chance you can also push this through a test-run?

I try with publishing fbx. Errored out. image

Successfully submitted mantra ifd(local and farm) image

image image

BigRoy commented 3 months ago

I try with publishing fbx. Errored out.

Fixed with https://github.com/ynput/ayon-core/pull/676/commits/187b2e98c53e49819c807eab9829f436b16af676

moonyuet commented 3 months ago

There is also an issue with camera publishing image pointcache local publish ok Encounter same issue with mustafa when testing pointcache farm publish

BigRoy commented 3 months ago

There is also an issue with camera publishing

Fixed with https://github.com/ynput/ayon-core/pull/676/commits/32ebf9c34f91e8f4cc88ed6f98d97c2d6d600fd5 - sorry about that. I kept the extractor list next to it and I still missed that it lacked camera. Just proves once again that I should always rely on the computer to tell me the differences between the lists. :D

Encounter same issue with mustafa when testing pointcache farm publish

I have a feeling this issue may exist outside of this PR as well - I don't have Deadline configured on my dev setup currently. Anyone of you able to confirm this quickly using develop?

moonyuet commented 3 months ago

There is also an issue with camera publishing

Fixed with 32ebf9c - sorry about that. I kept the extractor list next to it and I still missed that it lacked camera. Just proves once again that I should always rely on the computer to tell me the differences between the lists. :D

Encounter same issue with mustafa when testing pointcache farm publish

I have a feeling this issue may exist outside of this PR as well - I don't have Deadline configured on my dev setup currently. Anyone of you able to confirm this quickly using develop?

I tested with the develop branch, guess the issue of the pointcache is not related to this PR. But the camera publishing works with the branch.

BigRoy commented 3 months ago

I'd say looking at the tests this should be safe to merge.