Moving nodes in the NodeGraph should be undoable

[johnhaddon]

This will probably require #422, so that transitioning between movement in one axis and movement in two axes will still merge into a single undo event appropriately. While doing this, test with a very large number of nodes to determine the performance implications of creating and merging a huge number of undo events.

Alternatively we could cheat and just make a single undoable event at drag end.

[andrewkaufman]

Are we sure moving nodes should be undoable? I'd be tempted to leave it out personally, at least until some serious user testing shows it to be necessary. I get annoyed in other packages when trying to undo actual operations, and my queue is full of nonsense node shifts that I did without thinking.

[johnhaddon]

I've found myself being annoyed that it wasn't undoable on occasion. It does seem like a pretty solid general principle that anything the user does to their script directly should be undoable and should set the unsavedChanges plug - currently (with #419) moving nodes does neither. Maybe you need to stop making nonsense node shifts without thinking?

[andrewkaufman]

They're nonsense in that I don't care about them being undone. They're legitimate in that I like to have a nice pretty node layout, and my opinion of what that is shifts depending on my mood. If Ben feels strongly, I won't oppose it. Just saying that in other software that does this, I find it quite annoying / unnecessary for the type of work I do.

[johnhaddon]

I'm not in any rush to do this one so we can wait for some more input. I just wanted to make a note of it (and the relation to #422) while it was fresh on my mind from the other undo work.

[danieldresser]

I'll just duck in here to state my opinion:

+1 for undoable node moves
+1 for Andrew should stop making nonsense node shifts

[bentoogood]

I was just showing John one of my shader graphs this morning, moved some nodes off to the side to clarify the connections, then pressed ctrl-z to move them back.
was a little surprised to see nothing happen, seems like a very natural thing to expect.
so another +1 for undoable node moves

[johnhaddon]

Just noting that when we come to do this, we also need to make Backdrop resizing undoable.

[ivanimanishi]

I'm not that interested in the undo functionality itself, but I do think that changes to node positions should mark the "unsavedChanges" plug to True.
It is quite reasonable to expect that a user would spend a whole session just re-organizing the graph without creating new nodes or changing plug values, and still have something that should be considered "unsavedChanges".
In particular, I was trying to add functionality that would check for unsaved changes to either prevent unnecessary saves, or suggest a save before closing the application, or opening another script. And right now I can't go ahead with that as I may miss the kind of changes described above.

Not a critical issue right now, but I'd like to add my vote in favor of addressing this issue.

[Chadt54]

Did you guys ever address this issue? It seems like resizing backdrops is undoable, but moving nodes is not. At least in the version I'm using (gaffer-0.52.3.1-linux)

[johnhaddon]

Nope - ticket is still open...