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

Conversation

ClementHector
Copy link
Contributor

@ClementHector ClementHector commented Jun 27, 2023

Changelog Description

New PR that brings together the work of:
#4793
#4996
The two PR is no longer functional, it now works with the latest update_frame_range changes.

Testing notes:

  1. Open Maya Scene with wrong frame range in instances
  2. Click set frame range button to solve this problem
    Or
  3. Build scene with Build workfile template
  4. See all instances with the right frame range

@ynbot ynbot added host: Maya size/S Denotes a PR changes 100-499 lines, ignoring general files labels Jun 27, 2023
Author: Thomas Fricard <[email protected]>
Date:   Mon May 15 17:12:13 2023 +0200

    cosmetic refactoring

commit 6fcb0a6
Author: Thomas Fricard <[email protected]>
Date:   Mon May 15 16:55:19 2023 +0200

    check id attribute before family attribute

commit df6cc8c
Author: Thomas Fricard <[email protected]>
Date:   Fri May 12 16:01:52 2023 +0200

    verify that 'family' attributes exists and equals to 'render'

commit 5ae1847
Author: Thomas Fricard <[email protected]>
Date:   Fri May 12 15:58:05 2023 +0200

    change function name + docstrings

commit ff09bbb
Author: Thomas Fricard <[email protected]>
Date:   Fri May 12 11:48:43 2023 +0200

    add a 'type' key to the 'include handles' setting of maya

commit ec96832
Author: Thomas Fricard <[email protected]>
Date:   Wed May 3 15:32:06 2023 +0200

    merge and refactor functions to updte asset frame range

commit 661f494
Author: Thomas Fricard <[email protected]>
Date:   Tue Apr 18 10:03:24 2023 +0200

    add 'instances' to reset_frame_range function arguments and make sure it does not update frame range instances on creating render settings

commit a10ecfc
Merge: 19836b3 196f698
Author: Thomas Fricard <[email protected]>
Date:   Tue Apr 18 09:32:32 2023 +0200

    Merge branch '346-fix-studio-openpype-udpate-frame-range-of-scene-doesn-t-update-frame-range-in-animation-instance' of github.com:quadproduction/OpenPype into 346-fix-studio-openpype-udpate-frame-range-of-scene-doesn-t-update-frame-range-in-animation-instance

commit 19836b3
Author: Thomas Fricard <[email protected]>
Date:   Tue Apr 18 09:31:27 2023 +0200

    change key and label to make the setting more understandable

commit 196f698
Merge: 155a0c2 515c0ac
Author: Thomas Fricard <[email protected]>
Date:   Thu Apr 13 11:06:26 2023 +0200

    Merge branch 'develop' into 346-fix-studio-openpype-udpate-frame-range-of-scene-doesn-t-update-frame-range-in-animation-instance

commit 155a0c2
Author: Thomas Fricard <[email protected]>
Date:   Thu Apr 13 10:57:53 2023 +0200

    resolve conflicts

commit 6a6c9e0
Author: Thomas Fricard <[email protected]>
Date:   Wed Apr 12 14:07:19 2023 +0200

    check if update instances is enabled in settings

commit 9211c4a
Author: Thomas Fricard <[email protected]>
Date:   Wed Apr 12 13:58:40 2023 +0200

    add a setting in maya project settings to enable updating animation instances

commit fde0b50
Author: Thomas Fricard <[email protected]>
Date:   Tue Apr 11 16:58:37 2023 +0200

    check if attribute exsits before modifying it

commit 38eed31
Author: Thomas Fricard <[email protected]>
Date:   Wed Apr 5 15:26:04 2023 +0200

    update animation instance frames attribute on reset frame range

commit dc47ca9
Author: Thomas Fricard <[email protected]>
Date:   Fri Mar 31 15:56:19 2023 +0200

    get instances and set frameStart attribute

commit 0932341
Author: Thomas Fricard <[email protected]>
Date:   Wed May 17 12:47:17 2023 +0200

    call reset_frame_range function for instances after importing

commit 4de8e6e
Author: Thomas Fricard <[email protected]>
Date:   Mon May 15 17:12:13 2023 +0200

    cosmetic refactoring

commit efe2dbb
Author: Thomas Fricard <[email protected]>
Date:   Mon May 15 16:55:19 2023 +0200

    check id attribute before family attribute

commit 2379c5c
Author: Thomas Fricard <[email protected]>
Date:   Fri May 12 16:01:52 2023 +0200

    verify that 'family' attributes exists and equals to 'render'

commit 3a57274
Author: Thomas Fricard <[email protected]>
Date:   Fri May 12 15:58:05 2023 +0200

    change function name + docstrings

commit 7f68109
Author: Thomas Fricard <[email protected]>
Date:   Fri May 12 11:48:43 2023 +0200

    add a 'type' key to the 'include handles' setting of maya

commit 58fb563
Author: Thomas Fricard <[email protected]>
Date:   Wed May 3 15:32:06 2023 +0200

    merge and refactor functions to updte asset frame range

commit 49401c7
Author: Thomas Fricard <[email protected]>
Date:   Tue Apr 18 10:03:24 2023 +0200

    add 'instances' to reset_frame_range function arguments and make sure it does not update frame range instances on creating render settings

commit 8c0c60d
Author: Thomas Fricard <[email protected]>
Date:   Tue Apr 18 09:31:27 2023 +0200

    change key and label to make the setting more understandable

commit bdcdab2
Author: Thomas Fricard <[email protected]>
Date:   Thu Apr 13 10:57:53 2023 +0200

    resolve conflicts

commit 9040302
Author: Thomas Fricard <[email protected]>
Date:   Wed Apr 12 14:07:19 2023 +0200

    check if update instances is enabled in settings

commit cd884e8
Author: Thomas Fricard <[email protected]>
Date:   Wed Apr 12 13:58:40 2023 +0200

    add a setting in maya project settings to enable updating animation instances

commit 981c0b1
Author: Thomas Fricard <[email protected]>
Date:   Tue Apr 11 16:58:37 2023 +0200

    check if attribute exsits before modifying it

commit 1755ce4
Author: Thomas Fricard <[email protected]>
Date:   Wed Apr 5 15:26:04 2023 +0200

    update animation instance frames attribute on reset frame range

commit fdf6f43
Author: Thomas Fricard <[email protected]>
Date:   Fri Mar 31 15:56:19 2023 +0200

    get instances and set frameStart attribute
@ClementHector ClementHector force-pushed the 483-merge-feature-345-and-389-to-set-frame-range-on-animation-instance branch from d401984 to 92f5046 Compare June 27, 2023 18:23
@ClementHector ClementHector changed the title 483 merge feature 345 and 389 to set frame range on animation instance 483 merge feature 345 and 389 to set frame range on instances Jun 27, 2023
@ClementHector ClementHector changed the title 483 merge feature 345 and 389 to set frame range on instances Merge feature from #345 and #389 to set frame range on instances Jun 27, 2023
@ClementHector ClementHector changed the title Merge feature from #345 and #389 to set frame range on instances Merge feature from #4793 and #4996 to set frame range on instances Jun 28, 2023
@ClementHector ClementHector marked this pull request as ready for review June 28, 2023 08:47
@tokejepsen
Copy link
Member

Tested in Maya 2023 successfully. Code looks good to me.

@tokejepsen tokejepsen self-requested a review June 28, 2023 09:11
@tokejepsen tokejepsen assigned tokejepsen and antirotor and unassigned tokejepsen Jun 28, 2023
Copy link
Contributor

@Minkiu Minkiu left a comment

Choose a reason for hiding this comment

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

Haven't tested it, but code looks good.

openpype/hosts/maya/api/lib.py Outdated Show resolved Hide resolved
openpype/hosts/maya/api/lib.py Outdated Show resolved Hide resolved
openpype/hosts/maya/api/lib.py Outdated Show resolved Hide resolved
openpype/hosts/maya/api/lib.py Outdated Show resolved Hide resolved
openpype/hosts/maya/api/workfile_template_builder.py Outdated Show resolved Hide resolved
openpype/hosts/maya/api/lib.py Outdated Show resolved Hide resolved
Comment on lines 260 to 261
update_instances_frame_range()
update_instances_asset_attribute()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are we sure this is in the right place. cleanup_placeholder sounds like it runs per placeholder instead of at the very end of workfile build. We only want to run this once, right? Not every single time.

I feel like this is probably in the wrong place.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well seen, it is true that it is executed at each placeholder. I think we should move this to here with a post_populate_scene_placeholders method but we don't can't call maya commands here, maybe we should implement instance-related commands in host class? maybe it doesn't make sense with the new publisher (which I don't know)?
Tell me what would make more sense.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel like the abstract workfile builder is missing some methods that trigger before the build and after the build. We could expose a after_build_process() and pre_build_process() so that each DCC could implement its own callbacks that run before or after ALL placeholders. Not sure if the best approach and whether it's really missing - but couldn't quickly find a feature that does that.

Another way could be for the workfile building to emit events emit_event("workfile.before.build") and emit_event("workfile.after.build") so that any module could attach callbacks to the events using register_event_callback - which the host itself could do e.g. on install() etc.

Not sure which would be the best approach. @iLLiCiTiT @tokejepsen @antirotor thoughts?

@ClementHector ClementHector force-pushed the 483-merge-feature-345-and-389-to-set-frame-range-on-animation-instance branch from b6e955f to 1c19860 Compare July 6, 2023 12:07
Comment on lines +3169 to +3198
def update_instances_frame_range():
"""Update 'frameStart', 'frameEnd', 'handleStart', 'handleEnd' and 'fps'
attributes of publishable instances (their objectSets) that got one.
"""

attributes = ["frameStart", "frameEnd", "handleStart", "handleEnd", "fps"]

attrs_per_instance = {}
for instance in iter_publish_instances():
instance_attrs = [
attr for attr in attributes
if cmds.attributeQuery(attr, node=instance, exists=True)
]

if instance_attrs:
attrs_per_instance[instance] = instance_attrs

if not attrs_per_instance:
# no instances with any frame related attributes
return

fields = ["data.{}".format(key) for key in attributes]
asset_doc = get_current_project_asset(fields=fields)
asset_data = asset_doc["data"]

for node, attrs in attrs_per_instance.items():
for attr in attrs:
plug = "{}.{}".format(node, attr)
value = asset_data[attr]
cmds.setAttr(plug, value)
Copy link
Collaborator

Choose a reason for hiding this comment

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

One minor note about this - say you have an instance that does not match the current shot. :) Would we want it to reset to the fream range of the asset it's publishing to, or the one of the currently publishing from?

Currently it does the latter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this case it seems that the option which deactivates the update on the instances is not sufficient! Should this option be a filter on the name of the instances?

Copy link
Collaborator

Choose a reason for hiding this comment

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

In this case it seems that the option which deactivates the update on the instances is not sufficient! Should this option be a filter on the name of the instances?

Sorry, could you elaborate? Not sure what you mean with it being not sufficient?

@iLLiCiTiT
Copy link
Member

I have maybe stupid comment, but isn't this PR outdated since this PR #4388 ?

@BigRoy
Copy link
Collaborator

BigRoy commented Jul 13, 2023

I have maybe stupid comment, but isn't this PR outdated since this PR #4388 ?

Yes, I think so. Some of the logic still makes sense to keep - like updating the frame ranges, etc. but likely will need a decent overhaul to work with the new Creator system instead.

@tokejepsen tokejepsen marked this pull request as draft July 17, 2023 16:46
@tokejepsen
Copy link
Member

@ClementHector what is the status of this PR? Are you using the new Creator/Publisher?

@mkolar
Copy link
Member

mkolar commented Jan 25, 2024

@ClementHector we're closing this PR as it is now stale and obsolete. If there is any functionality from this you'd like to contribute again, feel free to re-open this again, targeting AYON

@mkolar mkolar closed this Jan 25, 2024
@ynbot ynbot added this to the next-patch milestone Jan 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
community contribution host: Maya size/S Denotes a PR changes 100-499 lines, ignoring general files
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

8 participants