Skip to content

Conversation

@lee-aandrew
Copy link
Contributor

Purpose of this PR

Ticket/Jira #: https://jira.unity3d.com/browse/USDU-274

Adds new test cases for functions within ImportHelper.cs script to increase coverage and to act as an onboarding task for @lee-aandrew

Testing

Functional Testing status:
N/A

Performance Testing status:
N/A

Overall Product Risks

N/A

Complexity:
1

Halo Effect:
1

Additional information

Note to reviewers:
Not sure if this is a bug or a intended way of doing things, but USD.Scene.Write, when given an object of type Parent Class, it will not write anything
For example:

SampleBase testArray[] { SphereSample sphere1, SphereSample sphere2 } // SampleBase is the root parent of SphereSample
Scene.Write("/root/sphere" testArray[0])

The Write in this case would not write anything to the scene

Copy link
Contributor

@vickycl vickycl left a comment

Choose a reason for hiding this comment

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

Added some suggestions, overall nice tests and great work!

var usdAsObjects = AssetDatabase.LoadAllAssetsAtPath(assetPath);

// ExpectedGameObjectCount: The Root GameObject
// ExpectedPrimSourceCount: 0 TODO: Shouldnt there be a prim source object for the root object?
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this TODO still valid?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Its more of a question than a TODO - im not sure why there isnt a prim source object for the root object when imported as a Timeline Clip

Copy link
Contributor

Choose a reason for hiding this comment

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

Possibly a question for @mirceaispas , I think there is no prim source because the root isn't considered an actual prim in USD terms?

@lee-aandrew lee-aandrew requested a review from vickycl January 5, 2023 13:49
Copy link
Contributor

@vickycl vickycl left a comment

Choose a reason for hiding this comment

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

New changes LGTM :)

@lee-aandrew lee-aandrew merged commit 19ea387 into dev Jan 17, 2023
@lee-aandrew
Copy link
Contributor Author

Thank you for the reviews @vickycl @unity-chris !
merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants