Get Undo.DestroyObjectImmediate to work nicely when removing a List/array element that's pointing to a MonoBehaviour?

Hello,

so I’ve been stuck with some really annoying bug for the last 2 days, helpless!

‘Basically’, what I have is a FSMTrigger with a List - The FSMTrigger is a MonoBehaviour that gets attached to a gameObject. It has a custom editor to add new states. FSMTriggerState is also an MB. When I create a new state, I create a new GO as a child to the FSM and attach the state script on it.

Using serialized properties, everything works nice and cozy. I can add a state and then undo, the state gets removed from the list, and the created GO gets destroyed as well (using Undo.RegisterCreatedObjectUndo)

The problem is when I remove a state - Just like adding a new state adds a new GO and a state to the list of states, removing a state does the opposite, it removes the state GO and removes the state entry for the list of states in the fsm.

Here’s the editor, in a very basic form (showing only what’s related):

[CustomEditor(typeof(Test))]
class TestEditor : Editor
{
	public override void OnInspectorGUI()
	{
		base.OnInspectorGUI();

		serializedObject.Update();

		var sp_states = serializedObject.FindProperty("states");

		if (GUILayout.Button("Add")) {
			var state = Utils.CreateGoWithMb<FSMTriggerState>("TEST STATE " + sp_states.arraySize, HideFlags.None, (target as Test).transform);
			Undo.RegisterCreatedObjectUndo(state.gameObject, "Created new state");
			sp_states.Add(state);
		}

		for (int i = 0; i < sp_states.arraySize; ) {

			EditorGUILayout.BeginHorizontal();
			{
				var sp_STATE = sp_states.GetAt(i);

				GUILayout.Label(sp_STATE.objectReferenceValue.name);

				if (GUIHelper.RemoveButton("Remove")) {
					Debug.Log("Size before removal: " + sp_states.arraySize);
					foreach (SerializedProperty s in sp_states) {
						Debug.Log(s.objectReferenceValue.name);
					}

					Undo.DestroyObjectImmediate(sp_STATE.gameObject());
					sp_states.RemoveAt(i);

					Debug.Log("Size after removal: " + sp_states.arraySize);
					foreach (SerializedProperty s in sp_states) {
						Debug.Log(s.objectReferenceValue.name);
					}
					continue;
				}
			}
			EditorGUILayout.EndHorizontal();

			i++;
		}

		serializedObject.ApplyModifiedProperties();
	}
}

If I don’t destroy the state’s GO, everything works fine. The state gets removed from the list, I could undo/redo, etc no problems whatsoever.

However, destroying the state’s GO will bring all sorts of problems. First, notice the two debug logs that’s showing the arraySize before/after the destroy, it will display the same size before and after! And also NullRefs… - I’m pretty sure there’s no problem with the RemoveAt method (that is, it ‘usually’ works fine), it does a double DeleteElementAtIndex (I also tried another manual method) - but if you’re doubting it, here it is:

public static void RemoveAt(this SerializedProperty prop, int atIndex)
{
	// Credits to Jamora @UA http://answers.unity3d.com/users/103522/Jamora.html
	// he was asking me about my remove method (the code commented below, and then said:
	/*"
	 * there is an alternative way, which I'm not sure if is a bug or not...
	 * doing SerializedProperty.DeleteArrayElementAtIndex to a null value removes the index
	 * and moves the remaining values down so nothing is lost.
	 "*/
	// This is true only for reference types
	var at = prop.GetAt(atIndex);
	if (at.IsReferenceType() && at.objectReferenceValue != null)
		prop.DeleteArrayElementAtIndex(atIndex);
	prop.DeleteArrayElementAtIndex(atIndex);

	// This is my old method
	//for (int i = atIndex, size = prop.arraySize; i < size - 1; i++) {
	//	prop.SetObjectValueAt(i, prop.GetObjectValueAt(i + 1));
	//}
	//prop.arraySize--;
}

Funny thing is though, if I try the other removal method (the one commented), it actually works well - I don’t get a NullRefExc when I destroy the state’s object, and when I undo, I do get the state’s GO back AND the state entry at the list, but guess what? it’s null! the link is broken between the state reference in the list and the actual state MonoBehaviour!

I made a video about this whole thing, but had some very bad luck editing it, tried more than 4 editors. There was a problem along the way for each of them.

So I just uploaded the relevant parts to the problem, 3 parts. [First][1], [second][2], [third][3]. (At first I show that everything works fine if I don’t destroy, the rest parts shows what happens when I do Destroy…)

To recap, what I’m asking for is: If you have a list of MonoBehaviours referenced by a serialized property, how do you get that to work nicely with undoing such that removing a list element removes the MB GO as well?

I really appreciate any help, thanks very much!

EDIT:

Test.cs

using UnityEngine;
using System.Collections.Generic;

public class Test : MonoBehaviour
{
	public List<FSMTriggerState> states = new List<FSMTriggerState>();
}

EDIT:

It seems that the problem doesn’t have to do with SerializedProperties, I just tried using target and modifying stuff directly and manually registering undo, same result! (as seen in the last video)

	for (int i = 0; i < states.Count; ) {

		isRemoved = false;

		var state = states*;*
  •   	EditorGUILayout.BeginHorizontal();*
    
  •   	{*
    
  •   		GUILayout.Label(state.name);*
    
  •   		if (GUIHelper.RemoveButton("Remove")) {*
    
  •   			Debug.Log("Size before removal: " + states.Count);*
    
  •   			foreach (var s in states) {*
    
  •   				Debug.Log(s.name);*
    
  •   			}*
    
  •   			Undo.RecordObject(target, "Removed a state");*
    
  •   			Undo.DestroyObjectImmediate(state.gameObject);*
    
  •   			states.RemoveAt(i);*
    
  •   			Debug.Log("Size after removal: " + states.Count);*
    
  •   			foreach (var s in states) {*
    
  •   				if (s != null)*
    
  •   					Debug.Log(s.name);*
    
  •   			}*
    
  •   			isRemoved = true;*
    
  •   		}*
    
  •   	}*
    
  •   	EditorGUILayout.EndHorizontal();*
    
  •   	if (!isRemoved)*
    
  •   		i++;*
    
  •   }*
    

It seems like it’s taking a Null snapshot of the state or something, not sure…
EDIT:
So with the great help of @whydoidoit (who mentioned the idea of delayCall), and @immersiveGamer (who mentioned caching the GO to be destroyed) I was able to make progress, I got really close this time… So here’s what’s happening now: when I click on remove, the state element gets removed from the list, and the state’s GO gets destroyed successfully. If I undo now, sometimes everything gets back intact, other times it doesn’t, I don’t know why, I can’t really explain it…
Here’s what I have now:

  •   for (int i = 0; i < sp_states.arraySize; ) {*
    
  •   	var sp_STATE = sp_states.GetAt(i);*
    
  •   	EditorGUILayout.BeginHorizontal();*
    
  •   	{*
    
  •   		GUILayout.Label(sp_STATE.objectReferenceValue.name);*
    
  •   		if (GUIHelper.RemoveButton("state")) {*
    
  •   			Debug.Log("Size before removal: " + sp_states.arraySize);*
    
  •   			currentGoToDestroy = sp_STATE.gameObject();*
    
  •   			remove = () =>*
    
  •   			{*
    
  •   				Undo.DestroyObjectImmediate(currentGoToDestroy);*
    
  •   				EditorApplication.delayCall -= remove;*
    
  •   			};*
    
  •   			EditorApplication.delayCall += remove;*
    
  •   			sp_states.RemoveAt(i);*
    
  •   			Debug.Log("Size after removal: " + sp_states.arraySize);*
    
  •   			continue;*
    
  •   		}*
    
  •   	}*
    
  •   	EditorGUILayout.EndHorizontal();*
    
  •   	i++;*
    
  •   }*
    

I tried to reverse the order, i.e. call RemoveAt and then Destroy, both orders gave the same results. I tried other combinations, like putting both Destroy and RemoveAt inside the delegate, but that didn’t go too well - same as with Destroying outside, and RemoveAt inside.
[1]: - YouTube
[2]: - YouTube
[3]: - YouTube

Using Delegate

Ok, so I figure it out. Indeed you can undo it all. I had tested out manually deleting the component and then the game object. And Unity was able to do it. What you need to do is create a delegate using EditorApplication.CallBaclFunction. First store the game object, then remove the reference and the call the Undo.DestroyObjectImmediate. Here is my code.

using UnityEngine;
using UnityEditor;
using System.Collections;

[CustomEditor(typeof(ContainerScript))]
public class ConatinerEditor : Editor {

	ContainerScript script;

	GameObject clone;
	int index;

	EditorApplication.CallbackFunction dlg;

	void OnEnable()
	{
		dlg = new EditorApplication.CallbackFunction(Remove);
		script = target as ContainerScript;
	}
	
	public override void OnInspectorGUI ()
	{
		DrawDefaultInspector();

		if(GUILayout.Button("Add"))
		{
			clone = new GameObject();
			clone.AddComponent<ClassScript>();
			clone.transform.parent = script.gameObject.transform;
			Undo.RegisterCreatedObjectUndo(clone, "");
			Undo.RecordObject(script, "");
			script.list.Add(clone.GetComponent<ClassScript>());
		}

		for(int i = 0; i < script.list.Count; i++)
		{
			if(GUILayout.Button("Remove at: "+i))
			{
				Debug.Log("pressed");
				clone = script.list*.gameObject;*
  •  		index = i;*
    
  •  		Undo.RecordObject(script, "");*
    
  •  		script.list.RemoveAt(index);*
    
  •  		EditorApplication.delayCall += dlg;*
    
  •  	}*
    
  •  }*
    
  • }*

  • public void Remove()*

  • {*

  •  Undo.DestroyObjectImmediate(clone);*
    
  •  EditorApplication.delayCall -= dlg;*
    
  • }*

}

## First Answer ##
This was my first answer
I’ve gone ahead and tried to solve the problem myself. Though it wasn’t a exhaustible text, it was thorough. I believe that it is just impossible for unity to undo this kind of action. It works when adding new objects because when unity retraces it’s steps it creates a new object and assigns the mono behavior to the list. While it might make sense that deleting should be the same it is not. So you remove the reference and delete the object, but here is the catch. When Unity undos and retraces it’s steps it creates a new object, this object isn’t the same as the one destroyed. Might be exactly same in form but it’s essence is different (probably the internal id is a new one). So when Unity then tries to undo the reference removal that object is no longer there (even though it seems like it). I tried not destroying the object and it worked just fine. Maybe if you could figure out a way to not destroy the object but hide it and set it so that it is cleaned up when the application exits (maybe changing hide flags?) then it might work.
As I said, this is just what I think is happening, but would make sense if that were the case.

Ok so this works as a very simple way of adding and removing with no issues on Undo/Redo

#EditHolder.cs

using UnityEngine;
using System.Collections;
using UnityEditor;

[CustomEditor(typeof(Holder))]
public class EditHolder : Editor {

	public override void OnInspectorGUI ()
	{
		var holder = target as Holder;
		if(GUILayout.Button ("+")) {
			Undo.RecordObject(holder, "Added");
			var newOne = Instantiate(holder.prefab) as GameObject;
			Undo.RegisterCreatedObjectUndo(newOne, "Added");
			Undo.SetTransformParent(newOne.transform, holder.transform, "Added");
			holder.instances.Add (newOne.GetComponent<Instance>());
			Undo.RecordObject(holder, "Added");
			Undo.CollapseUndoOperations(Undo.GetCurrentGroup());
		}
		if(GUILayout.Button ("-") && holder.instances.Count > 0) {
			var itemToRemove = holder.instances[0].gameObject;
			Undo.RecordObjects( new UnityEngine.Object [] { itemToRemove, holder }, "Removed");
			Undo.RecordObjects( new UnityEngine.Object [] {  holder }, "Removed");
			holder.instances.RemoveAt(0);
			Undo.DestroyObjectImmediate(itemToRemove);
			Undo.RecordObjects( new UnityEngine.Object [] {  holder }, "Removed");
			Undo.CollapseUndoOperations(Undo.GetCurrentGroup());
			Repaint ();
		}
		base.OnInspectorGUI ();//
	}
}

#Holder.cs

using UnityEngine;
using System.Collections;

public class Holder : MonoBehaviour {

    public GameObject prefab;
	public System.Collections.Generic.List<Instance> instances;

}

#Instance.cs

using UnityEngine;
using System.Collections;

public class Instance : MonoBehaviour {

}

Hi the option of caching the object worked for me! here is my implementation

ticksIngraph a list of gameobject that I want to delete and clear, and when I undo I want the list to be filled with the objects it had.
Attention this function is not running on a editor script but it works the same way

//converts list to array
		GameObject[] gos = ticksIngraph.ToArray ();

		//record the script
		Undo.RecordObject(this,"Stuff");

		//clears list
		ticksIngraph.Clear ();


		//iterates through array
		for (int i = 0; i < gos.Length; i++) {

			//not sure if I needed to this but well it works the same
				GameObject currentGoToDestroy = gos*;*
  •  	 	Undo.DestroyObjectImmediate(currentGoToDestroy);*
    
  •  }*
    

I’m working on similar problems recently, and here is my finding on Unity5.0.0f3;

first, the simplified settings:

public class SOA : ScriptableObject
{
    public int intdata = 1;
    public SOB sobInst;
}
public class SOB : ScriptableObject
{
    public float floatdata = 2.0f;
}
public class testWnd : EditorWindow
{
    public SOA m_soa; 
    public List<SOA > m_soas = new List<SOA>(); //will add 3 elem later
}

And the findings:
(1) Record first, change the field, destroy at last;
(2) if you want to track changes of field after any call of DestroyObjectImmediate(), need to call RecordObject again;

Examples:

  1. destroy the hierarchy of objects:

Undo.RecordObject(this, “delete soa”);
var soa = m_soa;
m_soa = null;
Undo.RecordObject(soa, “delete soa.sob”);
var sob = soa.sobInst;
soa.sobInst = null;
Undo.DestroyObjectImmediate(sob);
Undo.DestroyObjectImmediate(soa);

  1. destroy an object from a list:

Undo.RecordObject (this , “delete” ); //necessary as there’s DestroyObjectImmediate() above
var todel = m_soas [1];
m_soas. RemoveAt(1);
Undo.DestroyObjectImmediate (todel);

  1. destroy all objects from a list:

//right
Undo.RecordObject(this, “delete”);
var dummy = new List(); dummy.AddRange(m_soas);
m_soas.Clear();
foreach (var o in dummy)
{
Undo.DestroyObjectImmediate(o);
}

////wrong, the instances will all be recovered, but only the last slot of list is recovered
//Undo.RecordObject (this , "delete" );
//while ( m_soas. Count > 0)
//{
//       var o = m_soas[ m_soas. Count-1];
//       m_soas. RemoveAt(m_soas .Count - 1);
//        Undo.DestroyObjectImmediate (o);
// }