Skip to content

Conversation

@pavithiraraj
Copy link

Changelog Description

This exports the anim data of the characters so when the next department picks up the file they will have the option to load the anim and continue animation

Additional review information

Have added collect_anim_curve so it exports the anim data and adds to the representation. And to load back the anim you can right click on the anim representation in the loader and click "Load Anim", if the reference is present it will add the anim data else it will reference the asset and add the anim data.
the anim data that is exported is in a json format, as the atom export was not able to load the anim if the ctrls are selected in a different order.

Testing notes:

  1. publish an layout or ra step in maya and load it using the loader.

@MustafaJafar MustafaJafar added type: enhancement Improvement of existing functionality or minor addition community Issues and PRs coming from the community members labels May 6, 2025
@MustafaJafar MustafaJafar requested review from BigRoy and antirotor May 6, 2025 07:10
@MustafaJafar
Copy link
Member

Could you rename this file collect_anim_curve.py to export_anim_curve.py

@MustafaJafar
Copy link
Member

Does anim export re-use an existing creator?

@pavithiraraj
Copy link
Author

Could you rename this file collect_anim_curve.py to export_anim_curve.py

Have renamed the files.

@pavithiraraj
Copy link
Author

Does anim export re-use an existing creator?

i dont get it, which existing creator?

@moonyuet
Copy link
Member

moonyuet commented Jun 3, 2025

Does anim export re-use an existing creator?

the animation instance would be automatically created by default when you load the referenced rig with AYON.

@moonyuet moonyuet self-requested a review June 3, 2025 11:26
staging_dir = self.staging_dir(instance)
filename = "{0}.anim".format(instance_data['variant'])
out_path = os.path.join(staging_dir, filename)
controls = [x for x in instance_data['setMembers'] if x.endswith(":rigMain_controls_SET")]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
controls = [x for x in instance_data['setMembers'] if x.endswith(":rigMain_controls_SET")]
controls = [x for x in instance_data['setMembers'] if x.endswith("controls_SET")]

Copy link
Member

@moonyuet moonyuet Jun 3, 2025

Choose a reason for hiding this comment

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

:rigMain_controls_SET is a bit hardcoded. The loaded rig product type can be with different variants, e.g. rigTest, rigTech_carA etc.


def load(self, context, name, namespace, data):
project_name = context['project']['name']
anim_file = context['representation']['attrib']['path']
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
anim_file = context['representation']['attrib']['path']
anim_file = self.filepath_from_context(context)

_plugin = ReferenceLoader()
_plugin.load(context=context, name=context['product']['name'],
namespace=name_space, options=data)
ctrl_set = pm.ls(name_space + ":rigMain_controls_SET")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
ctrl_set = pm.ls(name_space + ":rigMain_controls_SET")
ctrl_set = pm.ls(name_space + ":rigMain_controls_SET")

Keep in mind there is the loaded instance with different variant, e.g. rigTest_controls_SET, rigTechCarA_controls_SET

logger = logging.getLogger(__name__)


class AnimLoader(plugin.Loader):
Copy link
Member

@moonyuet moonyuet Jun 3, 2025

Choose a reason for hiding this comment

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

EDIT: Please take references from how the setdress and layout loader implemented, super helpful for your case.
SetDress: https://github.com/ynput/ayon-maya/blob/develop/client/ayon_maya/plugins/load/load_assembly.py
Layout: https://github.com/ynput/ayon-maya/blob/develop/client/ayon_maya/plugins/load/load_layout.py

Comment on lines 54 to 56
_plugin = ReferenceLoader()
_plugin.load(context=context, name=context['product']['name'],
namespace=name_space, options=data)
Copy link
Member

Choose a reason for hiding this comment

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

Comment on lines 40 to 52
asset_data = ayon_api.get_folder_by_name(project_name=project_name, folder_name=current_asset['asset_name'])
versions = ayon_api.get_last_version_by_product_name(project_name, "rigMain", asset_data['id'])
representations = ayon_api.get_representations(
project_name=project_name,
version_ids={versions['id']},
fields={"id", "name", "files.path"}
)
rep_id = None
for rep in representations:
if rep['name'] == 'ma':
rep_id = rep['id']
break
context = get_representation_context(project_name, rep_id)
Copy link
Member

@moonyuet moonyuet Jun 3, 2025

Choose a reason for hiding this comment

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

If you refer to the current representation data from the loaded asset

Suggested change
asset_data = ayon_api.get_folder_by_name(project_name=project_name, folder_name=current_asset['asset_name'])
versions = ayon_api.get_last_version_by_product_name(project_name, "rigMain", asset_data['id'])
representations = ayon_api.get_representations(
project_name=project_name,
version_ids={versions['id']},
fields={"id", "name", "files.path"}
)
rep_id = None
for rep in representations:
if rep['name'] == 'ma':
rep_id = rep['id']
break
context = get_representation_context(project_name, rep_id)
repre_entity = context["representation"]
rep_id = repre_entity["id"]
context = get_representation_context(project_name, rep_id)

EDIT: if you inherited from ReferenceLoader, it shouldn't not be included in the code, as the context got from the get_representation_context should be the same with the context

Comment on lines 65 to 67
self.log.debug(f"ctrls: {ctrls}")
self.log.debug(f"namespace: {namespace}")
self.log.debug(f"anim_file: {anim_file}")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
self.log.debug(f"ctrls: {ctrls}")
self.log.debug(f"namespace: {namespace}")
self.log.debug(f"anim_file: {anim_file}")

Comment on lines 30 to 34
name_space = context['representation']['data']['context']['product']['name'].replace(
context['representation']['data']['context']['product']['type'], '')
if not cmds.namespace(exists=name_space):
assets = context['version']['data'].get("assets", [])
self.log.info(f"assets: {assets}")
Copy link
Member

Choose a reason for hiding this comment

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

You can take reference from other loaders in /load folder to get the namespace

Suggested change
name_space = context['representation']['data']['context']['product']['name'].replace(
context['representation']['data']['context']['product']['type'], '')
if not cmds.namespace(exists=name_space):
assets = context['version']['data'].get("assets", [])
self.log.info(f"assets: {assets}")
folder_name = context["folder"]["name"]
namespace = namespace or lib.unique_namespace(
folder_name + "_",
prefix="_" if folder_name[0].isdigit() else "",
suffix="_",
)

return
current_asset = current_asset[0]
asset_data = ayon_api.get_folder_by_name(project_name=project_name, folder_name=current_asset['asset_name'])
versions = ayon_api.get_last_version_by_product_name(project_name, "rigMain", asset_data['id'])
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
versions = ayon_api.get_last_version_by_product_name(project_name, "rigMain", asset_data['id'])
versions = ayon_api.get_last_version_by_product_name(project_name, "rigMain", asset_data['id'])

We can have the product name with different variants, e.g. rigTest, rigTechCarA etc.

Comment on lines 36 to 40
representation = {
'name': 'anim', 'ext': 'anim',
'files': os.path.basename(out_path),
'stagingDir': staging_dir.replace("\\", "/")
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
representation = {
'name': 'anim', 'ext': 'anim',
'files': os.path.basename(out_path),
'stagingDir': staging_dir.replace("\\", "/")
}
representation = {
'name': 'anim',
'ext': 'anim',
'files': os.path.basename(out_path),
'stagingDir': staging_dir.replace("\\", "/")
}

Copy link
Author

Choose a reason for hiding this comment

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

This was starting to give errors so i removed this block to add representation, but still the anim got added to the published version, any idea as to why this is happening

Copy link
Member

Choose a reason for hiding this comment

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

This was starting to give errors so i removed this block to add representation, but still the anim got added to the published version, any idea as to why this is happening

The suggested change only does the cosmetic adjustment.
Can you enclose with the publish report to show what the error you have encountered?

Copy link
Author

Choose a reason for hiding this comment

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

i was getting that the representation already exists, so i tried renaming to anim_curve and that seems to create two entries
image

so once i removed it , it was back to normal

Copy link
Member

Choose a reason for hiding this comment

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

i was getting that the representation already exists, so i tried renaming to anim_curve and that seems to create two entries image

so once i removed it , it was back to normal

Sorry I am a little bit confused what you mean. Do you mean you have published two sets of representation data (with the anim-related and anim_curves-related publish respectively) while you just implement only one representation data to be published in this extractor?

I am wondering where anim_curves comes from, is it from another set of representation data with the anim as extension? It is supposed that you can get the published data with the name anim and with the extension anim in the code you showed here.

Copy link
Author

Choose a reason for hiding this comment

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

i am guessing that the publish code automatically adds the representation based on the instance, without the need to define it in the code.

Copy link
Member

@moonyuet moonyuet Jun 4, 2025

Choose a reason for hiding this comment

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

i am guessing that the publish code automatically adds the representation based on the instance, without the need to define it in the code.

The Extractor plugins you have inherited from MayaExtractorPlugin are also used for inheritance in other extractors(e.g. extract_obj.py etc). Please see the example here:

if "representation" not in instance.data:
instance.data["representation"] = []
representation = {
'name': 'obj',
'ext': 'obj',
'files': filename,
"stagingDir": staging_dir,
}
instance.data["representations"].append(representation)

They are all with the similar implementation of the representation data

It would be great if you can enclose the publish report by exporting report in Publisher UI and we can take the deep look to see where the errors come from.
image

Copy link
Author

Choose a reason for hiding this comment

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

publish-report-250604-16-31.json
this is the error report i get and this is what i get
image

Copy link
Contributor

Choose a reason for hiding this comment

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

Bottom left in publisher UI, click the clipboard icon.
Export the report to a json file. Share it here. That way we can debug the full publish process.

This error really feels like you're adding the representation twice. Which in your case is true, as your screenshot shows... because you seem to have two extractor plug-ins:

It has this twice:

  • Extract Animation curves
  • Extract Animation curves

Copy link
Author

Choose a reason for hiding this comment

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

I have attached it already
publish-report-250604-16-31.json
But i found the issue, we had a separate package to do the anim export for now, and seems that addon was added to the bundle. (feeling dumb)

if not len(static_name) > 1:
continue
static_name = static_name[1]
# static_name = static_chan.name().split(obj_shot_name + '.')[1]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# static_name = static_chan.name().split(obj_shot_name + '.')[1]

from pyblish.api import ExtractorOrder


class ExtractAnimCrv(plugin.MayaExtractorPlugin):
Copy link
Member

@moonyuet moonyuet Jun 3, 2025

Choose a reason for hiding this comment

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

Suggested change
class ExtractAnimCrv(plugin.MayaExtractorPlugin):
class ExtractAnimCurve(plugin.MayaExtractorPlugin):

@moonyuet
Copy link
Member

moonyuet commented Jun 3, 2025

@pavithiraraj Thank you for contribution, I double checked the code. I believe the best case to take reference for the loader is the setdress loader and the layout loader from the maya. Both of them read the data from json and applied it into the loaded asset. (Guess the anim data is similar to the json data as you used json.dumps)

@BigRoy do we need to avoid pymel? I remember at some point we have discussed to unionize the implementation and use import from maya.cmds instead.

Copy link
Contributor

@BigRoy BigRoy left a comment

Choose a reason for hiding this comment

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

So, I don't think this will end up being merged if it remains close to what it is now. However, the feature itself is great - so let's work towards making it mergable.

The issues;

  • Pymel dependendency. The code is now allowed to use pymel. so please avoid using it.
  • Custom animation data file format. This introduces a completely custom animation data file format - which I think is a bad thing. There are tools and formats out there that I'd rather rely on or mimic, like atom or matching e.g. studiolibrary export/import or even animbot's anim export/import. Preferably it's a freely available one, and bonus points if it's by default included with Maya... but rolling our own I think is just plain overkill, plus not good for any interchangeable efforts down the line.

hosts = ["maya"]

def process(self, instance):
instance_data = instance.data
Copy link
Contributor

Choose a reason for hiding this comment

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

Why? :)

project_name = context['project']['name']
anim_file = context['representation']['attrib']['path']
self.log.info(f"anim_file: {anim_file}")
name_space = context['representation']['data']['context']['product']['name'].replace(
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a comment or docstring to this loader as to what it's doing, how and why. Relying this heavily on a namespace feels very frowned upon.

I think you're much better applying the animation just to the target namespace, regardless of the source namespace. That way, if you'd load it twice... it would still work without much effort.

import ayon_api
from ayon_maya.api import plugin
from ayon_core.pipeline.load.utils import get_representation_context
from ayon_maya.plugins.load.load_reference import ReferenceLoader
Copy link
Contributor

Choose a reason for hiding this comment

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

Please do not explicitly import Loader plug-ins. These are dynamically loaded by the plugin system (similar to what pyblish does), and not cached in memory. By importing it we'll get into edge cases where you're referring to another ref loader than other code.

You're better of doing a get_loaders_by_name call (it's in ayon core) to get the loader that way. It at least finds it dynamically using the discover logic.

read_anim(filepath=anim_file, objects=ctrls)


def read_anim(filepath='C:/temp/anim.anim', objects=pm.selected(), namespace=None):
Copy link
Contributor

Choose a reason for hiding this comment

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

Default path should not be there ;)

Also, pm.selected() default value evaluates on function definition (when the code gets imported) and not on the function call itself.. .so it's not doing what you expect it to.

Comment on lines 11 to 12
logging.basicConfig(level=logging.DEBUG)
logger = logging.getLogger(__name__)
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to create a logger here - just pass the logger from the plug-in if you need one.

Comment on lines 42 to 51
representations = ayon_api.get_representations(
project_name=project_name,
version_ids={versions['id']},
fields={"id", "name", "files.path"}
)
rep_id = None
for rep in representations:
if rep['name'] == 'ma':
rep_id = rep['id']
break
Copy link
Contributor

Choose a reason for hiding this comment

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

You may just want to use ayon_api.get_representation_by_name

}
version_data["assets"].append(asset_data)
instance_data["data"] = [name_space + ':' + reference_node.path.__str__()]
instance.data["assets"] = [name_space + ':' + reference_node.path.__str__()]
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like arbitrary data?
Why not store the rig's representation id instead? So that on load, you're also loading the same matching rig production version (and representation). It'd greatly simplify loading too, because you can just directly load it from the representation id. :)

Copy link
Author

@pavithiraraj pavithiraraj Jun 4, 2025

Choose a reason for hiding this comment

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

That sound like a solid plan, how do i get the representation id from the instance?
is it the inputRepresentations in the instance.data ?

Copy link
Member

@moonyuet moonyuet Jun 4, 2025

Choose a reason for hiding this comment

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

That sound like a solid plan, how do i get the representation id from the instance? is it the inputRepresentations in the instance.data ?

You can take reference from extract layout:

for container, container_root in container_dict.items():
representation_id = cmds.getAttr(
"{}.representation".format(container))
# Ignore invalid UUID is the representation for whatever reason
# is invalid
if not is_valid_uuid(representation_id):
self.log.warning(
f"Skipping container with invalid UUID: {container}")
continue
# TODO: Once we support managed products from another project
# we should be querying here using the project name from the
# container instead.
representation = get_representation_by_id(
project_name,
representation_id,
fields={"versionId", "context", "name"}
)

@pavithiraraj
Copy link
Author

I have made most of the changes Can you have a look at it now, do you want me to change the way we write out the data, from a json to atom?
and do i need to remove pymel in the code?

@moonyuet
Copy link
Member

moonyuet commented Jun 5, 2025

I have made most of the changes Can you have a look at it now, do you want me to change the way we write out the data, from a json to atom? and do i need to remove pymel in the code?

@BigRoy and I suggest you refactor the extractor and the loader(to align what we did for the workflow in layout and setdress), besides the removal of pymel. But it requires some amount of time to do so. So take your time

@pavithiraraj
Copy link
Author

Ah ok , i will try and update this based on the layout loader, just curious, when they publish a next version, will the container know about the update? as we are loading the asset as a container and the anim data on top of that right?

@moonyuet
Copy link
Member

moonyuet commented Jun 5, 2025

Ah ok , i will try and update this based on the layout loader, just curious, when they publish a next version, will the container know about the update? as we are loading the asset as a container and the anim data on top of that right?

You can update your loaded asset through the scene inventory. You can find the hints from setdress/layout loader.

@antirotor antirotor requested a review from BigRoy October 17, 2025 12:54
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 type: enhancement Improvement of existing functionality or minor addition

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants