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

Fix for UFE v1 code path with missing "|world". #1050

Merged
merged 5 commits into from
Jan 21, 2021

Conversation

seando-adsk
Copy link
Collaborator

No description provided.

@seando-adsk seando-adsk requested a review from ppt-adsk January 8, 2021 19:29
@seando-adsk seando-adsk added bug Something isn't working ufe-usd Related to UFE-USD plugin in Maya-Usd labels Jan 8, 2021
// with "|world" and will pop it off. So make sure our string has it.
// Note: std::string starts_with only added for C++20. So use diff trick.
std::string proxyPath;
if (ufePathString.rfind("|world", 0) == std::string::npos) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice trick.

Copy link

@kxl-adsk kxl-adsk left a comment

Choose a reason for hiding this comment

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

Please add regression test and cover all ways we can access the stage (with and without ufe)

Copy link
Collaborator

@ppt-adsk ppt-adsk left a comment

Choose a reason for hiding this comment

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

Great test, thanks for doing this!

@seando-adsk
Copy link
Collaborator Author

@mattyjams I added to you review the change I made to test/lib/ufe/testUfePythonImport.py

Comment on lines 125 to +129
#ifdef UFE_V2_FEATURES_AVAILABLE
def("getPrimFromRawItem", getPrimFromRawItem);
def("getNodeNameFromRawItem", getNodeNameFromRawItem);
#endif

def("getNodeTypeFromRawItem", getNodeTypeFromRawItem);
#endif
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 raw item (for ufe scene item) was only added in UFE v2. So these three wrappers that use raw item should only be avail in v2.

''' Called initially to set up the Maya test environment '''
self.assertTrue(self.pluginsLoaded)

def testWrappers(self):
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

New test where I test all the maya-usd ufe wrapper methods.

Comment on lines 36 to 41
if ufe.VersionInfo.getMajorVersion() >= 2:
# raw item only introduced in UFE v2
invalidPrim = mayaUsdUfe.getPrimFromRawItem(0)
else:
# In UFE v2 if the Nb segments == 1, returns invalid prim
invalidPrim = mayaUsdUfe.ufePathToPrim('a')
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This test is avail in UFE v1, but it was using a ufe wrapper method that I've made only avail in UFE v2. So for the v1 case use a different wrapper method. We cannot just use ufePathToPrim for both UFE versions, since in UFE v2 ufePathToPrim uses different code that will assert with an invalid input path.

Copy link

@kxl-adsk kxl-adsk left a comment

Choose a reason for hiding this comment

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

Excellent test, thank you!

Copy link
Contributor

@mattyjams mattyjams left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@@ -23,6 +23,7 @@
import mayaUsd.ufe

import ufe
import ufeUtils
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oops I forgot to include this to have access for line 45. Since this usd utils already includes ufe, including the ufeUtils should be okay.

@seando-adsk seando-adsk requested a review from ppt-adsk January 21, 2021 15:03
@seando-adsk seando-adsk added the ready-for-merge Development process is finished, PR is ready for merge label Jan 21, 2021
@kxl-adsk kxl-adsk merged commit d923bf7 into dev Jan 21, 2021
@kxl-adsk kxl-adsk deleted the donnels/fix_for_missing_world branch January 21, 2021 23:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ready-for-merge Development process is finished, PR is ready for merge ufe-usd Related to UFE-USD plugin in Maya-Usd
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants