Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

USD contribution: Set up different default values based on profiles #973

Draft
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

BigRoy
Copy link
Collaborator

@BigRoy BigRoy commented Oct 26, 2024

Changelog Description

Follow up PR to: #994 so that USD products will use different attribute defaults based on profiles.

Additional info

TODO

  • Implement profiles in settings
  • Allow to also filter profiles against e.g. task type

Testing notes:

  1. Use together with PR here: Allow creating more product types as USD with USD contribution workflow and better defaults. ayon-houdini#144
  2. In Houdini, start publishing USD model, groom and look products. Each product should have different defaults and target the right 'default layer'

for _ in pyblish.logic.plugins_by_families(
[cls], [instance.product_type]
):
families = [instance.product_type]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be honest I don't agree with this change. I still think we should not use families during create phase, but other data driven workflow. Either it would be is_usd: True on instance data or more complex structure, but it should be usd specific so it can be targeted and deterministic, with possibly more information, instead of arbitrary list of strings.

Example data on instance

{
    "usdData": {
        "someKey": "someValue",
        "renderer": "supaR2D3",
    }
}

If the values would not change during create phase and would be read only, they could be in transient data so we don't overfill stored data to scene.

This is my personal opinion, maybe caused by me not having full view into the issue.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly - most of these things I'm explaining here should also 1:1 define whether the plug-in should process the instance. Hence assigning families as tag is the way to go. But yes, there will be edge case where we may want to act on specific metadata and more complex values - but I'd say that's WAY harder to track in consistency, especially across multiple addon repositories instead of just checking "does it have this tag".

However, I just want to state that I'd like to PREFIX all these families, e.g. traits/reviewable or traits/frame_range or node/AlembicROP or whatever to differentiate clearly from the families that are intended as a product type. Also, because it allows us to transition smoother from the legacy system over time.

Also wanted to link this Community topic where I mentioned families versus product type as well: https://community.ynput.io/t/difference-between-family-and-product-type/1923

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Be carefull about traits, as that will be new concept in representations, it might be confusin to mix those.

BTW does trait in this case make sense? Shouldn't be traits defined based on set up attributes instead of set up attributes based on traits?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW does trait in this case make sense? Shouldn't be traits defined based on set up attributes instead of set up attributes based on traits?

The idea is indeed that traits and attributes would be intertwined - in some case a toggle may enable/disable a trait, but to me that's essentially the same case with these families. A creator attribute may add/remove such a 'family'. But yes, whether traits is the best name for it I'm not sure. I just used it because it clarified the subject since there's huge overlap in that area (a much better overlap than "product type" is)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like I've said, traits are dangerous, if you want to use families then use families, and don't call families traits.

BTW if there are families used only during create phase then I have another argument why not use families. Wouldn't make sense to add usdTraits instead of families to the instance data? So it is clear it is related to USD?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The concept I'm trying to portray is that this is not USD specific - the concept expands beyond that and is completely embedded into ayon. E.g. being able to target an instance to be farm compatible, or something to have a framerange or alike so that we can have a generic frame range validation (or something of the sort). The forum post I linked explains that well.

I understand your point regarding "traits" and the potential confusion. All I really mean to say is that I really want to differentiate from what 'families' are actually about.

I'd really hate these scattered attributes appearing:

isUsd: True
isFbx: True
usdTraits: a, b
etc.

Using e.g. file/usd as a family may at least set a standard so that whenever we target something to a filetype, that we prefix at such. So that whenever you see file/fbx you know what it's doing and that it is a file type being referred to. Personally I'd even like producttype/review (although bit lengthy) but it's way clearer that "review" family is being targeted based on a product type as opposed to some other scattered data.

Anyway, I've requested a discussion/call this week to discuss this more in-depth. Would be good to have you there too! :)

Copy link
Member

@iLLiCiTiT iLLiCiTiT Oct 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you discuss the concept with anyone else before?

Copy link
Collaborator Author

@BigRoy BigRoy Oct 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I discussed it with Milan - but more as "my proposal". And I mentioned it to Dee and Ondrej last week in a meeting. But mostly from the perspective that we should discuss this in a brainstorm - which I believe current indicative planning is Thursday morning. So this is indeed "to be discussed"

Also want to mention that to me @MustafaJafar 's PR mentioned here #973 (comment) shows the issue at hand. It's a good fix, but now suddenly we've flooded families with opengl and review - not knowing which refers to a product type, which refers to targeting a rop node in houdini, etc. Preferably we 'tag' that clearer so its absolutely obvious from reading the code - which could e.g. also be rop/opengl or rop.opengl.

@MustafaJafar
Copy link
Contributor

MustafaJafar commented Oct 29, 2024

Hey,
This PR enhances many aspects:

  • Dev: From pyblish plugins dev perspective, this PR helps understanding the intention of the creators. also, it helps making the system more modular as I can add as many attributes using different plugins.
  • Artist: I can find attributes related to my publish instance.

Note

I made a draft PR ynput/ayon-houdini#148 to show case my test for this PR. (It shows the issue of the difference between product type and families as mentioned here #973 (comment). but, it doesn't leverage this PR), so, I tried one more time.

This example show cases the benefit from this PR.
where I needed to

  1. add the necessary families to get_publish_families
  2. implement the proper callbacks to show and hide attributes in the publisher (taking USD Contribution Workflow: Show/hide attributes per instance based on status of other toggles #960 as reference).

The draft PRs where I took this screengrab are: ynput/ayon-houdini#149, ynput/ayon-deadline#54
Animation_17

@BigRoy BigRoy changed the title Show publish plug-in attributes based on families instead of only productType USD contribution: Set up different default values based on profiles Nov 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/S type: enhancement Improvement of existing functionality or minor addition
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants