-
Notifications
You must be signed in to change notification settings - Fork 129
Maya: Update frame range of instances when clicking on "Reset frame range" #4793
Conversation
Interesting! What is the use case for this? |
I agree with @tokejepsen - this doesn't seem to be the correct intended behavior. Also, be aware that the logic will not work for all instances. There will be instances who do not have frame range attributes. There will also be instances that are time-dependent but don't expose their time settings on the instance (e.g. renderlayers). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned I suspect this PR won't get merged since it doesn't seem to be the intended behavior of the method. However, here are some purely code based notes.
Hello, They will use the reset frame range to get the last frame range from production tracking. This will get change for the frame in / out / handle of the scene itself but not on the exported entities (animation, camera , etc ) If publish as is, the sanity check is going to complain that things are not right. Since we are already doing something (reset frame range) regarding the time we could ensure that all the entities that are dependent of time in the scene are changed to the correct known value. Otherwise, reset frame range should not be there and all should be repaired/ fix from the sanity check. |
This validator Validate Frame Range, should fail if the instance frame range is out of sync with the asset. Does this not happen for your pipeline? |
It does happen ! but we could make it faster to publish scene by changing also the instance to what we know to be the correct value when we use the "reset frame range" in the openpype menu. That's for sure a "comfort" request, that lead to less complaints of artist and more adhesion of pipeline |
I understand the PR and definitely understand the reasoning too. However, we tend to have many instances which by choice do not adhere to the asset's full frame range. We might cache only a part of e.g. a character which is only visible the first 20 frames of a shot as an optimization (plus also more explicit information about what part is actually animated for e.g. an FX artist) - this would then suddenly override it. So we would disable this feature on our end since it goes against a lot of our actually intended frame differences between instances. It's interesting actually that the artists you mention do click "Reset frame range" more frequently then getting caught in validations during publishing. Clicking "Reset frame range" is exactly something an artists tends to forget unless someone's told them to update their shot's frame range. Purely out of curiosity, when do your artists tend to click on the button? When they are explicitly told to sync frame ranges? Or? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@friquette this feature will need to be behind a setting
Hello, And for sure we don't optimise cache time for exemple, we aim for stability (when the character is not visible there may be shadow, etc etc ) so we export always the full range with a little more even for motion blur raison |
@BigRoy what you think of this? This feature is behind a setting, so studios need to opt in now. I would also suggest to maybe widen the scope of this to no just |
ae3f9f7
to
155a0c2
Compare
Looks great to me. We would have it disabled but I can see it being useful for some. We might want to add a
It currently actually not only applies to The logic wouldn't work for resetting frame ranges of renderlayer instances, because those attributes aren't on the renderlayer instance but set in the scene with potential renderlayer overrides, etc.
That would make sense. It's good to note however that #4388 will already do that since there you'd get the instances from the Create Context instead. So I'm wondering whether it's worth it now. This also means that this feature will need refactoring in that PR to continue to work. |
@friquette could you implement this?
|
As far as I know it already works for ANY instance that has the frame attributes - it doesn't seem to check anything related to animation instances or alike. |
Yup, you are right. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general this makes pretty good sense in certain workflows. I'm not sure it actually add a whole lot of value compared to keeping identical functionality in the validators repair, but I have no major problem with it.
openpype/settings/entities/schemas/projects_schema/schema_project_maya.json
Outdated
Show resolved
Hide resolved
openpype/settings/entities/schemas/projects_schema/schema_project_maya.json
Outdated
Show resolved
Hide resolved
openpype/settings/entities/schemas/projects_schema/schema_project_maya.json
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add keyword instances=True
to reset_frame_range
and make sure it does not run on creating render settings.
@friquette is this ready to test? |
Yes it is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Getting this error when opening the settings:
Traceback (most recent call last):
File "C:\Users\tokejepsen\OpenPype\openpype\tools\settings\settings\categories.py", line 564, in reset
self._create_root_entity()
File "C:\Users\tokejepsen\OpenPype\openpype\tools\settings\settings\categories.py", line 979, in _create_root_entity
entity = ProjectSettings(
File "C:\Users\tokejepsen\OpenPype\openpype\settings\entities\root_entities.py", line 654, in __init__
super(ProjectSettings, self).__init__(schema_hub, reset)
File "C:\Users\tokejepsen\OpenPype\openpype\settings\entities\root_entities.py", line 76, in __init__
self._item_initialization()
File "C:\Users\tokejepsen\OpenPype\openpype\settings\entities\root_entities.py", line 202, in _item_initialization
self._add_children(self.schema_data)
File "C:\Users\tokejepsen\OpenPype\openpype\settings\entities\root_entities.py", line 172, in _add_children
child_obj = self.create_schema_object(children_schema, self)
File "C:\Users\tokejepsen\OpenPype\openpype\settings\entities\root_entities.py", line 231, in create_schema_object
return self.schema_hub.create_schema_object(
File "C:\Users\tokejepsen\OpenPype\openpype\settings\entities\lib.py", line 358, in create_schema_object
return klass(schema_data, *args, **kwargs)
File "C:\Users\tokejepsen\OpenPype\openpype\settings\entities\base_entity.py", line 916, in __init__
self._item_initialization()
File "C:\Users\tokejepsen\OpenPype\openpype\settings\entities\dict_immutable_keys_entity.py", line 196, in _item_initialization
self._add_children(self.schema_data)
File "C:\Users\tokejepsen\OpenPype\openpype\settings\entities\dict_immutable_keys_entity.py", line 163, in _add_children
child_obj = self.create_schema_object(children_schema, self)
File "C:\Users\tokejepsen\OpenPype\openpype\settings\entities\base_entity.py", line 962, in create_schema_object
return self.schema_hub.create_schema_object(*args, **kwargs)
File "C:\Users\tokejepsen\OpenPype\openpype\settings\entities\lib.py", line 358, in create_schema_object
return klass(schema_data, *args, **kwargs)
File "C:\Users\tokejepsen\OpenPype\openpype\settings\entities\base_entity.py", line 916, in __init__
self._item_initialization()
File "C:\Users\tokejepsen\OpenPype\openpype\settings\entities\dict_immutable_keys_entity.py", line 196, in _item_initialization
self._add_children(self.schema_data)
File "C:\Users\tokejepsen\OpenPype\openpype\settings\entities\dict_immutable_keys_entity.py", line 146, in _add_children
children_schemas = self.schema_hub.resolve_schema_data(
File "C:\Users\tokejepsen\OpenPype\openpype\settings\entities\lib.py", line 308, in resolve_schema_data
schema_type = schema_data["type"]
KeyError: 'type'
Just noticed that there's actually logic implemented that is VERY CLOSE to this PR here but that's only triggered on task changed. We could technically merge/clean up that functionality and also use it in reset frame range. Also, according to the reset frame range logic implemented here I think that might be setting the wrong frame ranges for some things if handles needed to be included? It's not setting start and end handles for example. Anyway, might be good to merge/cleanup since the logic is pretty much a duplicate. |
Good spot! Yeah, lets merge the logic/code. |
@friquette could you have a look at @BigRoy feedback here, then we can merge this PR? |
17f1f3b
to
661f494
Compare
… it does not update frame range instances on creating render settings
17e0080
to
5834a0f
Compare
…e-of-scene-doesn-t-update-frame-range-in-animation-instance
Can anyone confirm that this works to be merged? |
I was actually ready to merge it, but tons of commits came after the last approval, so we need to try it again. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor docs request.
Tested successfully in Maya 2023.
@BigRoy what do you think of the code? |
Co-authored-by: Roy Nieterau <[email protected]>
…e-of-scene-doesn-t-update-frame-range-in-animation-instance
New PR cleaner here: #5201 |
Changelog Description
Update "start frame", "end frame", "handle start" and "handle end" extra attributes of animation instances when using the "Reset frame range" of OP in Maya.