Default subsets from creator plugin

[davidlatwe]

Feature Propose

Make TDs able to predefine some commonly used subset names in the creator plugin.

Example creator:

import avalon.maya

class ModelCreator(avalon.maya.Creator):
    name = "modelDefault"
    label = "model"
    family = "pipeline.model"
    subsets = [
        "HighPoly",
        "LowPoly",
    ]

Those predefined subset names will be listed in the Creator App's subset menu for artists to pick up, along with previous published subset names.

[BigRoy]

I like it - though the subsets attribute is somewhat confusing. Or maybe more it's the name attribute? Looking at this I wonder why modelDefault is separate from HighPoly and LowPoly.

I like the idea of having recommended preset names, I believe at the beginning we imagined doing that a lot too even though we didn't end up using it. Only really with model outputs this makes quite some sense I believe.

Anyhow, can we simplify this somehow? Like merge the two? Or drop the support for name?

[davidlatwe]

though the subsets attribute is somewhat confusing.

This might relate to the concept of #269 : the subset name = family leaf name + variant name

So how about renaming the attribute subsets to variants ? That way, the name will still being as a default subset name, which precomposed without from family and default variant.

[BigRoy]

The issue I'm having is that the name is also listed among the "variants". I'd say drop name and just do:

subsets = ["default", "highPoly", "lowPoly"]

The first one in the list will be just the one that's displayed by default. I believe the name is not used anywhere else than to base the "default subset name" of. Should we maybe deprecate name (allow it to remain None - and maybe show a deprecation message?) and support solely passing subsets or variants or names?

• Tagging @mkolar as he might have some input too.

[mkolar]

The first one in the list will be just the one that's displayed by default.

I agree with this completely. having name in there feels like extra information for no reason. And is in danger of staying hovering around in the code similar to how family and families are both used in pyblish still even though family is practically not needed.

[davidlatwe]

I have made some changes:

• Attribute name in Creator class has been removed. (f8780bc, 9f13c8c)
• Making dimmed subset result text in GUI a bit brighter. (2110c97)
• Currently, "default" and "modelDefault" were all defined as subset name, I thought these two should be separated as variant name and subset name. (fc4e34b)
• Move subset name composing related logic to Creator class, because I think this should be defined in the pipeline instead of written in tools, and could be convenient for someone creating without GUI. (2cf4f52)
• The variant menu will respect the variant list order declared in plugin, and adding a menu separator between predefined and custom variant name. (e0e66c5, 08b51d5)

Here's the GIF:

At Model creator plugin, the "Dummy" is Artist self named variant so it's get separated.

---

Please let me know what you think about these changes, especially on commit fc4e34b and 2cf4f52.

[BigRoy]

The visual changes I like, so the GUI seems fine. However, by avoiding Creator.name and just supporting the multiple of recommended subsets I was hoping that the code itself would somewhat simplify - as name felt a bit odd to implement on the Creator. Instead, I feel it has slightly introduced more complexity.

For example the added classmethods for the pipeline on the Creator, could they scare off Creator plug-in developers? As it looks like now there's much more to implementing a Creator. Or am I just thinking wrong?

[davidlatwe]

the added classmethods for the pipeline on the Creator, could they scare off Creator plug-in developers? As it looks like now there's much more to implementing a Creator.

Yeah, I was worried about that, too. My motive was to implement the concept of subset = family + variant into code, and able to reuse elsewhere with confidence when you need to alter subset name, since it's a pipeline logic I think.

For example, inheriting variant name from parent subset which belongs to different family: generating texture instance from look instance in publish run-time.

But it's true that implementing this inside creator plugin may complicate things for starter, perhaps leave them as the functions of avalon.pipeline (outside of the Creator class) ? Or documenting the concept would be enough (no need to implement it) ?

[davidlatwe]

Sorry, this actually should be in another issue or PR, I'd better revert commit fc4e34b and 2cf4f52 for now :/

[BigRoy]

Thanks @davidlatwe - looks pretty good to me.

[davidlatwe]

I have added one last commit for ensuring capitalize subset name and ignoring letter case. Ready to merge !

[BigRoy]

I have added one last commit for ensuring capitalize subset name and ignoring letter case.

Actually, this might currently be wrong. It should only uppercase the first character, right? Capitalize would change backgroundCharacters to Backgroundcharacters. It will remove all uppercase except for the first character, whereas the remainder of uppercase characters should be preserved I believe. Or do we have good reason not to do so?

We currently have some use cases where multiple capital letters are included for a clear name.

[davidlatwe]

It should only uppercase the first character, right?

Oh yes, you are right ! I missed that use case.

I was meant to prevent the possible subset name duplication in the menu due to different letter cases. Since the subset name parsed from database would have first letter uppercased, and if the defaults from the creator plugin were first letter lowercased, the menu will have duplicated names.

I have reverted the last commit and changed to another implementation.

[davidlatwe]

Merging this if no any further doubts or objections :) Thank you all !

[davidlatwe]

Merged :)

[BigRoy]

Issue

Just wanted to let you know that this PR introduced an issue for us for plug-ins that create instances that weren't intended to be picked up by the default collector. Now with this PR, it feels like a hack because it's broken so it's likely a bit hacky on our end. ;)

For example these plug-ins:

• https://github.com/Colorbleed/colorbleed-config/blob/72b8aa5cd0612b8a80ad4afaa70efd53bfcd821a/colorbleed/plugins/maya/create/colorbleed_vrayscene.py
• https://github.com/Colorbleed/colorbleed-config/blob/72b8aa5cd0612b8a80ad4afaa70efd53bfcd821a/colorbleed/plugins/maya/create/colorbleed_renderglobals.py

Note how they are popping the subset data as this particular instance did not need it.

Yet it now fails due to self.data["subset"] in the default Creator here

Any pointers on which way this could be fixed best? We'd love to maintain the ability to create this more "global" publish instances through the Creator.

---

Moving the hack to "process()"?

Maybe the hack now makes more sense to push into process() and delete the attribute from the created instance instead. :)

Something like:

    def process(self):

        from maya import cmds

        with lib.undo_chunk():
            instance = super(CreateVRayScene, self).process()

            # We don't need subset, asset or active attributes
            remove = ["subset", "asset", "active"]
            for attr in remove:
                if cmds.attributeQuery(attr, node=instance, exists=True):
                    cmds.deleteAttr("{0}.{1}".format(instance, attr))

It does feel a bit clumsier than just:

        # We don't need subset or asset attributes
        self.data.pop("subset", None)
        self.data.pop("asset", None)
        self.data.pop("active", None)

---

Better proposal, keep self.name usage?

Maybe actually using self.name still makes more sense? That way a config could also override the names automatically for created instances, instead of forcing it to be named exactly like the subset?

• It's backwards compatible :)
• Easily allows node name customization for instances in creators.

[davidlatwe]

Oh ! This seems the best option here :

Better proposal, keep self.name usage

Yeah, that's better and workable for all of us !

That way a config could also override the names automatically for created instances, instead of forcing it to be named exactly like the subset

Easily allows node name customization for instances in creators

Good points, that gives a bit more space for custom implementations.