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

author kind=group on the UsdPrim created by the group command when its parent is in the model hierarchy #1094

Conversation

mattyjams
Copy link
Contributor

Authoring the kind metadata on the group Xform prim ensures that the model hierarchy remains contiguous between the parent and children of the new group prim. Prims underneath the group prim may still be in the model hierarchy if they also have kind authored.

If the parent prim of the group prim is not part of the model hierarchy, no kind is authored on the group prim.

More info about model hierarchy can be found here:
https://graphics.pixar.com/usd/docs/USD-Glossary.html#USDGlossary-ModelHierarchy

…arent is in the model hierarchy

Authoring the kind metadata on the group Xform prim ensures that the model
hierarchy remains contiguous. Prims underneath the group prim may still be in
the model hierarchy if they also have kind authored.

If the parent prim of the group prim is not part of the model hierarchy, no
kind is authored on the group prim.
@mattyjams mattyjams added the ufe-usd Related to UFE-USD plugin in Maya-Usd label Jan 20, 2021
Comment on lines +41 to +44
UsdUndoSetKindCommand(const UsdUndoSetKindCommand&) = delete;
UsdUndoSetKindCommand& operator=(const UsdUndoSetKindCommand&) = delete;
UsdUndoSetKindCommand(UsdUndoSetKindCommand&&) = delete;
UsdUndoSetKindCommand& operator=(UsdUndoSetKindCommand&&) = delete;

Choose a reason for hiding this comment

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

I see we copy this pattern around making the declaration of this trivial class busier than it has to be.

The base class Ufe::UndoableCommand already explicitly deletes the copy constructor. This will make the compiler implicitly delete any derived class copy constructors. So line 41 is not needed.
Lack of a copy constructor will also make the compiler implicitly delete the move constructor. So line 43 is not needed.

Finally, we should do a better job in the base class and delete the copy assignment operator, this would make the compiler implicitly delete the copy assignment operator and move the assignment operator from the derived class. This is not currently done, so lines 42 and 44 have to stay.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's a good point. I really don't have a strong opinion here. I personally am not a huge fan of implicit philosophy whether it is C++ or any other languages.

Copy link
Contributor

@HamedSabri-adsk HamedSabri-adsk left a comment

Choose a reason for hiding this comment

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

Looking good and thank you for adding the test.

@kxl-adsk kxl-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 78ff9b3 into Autodesk:dev Jan 21, 2021
@mattyjams mattyjams deleted the pr/author_group_kind_when_grouping_in_model_hierarchy branch January 21, 2021 23:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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