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 Workflow: Enable asset contributions to write AYON Entity URIs #754

Merged
merged 9 commits into from
Aug 29, 2024

Conversation

BigRoy
Copy link
Collaborator

@BigRoy BigRoy commented Jul 5, 2024

Changelog Description

Enable the writing of Ayon Entity URIs for asset/shot contribution workflows for the AYON USD resolver to resolve.

Additional info

Must be tested using the AYON USD Resolver

TODO

  • Implement toggle in settings to enable/disable.

Testing notes:

  1. Enable the use_ayon_entity_uri Entity URI in settings.
ayon+settings://core/publish/ExtractUSDAssetContribution/use_ayon_entity_uri
ayon+settings://core/publish/ExtractUSDLayerContribution/use_ayon_entity_uri
  1. Publish USD files with asset or shot contributions (e.g. from Houdini, but should also work from Maya and Blender!)

@ynbot ynbot added type: enhancement Improvement of existing functionality or minor addition size/XS labels Jul 5, 2024
@Lypsolon Lypsolon self-requested a review July 24, 2024 16:25
@BigRoy BigRoy marked this pull request as ready for review July 25, 2024 12:17
@Lypsolon
Copy link

I believe the current logic breaks the publisher's versioning.

Here is what happens and what I think should happen.
Currently, all the URIs use version=latest, which causes every version to resolve to the newest version.
For example, usdAsset v001 will resolve to usdModel v005 if v5 is the newest version. If you recently published something broken, you can't just load an older usdAsset version and be fine.

I believe every URI should use a version if not explicitly stated. This way, usdAsset v001 will be an accurate representation of how the asset looked at the time of Publish.

I even believe there might be a value to "Resolve" =latest to the current newest version on publish if you're contributing inside of an asset (this I'm not sure of)

@BigRoy BigRoy assigned BigRoy and unassigned antirotor and Lypsolon Jul 25, 2024
@BigRoy
Copy link
Collaborator Author

BigRoy commented Jul 26, 2024

@Lypsolon could you try again? It should now write the AYON URI with the version numbers in them.

I do wonder though if there are edge cases where instance.data["version"] may not be set - but since it's defined here at the end of the CollectorOrder we should be good.

By the way, if you perform a "Review" on Github and "submit the review" it allows me to just re-request a review instead of having to comment and tag. Maybe that's nicer workflow? Let me know!

Copy link

@Lypsolon Lypsolon left a comment

Choose a reason for hiding this comment

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

You mentioned that you made changes to the way the URI is constructed.
My testing might be a bit messed up I was playing around with the system while having the Use AYON Entitiy URI button Off and enabled it when publishing version 2 of my look. (if this is UB, tell me, and I will test again)

But what I noticed is.
The ayon_uri in the customData has the proper version set, but the actual reference dose still uses the latest. I am not sure why

    variantSet "look" = {
        "Main" (
            prepend references = [
                @ayon://Usd_Base//testRoyJul29/char_gul?product=lookMain&version=latest&representation=usd@ (
                    customData = {
                        int ayon_order = 300
                        string ayon_uri = "ayon://Usd_Base//testRoyJul29/char_gul?product=lookMain&version=2&representation=usd"
                    }
                )
            ]
        ) {

        }
    }
}

@BigRoy BigRoy requested a review from Lypsolon August 27, 2024 12:51
@BigRoy
Copy link
Collaborator Author

BigRoy commented Aug 27, 2024

But what I noticed is.
The ayon_uri in the customData has the proper version set, but the actual reference dose still uses the latest. I am not sure why

Interesting case - but I've tried it now, on my end, with some clean assets trying to break it in a similar manner.. but so far have been unable to reproduce and that paths all turned out versioned. I think we're best off considering this a valid first version - it does what it should - and if we encounter edge cases hopefully by that time we know how to reproduce it exactly and we can tackle it as a dedicated edge case.

So I've re-requested review from you, but do let me know if you still have issues with approving this.

…tings do not exist (e.g. dev mode using older addon on server in bundle) that it still runs backwards compatible and has this disabled by default
Copy link

@Lypsolon Lypsolon left a comment

Choose a reason for hiding this comment

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

i re tested this pr and it works.

i will mark tho: we need to get those things programmatic tested. the current strategy is okay for QA but other than that its more hope hand testing.

@BigRoy
Copy link
Collaborator Author

BigRoy commented Aug 29, 2024

i will mark tho: we need to get those things programmatic tested. the current strategy is okay for QA but other than that its more hope hand testing.

Yup - it's not just here, but around the full scope of AYON addons really.

@BigRoy BigRoy merged commit 83befb2 into ynput:develop Aug 29, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/XS type: enhancement Improvement of existing functionality or minor addition
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants