-
Notifications
You must be signed in to change notification settings - Fork 201
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
LOOKDEVX-1592 - Clean up references, targets, and connections on delete #3194
LOOKDEVX-1592 - Clean up references, targets, and connections on delete #3194
Conversation
Similar to what is done on Rename, we traverse the scene and clean up any reference, target, or connection that will become invalid once the prim is removed.
const SdfReferencesProxy& referencesList, | ||
SdfListOpType op) | ||
{ | ||
// set the listProxy based on the SdfListOpType |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this file, please always compare with the function just above the new one to see the difference between cleaning up and renaming.
prepend apiSchemas = ["MaterialBindingAPI"] | ||
) | ||
{ | ||
rel material:binding = </mtl/BrownMat> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This material will be propagated to all children.
prepend apiSchemas = ["MaterialBindingAPI"] | ||
) | ||
{ | ||
rel material:binding = </mtlFoliage/GreenMat> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This material wins over the parent material.
SdfReferencesProxy referencesList = primSpec->GetReferenceList(); | ||
|
||
// update append/prepend lists individually | ||
removeInternalReferencePath(deletedPrim, referencesList, SdfListOpTypeAppended); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even in the previously-existing version for rename, IDK why we don't treat the SdfListOpTypeOrdered, SdfListOpTypeAdded, SdfListOpTypeExplicit and SdfListOpTypeDeleted. MayaUSd might not author those, but we can edit USD file that may contain them.
# Ref2 has an "external" reference, but to this file. It will not be | ||
# cleaned up because external, baut maybe it should have since the | ||
# referenced file happens to be the root layer? | ||
self.assertTrue(stage.GetPrimAtPath('/TreeBase/leaf_ref_2').HasAuthoredReferences()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, might be good to file a story to improve this, along with my comments about other list ops.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also small typo baut -> but, no need to respin teh PR just for that in a unit test though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but I think we have ground to create a story about improving the code further in the future.
Thanks Pierre, I have opened EMSUSD-251 to track improvements. |
Similar to what is done on Rename, we traverse the scene and clean up any reference, target, or connection that will become invalid once the prim is removed.