- Home /
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, second, third. (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[i];
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.
If there's anything ambiguous, please ask. And if you like, I could upload the rest of the clips explaining the first parts of the code if that helps.
Bump :( ... Now I'm just showing a confirmation box to the user saying that the deletion can't be undone.. I still haven't found a solution to my problem.
Ok so you are running your loop forwards but removing things from the array - generally you should iterate backwards through an array when you are deleting things as holes can appear in the array when you remove stuff and then step over.
And which line is really line 29 that your null ref appears on?
Answer by immersiveGamer · Feb 23, 2014 at 10:00 AM
Using Delegate
k, 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[i].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.
Well first thanks for your feedback on my questions today, and thanks for taking the time trying this thing out yourself, appreciate that!
I do think it is possible, if you check out the last couple of comments between me and whydoidoit - he suggested that I'd use EditorApplication.delayCall
to delay the destruction to the next frame, it yielded different results - it deleted a bunch of states not just one (it should be just one) - and the funny thing is, when I undid everything went back intact! - so there must be a hack or something to get this right...
Also that strange behaviour when trying out the first deletion method (don't know if you saw the videos...)
Updated answer. You were right, there was a way. A pain that you have to do a delayed call of that undo function.
Hmm, looking at you updated answer I can immediately spot a flaw. You're always destroying the last GO you create - you need to destroy the GO of the current element of your list. It's also shaky to remove the element and then destroy, it makes sense to do the opposite, but from what I've seen there's no sense related to this problem. I'm gonna give that callback a try, but I'm not sure if it will yield any different results than what I've tried yesterday. I wonder if this is the solution but the problem is with the removal methods that I'm using for my SerializedProperties. $$anonymous$$aybe it worked for you cause you used a normal list?
Never $$anonymous$$d what i Just said ^ - I don't know if I missed it or you updated your answer, but I just saw
clone = script.list[i].gameObject;
Just to give you an update, this is indeed working. But unfortunately not always, I don't know why, I can't explain it - but sometimes it works, other times it won't (ie sometimes the GO and reference gets back in tact, or the GO gets back but the ref is still null) - It's very unpredictable and hard to reproduce at a certain knowledge. I don't know if you experienced that or not...
still experimenting atm...
not sure why you need
index = i;
Out of interest - there is no need to -= on delayCall - delayCall is cleared every time it is used.
And you can just use -
EditorApplication.delayCall += ()=> Undo.DestroyObjectImmediate(clone);
No need for all of that creation of delegates stuff any more.
Answer by whydoidoit · Feb 26, 2014 at 10:45 AM
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 {
}
Thanks! But, you're not actually destroying the gameObject of your instance, but just the instance which is a $$anonymous$$B - I don't have problems with that. Problems I have is when I destroy the GO. Not sure about this collapsing method, the doc didn't do a good job explaining it, but anyway... currently I'm doing this:
if (GUIHelper.RemoveButton("state")) {
Debug.Log("Size before removal: " + sp_states.arraySize);
currentGoToDestroy = sp_STAT$$anonymous$$gameObject();
EditorApplication.delayCall += () => Undo.DestroyObjectImmediate(currentGoToDestroy);
sp_states.RemoveAt(i);
Undo.CollapseUndoOperations(Undo.GetCurrentGroup());
Debug.Log("Size after removal: " + sp_states.arraySize);
continue;
}
As I mentioned in one of my comments to @immersiveGamer, this works almost 50% when I undo, sometimes everything gets back in tact, and sometimes not...
Good point - updated with a version that works with adding gameobjects - the trick to stop the nulls was the multiple calls to RecordObject. However If you pass through 0 objects in the list the next undo leaves a null in the entry once. This would appear to be something about the fact the undo buffer is probably not recording the list at all in this case - perhaps you make it so you just can't take the list back to 0 with the "-" buttons?
Sorry, but again, you're not destroying the game object of your instance! - I don't have problems with adding a gameObject and destroying the instance ($$anonymous$$B) just like you did with your last update.
$$anonymous$$aybe I wasn't clear enough?
try destroying the gameObject of your instance, not the actual instance i.e.
var itemToRemove = holder.instances[0].gameObject;
Undo.RecordObjects( new UnityEngine.Object [] { itemToRemove, holder }, "Removed");
Undo.DestroyObjectImmediate(itemToRemove);
The whole question/problem is about destroying the gameObject of 'instance' and the instance itself, and when undoing both the gameObject and the reference to its $$anonymous$$B should get back intact.
as I mentioned in my last comment, I had 50/50 success using the code I posted.
No you're perfectly clear - I posted the wrong code :S Updated
ok thanks. are you having any problems with that? try removing/undoing a bunch of times - do you 'always' get everything back intact? (in the previous code I used, sometimes I do it about 10 times, and on the 11th it fails...)
Answer by DonHaul · Aug 17, 2017 at 06:02 PM
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[i];
Undo.DestroyObjectImmediate(currentGoToDestroy);
}
Thanks a lot. Simple and working well. I used it to clear whole array.
Answer by TMPxyz · Oct 03, 2015 at 10:00 AM
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:
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);
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);
destroy all objects from a list:
//right Undo.RecordObject(this, "delete"); var dummy = new List<SOA>(); 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); // }
Your answer
Follow this Question
Related Questions
How to initialize array[][] via SerializedProperty? 1 Answer
How to work with custom class (ScriptableObject) SerializedProperty? 1 Answer
What would be the best way to go about loading and saving a Serializable object in JSON with Unity? 0 Answers
How to record hideFlags for Undo/Redo 0 Answers
How to deal with object deletion Undo states and variables referencing the objects involved? 2 Answers