ayon-core Create Context: Per instance attributes and Callbacks

Changelog Description

Create context now allows to define attribute definitions per instance and allows to register callbacks listening to changes (e.g. on instances).

Additional info

Create plugin can define different attribute definitions on each instance, for example only different default values using get_attr_defs_for_instance. Publish plugins can define attribute definitions for any instance, and both instance and context plugins can define attribute definitions for both instances and context, so context plugin can define attribute definitions for instances. That can be achieved by implementing get_attr_defs_for_instance abd get_attribute_defs_for_context. Everything should be backwards compatible and work as before this PR.

Added callbacks allow to do adhoc live changes. That allows to create/publish plugin to change attribute definitions when a value changes. There is almost no limit to this usage. For example when a folder and task of instance A changes it is possible to propagate the same change to instances B, C, D. When create attribute changes value, publish attribute can be hidden or shown again. Registered callbacks are reset on each reset of CreateContext.

To register callbacks use listen_to_added_instances, listen_to_removed_instances, listen_to_value_changes, listen_to_pre_create_attr_defs_change, listen_to_create_attr_defs_change, listen_to_publish_attr_defs_change on CreateContext. To change create attributes use set_create_attr_defs, to update publish attributes use set_publish_plugin_attr_defs on CreatedInstance and to update context publish attributes use set_context_publish_plugin_attr_defs on CreateContext .

Callbacks are not targeted to the plugin, so plugin has to filter out what is relevant for him, if anything is relevant.

NOTE it is possible, but has caviads, the plugins must care that everything is set up correctly when they decide they do it. For example when enum will rename one value it should make sure the value kept selected on instances that did have it selected.

Also note that callbacks should not be used to live propagation to scene. The changes can be reverted if save_changes is not called on CreateContext.

Dev note

I did a lot of changes on the fly and maybe forgot some lines that might not be necessary or are undocumented, if you find those please comment in the code. I will try to cleanup it and fill in docstrings, but it won't hurt me if you mark something as "unclear", "confusing", "unnecessary". Any suggestions welcommed.

Traypublisher

Traypublisher is making issues at this moment as it changes window behavior and is using controller methods that should be removed but can't. We should remove option to change project in traypublisher and instead allow to change project only on start.

Testing notes

All changes are important mainly for publisher UI, so please choose DCC/host of your preference and use publisher.

All existing plugins should work as before.

None of new features are use in code so it requires to implement them to test hem out. If you'll find a plugin in ayon core that would benefit from these changes, we can change it in this PR as example.

This opens a lot of possibilities, so I need creative approaches and real life use-cases to be applied with this PR more than fake testing.

My testing steps using fake plugins

All tests were done in traypublisher.

Per instance attribute definitions

Added one create plugin and one publish plugin that randomly changes what attribute definitions are defined per instance and what are default values.
Opened traypublisher.
Created few instances using the creator and multiselect them.
It should show all of them, and those who have same definitions (except for default value) should be merged into one.
Changing value on them should work as before.

Callbacks

We can reuse plugins from previous step.
Implement register_callbacks on create plugin and classmethod register_create_context_callbacks on publish plugin.
Register callbacks based on you usage, you can use methods on create context listen_to_added_instances, listen_to_removed_instances, listen_to_value_changes, listen_to_pre_create_attr_defs_change, listen_to_create_attr_defs_change, listen_to_publish_attr_defs_change.
In you're callback you should be able to change attribute definitions or values of attributes on existin instances. You can also trigger refresh of pre-create attributes if needed. To change create attributes use set_create_attr_defs, to update publish attributes use set_publish_plugin_attr_defs on CreatedInstance. To change values of instance just use instance as dictionary.

Oct 03 '24 10:10 iLLiCiTiT

Task linked: AY-2420 Callbacks and groups with Publisher attributes

Oct 03 '24 10:10 ynbot

I scratched my head around this PR for some time. and I've few questions if you don't mind.

how to implement register_callbacks ? should we use ayon_core.lib.register_event_callback?
where to implement the callback functions?
how publish plugins are related to this PR?
could you push a branch with some examples. so that we can fetch locally and play with it?

Oct 03 '24 23:10 MustafaJafar

I scratched my head around this PR for some time. and I've few questions if you don't mind.

@MustafaJafar On any Publish plug-in that inherits from ayon_core.pipeline.publish.AYONPyblishPluginMixin add these methods:

    @classmethod
    def on_value_change(self, event):
        print(event)
        changes = event.data.get("changes", {})
        print(changes)

    @classmethod
    def register_create_context_callbacks(cls, create_context):
        create_context.listen_to_value_changes(cls.on_value_change)

Now start toggling attributes! :) You will get printed results.

The methods available on create context are:

listen_to_added_instances
listen_to_removed_instances
listen_to_value_changes
listen_to_pre_create_attr_defs_change
listen_to_create_attr_defs_change
listen_to_publish_attr_defs_change

However, that does raise the question - from that callback. How do I update the attribute definitions? :) Say I want to hide certain attributes or disable them?

The callbacks also trigger for plug-ins that are NOT related to active instances. Is that intentional? As in, say I have a plugin for families = ["neverexists"] they still get triggered.

Oct 03 '24 23:10 BigRoy

The callbacks also trigger for plug-ins that are NOT related to active instances. Is that intentional? As in, say I have a plugin for families = ["neverexists"] they still get triggered.

It is intentional. Everything should be in PR description.

Publish plugins can define attribute definitions for any instance, and both instance and context plugins can define attribute definitions for both instances and context

To change create attributes use set_create_attr_defs, to update publish attributes use set_publish_plugin_attr_defs on CreatedInstance. To change values of instance just use instance as dictionary.

Oct 04 '24 00:10 iLLiCiTiT

I have taken another look but I still can't seem to figure out how to do something simple like:

I currently have a plug-in that defines:



    @classmethod
    def get_attribute_defs(cls):

        return [
            BoolDef("enabled",
                    label="Enable",
                    default=True),
            BoolDef("A",
                    label="A",
                    default=True),
            BoolDef("B",
                    label="B",
                    default=True)
        ]

Now what I want to do is that whenever enabled is false on an instance - I want A and B either hidden or disabled in the UI. How do I do this?

I'm not sure how to update those per instance?

Oct 04 '24 11:10 BigRoy

Oh, I forgot to mention new classmethods get_attr_defs_for_instance and get_attr_defs_for_context on publish plugins.

EDITED: Added instance_matches_plugin_families method.

@classmethod
def get_attr_defs_for_instance(cls, create_context, instance):
    # Filtering of instance, if needed, can be customized
    if not cls.instance_matches_plugin_families(instance):
        return []

    # Attributes logic
    disabled = False
    if not instance["publish_attributes"].get(cls.__name__, {}).get("enabled", True):
        disabled = True
    return [
        BoolDef("enabled", label="Enable", default=True),
        BoolDef("A", label="A", default=True, disabled=disabled),
        BoolDef("B", label="B", default=True, disabled=disabled)
    ]

@classmethod
def on_value_change(cls, event):
    for instance_change in event["changes"];
        if not cls.instance_matches_plugin_families(instance):
            continue
        value_changes = instance_change["changes"]
        if "enabled" not in value_changes:
            continue
        instance = instance_change["instance"]
        new_attrs = cls.get_attr_defs_for_instance(
            event["create_context"], instance
        )
        instance.set_publish_plugin_attr_defs(cls.__name__, new_attrs)

Oct 04 '24 12:10 iLLiCiTiT

It works ✅

https://github.com/user-attachments/assets/2c246941-293e-463f-b732-a73849949488

Notes

I'm not completely convinced by the API for it yet. It's cumbersome and we'll definitely need helper functionality to streamline it over time because the code looks crazy. But what we need there can easily be developed and figured out over time.
When you change the disabled state of attribute definitions per instance - then the multiselection starts behaving unpredictably. Whether attributes are disabled or not is based on which of the instances you select first.
Not sure how, but I'm now often getting the UI in this state:

(The stop button is 'active' even though publishing isn't currently happening. It remains like this even after clicking reset. It should be greyed out.)

Oct 04 '24 15:10 BigRoy

I made three PRs that 'use' this feature as a demo:

https://github.com/ynput/ayon-fusion/pull/21
https://github.com/ynput/ayon-maya/pull/138
https://github.com/ynput/ayon-core/pull/943

That may help to discuss whether the API needs changing or is ok as a first step. I'll let maybe @antirotor join in from that perspective.
@MustafaJafar those PRs may help you to understand how to potentially use this?
And with a bit of luck @iLLiCiTiT can use those to feedback my code and make it even more streamlined/better.

Oct 06 '24 13:10 BigRoy

It seems that all your 'demo' implementation contain a lot of boiler plate code for on_values_changed. Maybe we could have default implementation that would trigger get/set attr_defs_for_instance by optional list of "triggerring" events in on_values_changed signature?

Oct 07 '24 11:10 kalisp

It seems that all your 'demo' implementation contain a lot of boiler plate code for on_values_changed. Maybe we could have default implementation that would trigger get/set attr_defs_for_instance by optional list of "triggerring" events in on_values_changed signature?

The API currently exposed and listened to is built so that you can listen to all events (even those from other instances). I agree that it makes the 'most used' cases contain quite elaborate code; yet at the same time we don't want to make it "too stupid" so that we can't allow that case.

As you described, potentially adding a filter to the adding of the callbacks which would just 'insert' a filtering function in between should be trivial do to.

E.g.

# pseudocode
def add_filtered_value_change_callback(self, callback, instances, attributes):

    def _callback(event):
        for event_change in event.data["changes"]:
            if event_change["instance"] not in instances:
                continue
            if not any(attrib in event_change["changes"] for attrib in attributes):
                continue
            # Event contains changes we care about
            callback(event)
            return

    self.create_context.add_value_changed_callback(_callback)

Or even just a add_attr_def_refresh_on_instance_value_changes_callback(self, attributes_filter) which just ends up forcing a refresh for an instance if it happens to have changes to the the attributes_filter.

Oct 07 '24 12:10 BigRoy

I didn't want to add the helper until I have some real use-cases.

For create plugins we might add simplified option to filter only changes related to instances of the create plugin, question is how, and if it has to be in this PR.

For publish plugins I'm not that sure. I think they'll use more complex logic for filtering than product type (implementation before this PR). The USD plugin for example, I am sure it will benefit if it would not be based on product type at all... And in that case it needs custom filtering, that has to be implemented on the plugin anyways, so why to add helper function that would actually not help "generally" as the plugin has to implement it anyways.

The pseudocode has few issues, for example you would have to be able to add instances to the filter when new is created, adding callback for each instance will make it uber slow, and event system does not support callbacks that are garbage collected. In case of passing filter function in, well you could use the filter function in the callback function (no benefit there).

So the question is, what is reasonable helper to filter events? For create plugin it would be helper to filter instances by create identifier(s), or just easier way to say "refresh instance attributes if value changes matching this filter function returns True". For publish plugins, I don't know. I don't have ambition to make super flexible dynamic filtering for events related to publish plugins, I would keep it minimalistic, and the complex situations has to be done in plugins that need the complexity, or wait for use-cases that are often repeated?

My end goal of this PR is to fix all necessary parts (like hidden and disabled attributes issues), and all other enhancements in different PRs.

Oct 07 '24 15:10 iLLiCiTiT

Ok, func fact. Publish attribute definitions were taken only from first instance, so it did magically work by an accident...

Oct 08 '24 12:10 iLLiCiTiT

Ok, func fact. Publish attribute definitions were taken only from first instance, so it did magically work by an accident...

So - what does that mean for the current state? It's broken, or?

Oct 08 '24 19:10 BigRoy

So - what does that mean for the current state? It's broken, or?

It is not ready to use in current state. But if you don't have completelly different attribute definitions per instance, then it is ok.

What might be broken:

One instance has one additional attribute that other instances don't have, or does not have the attribute, that others have.
If you change attribute that is disabled/hidden on instance B, but is shown because instance A has different states, the value will change on both if you multiselect them.
Different options in enum attribute would not be available -> this could actually cause error.

Oct 09 '24 08:10 iLLiCiTiT

Now it should be able to handle edge cases, maybe not "the nicest and clearest way", but it is possible. I would not bother until we hit issues related to that.

I had to add some implementations to attribute definitions to make it possible.

Oct 09 '24 13:10 iLLiCiTiT

Issue reported by @BigRoy . Publish buttons are disabled after reset, seems like it is related to instance context validation.

Oct 11 '24 12:10 iLLiCiTiT

Discovered few more issues. Labels of instances were not updating and change of active state on instances was not propagated. Should be resolved now.

Oct 14 '24 16:10 iLLiCiTiT

Should be fixed.

Oct 21 '24 09:10 iLLiCiTiT