Store selection in preFileSave(), and restore it in postFileSave()

[wnxntn]

Description (this won't be part of the changelog)

As mentioned in this issue,
Currently, the AL_USDMaya plugin clears the selection during the save. This deviates from the expected Maya behaviour and breaks publishers—such as Shotgun Toolkit—which have selection based publishes, where:

1. The user selects the DAG oject in the Outliner they want to publish, hit "publish"
2. The maya scene is saved and published
3. The selected object is exported and published, and flagged as an output product of the published maya scene

To solve this issuse, we store the selection list in preFileSave(), still clear the selection list, but restore the selection in postFileSave(). AL_USD proxies will also be ignored in the storing of the selection, as they will never be part of selection based publishes

Changelog

These entries will be used to update the changelog.

Use Markdown list syntax for each entry.

Filled out fields will be added to the changelog.

Added

Changed

• Selection(is stored preFileSave() in a MSelectionList, not storing AL_USD proxies
• Selection is restored postFileSave()

Deprecated

Removed

Fixed

Checklist (Please do not remove this line)

• Make sure the Title and Description of the PR make sense and provide sufficient context for your work
• Do any added files have the correct AL Apache Licence Header?
• Are there Doxygen comments in the headers?
• Are any new features, behaviour changes documented in the .md format documentation?
• Have you added, updated tests to cover new features and behaviour changes?
• Have you filled out at least one changelog entry?

[robthebloke]

This should probably be a TF_DEBUG message (so we don't spam the command prompt with coder speak)

[robthebloke]

This may fail (obj might not be a dag node)

[robthebloke]

Might be better to use the getDagPath version? (and check the return status, so as to filter out non-dag node types?). This would then take into account selected instances...

[robthebloke]

I don't think this would be the best approach. You're probably better off simply calling MSelectionList::remove() instead (since you're simply filtering the selection list, and the entire maya selection will be cleared later anyway).

[robthebloke]

I'm not sure the logic here is correct? The following would pass this logic and leave the selection untouched:

select -r `ls -type "AL_usdmaya_ProxyShape"`

This should do more or less the same thing, but with a slight lean towards a more efficient approach. Only issue I can see with this code, is if a parent->parent->proxyshape is selected (since neither approach catches that case? is that important?)

// some utils that test for AL types, but which only initialise function sets when it's possible
// that the type may be a plugin shape or transform. Avoids a tonne of function set initialisations
// and string compares on the types. 
auto isProxyShape = [] (MDagPath p) {
  if(p.node().hasFn(MFn::kPluginShape)) {
     return MFnDagNode(p).typeName() == "AL_usdmaya_ProxyShape";
  }
  return false;
};
auto isTransform = [] (MDagPath p) {
  if(p.node().hasFn(MFn::kPluginTransform)) {
     return MFnDagNode(p).typeName() == "AL_usdmaya_Transform";
  }
  return false;
};

for(int i = 0; i < selected.length(); /* empty */ )
{
  // grab item as a dag path (skip over materials/textures/etc)
  MDagPath selectedPath; 
  if(!selection.getDagPath(i, selectedPath))
  {
    ++i;
    continue;
  }

  // test for any selected proxy shapes or transform nodes
  if(isProxyShape(selectedPath) || isTransform(selectedPath))
  {
    // remove node from selection list
    selection.remove(i);
    continue;
  }

  // test for any parents of proxy shapes selected (don't iterate over all children, just the shape nodes below)
  uint32_t num = 0, j;
  selectedPath.numberOfShapesDirectlyBelow(num);
  for(j = 0; j < num; ++j)
  {
    MDagPath child = selectedPath;
    child.extendToShapeDirectlyBelow(j); //< only care about shape nodes (rather than ALL children!)
    if(isProxyShape(child))
    {
      selection.remove(i);
      removed = true;
      break;
    }
  }

  // if none found, increment count
  if(j == num)
  {
    ++i;
  }
}

[robthebloke]

Initialising a function set and then performing a string compare for every child of every selected node seems like overkill?

[robthebloke]

Is there any reason you are not using MGlobal::setActiveSelectionList(selected); here? This method would fail for selected instances...

[robthebloke]

If you simply remove the offending items from selected instead, we can avoid the numerous calls to deselect the offending nodes (In the short/medium term, the select/unselect mechanism for AL_usdmaya_Transform nodes isn't the most lightweight of processes sadly). The entire selection list will be wiped after this function completes, which should be a little better from a performance POV.

[robthebloke]

TF_DEBUG ?

[robthebloke]

To avoid exporting a DSO symbol, and to make it clear we have a global variable....

static MSelectionList g_selected;

[wnxntn]

Thanks @robthebloke, implemented your changes.

[fdan]

Have you folks had a chance to review Wen's PR fixes? As it stands AL_USDMaya breaks behaviour of Shotgun Toolkit's publishers as the selection list is cleared on save. In order to use your plugin we have to maintain a fork of each version, it'd be nice if this behaviour could be rolled in.