getavalon / core

The safe post-production pipeline - https://getavalon.github.io/2.0
MIT License
218 stars 48 forks source link

nuke workflow / pypeclub update #370

Closed mkolar closed 5 years ago

mkolar commented 5 years ago

So we took #329 and pushed it a bit further, so this should have most that's needed for nuke implementation. We've closed our previous PR and reopening here as we had to do a lot of cleanup on our side. This builds on top of #329 so if everyone is fine with this PR, the old can be closed I think.

What's here: Avalon Menu in nuke with all the main options integrated key functions in ./avalon/nuke/pipeline ( ls, publish, containerise, parse_container, update_container, viewer_update_and_undo_stop) added some useful functions into ./avalon/nuke/lib (add_publish_knob, ls_img_sequence, maintained_selection) workfiles integration for Nuke

We tried to keep the integration as close to fusion and maya standard as we could. If anyone spots inconsistencies please shout out, we'll gladly fix them.

Creator Attaches avalon tab to a node with avalon data string knob to which we serialize all the necessary data and then read it back during collection from this one attribute. we discussed instead adding more separate attributes like in maya, but this seemed a bit cleaner and user can't screw with the data on the node.

Containers Work the same as above. When loading we add avalon data to a new tab on the node, so everything that has this tab with avalon data and correct id is listed as container and can be updated.

I'm hoping there are no outside dependencies left at all now.

tokejepsen commented 5 years ago

Awesome stuff! Unfortunately can't test it, but will close the other issue to clean up.

BigRoy commented 5 years ago

Great stuff @mkolar! I commented one some things that stood out to me, though note that I am not a Nuke user.

Attaches avalon tab to a node with avalon data string knob to which we serialize all the necessary data and then read it back during collection from this one attribute. we discussed instead adding more separate attributes like in maya, but this seemed a bit cleaner and user can't screw with the data on the node.

Personally I don't feel the need for serializing it into a single attribute, but if it makes most sense from a Nuke UX side I am not opposed to it. As a note, in Maya the attributes are also "editable" but I've never seen anyone break it.

mkolar commented 5 years ago

I commented one some things that stood out to me

awesome thanks. I'm pinging @jezscha who did most of it about those points. But all of them sound like very good catches and should be fixed I think.

mkolar commented 5 years ago

has anyone had a chance to try this? we have it deployed and it works ok. Albeit over time we'll certainly find places for many improvements. I think if no-one has any complaints we could merge. I've fixed all the comments above I believe

BigRoy commented 5 years ago

Unfortunately I'm unable to test this as I don't have any Nuke licenses. Maybe @davidlatwe is able to chime on this one to do a test?

If it works for you and your clients, noone else can test it and the code looks clean I guess it should be ready to merge. It looks great.

By the way, do you have any pictures/gifs/videos of it in action?

mkolar commented 5 years ago

By the way, do you have any pictures/gifs/videos of it in action?

nope but will do.

mottosso commented 5 years ago

Also can't test this, but happy to leave it with you guys. Agree with @BigRoy that we need gifs or video of what it looks like and how it works.

BigRoy commented 5 years ago

@mkolar Is this still the up-to-date state of a working Nuke integration? Plus, are you able to produce GIFs/JPEGS or some other type of image/video to showcase the working integration.

BigRoy commented 5 years ago

@tokejepsen @mkolar @jezscha is any of you able to share some GIFs/videos of a working Nuke integration of Avalon plus make sure the up-to-date code for core is ready to go on this PR or another? As part of Avalon 6.0 I think it'd be great to get the Nuke integration merged and ready, plus potentially documented - yet unfortunately I myself do not work with Nuke. ;)

tokejepsen commented 5 years ago

As far as I know Nuke integration is still being worked on. Or maybe it was NukeStudio?

BigRoy commented 5 years ago

Well, there do seem to be some updates not in this PR, like for Nuke https://github.com/pypeclub/avalon-core/commit/3c4ae51b1ced638c851896f38409608fe5933bd8 and for Nuke Studio https://github.com/pypeclub/avalon-core/commit/a9b794a81d39c61c67140d97a6b59cd207715589 -- however, changes like these seem very odd as it prints all node names, even those that are not avalon containers, on every ls() which I think you (very rarely!) want? Seems like remaining debug code.

mkolar commented 5 years ago

I believe that this PR is still valid for the basic implementation. Yes we were adding some more details in different branches and features on our side, but it's not production tested yet. I'll double check with @jezscha today

mottosso commented 5 years ago

Having a look at this one now. The workfiles app seems to have been updated inbetween this PR and now, I'll have a look at these now.

mottosso commented 5 years ago

I've implemented the workio.py file consistent with maya, fusion et. al. based on the implementations found in in the workfiles app from this PR.

The only one I wasn't sure of was workio.work_root(), which I've made os.path.dirname(workio.current_file()), @tokejepsen do you know if that is right? :S

BigRoy commented 5 years ago

The only one I wasn't sure of was workio.work_root(), which I've made os.path.dirname(workio.current_file())

@mottosso that's definitely wrong. See documentation here

Also, in those cases you should be able to one-to-one copy it from another host that does the same. In this case, Houdini and Fusion. Actually Maya has only been the one that defines its own work root through the means of workspaces.

mottosso commented 5 years ago

Ah, yes I was looking at Maya for guidance, but couldn't translate its idea of "workspace". That doc is great, but isn't it saying to return the parent directory of he current file? What should the function return instead?

BigRoy commented 5 years ago

It should not use a current file as it could actually even be queried when there is no currently saved file.

Anyway, documentation there does seem to lack the code example for those cases, however it should be similar to this

mottosso commented 5 years ago

Thanks, it's been fixed.

tokejepsen commented 5 years ago

The only one I wasn't sure of was workio.work_root(), which I've made os.path.dirname(workio.current_file()), @tokejepsen do you know if that is right? :S

We have worked with this; https://github.com/pypeclub/avalon-core/blob/develop/avalon/nuke/workio.py

mottosso commented 5 years ago

Thanks, fixed. Curious, is nuke.root() the same as nuke.Root()? The previous version used lower-case. :S

tokejepsen commented 5 years ago

Thanks, fixed. Curious, is nuke.root() the same as nuke.Root()? The previous version used lower-case. :S

Honestly dont know. Think both works.