EMSUSD-533 - Incremental naming doesn't take zero into consideration #3309
Conversation
* Added optional DCC function for uniqueChildName() and override the UsdUfe one with Maya version: uniqueChildNameMayaStandard(). * Split out numerical suffix handling in UsdUfe::uniqueName() into separate helper function. * When creating new unique name take into account length of numerical suffix (when padded with 0's). * Added new unit tests to testDuplicateCmd to test all these naming rules.
| @@ -164,6 +164,7 @@ MStatus initialize() | |||
| dccFunctions.timeAccessorFn = MayaUsd::ufe::getTime; | |||
| dccFunctions.isAttributeLockedFn = MayaUsd::Editability::isAttributeLocked; | |||
| dccFunctions.saveStageLoadRulesFn = MayaUsd::MayaUsdProxyShapeStageExtraData::saveLoadRules; | |||
| dccFunctions.uniqueChildNameFn = MayaUsd::ufe::uniqueChildNameMayaStandard; | |||
There was a problem hiding this comment.
From MayaUsd side, set optional uniqueChildName function to be used instead of the default one in UsdUfe.
| //! Returns a unique child name following the Maya standard naming rules. | ||
| MAYAUSD_CORE_PUBLIC | ||
| std::string uniqueChildNameMayaStandard(const PXR_NS::UsdPrim& usdParent, const std::string& name); |
There was a problem hiding this comment.
Removed the two inline mayaUsd versions of these functions (modified calling locations to use them from UsdUfe).
Added new mayaUsd verision of uniqueChildName that follows the Maya standard of naming.
| : uniqueChildNameDefault(usdParent, name); | ||
| } | ||
|
|
||
| std::string uniqueChildNameDefault(const UsdPrim& usdParent, const std::string& name) |
There was a problem hiding this comment.
Left this function alone (other than adding "Default" to name). If you have children:
Capsule1
Capsule6
and you duplicate Capsule1 the new child will be Capsule2. This isn't how Maya naming works. Which is why I added the ability to override unqiueChildName function with your own.
| @@ -278,27 +279,57 @@ int ufePathToInstanceIndex(const Ufe::Path& path, UsdPrim* prim) | |||
| return instanceIndex; | |||
| } | |||
|
|
|||
| std::string uniqueName(const TfToken::HashSet& existingNames, std::string srcName) | |||
| bool splitNumericalSuffix(const std::string srcName, std::string& base, std::string& suffix) | |||
There was a problem hiding this comment.
Split out the first part of the original unqiueName function which separated the input name into base and numerical suffix. Made into helper so I can call it elsewhere.
| // Create a suffix string from the number keeping the same number of digits as | ||
| // numerical suffix from input srcName (padding with 0's if needed). | ||
| suffixStr = std::to_string(suffix); | ||
| suffixStr = std::string(lenSuffix - std::min(lenSuffix, suffixStr.length()), '0') + suffixStr; |
There was a problem hiding this comment.
Modified naming code here to maintain the 0 padded numerical suffix. So duplicating Capsule001 will give you Capsule002
| // By sending in the largest matching name (instead of the input name) | ||
| // the unique name function will increment its numerical suffix by 1 | ||
| // and thus it will be unique and follow Maya naming standard. | ||
| childName = uniqueName(allChildrenNames, largestMatching.first); |
There was a problem hiding this comment.
In this mayaUsd version I find only the greatest matching child (in the list of all children) and send that to the uniqueName function. In that way it will follow the Maya naming. So if you have:
Capsule001
Capsule006
and you duplicate Capsule001, the new child will be Capsule007
| def testDuplicateUniqueName(self): | ||
| '''Test the duplicate of prims and ensure the new name follows Maya | ||
| unique new name standard.''' |
There was a problem hiding this comment.
Add a bunch of new test cases to verify my code changes for Usd prims, follow how Maya naming works.
* Code review feedback - fixed case where zero padding was missing. * Added python binding for 'uniqueName' and added new tests to cover case above. * Added missing tests for python bindings.
| @@ -104,11 +101,12 @@ void wrapUtils() | |||
| def("usdPathToUfePathSegment", | |||
| _usdPathToUfePathSegment, | |||
| (arg("usdPath"), arg("instanceIndex") = PXR_NS::UsdImagingDelegate::ALL_INSTANCES)); | |||
| def("uniqueChildName", _uniqueChildName); | |||
| def("uniqueName", _uniqueName); | |||
There was a problem hiding this comment.
Added missing python wrapper for this function.
| @@ -104,11 +101,12 @@ void wrapUtils() | |||
| def("usdPathToUfePathSegment", | |||
| _usdPathToUfePathSegment, | |||
| (arg("usdPath"), arg("instanceIndex") = PXR_NS::UsdImagingDelegate::ALL_INSTANCES)); | |||
| def("uniqueChildName", _uniqueChildName); | |||
| def("uniqueName", _uniqueName); | |||
| def("uniqueChildName", UsdUfe::uniqueChildName); | |||
There was a problem hiding this comment.
For some of these we don't need the extra _ helper defined here. Can just call the UsdUfe function directly.
| # Test maya-usd usdPathToUfePathSegment wrapper. | ||
| ufePath = mayaUsd.ufe.usdPathToUfePathSegment('/Capsule1') | ||
| self.assertEqual(ufePath, str(usdUtils.createUfePathSegment('/Capsule1'))) |
There was a problem hiding this comment.
Added missing test for python wrapper.
| # stripInstanceIndexFromUfePath/ufePathToInstanceIndex wrappers are tested | ||
| # by testPointInstances. | ||
|
|
||
| # isEditTargetLayerModifiable wrapper is tested by testBlockedLayerEdit. | ||
|
|
||
| # Test the getTime wrapper. | ||
| cmds.currentTime(10) | ||
| t = mayaUsd.ufe.getTime(stagePath) | ||
| self.assertEqual(t, Usd.TimeCode(10)) | ||
|
|
||
| # isAttributeEditAllowed wrapper is tested by testAttribute. |
There was a problem hiding this comment.
Added missing test for python wrapper.
| # Test the maya-usd getPrimFromRawItem() wrapper. | ||
| capsulePath = proxyShapePath + usdUtils.createUfePathSegment('/Capsule1') | ||
| capsuleItem = ufe.Hierarchy.createItem(capsulePath) | ||
| rawItem = capsuleItem.getRawAddress() | ||
| capsulePrim2 = mayaUsd.ufe.getPrimFromRawItem(rawItem) | ||
| self.assertIsNotNone(capsulePrim2) | ||
| self.assertEqual(capsulePrim, capsulePrim2) | ||
|
|
||
| # Test the maya-usd getNodeTypeFromRawItem() wrapper. | ||
| nodeType = mayaUsd.ufe.getNodeTypeFromRawItem(rawItem) | ||
| self.assertIsNotNone(nodeType) | ||
|
|
||
| # Test the maya-usd getNodeNameFromRawItem() wrapper. | ||
| nodeName = mayaUsd.ufe.getNodeNameFromRawItem(rawItem) | ||
| self.assertIsNotNone(nodeName) |
There was a problem hiding this comment.
This is code from before - I just removed the if ufe >=2
| newName = mayaUsd.ufe.uniqueName([], 'Capsule') | ||
| self.assertEqual(newName, 'Capsule1') | ||
| newName = mayaUsd.ufe.uniqueName([], 'Capsule1') | ||
| self.assertEqual(newName, 'Capsule2') | ||
| newName = mayaUsd.ufe.uniqueName(['Cone1'], 'Capsule1') | ||
| self.assertEqual(newName, 'Capsule2') | ||
| newName = mayaUsd.ufe.uniqueName(['Capsule1'], 'Capsule1') | ||
| self.assertEqual(newName, 'Capsule2') | ||
| newName = mayaUsd.ufe.uniqueName(['Capsule1', 'Capsule5'], 'Capsule1') | ||
| self.assertEqual(newName, 'Capsule2') | ||
| newName = mayaUsd.ufe.uniqueName(['Capsule001'], 'Capsule001') | ||
| self.assertEqual(newName, 'Capsule002') | ||
| newName = mayaUsd.ufe.uniqueName(['Capsule001', 'Capsule002'], 'Capsule001') | ||
| self.assertEqual(newName, 'Capsule003') | ||
| newName = mayaUsd.ufe.uniqueName(['Capsule001', 'Capsule005'], 'Capsule001') | ||
| self.assertEqual(newName, 'Capsule002') | ||
|
|
||
| # Test uniqueChildName wrapper. | ||
| newName = mayaUsd.ufe.uniqueChildName(capsulePrim, 'Cone1') | ||
| self.assertEqual(newName, 'Cone1') | ||
| mayaUsdStage.DefinePrim("/Capsule1/Cone1", "Cone") | ||
| newName = mayaUsd.ufe.uniqueChildName(capsulePrim, 'Cone1') | ||
| self.assertEqual(newName, 'Cone2') | ||
| mayaUsdStage.DefinePrim("/Capsule1/Sphere001", "Sphere") | ||
| newName = mayaUsd.ufe.uniqueChildName(capsulePrim, 'Sphere001') | ||
| self.assertEqual(newName, 'Sphere002') |
There was a problem hiding this comment.
New tests to cover uniqueName/uniqueChildName, especially the missing numerical padding case you caught with my original code.
* Fix Linux/Mac build issues and broken unit tests.
* Add variable initialization to fix random problem. The iteration order of TfToken::HashSet is not guaranteed because the hash is by memory address.
|
@pierrebai-adsk Sorry had some trouble with Linux/Mac builds and the unit test. Had to make some extra fixes. |
seando-adsk
added
the
ready-for-merge
seando-adsk
deleted the
donnels/EMSUSD-533/incremental_naming_with_zero_padding
branch
EMSUSD-533 - Incremental naming doesn't take zero into consideration