Question Undo system not registering changes

Hi,

I have code that looks like this:
Undo.RecordObject(object, "undo message");
object.aProperty = 123; // a new value

But it is not registering in the undo history. I have also tried adding Undo.FlushUndoRecordObjects(); but that doesn't help.

The object is serialisable. I have confirmed this with JsonUtility.ToJson(object); and this gives the expected result.
I have done this before and after my change and confirmed the change in the resulting JSON.

But it just will not add it to the undo stack.

Any ideas?

Thanks


Edit: Here is the full code of one part of my code that uses the undo system in case the summary above is not enough.
Also a link to the result of JsonUtility.ToJson(iconEditor.window): https://codebeautify.org/jsonviewer/y23c019d6
As you can see, the icon fields are successfully serialised (iconEditor->currentIcon)

If you are wondering why I record the entire window, it is because the Icon object itself is just a regular C# object, does not derive from unity object or scriptableobject, so cannot be passed into Undo.RecordObject so this is my workaround. I have had something similar before that worked.
I did also try deriving the Icon class from ScriptableObject and then passing that into RecordObject, but it still wasn't added to the stack.

Code (CSharp):

1. public static void DrawObjectControls(IconEditor iconEditor)
2.         {
3.             //---Begin change check---//
4.             EditorGUI.BeginChangeCheck();
5.  
6.             //---Draw position field---//
7.             Vector3 tmpObjPos = EditorGUILayout.Vector3Field("Position", iconEditor.currentIcon.objectPosition);
8.             Rect posR = GUILayoutUtility.GetLastRect();
9.             posR.x += 50;
10.             posR.height = 18;
11.             posR.width = 50;
12.  
13.             //---Draw auto button for position---//
14.             if (GUI.Button(posR, "Auto"))
15.             {
16.                 tmpObjPos = Utils.GetObjectAutoOffset(iconEditor.currentIcon, Utils.GetObjectBounds(iconEditor.currentIcon));
17.             }
18.  
19.             //---Draw auto all selected button for position---//
20.             posR.x += 50;
21.             posR.width = 150;
22.             if (GUI.Button(posR, "Auto All Selected Icons"))
23.             {
24.                 //---Loop through all selected icons---//
25.                 int index = 1;
26.                 foreach (Icon icon in iconEditor.assetGrid.selectedIcons)
27.                 {
28.                     //---Draw progress bar---//
29.                     EditorUtility.DisplayProgressBar("Updating Icons (" + index + "/" + (iconEditor.assetGrid.selectedIcons.Count - 1) + ")", icon.assetPath, ((float)index++ / (iconEditor.assetGrid.selectedIcons.Count - 1)));
30.  
31.                     //---Get auto offset position---//
32.                     if (icon != iconEditor.currentIcon)
33.                     {
34.                         icon.objectPosition = Utils.GetObjectAutoOffset(icon, Utils.GetObjectBounds(icon));
35.                         Utils.UpdateIcon(icon, iconEditor);
36.                     }
37.                     else
38.                     {
39.                         tmpObjPos = Utils.GetObjectAutoOffset(icon, Utils.GetObjectBounds(icon));
40.                     }
41.                 }
42.  
43.                 //---Clear progresss bar---//
44.                 EditorUtility.ClearProgressBar();
45.             }
46.  
47.             //---Draw object rotation and scale fields---//
48.             Vector3 tmpObjRot = EditorGUILayout.Vector3Field("Rotation", iconEditor.currentIcon.objectRotation);
49.             Vector3 tmpObjScale = EditorGUILayout.Vector3Field("Scale", iconEditor.currentIcon.objectScale);
50.  
51.             //---Draw link scale toggle---//
52.             Rect r = GUILayoutUtility.GetLastRect();
53.             r.position += new Vector2(40, 0);
54.             if (GUI.Button(r, iconEditor.linkScale ? iconEditor.scaleLinkOnImage : iconEditor.scaleLinkOffImage, GUIStyle.none))
55.             {
56.                 iconEditor.linkScale = !iconEditor.linkScale;
57.             }
58.  
59.             //---If any fields changed---//
60.             if (EditorGUI.EndChangeCheck())
61.             {
62.                 //---Record object for undo---//
63.                 Undo.RecordObject(iconEditor.window, "Edit Icon Object");
64.  
65.                 //---Apply position and rotation---//
66.                 iconEditor.currentIcon.objectPosition = tmpObjPos;
67.                 iconEditor.currentIcon.objectRotation = tmpObjRot;
68.  
69.                 //---If link scale disabled, apply scale---//
70.                 if (!iconEditor.linkScale)
71.                 {
72.                     iconEditor.currentIcon.objectScale = tmpObjScale;
73.                 }
74.                 else
75.                 {
76.                     //---If link scale enabled, detect which axis changed and apply to all axes---//
77.                     if (tmpObjScale.x != iconEditor.currentIcon.objectScale.x)
78.                     {
79.                         iconEditor.currentIcon.objectScale = tmpObjScale.x * Vector3.one;
80.                     }
81.                     else if (tmpObjScale.y != iconEditor.currentIcon.objectScale.y)
82.                     {
83.                         iconEditor.currentIcon.objectScale = tmpObjScale.y * Vector3.one;
84.                     }
85.                     else if (tmpObjScale.z != iconEditor.currentIcon.objectScale.z)
86.                     {
87.                         iconEditor.currentIcon.objectScale = tmpObjScale.z * Vector3.one;
88.                     }
89.                 }
90.  
91.                 //---Save and update icon---//
92.                 iconEditor.currentIcon.saveData = true;
93.                 iconEditor.updateFlag = true;
94.             }
95.         }

 

Unfortunately that didn't help

 

No, the object is contained in the window. As you can see from the linked json.

 

That json file is very big, and I don't think you can reasonably expect people here to read through it to understand your class structure

I think what Karl probably meant is these lines seem weird:

Code (CSharp):

1.  
2.                 Undo.RecordObject(iconEditor.window, "Edit Icon Object");
3.                 //---Apply position and rotation---//
4.                 iconEditor.currentIcon.objectPosition = tmpObjPos;
5.                 iconEditor.currentIcon.objectRotation = tmpObjRot;
6.  

You're recording undo for "iconEditor.window" but then changing "iconEditor.currentIcon".

If "window" is what you're serialising, I would expect the icon to be inside it, something like:

Code (CSharp):

1.  
2.                 Undo.RecordObject(iconEditor.window, "Edit Icon Object");
3.  
4.                 //---Apply position and rotation---//
5.                 iconEditor.window.currentIcon.objectPosition = tmpObjPos;
6.                 iconEditor.window.currentIcon.objectRotation = tmpObjRot;
7.  

 

Well I pointed out exactly where in the json it is in the original post. You don't need to read it. Window contains a reference to iconEditor, which contains a reference to currentIcon.

So it's window.iconEditor.currentIcon.

This is also irrelevant anyway as even if I make Icon a scriptable object and pass it directly into RecordObject, it still does no get added to the Undo stack. And the json does in fact confirm that the window Object changes before/after the call to RecordObject anyway. Thus is not the issue.

 

Yes its an editor window but I think it's not worth getting hung up on this as I have also tried passing the icon directly and that doesn't work. There is no such thing as window.currentIcon. The reference to currentIcon is in iconEditor.

I made the icon class a ScriptableObject, then used Undo.RecordObject(iconEditor.currentIcon). But this still doesn't work. So the issue is unrelated to the window. And as I've already proven it is unrelated to a serialisation issue.

I also I have had an almost identical setup before that worked just fine.

 

According to the Json your object hierarchy is as follows:

• iconEditor
  • currentIcon
  • window

It seems incorrect to record undo for the window, it should record undo on the icon editor instead.

 

Sure, but Unity only lets you record objects, and IconEditor is not an object. As I've said 5+ times now, this is not the issue. It has nothing to do with what object is being passed in. As I have tried directly passing in the Icon scriptable object - the object that is actually being changed - and it is still not recored. The issue must be deeper.

And again, this exact setup has worked before

 

Could you lay out again what your classes inherit from respectively what their built-in type is?
Specifically: IconEditor, IconEditor's CustomEditor type and its inherited type, window, currentIcon

And what class and method do you call DrawObjectControls from?

It looks like an OnGUI thing and what I've seen users fail to understand sometimes is that if you have an Editor script, you cannot just modify a property or public field of an object - you may need to get the SerializedObject and modify that. Perhaps this is where things go wrong here.

 

window derives from EditorWindow. Here is what that window looks like


There is then an IconEditor class which handles everything in the right hand pane of that window - it is not a monobehaviour. There is no CustomEditor for it. There are also other classes to handle the other elements, such as the asset list, the grid of previews, and the draggable separators between these panes.

In the window OnGui I call iconEditor.Draw() which handles all the GUI elements in that pane. DrawObjectControls is a function which draws the GUI elements for the "Object" tab, i.e. those three vector3 fields.

currentIcon is a reference to an Icon class. The Icon class contains all the information about the icon (such as those object fields).

Those fields modify properties in the Icon class. I want to be able to undo those. Hence why I am using Undo.RecordObject(). But no matter whether I pass the whole window (which includes the icon), or just the Icon ScriptableObject iteself (currentIcon). There is nothing being added to the Undo stack.


The window class contains a reference to the IconEditor class, as well as all the other element classes.

The iconEditor also has a reference back to the window.

So iconEditor.window.iconEditor is the same as iconEditor.
iconEditor.currentIcon is the same as window.iconEditor.currentIcon
etc.

I've worked with the undo system in unity quite a considerable amount and never had this issue

 

Is the IconEditor a serialized field of the window?

IconEditor is a subclass of Editor I suppose?

If Icon is a ScriptableObject, and you modify a [SerializeField] or public field of it, this may still need to be done through the SerializedObject version of the SO. I'm not sure how this works for custom windows/editors but you cannot just change fields of an object instance that has a custom Inspector editor. So this may be the same issue here.

Or you are modifying a property that is not serialized?

 

Everything is serialised as expected, as proved by the JSON.
IconEditor is not a subclass of anything.
All the fields that are modified are public fields, and therefore are serialised by default.
I can have a look at getting the serialised object but this setup has worked before without doing that.


Edit: still does not work with this code - no undo recorded:

Code (CSharp):

1.                 //---Record object for undo---//
2.                 Undo.RecordObject(iconEditor.currentIcon, "Edit Icon Object");
3.  
4.                 //---Apply position and rotation---//
5.                 SerializedObject obj = new SerializedObject(iconEditor.currentIcon);
6.                 var rot = obj.FindProperty("objectRotation");
7.                 rot.vector3Value = tmpObjRot;
8.                 obj.ApplyModifiedProperties();

 

That may be your issue!

The signature for RecordObject requires you to pass a UnityEngine.Object:

Code (CSharp):

1. public static void RecordObject(Object objectToUndo, string name);

From what I remember, you cannot serialize custom classes, not even if they are marked [Serializable]. They will be serialize and undoable however if you put them as a serializable field in a class that inherits from UnityEngine.Object, and then you call RecordObject on that container class.

 

It's really not the issue. You can serialise custom classes in Unity and as I have already proven there are no issues with serialisation. And it doesn't work even if I pass directly the icon object. I feel like I'm just repeating myself with every comment.

That is already what I am doing by passing in the window object.


No issues with serialisation, here is the result of serialising the window object, and clearly here are the properties that are being changed:

 

Ignore your json serialization for once! It may not be representative for what Unity's (undo) serializer serializes.

 

I give up tbh. This exact code used to work (literally checked it out from my git repo). I know it's compatible and I don't want to keep repeating myself. JSON utlity uses Unity's serialiser - the output is the same.

The undo system is even no longer working with completely unrelated code that has always worked. My point is, the issue I am having is much deeper but people keep repeating the same irrelevant comments over and over again.

 

In that case it seems likely that some editor script breaks undo for you.

I come to think of AssetDatabase.Start/StopAssetEditing not enclosed in try/catch - there may be something like that for Undo which prevents undo from recording new objects. Undo groups come to mind, just like collapsing and clearing undo buffer methods, and so on. You may want to hook into all undo events and log them, and experiment with "register complete undo" and what other methods there are.

Try to create a minimal example in an empty project to see if at least the current object hierarchy you have does work with undo. Or take the other route and strip down your current code (in a copy of the project) until only the serializable data remains while being able to make a change to the fields you want to have undo-able (for example by changing values through keyboard/mouse events).

You may also want to try your code in a (very) different Unity version to rule out any chance of this being a Unity editor bug in the particular version of Unity you're working, but generally it's more likely to be an issue within the current project unless you can prove there's an issue even with a minimal code example.

 

What does this mean exactly - did it work on an earlier Unity version? Or the exact same project on the exact same version once worked and now doesn't?

Sounds like you have a lot of technical experience, so you probably know if you have a bit of code that worked and then stopped - something must've changed to cause that. Either the Unity Editor version, new packages, some new code you wrote that interacts with it, etc.

And honestly if you find you have code that worked, and the exact same thing doesn't work in a newer Unity version, that sounds like a great bug report to Unity.

 

I am also facing similar issue, I am on 2022.3.7f1. The documentation doesn't really help. To clarify, I have only used Unity in past 2 months and trying to create a custom editor. I assume Undo means CTRL+Z and Undo.RecordObject() creates an entry in the Undo History CTRL+U window.

When I move the points, the value changes but nothing happens to the undo history.
Can anyone please help spot what the problem is? Thanks!



My test component:

Code (CSharp):

1. using System.Collections.Generic;
2. using UnityEngine;
3.  
4. public class MyComponent : MonoBehaviour
5. {
6.     [SerializeField]
7.     public List<Vector3> points { get; private set; }
8.  
9.     public void CreateTool() {
10.         points = new List<Vector3>();
11.         points.Add(transform.position + Vector3.left);
12.         points.Add(transform.position + Vector3.right);
13.         Debug.Log($"CreateTool");
14.     }
15. }

And my editor:

Code (CSharp):

1. using UnityEngine;
2. using UnityEditor;
3.  
4.  
5. [CustomEditor(typeof(MyComponent))]
6. public class MyComponentEditor : Editor
7. {
8.     MyComponent component;
9.  
10.     private void OnSceneGUI() {
11.  
12.         for(int i = 0; i < component.points.Count; i++) {
13.             Vector3 newPos = Handles.FreeMoveHandle(component.points[i], 0.1f, Vector3.zero, Handles.CylinderHandleCap);
14.             if(component.points[i] != newPos) {
15.                 Undo.RecordObject(component, "Move Point");
16.                 component.points[i] = newPos;
17.                 Debug.Log($"I am inside here changing position: {component.points[i]}");
18.             }
19.         }
20.     }
21.  
22.     void OnEnable() {
23.         component = (MyComponent)target;
24.  
25.         if(component.points == null) {
26.             component.CreateTool();
27.         }
28.     }
29. }
30.  

And my Debug log and Undo History