Open davidlatwe opened 4 years ago
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.
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. :relaxed: Pinning @jezscha :telephone_receiver: , is that okay if we drop the old containerization implementation ?
I was wondering whether this is necessary?
I think I'll keep the backward comapt for now, untill everyone who use Nuke can confirm that we don't need it.
And I've changed the name from AVALON_NUKE_CONTAINERS_AT_LARGE
into AVALON_NUKE_OLD_CONTAINER
, the new name should be much easier to know what's it for.
Currently I am still working on my Nuke config, but what's inside this PR is working fine here, so hope we can merge this soon if no objections.
Would be great if anyone can have a try out. :)
What's the status of this long open PR? :) Should this be ready to merge or are there reasons not to?
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. 🤔
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.
Testing this PR out atm, and will dump any thoughts and feedback here.
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
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.
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.
@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 ? 😮
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.
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.
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. 🤔
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.
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.
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).
This PR is aimed to implement #474, I'll try to be as clear as possible.
What's changed
lib.reset_selection
(919f1ca)INFO
(56f55fc)lsattr
andlsattrs
for finding nodes by knob and value (1ff01fa)models.TreeModel
andsceneinventory.app
for Qt4 compat (98d3562)containerize
andls
by env varAVALON_NUKE_OLD_CONTAINER
(bd88c1a, a19b066)lib.imprint
,lib.read
with vendorizedknobby
(05b5086, f217fa4, 875f83f)lsattr, lsattrs
These two functions have three extra args compares to other hosts,
type=None
,group=None
andrecursive=False
. These three args will be passed intonuke.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
andls
of this workflow is not backwards compatible, you have to setAVALON_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
andcontainerId
imprinted, thecontainerId
is the container'savalonId
. And we look for copies by matching node type andavalonId
.To use it in Loader plugin, like
containerise
inLoader.load
, you would call it inLoader.update
, for example:Vendoring
knobby
There were some issues in previous implementation of knob imprinting, like:
imprint
might not good forread
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 innuke.vendor
, I would love to move it back.So what
knobby
will do for us ? TL;DRtab
existed.mold
is likeread
, 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
knobI 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. :)