Implement #474

[davidlatwe]

This PR is aimed to implement #474, I'll try to be as clear as possible.

What's changed

1. Fixed a bug in lib.reset_selection (919f1ca)
2. Set logging message level to INFO (56f55fc)
3. Implemented lsattr and lsattrs for finding nodes by knob and value (1ff01fa)
4. Re-implemented subset containerizing (9df1197, 75e82c8, 3c685d0, 134b4cd, 6a11ea7)
5. Implemented node update/override system (c84473b)
6. Changed models.TreeModel and sceneinventory.app for Qt4 compat (98d3562)
7. Keep old style containerize and ls by env var AVALON_NUKE_OLD_CONTAINER (bd88c1a, a19b066)
8. Replaced lib.imprint, lib.read with vendorized knobby (05b5086, f217fa4, 875f83f)
9. Removed avalonDataGroup knob from Avalon Tab (f11e595)

lsattr, lsattrs

These two functions have three extra args compares to other hosts, type=None, group=None and recursive=False. These three args will be passed into nuke.allNodes for defining the node search range.

Subset containerizing

The new workflow of subset containerizing, will keep a copy (not clone) of loaded subset inside a AVALON_CONTAINERS group node.

Since the new containerise and ls of this workflow is not backwards compatible, you have to set AVALON_NUKE_OLD_CONTAINER in environment so the old style will be enabled.

But I am not sure about the name of that environment variable, so any suggestion is welcome.

Node update/override system

Since the subset will be containerized, we have at least one copy of subset being presented outside of main container for artist to operate on. So we need a way to update all copies when updating subset.

This feature was mentioned here, the implementation is a context manager lib.sync_copies.

Every nodes that being containerized will have avalonId and containerId imprinted, the containerId is the container's avalonId. And we look for copies by matching node type and avalonId.

To use it in Loader plugin, like containerise in Loader.load, you would call it in Loader.update, for example:

class Loader(avalon.api.Loader):

    def update(self, container, representation):

        with lib.sync_copies(nodes):
            # Update subset
            for node in nodes:
                self.update_file_path(node)
                ...
        # All copies of `nodes` are auto updated by matching `avalonId`

        with lib.sync_copies([container_node], force=True):
            # Update container data
            ...
        # All copies of `container_node` updated

Vendoring knobby

There were some issues in previous implementation of knob imprinting, like:

1. Creating duplicate named knob without raising any error from Nuke
2. Knob name prefix being an option on imprint might not good for read

I have tried to improve and resolved, but the code was a bit complicate for being in lib. Then I thought maybe to move it out as a standalone module would be better. If you don't like it to be in nuke.vendor, I would love to move it back.

So what knobby will do for us ? TL;DR

1. It will merge new and old knobs then re-build all user knobs if the imprinting tab existed.
2. Knob will be auto prefixed with it's tab name.
3. New method mold is like read, but will preserve the tab hierarchy.

I think this is much stable and should be a bit more straight forward when writing/reading data to/from node than previous implementation.

Drop avalonDataGroup knob

I though the Avalon data tab could be simplified, so I have removed it from set_avalon_knob_data. If you don't mind.

Qt4 support

I was tested in Nuke 9 which still using PySide, so I had encountered some errors while using scene inventory app. Fixed them all together.

---

Please have a test if you could. :)

[tokejepsen]

Looks very good!

Since the new containerise and ls of this workflow is not backwards compatible, you have to set AVALON_NUKE_CONTAINERS_AT_LARGE in environment so the old style will be enabled.

I was wondering whether this is necessary?
I know backward compatibility is a must, but the Nuke implementation for Avalon seems somewhat in a beta stage where we are still ironing things out, so the question is whether enough people are using it to warrant the added complexity of this.

[davidlatwe]

Yeah, it's true that Nuke's implementation in Avalon official release should be considered as beta stage.

The main reason I add that environment variable was actually for pypeclub implementation, so they could pick up without to much trouble. (I hope !)

Would be great if we don't need that. ☺️
Pinning @jezscha 📞 , is that okay if we drop the old containerization implementation ?

[BigRoy]

What's the status of this long open PR? :) Should this be ready to merge or are there reasons not to?

[davidlatwe]

Should this be ready to merge or are there reasons not to?

Here in my studio, the Avalon-Nuke implementation was just started to run in production, so I am not sure about the implementation of AVALON_CONTAINER is good enough or not (exposing it in the node graph). Also, not sure how this PR will compat with the implementation that Pype have.

So I guess we might need to wait a bit. 🤔

[mkolar]

To be perfectly honest, we didn't find time to test this properly. Code wise it looks very nice, but we were swamped and couldn't afford the time to test. I'm truly hoping that next week we can give it a day.

[tokejepsen]

Testing this PR out atm, and will dump any thoughts and feedback here.

[tokejepsen]

I think the backdrop workflow within the AVALON_CONTAINERS group could use some documentation and code. For example as a developer how do I get the nodes for a container in the group?
I would support the documentation with an exposed method like this;

def get_container_nodes(container_name):
    # Enter container group.
    groupNode = nuke.toNode("AVALON_CONTAINERS")
    groupNode.begin()
    backdrop = nuke.toNode(container_name)
    
    # Clear selection.
    for node in nuke.allNodes():
        node["selected"].setValue(False)
    backdrop.selectNodes()
    
    # Get backdrop nodes.
    backdrop_nodes = []
    for node in nuke.allNodes():
        if node["selected"].getValue():
            backdrop_nodes.append(node)
    groupNode.end()

    return backdrop_nodes

[tokejepsen]

Finding working with a context manager somewhat confusing. Take this example:

container_nodes = get_container_nodes(container["objectName"])
with avalon.nuke.sync_copies(container_nodes):
    pass

Would it not be easier read if we had just a method to call:

container_nodes = get_container_nodes(container["objectName"])
avalon.nuke.sync_copies(container_nodes)

Also it might be better to have the sync_copies mirror find_copies when it comes to arguments. find_copies takes a single node as source, but sync_copies takes a list as source.

[davidlatwe]

Just found out that sync_copies may failed if currrent Nuke session has multiple views, since the knob name returned from knob.fullyQualifiedName() will append view name at the end.

7374745 resolved this.

[davidlatwe]

@tokejepsen

Finding working with a context manager somewhat confusing.

The reason why I made it a context was because, it need to firstly compare which copied knob hasn't been changed against source knob before updating them. Or it doesn't need to be that way ? 😮

[davidlatwe]

@tokejepsen

I think the backdrop workflow within the AVALON_CONTAINERS group could use some documentation and code. For example as a developer how do I get the nodes for a container in the group?

Yeah, you are right, that seems very implicit. Currently it looks like you can only get them from the entry "_member" in the parsed container.

Can't remember why I implement it like this. :/
Any suggestion ?

[tokejepsen]

Hmm, long time since I posted this, but I think generally I found it confusing about how to fetch nodes to pass to sync_copies.
Ideally we would work with similar arguments to get the nodes in the container group and what to pass to sync_copies.

[davidlatwe]

Not sure if this helps, currently I only use it inside our loader plugin's update method, like this

https://github.com/MoonShineVFX/reveries-config/blob/a5c56c5d3ee6e87498b024004a41e0b80fc48501/plugins/nuke/load/load_renderlayer.py#L244-L257

Haven't had the needs to find backdrop(container) nodes in other cases.
Maybe we could decouple this

# Get container backdrop child nodes
lib.lsattr("avalon:containerId",
           value=data["avalonId"],  # container's id
           group=nuke.toNode(AVALON_CONTAINERS),
           recursive=True)

into another function and expose it ?

[tokejepsen]

Yeah, sounds like a good idea.

[tokejepsen]

The reason why I made it a context was because, it need to firstly compare which copied knob hasn't been changed against source knob before updating them. Or it doesn't need to be that way ?

I just cant see why we are using a context, since nothing gets reverted after the context finishes?
We get the same behaviour from just calling a method.

[davidlatwe]

Here I made an example to show the sync workflow:

from avalon.nuke import lib

def copy_one(node):
    node["selected"].setValue(True)
    nuke.nodeCopy("_copy_")
    nuke.nodePaste("_copy_")
    copied = nuke.selectedNodes()[0]
    copied["selected"].setValue(False)
    return copied

# Make a dummy that will be containerized in real production
source = nuke.createNode("NoOp", inpanel=False)

# Tag an id so we could find it's copies later
lib.set_id(source)

# Make copies
copied_1 = copy_one(source)
copied_2 = copy_one(source)

# Change the second copy to simulate artist input
copied_2["label"].setValue("hi")

# Update source and sync
with lib.sync_copies([source]):
    source["label"].setValue("hello")

assert source["label"].value() == copied_1["label"].value()
assert source["label"].value() != copied_2["label"].value()

As you may see, the second copy's label that was touched by artist is stay unchaged since it has been identified as an artist's demand while entering the context, but the first copy is synced at the end of the context.

[davidlatwe]

As you may see, the second copy's label that was touched by artist is stay unchaged since it has been identified as an artist's demand while entering the context, but the first copy is synced at the end of the context.

This was meant to behave just like Maya applying reference edits. 🤔

[davidlatwe]

Sorry, one additional note. Without context manager to compare current state of source and copies just right before updating, we will not able to know which knob that has been changed by artist, and ends up like a forced update.

[tokejepsen]

Sorry, one additional note. Without context manager to compare current state of source and copies just right before updating, we will not able to know which knob that has been changed by artist, and ends up like a forced update.

Good point! I see the need for the context now.

[davidlatwe]

Looks like I did not carefully test on forced sync !
Just found a bug that node name will be synced when force enabled, which no one will expect that happen !
Resolved with 3129b24 (and a6cd92b, sorry).