Search Unity

[WIP, Open-Source] UndoPro - Command-pattern Undo integration

Discussion in 'Immediate Mode GUI (IMGUI)' started by Seneral, May 23, 2016.

  1. Seneral

    Seneral

    Joined:
    Jun 2, 2014
    Posts:
    1,206
    The Problem
    If you're an editor developer like me you've likely already stumbled over Undo. It's essential for basically every editor extension/tool. But implementing it can be a real pain, because of the nature of the Undo system - you have predefined common actions like creation and deletion of objects and a general purpose modification of objects. That's it, and eg. recording list changes requires you to record the whole object containing the list, which is often total bullshit because there's the greatly better, performance-wise faster and less complicated way: An action based Undo system.
    There are solutions like VFW by vexe... But until now they were all seperate systems that required the developer to force their users into a new workflow, as the common shortcuts were used by the default Undo system.

    for TL;DR:
    The Solution
    So I've tried hard and worked on an integration of the Command-Pattern (or Action-based) Undo system into the default Unity Undo system. It is in a mostly stable state, although new changes could at any point throw the tracking off - at which point the undo might trigger not at the original intended record, but a few off (but it will always trigger).

    [REPO]

    Features
    - Extended Callbacks for Undo: Undo/Redo seperated, OnAddUndoRecord, ...
    - Add action-based undo records with both undo- and redoactions
    - Includes a pretty solid and stable serializable action
    -> Supports all serializable objects (of both UnityEngine.Object and System.Object) and unserializable objects partially (one layer serializable member serialization), all other objects get defaulted
    -> Supports even most anonymous actions (no unserializable found yet)! You can fully use the context and reference nearly all local variables (conditions outlined above apply)!

    Functionality behind
    Provided about the default Undo system is only the current ID (not unique for a record, but a steadily increasing one), the current group name and with reflection the complete Undo/Redo stack only by name (not unique). As the behaviour of the default Undo system is nearly unpredictable (records may duplicate in certain conditions when undone/redone, or vanish), it is very hard, but a requirement to make a solid tracking algorithm. Additionally, the addition of new records has to be detected.
    I'm making use of Update to check for new records and the UndoRedoPerformed callback to query the stacks for the undone/redone records. Then I'll update the internal UndoPro records, represented by a dummy record in the default undo system, accordingly.

    Usage
    UndoPro gives two fundamental capabilities to enable an command-based undo pattern:
    1. It integrates custom undo records into the default undo system by creating dummies, and keeping track of them in order to trigger the provided undo/redo function whenever it's associated dummy record is called.
    This by itself could support a separate undo system through static function calls, but we want more than that - a command based system - so:
    2. Instead of static functions, anonymous functions are also supported, so that each record can keep it's own data encoded into the anonymous functions.
    Why is this a big deal? Static functions only tell you 'Undo that', without context and associated data (e.g. undo node delete, but without knowing which node, what it was connected to, etc.).
    Instead, you can choose data to be stored alongside the action.

    Of course, conditions apply. That data has to be explicitly assigned to own local variables right before the anonymous action is defined (in the same command level, e.g. if-block, as well!) to ensure it is correctly serialized.
    Code (csharp):
    1.  
    2. ConnectionPort port1 = this, port2 = port;
    3. UndoPro.UndoProManager.RecordOperation(
    4.     () => port1.TryApplyConnection(port2, true),
    5.     () => port1.RemoveConnection(port2, true),
    6.     "Create Connection");
    7.  
    Would be valid (stored data: two SerializableObjects in this case),
    Code (csharp):
    1.  
    2. ConnectionPort port1 = this, port2 = port;
    3. {
    4.     UndoPro.UndoProManager.RecordOperation(
    5.         () => port1.TryApplyConnection(port2, true),
    6.         () => port1.RemoveConnection(port2, true),
    7.         "Create Connection");
    8. }
    9.  
    would be invalid. It would result the variables to be stored elsewhere, which can give complications.

    As for the TYPE of the variables, these are supported:
    UnityEngine.Object, Serializable System.object, ICollections of aforementioned things, unserializable types with serializable members
    And these are NOT supported:
    Nested Lists, Non-serializable structures (e.g. Vector2)
    Null-values are supported, if a type is NOT supported, it will be created to its default state.

    Why is serialization important?
    These actions have to survive script compilations and playmode change (not scene change), so they need to be serialized.
    Note that your references WILL be broken, so if you care about references, use ScriptableObjects, which will retain their reference (Provided that they still exist after the reload, as for the NEF I had to change a bit to comply to this restriction).

    Usage Example
    A comprehensive example provides the implementation of Undo in the Node Editor Framework

    Current Problems
    As of now, there are no constellations that break the tracking system of UndoPro

    Any feedback welcome!
     
    Last edited: Oct 15, 2019
    elmar1028 likes this.
  2. crispybeans

    crispybeans

    Joined:
    Apr 13, 2015
    Posts:
    210
    I'd be happy to take a look at it since we did alot of Editor extensions in the past for projects It would be nice to see what you have cooked up.
     
  3. Seneral

    Seneral

    Joined:
    Jun 2, 2014
    Posts:
    1,206
    Ok, I will cook a unitypackage this evening when I'm back home:)
     
  4. Seneral

    Seneral

    Joined:
    Jun 2, 2014
    Posts:
    1,206
    @crispybeans
    Sorry for the late reply. I attached a bundle with the beta:)
    I previously mentioned known issues regarding playmode change, but I seem to have fixed that fortunately. I currently find no way of breaking it, it survives playmode changes, script reloads, anomalies regarding reparenting the selected object, etc... and on scene change, the stack is cleared, just as it should.
    If you find anything that breaks it, please tell me!

    Note: It is considered as breaking when the tracking failed, means in the test window the index of the internal record (bottom) does not match the corresponding tracked record in the default system (top). That would result in a shifted triggering of the custom record:(

    It may also help to enable debug, just uncomment the #define UNDO_DEBUG in UndoProManager.cs!
    I also included two test windows, one regarding Action serialization and the other that enables you to quickly create action based undos and debug the stacks.

    Hope you will find it easier to work with than the standard undo system:)
    Seneral
     

    Attached Files:

    elmar1028 likes this.
  5. elmar1028

    elmar1028

    Joined:
    Nov 21, 2013
    Posts:
    2,359
    Hi,

    Looks pretty good. Few questions though:

    1) Are you planning to make it available on GitHub (given that it's an open source project)?
    2) What's the license of the project? Do I need to pay for it to be implemented into commercial editor extension (e.g sell it on Asset Store)

    Thanks!
     
  6. Seneral

    Seneral

    Joined:
    Jun 2, 2014
    Posts:
    1,206
    I've not thought much about the license, but I'll probably set it up on GitHub, under the MIT license (similar to the Node Editor Framework project). I won't take any money because I think it's more like a fix than a feature;)

    Did you already test it, and what do you think?
     
  7. elmar1028

    elmar1028

    Joined:
    Nov 21, 2013
    Posts:
    2,359
    Didn't get an opportunity to try it yet. I promise I will ;)

    What's the recommended Unity version? What Unity version did you use?
     
  8. Seneral

    Seneral

    Joined:
    Jun 2, 2014
    Posts:
    1,206
    I used 5.3.5f1, though it should be compatible with most versions (atleast past 5.0).
    Scene stuff for example should also be compatible with < 5.3
     
    Last edited: Jun 6, 2016
  9. Seneral

    Seneral

    Joined:
    Jun 2, 2014
    Posts:
    1,206
    I set up a repo for this project, check it out here:)
     
  10. rigidbuddy

    rigidbuddy

    Joined:
    Feb 25, 2014
    Posts:
    39
    Hi Seneral! First of all let me thank you for solving such a pain as unity's Undo system.
    But I've got errors after assembly reload.
    How to reproduce:
    1) Window->SerializableActionTest
    2) Create some actions
    3) Create script in the project view (to force assembly reload). Got about 52 errors after deserialize.

    Regards,
    Mikhail
     
  11. rigidbuddy

    rigidbuddy

    Joined:
    Feb 25, 2014
    Posts:
    39
    The same happens after turning playmode on.
    And UndoPro clears after unloading scene.
    Maybe setting hideflags would help but not sure it's the best way solving it.
     
  12. Seneral

    Seneral

    Joined:
    Jun 2, 2014
    Posts:
    1,206
    I just tried but cannot reproduce this problem, everything is serializing fine for me. It's most probably platform specific, can you tell me your unity version, target platform and operating system?
    For reference, I'm using 5.3.6 f1, but tested it on some previous versions aswell. I'm on Windows 7 and Build target is Standalone.
    Or do you use the new 5.4? I'll download it now to test, have to do it for my other projects either way;)

    You second note, that UndoPro clears on scene change, is actually intended. It's the behaviour of the default Undo system and it's also quite logic. Undo records are only important for the current scene;)

    Seneral
     
  13. Seneral

    Seneral

    Joined:
    Jun 2, 2014
    Posts:
    1,206
    Ok tested with 5.4, I was able to reproduce it there. I'll post updates on the issue thread you posted on the repo:)
     
  14. rigidbuddy

    rigidbuddy

    Joined:
    Feb 25, 2014
    Posts:
    39
    Yeah, it's such a pain =/
    Good luck solving it!
     
  15. Fullymetal

    Fullymetal

    Joined:
    Mar 4, 2014
    Posts:
    4
    Hi Seneral,

    I have a problem with "Selection Change" of the Unity undo stack. It is possible to track "Selection Change" in UndoPro Undo stack ? Or don't track "Selection Change" in Unity undo stack ?

    Thank you.

    Fully
     
  16. Seneral

    Seneral

    Joined:
    Jun 2, 2014
    Posts:
    1,206
    Can you explain in more detail what you mean? Are you referring to the callbacks?
     
  17. Fullymetal

    Fullymetal

    Joined:
    Mar 4, 2014
    Posts:
    4
    I work in a BehaviorTree editor, and I want implement Undo/Redo system. I implement a switch method in behavior, for example ActiveSequence become ActiveSelector, so I have a custom entry in UndoPro stack and "Selection Change" entry in Unity stack (because Selection.activeObject changed). When I Ctrl-Z, Unity undo only "Selection Change" and I like undo "Selection Change" and custom Switch action with only one Ctrl-Z.

    I try lot of thing but nothing work. Are you a idea ?

    Thank you
     
  18. Seneral

    Seneral

    Joined:
    Jun 2, 2014
    Posts:
    1,206
    @Fullymetal Ok now I understood:) Basically when a custom record is added UndoPro makes sure the current group is closed, so the added record is one of it's own. If you don't know what that means, the Undo records in unity are grouped together so that they will be undone/redone together. This is what you would want in your case... when I prevented this grouping of UndoPro records with others I didn't think about that case, so I'll fix this:)
     
  19. Seneral

    Seneral

    Joined:
    Jun 2, 2014
    Posts:
    1,206
    Ok it was as easy as I thought, removing one line did the trick:) But when building an example (in the UndoPro test window) to check if it gets the same effect you want to get, I finally found a solid way to produce an error I was occasionally getting a long time ago. Currently, it only throws a warning, but now that I am able to reproduce it, I can get on fixing it.
    It is very nasty, basically what happens is that when undoing, a random record gets added (usually SelectionChange), so that on the undo-stack when there are 3 records removed at once (when they are grouped), 4 records would be added to the redo stack... it only now affects the system now when I did the change to allow UndoPro records to be grouped with normal records - because if these normal records would trigger that bug, the UndoPro record that was grouped with them may have been shifted.
    If you don't want to wait for me to fix the bug (it won't affect every record with the grouping enabled), then in line 168, uncomment Undo.IncrementCurrentGroup (); :)
     
  20. Fullymetal

    Fullymetal

    Joined:
    Mar 4, 2014
    Posts:
    4
    Thank you very much Seneral.
     
  21. deltamish

    deltamish

    Joined:
    Nov 1, 2012
    Posts:
    58
    Can the API be used for ruin time use ?
    Say In-case of a game level editor where user may change materials and Create new objects the system should be able to track those actions and delete/revert them if called.

    Thanks
     
  22. Seneral

    Seneral

    Joined:
    Jun 2, 2014
    Posts:
    1,206
    No, this is not an undo system by itself, just a new way to use the existing editor undo.
    I think vexe has a very nice undo system for editor and runtime use, don't know what it's called though atm. You'll find it when you look it up:)
     
  23. Seneral

    Seneral

    Joined:
    Jun 2, 2014
    Posts:
    1,206
    Ok so I've been encouraged to finally make an Undo system for the Node Editor Framework with UndoPro.
    Out came some improvements:
    - Switched from ISerializationCallbackReceiver to ondemand serialization
    - Added support for Collection serialization for variables stored in anonymous functions
    - Other Bugfixes in serialization system
    - Reworked UndoProTestWindow to easily visualize if anything has gone wrong with tracking (use this to verify suspicions, if anything breaks, bold UndoPro records will get a normal font, split up and shift: 'Normal / Pro')
    - Added Begin-/EndRecordStack to be able to group singular UndoPro records together as a group

    In order to support lists in anonymous functions I unfortunately needed to make another (a third) level of serialization, which means I needed to duplicate the second level with some minor changes. While this is ugly, I haven't yet found a better way. Alternative would be to not treat lists in levels, but then unity would complain about loops and stuff.

    Also, I decided to actualy create a small guideline on how to use this (I've talked to a user who used UndoPro but not how I expected due to not knowing the capabilities is gives). I'll add it to the OP aswell.

    Usage
    UndoPro gives two fundamental capabilities to enable an command-based undo pattern:
    1. It integrates custom undo records into the default undo system by creating dummies, and keeping track of them in order to trigger the provided undo/redo function whenever it's associated dummy record is called.
    This by itself could support a separate undo system through static function calls, but we want more than that - a command based system - so:
    2. Instead of static functions, anonymous functions are also supported, so that each record can keep it's own data encoded into the anonymous functions.
    Why is this a big deal? Static functions only tell you 'Undo that', without context and associated data (e.g. undo node delete, but without knowing which node, what it was connected to, etc.).
    Instead, you can choose data to be stored alongside the action.

    Of course, conditions apply. That data has to be explicitly assigned to own local variables right before the anonymous action is defined (in the same command level, e.g. if-block, as well!) to ensure it is correctly serialized.
    Code (csharp):
    1.  
    2. ConnectionPort port1 = this, port2 = port;
    3. UndoPro.UndoProManager.RecordOperation(
    4.     () => port1.TryApplyConnection(port2, true),
    5.     () => port1.RemoveConnection(port2, true),
    6.     "Create Connection");
    7.  
    Would be valid (stored data: two SerializableObjects in this case),
    Code (csharp):
    1.  
    2. ConnectionPort port1 = this, port2 = port;
    3. {
    4.     UndoPro.UndoProManager.RecordOperation(
    5.         () => port1.TryApplyConnection(port2, true),
    6.         () => port1.RemoveConnection(port2, true),
    7.         "Create Connection");
    8. }
    9.  
    would be invalid. It would result the variables to be stored elsewhere, which can give complications.

    As for the TYPE of the variables, these are supported:
    UnityEngine.Object, Serializable System.object, ICollections of aforementioned things, unserializable types with serializable members
    And these are NOT supported:
    Nested Lists, Non-serializable structures (e.g. Vector2)
    Null-values are supported, if a type is NOT supported, it will be created to its default state.

    Why is serialization important?
    These actions have to survive script compilations and playmode change (not scene change), so they need to be serialized.
    Note that your references WILL be broken, so if you care about references, use ScriptableObjects, which will retain their reference (Provided that they still exist after the reload, as for the NEF I had to change a bit to comply to this restriction).

    Usage Example
    A comprehensive example provides the implementation of Undo in the Node Editor Framework
     
  24. stroibot

    stroibot

    Joined:
    Feb 15, 2017
    Posts:
    91
    Tries adding it to project but got `ArgumentException: method arguments are incompatible` immediately. Can't find anything that says for what version this was done for
     
  25. bullze

    bullze

    Joined:
    Aug 27, 2015
    Posts:
    1
    same Problem here! Using Unity 2021.3.18f1