getavalon / core

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

General Improvement #352

Closed davidlatwe closed 5 years ago

davidlatwe commented 5 years ago

Changed

  1. If RuntimeError raised while reading custom attributes in avalon.maya.lib.read(), parse source node name if has input connection. Useful for message type connection.

  2. Use creator plugin's name value as default, if avalon.pipeline.create()'s arg name receive "" or None.

  3. Remove unnecessary value remap in cbsceneinventory app, which causing wrong container data name passed to loader.update.

  4. Remove hard-coded container id "pyblish.avalon.container" in each host.

  5. Move collect_container_metadata from cbsceneinventory app to each host.ls, thought putting this logic to each host would making more sense.

Hoping these changes are making sense to you, please let me know what you think :)

BigRoy commented 5 years ago

Looks like some nice fixes. I need to double check coming week whether it doesn't break anything on our end, but it looks good. :)


If RuntimeError raised while reading custom attributes in avalon.maya.lib.read(), parse source node name if has input connection. Useful for message type connection.

Where/how are you using this? Are you using this on instances?

  1. Move collect_container_metadata from cbsceneinventory app to each host.ls, thought putting this logic to each host would making more sense.

The reason we ended up doing that is that I felt the data was purely intended for the UI - as such I felt only exposing the functionality there made sense. Also to ensure the ls() kept a high speed as a query, but that was probably a micro-optimization.

Both is fine with me.

BigRoy commented 5 years ago

The only minor red flag is where AVALON_CONTAINER_ID is coming from. On top of that, it's hard-coded in pipeline.py

Is it configurable? If it is, shouldn't that invalidate the current schema? I thought it was supposed to be a fixed value in Avalon.

mottosso commented 5 years ago

Good question. There's no reason to change this value, so maybe hard-coding it isn't a problem.

davidlatwe commented 5 years ago

@BigRoy

Where/how are you using this? Are you using this on instances?

I have another objectSet which functions alike container, but instead having all referenced/imported nodes as member, it only contain the root assembly node and other important nodes, like an interface for artists. And I connect the container to the correspond interface node with message type connection.

@mottosso

There's no reason to change this value, so maybe hard-coding it isn't a problem.

Then I shall ignore the change request ? :P The reason I made that change was because I was implementing a custom lookup for containers, but I wouldn't wont to hard-coding that id string in my config, so I thought making it as an attribute in the core would be nice.

Thanks !

mottosso commented 5 years ago

Then I shall ignore the change request ? :P

Haha, yes. Sorry. :)

davidlatwe commented 5 years ago

Merging this, thanks !