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

MAYA-105175: Rewrote the rename restriction algorithm from scratch to cover more edge cases. #786

Merged
merged 11 commits into from
Sep 24, 2020
49 changes: 27 additions & 22 deletions lib/mayaUsd/ufe/private/Utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@

#include <mayaUsdUtils/util.h>

#include <iostream>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

just noticed I left out to clean up this iostream. Will have it remove in another PR.


PXR_NAMESPACE_USING_DIRECTIVE

MAYAUSD_NS_DEF {
Expand Down Expand Up @@ -132,43 +134,46 @@ void applyCommandRestriction(const UsdPrim& prim, const std::string& commandName
auto primSpec = MayaUsdUtils::getPrimSpecAtEditTarget(prim);
auto primStack = prim.GetPrimStack();
std::string layerDisplayName;
std::string message{"It is defined on another layer"};

// iterate over the prim stack, starting at the highest-priority layer.
for (const auto& spec : primStack)
{
const auto& layerName = spec->GetLayer()->GetDisplayName();

if (primSpec != spec) {
layerDisplayName.append("[" + layerName + "]" + ",");
// skip if there is no primSpec for the selected prim in the current stage's local layer.
if(!primSpec){
// add "," separator for multiple layers
if(!layerDisplayName.empty()) { layerDisplayName.append(","); }
layerDisplayName.append("[" + layerName + "]");
continue;
}
else {
// if exist a primSpec with reference
if(spec->HasReferences()) {
break;
}

// if exists a def sepc
if (spec->GetSpecifier() == SdfSpecifierDef) {
// if exists a def spec is in another layer other than current stage's local layer.
if(primSpec->GetLayer() != spec->GetLayer()) {
layerDisplayName.append("[" + layerName + "]" + ",");
break;
}
continue;
}
// one reason for skipping the reference is to not clash
// with the over that may be created in the stage's sessioLayer.
// another reason is that one should be able to edit a referenced prim that
// is tagged over/def as long as it has a primSpec in the selected edit target layer.
if(spec->HasReferences()) {
break;
}

// if exists an over sepc
if (spec->GetSpecifier() == SdfSpecifierOver) {
layerDisplayName.append("[" + layerName + "]" + ",");
// if exists a def/over specs
if (spec->GetSpecifier() == SdfSpecifierDef || spec->GetSpecifier() == SdfSpecifierOver) {
// if exists in another layer other than current stage's local layer.
if(primSpec->GetLayer() != spec->GetLayer()) {
layerDisplayName.append("[" + layerName + "]");
message = "It has a stronger opinion on another layer";
break;
}
continue;
}
}

if(!layerDisplayName.empty()) {
layerDisplayName.pop_back();
std::string err = TfStringPrintf("Cannot %s [%s]. It is defined on another layer. Please set %s as the target layer to proceed.",
std::string err = TfStringPrintf("Cannot %s [%s]. %s. Please set %s as the target layer to proceed.",
commandName.c_str(),
prim.GetName().GetString().c_str(),
prim.GetName().GetString().c_str(),
message.c_str(),
layerDisplayName.c_str());
throw std::runtime_error(err.c_str());
}
Copy link
Contributor Author

@HamedSabri-adsk HamedSabri-adsk Sep 22, 2020

Choose a reason for hiding this comment

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

@ppt-adsk

  • polished the algorithm slightly to make it easier to reason about the code and added appropriate comments.
  • added proper messaging. we now only need to deal with two messages ( no more fancy words ) :
  1. Cannot rename something because It is defined on another layer
  2. Cannot rename something because It has a stronger opinion on another layer

please see 2472eec .

Also ignore my typo in the commit message: I meant "polished" not "published". Sigh..............

Expand Down