-
Notifications
You must be signed in to change notification settings - Fork 413
Undo Support #36
Comments
I'm going to give this a shot. Will make a PR later and modify where it's needed |
Hi! Thanks @TwoOfDiamonds, very thankful for that. Might be a daunting task though. If you feel you need more control than the default undo system we could consider using UndoPro in order to make use of undo actions instead of the preset commands of unity. |
I was thinking about adding 2 stacks to the canvas and have one for the Undo action and one for the Redo action (Ctrl+Z and Ctrl+Y). I would add the performed actions to the Undo stack when they are done. Like adding, removing, moving nodes, etc. |
Not a good idea, better to use standard undo system of Unity. Atleast for the editor, that is. Because the big problem would be that you can not use the shortcuts Ctrl+Z and Ctrl+Y that everyone is used to, and you would also loose all other interfaces. |
I will try with the Unity Undo system and if it works for the editor I will be happy |
Hi, We're trying to use something based on UndoPro for our implementation of the Node Editor (to create behaviour trees). We're ran into a big issue though : say you modify a value in a node, you'll get an Inspector entry in the Undo stack. If you then delete the node, the first Undo will work and get it back, but it broke the reference of the node and the second Undo doesn't restore the former value. Is there a way around that ? |
I'll try to explain whats going on... int oldValue = node.value; // This will ensure it saves the current value, not node.value
UndoProManager.RecordOperation (()=>node.value = newValue, ()=>node.value = oldValue, "Changed Value"); Then these following variables are saved along the record: A way around this is replacing the direct reference to int oldValue = node.value; // This will ensure it saves the current value
string nodeID = node.uniqueID; // This will ensure it saves the actual ID
UndoProManager.RecordOperation (()=>nodeCanvas.GetNodeByID(nodeID).value = newValue, ()=>nodeCanvas.GetNodeByID(nodeID).value = oldValue, "Changed Value"); Which would cause these variables to be serialized along the record: Currently, there's nothing like a unique node ID implemented in the framework, the ID you can access on the node is type-specific. So you'd need to ensure yourself that each node has it's unique ID (add it into Well, long read, hope you understood it:) Btw, I think most undo parts can be done with the default undo system, especially related to the GUI for each node. Maybe it's better to try relying on that wherever possible, but I can imagine that quite some things are very tricky with it, hence I built UndoPro;) |
We already have an ID in every node for our Undo in the Pro records stack, but modifications in the inspector add an 'Inspector' entry in the default undo stack, not our custom stack, and we have no control over it. Same for 'Selection change' entries. Theses entries are generic, they work for selection changes or modifications in the inspector anywhere, not just in the node canvas (ie. selecting a script in the project window). But when they apply to a node that was deleted/recreated they break. Edit: just tried creating a 3d sphere, changing stuff, deleting it and Undo. The sphere is created again and the Inspector entry works. So Unity can do it. But, how? And can it apply to something other than a GameObject (our Nodes are ScriptableObjects), or outside of the Scene? |
Oups, misunderstood you, thought you were using UndoPro for both the 'value changed' and 'deleted' entries... I just tested, the default undo system does not work correctly with assets, but in the editor using the cache we are directly working on assets, so using the default system is not applicable here:( Here's a test snippet on a sample Node asset showing what I said above (GUI part only): GUILayout.Label ("Node '" + (node==null? "null" : node.name) + "' rect '" + (node==null? "null" : node.rect.ToString ()) + "'");
if (GUILayout.Button ("Test"))
{
node = null;
// Does not work:
// Undo.RegisterCreatedObjectUndo (node, "Created Node asset");
// UndoPro works here:
UndoPro.UndoProManager.RecordOperationAndPerform (
()=> {
node = CreateInstance<NodeEditorFramework.Standard.CalcNode> ();
AssetDatabase.CreateAsset (node, "Assets/Test.asset");
Debug.Log ("REDO PRO: Node is " + (node==null? "null" : node.name));
}, ()=> {
ScriptableObject.DestroyImmediate (node, true);
AssetDatabase.DeleteAsset ("Assets/Test.asset");
Debug.Log ("UNDO PRO: Node is " + (node==null? "null" : node.name));
}, "Created Node asset PRO");
// Make sure to copy values! And do NOT resuse the variable or it will overwrite the record variable!
Rect newRect = new Rect (0, 0, 100, 100);
Rect prevRect = node.rect;
Debug.Log ("Now changing rect from " + prevRect.ToString () + " to " + newRect.ToString () + "!");
UndoPro.UndoProManager.RecordOperationAndPerform (
()=> {
node.rect = newRect;
Debug.Log ("REDO PRO: Node Rect is " + (node==null? "null" : node.rect.ToString ()));
}, ()=> {
node.rect = prevRect;
Debug.Log ("UNDO PRO: Node Rect is " + (node==null? "null" : node.rect.ToString ()));
}, "Changed node rect PRO");
// Now using the default undo system. Copying is not needed
Rect newRect2 = new Rect (100, -100, 200, 50);
Debug.Log ("Now changing rect to " + newRect2.ToString () + "!");
Undo.RegisterCompleteObjectUndo (this.node, "Changed node rect NORMAL");
node.rect = newRect2;
Debug.Log ("AFTER: Node Rect is " + (node==null? "null" : node.rect.ToString ()));
} I'll report once I have a correct 'rig' supporting both UndoPro and normal records on asset objects along each other:) |
Ok so for now I actually can't see a way of implementing Undo, UNLESS we completely disable the current cache system, which directly works on the asset objects. Hope you understand, and please tell me if you found a way around it. |
Well we don't know much about Unity's undo system apart from the fact that we need it to put our behaviour tree editor on the store. The solution for our implementation would be to do every edits (child priority, method choice, etc..) from custom controls on the nodes instead of the inspector. That way we have full control and can use IDs like we do for other actions. But I don't think it's viable for a general use as we heavily modified the node editor to fit our project and others might still need to be able to edit properties from the inspector ? |
Ok, so I think I will be developing a different (simpler) cache system in the future that does not work on assets directly and as such is faster and allows for undo at the cost of a bit of safety incase of Unity crashing... |
Yes, using something like UndoPro with explicit records is not really applicable for the node GUI, not only because of the inspector, but I don't want to care about Undo when I'm simply creating a node with standard controls... So I think I'll just create a different, much simpler cache system that does not work directly on assets, which has the benefit of significantly less lag when creating nodes and a possibility to add Undo but with the risk of loosing progress when Unity crashes. But I think we can accept that, don't you think? |
How much progress are we talking about ? The entire session ? The undo is a requirement for the Unity store, if it doesn't work with assets, I don't think we have a choice. |
We could auto-save every 5 or 2 mins of course. So it's not automatically saved while it is created as it is right now, but saved at intervals and of course when closing the window. Of course you're right about Undo... Do you have a release date set? I'll try to do at least the cache over the holidays, then Undo should be possible:) |
Implemented second cache system that should enable Undo support in 50b28a3 |
We'll have a look at it, thanks ! |
Great Work!! |
Oh hold on, this should be closed! Implemented long ago in 0f69501 |
Description: Undo support is a MUST. Using the default system, this would be absolutely tedious though. A command based pattern would be preferred. Maybe a mix of both is the best choice.
Progress: I've a WIP Command-Pattern Undo Integration (yes, integration into the default system:D ) we can use for the editor when it's finished (Progressing good, but currently on hold). For Runtime, VFW would probably be the best choice as the undo system (also command-pattern). Currently, theres no undo support though.
The text was updated successfully, but these errors were encountered: