Improve undo/redo - Githubissues

This is a big one. With undo/redo that actually works reliably, and that includes tests for the most common edge-cases.

Greatly improved Undo/redo
SAFE_MODE 2.0
Removed onDestroyed and onRemoved callbacks. They were unused and unsafe.
Removed node["attr"] = value during modifier-use, unused and not practical
Removed plug reuse, it just isn't safe, and didn't make much difference to performance
Refactored DagModifier.lockAttr() and .niceName() and keyableAttr for better integration with undo

Undo/Redo

In the previous version, and every version since then, cmdx has had a checkered past with undo/redo. It's a hard problem, especially when tackled outside of the intended MPxCommand environment for which the API was designed.

But it's done. It's safe, and it's fast.

Changed

Nodes no longer carry their parent modifier, this prevented nodes from getting destroyed when they really should have been. Turns out, the MDagModifier instance actually prevents a destructor from being called. That's bad in all sorts of ways, since the MObject reference remains alive and well.
Plugs are no longer reused. The performance difference is neglible, and the stability improvements are complete.
Undo now discards of commited undo/redo functions to facilitate garbage collection and ~destruction of nodes, both native and custom ones
API commands that intermingle with maya.cmds are now sandboxed appropriately, e.g. DagModifier.lockAttr

Examples


def test_modifier_undo():
    new_scene()

    with cmdx.DagModifier() as mod:
        mod.createNode("transform", name="nodeA")
        mod.createNode("transform", name="nodeB")
        mod.createNode("transform", name="nodeC")

    assert "|nodeC" in cmdx.ls()
    cmds.undo()
    assert "|nodeC" not in cmdx.ls()

def test_modifier_locked():
    """Modifiers properly undo setLocked"""

    new_scene()
    node = cmdx.createNode("transform")
    assert not node["translateX"].locked

    with cmdx.DagModifier() as mod:
        mod.setLocked(node["translateX"], True)

    assert node["translateX"].locked
    cmds.undo()
    assert not node["translateX"].locked
    cmds.redo()
    assert node["translateX"].locked
    cmds.undo()
    assert not node["translateX"].locked

def test_modifier_keyable():
    """Modifiers properly undo setKeyable"""

    new_scene()
    node = cmdx.createNode("transform")
    assert node["translateX"].keyable

    with cmdx.DagModifier() as mod:
        mod.setKeyable(node["translateX"], False)

    assert not node["translateX"].keyable
    cmds.undo()
    assert node["translateX"].keyable
    cmds.redo()
    assert not node["translateX"].keyable
    cmds.undo()
    assert node["translateX"].keyable

def test_modifier_nicename():
    """Modifiers properly undo setNiceName"""

    new_scene()
    node = cmdx.createNode("transform")
    node["myName"] = cmdx.Double()
    assert node["myName"].niceName == "My Name"

    with cmdx.DagModifier() as mod:
        mod.setNiceName(node["myName"], "Nice Name")

    assert node["myName"].niceName == "Nice Name"
    cmds.undo()
    assert node["myName"].niceName == "My Name"
    cmds.redo()
    assert node["myName"].niceName == "Nice Name"
    cmds.undo()
    assert node["myName"].niceName == "My Name"

def test_modifier_plug_cmds_undo():
    """cmds and Modifiers undo in the same chunk"""

    new_scene()
    with cmdx.DagModifier() as mod:
        mod.createNode("transform", name="cmdxNode")
        cmds.createNode("transform", name="cmdsNode")

    assert "|cmdxNode" in cmdx.ls()
    assert "|cmdsNode" in cmdx.ls()

    cmds.undo()

    assert "|cmdxNode" not in cmdx.ls()
    assert "|cmdsNode" not in cmdx.ls()

    cmds.redo()

    assert "|cmdxNode" in cmdx.ls()
    assert "|cmdsNode" in cmdx.ls()

    cmds.undo()

    assert "|cmdxNode" not in cmdx.ls()
    assert "|cmdsNode" not in cmdx.ls()

def test_commit_undo():
    """commit is as stable as Modifiers"""

    new_scene()

    # Maintain reference to this
    test_commit_undo.node = None

    def do():
        test_commit_undo.node = cmdx.createNode("transform", name="nodeA")

    do()

    def undo():
        cmdx.delete(test_commit_undo.node)

    cmdx.commit(undo=undo, redo=do)

    assert "|nodeA" in cmdx.ls()

    cmds.undo()

    assert "|nodeA" not in cmdx.ls()

    cmds.redo()

    assert "|nodeA" in cmdx.ls()

    cmds.undo()

    assert "|nodeA" not in cmdx.ls()

SAFE_MODE

There's always been a SAFE_MODE variable for cmdx, to try and produce a version that could not possibly crash. Any experimental feature - such as Undo in the early days - was put behind this flag.

This flag now makes this goal more true. It simply should not be able to crash when SAFE_MODE is active. It will however run about 10x slower due to the amount of added checking of validity it ends up doing.

import os
os.environ["CMDX_SAFE_MODE"] = "Yes please"
import cmdx

NOTE: The flag alters the contents of the module itself, so it can't be changed after import. Not unless you reload the module.

Callbacks

There were two callbacks registered to cope with reusability of MObject instances. We now check for validity via MObjectHandle instead which relieves the use of callbacks. (And won't slow down your scene over time the more nodes become known to cmdx, which is enough of a plus on its own)

# No longer possible
def on_removed():
  print("Some node was removed!")

node.onRemoved.append(on_removed)

Modifiers

A little-known - as well as complex/obscure - feature of the cmdx.MDagModifier is that you can use the same syntax for setting attributes from within a modifier context manager.

with cmdx.DagModifier as mod:
  node = mod.createNode("transform")
  node["tx"] = 5  # Actually calls mod.setAttr(node["tx"], 5)

In practice, this wasn't very smart. Consider this.

nodeB = cmdx.createNode("transform")
with cmdx.DagModifier as mod:
  nodeA = mod.createNode("transform")
  nodeB["tx"] = 5

You'd expect nodeB to get set using this modifier, so it can be undone, because it happens within the context manager. When in fact it won't, since it isn't actually created by this modifier. And so, to leverage this convenience, you'd have to mix the two.

nodeB = cmdx.createNode("transform")
with cmdx.DagModifier as mod:
  nodeA = mod.createNode("transform")
  nodeA["tx"] = 5  # OK  
  mod.setAttr(nodeB["tx"], 5)  # OK

And then there really isn't much point in having the convenience. This has now been removed, you now use mod.setAttr to set an attribute via the modifier, and node[] to set it outside of it. Simple.

Plug Reuse

MPlug instanced used to be reused whenever you re-accessed a plug. But it isn't safe.. we can't know when an attribute is part of a node that has been deleted, or if the attribute itself has been deleted. Not without calls to MObjectHandle.isAlive and MPlug.isNull which costs as much as just finding the attribute over again.

DagModifier.lockAttr

The modifier is essential for encapsulating a set of commands into a single undo block. But some things just aren't possible using a modifier, like locking an attribute, changing its nice name or editing its keyable state. These were tacked onto the cmdx modifier but aren't safe.. Locking of a dynamic attribute for example still can't happen via the API, and when we call cmds.setAttr in the midst of a MDagModifier, we've entangled the commands for undo. What should it do? Undo the locking first, and the modifier later? What if the modifier created the attribute we're locking? Not safe.

These have now been refactored to happen alongside a modifier and to play nice with undo.

Modifier is done, and commited to undo
Lock, keyable and nicenames are done via maya.cmds, which also commits to undo
Both commitments are added to the same undo chunk via cmds.undoInfo(openChunk)

And thus, they now play nice with undo.

The main limitation - aside from the performance penalty of translating a perfectly fine MObject into a full path and calling maya.cmds - is that cmds.addAttr(niceName=) only works on dynamic attributes. Bummer.

mottosso / cmdx

Improve undo/redo #56