I’ve been having trouble implementing UndoRedo for a custom editor tool. I’ve managed to get it to work perfectly, however if I perform a undo and then a redo and then save the scene and reload it or hit the play button, the scene heirarchy is broken and stays broken (as if the scene file was incorrectly modified).
I have reduced my problem to a simple reproduce-able test case.
I have the following Scene Hierarchy with a “TestObject” that has a TestMap component attached which only has a public List exposed:
Here are the two monobehaviours I am using in the example:
public class MapPiece : MonoBehaviour
{
public TestMap testMap;
public Vector3 vec;
public int number = 0;
}
public class TestMap : MonoBehaviour
{
public List<MapPiece> pieces = new List<MapPiece>();
}
TestMap is attached to the “TestObject” gameobject (as shown above).
MapPiece is attached to the root of a simple prefab created by creating a simple “primitive Cube” gameobject using the built-in gameobject context menu (Right Click > 3D Object > Cube). This prefab has a “primitive Sphere” gameobject as a child object.
Press Ctrl + Z to trigger Undo.
Result : Scene returns to normal.
Press Ctrl + Y to trigger Redo.
Result: Scene returns to the exact same state as in step 1 (as expected!) but don’t celebrate just yet…
Save the scene. Then either press the play button (reloads the scene) or reload the current scene in the editor.
Result : Scene is broken if reloaded in the editor. Unity crashes if you press the play button (you will have to manually delete the scene file using your file explorer in order to open Unity again without crashing).
Yeah, I’ve got nothing… you might have a Unity bug, but on the other hand you are in some pretty sticky complicated code with the unpacking prefabs stuff.
It appears you are undo-saving the right stuff, but I might suggest recording the mapPiece.gameObject instead, or otherwise trying combinations of other objects.
I would need to kinda diagram the object creation and mutation you got going to reason about if it is capturing everything it needs to or not.
When I do an Undoable sequence of multiple steps, I follow very clear steps as if they were independently-invokable commands. Your code doesn’t really read like there’s a clear bracketing of stuff to be done, at least not like I’ve seen before.
Undo.IncrementCurrentGroup();
Undo.SetCurrentGroupName("Do Things");
int undoID = Undo.GetCurrentGroup();
foreach (var gobj in gobjsToBeDone)
{
MyComp comp = gobj.GetComponent<MyComp>();
Undo.RecordObject(comp, "Do One Thing");
comp.DoThing();
}
Undo.CollapseUndoOperations(undoID);
@halley1 Thanks for the reply. I want to make multiple changes to multiple things and collapse it into a single action. I record all steps for each action individually and collapse them into the current group. I should then increment the group index but I didn’t bother as this is a minimal test example that should run once and work (plus I have a test context menu action that clears the undoredo stack).
As for my logic not having a “bracketing of stuff” (I’m not quite sure what you mean by this) but I think the logic makes sense and follows a simple set of steps:
Do the following 3 times:
A. Instantiate prefab
B. Since I created a gameobject in the scene, I register its creation via RegisterCreatedObjectUndo
C. Unpack the prefab, passing InteractionMode.UserAction as a parameter which registers this action with the undo system.
D. Start recording MapPiece object attached to the newly created instance and the testMap object that is in the scene
E. Make changes to the MapPiece object (assign testMap reference)
F. Make changes to the testMap object (assign mapPiece reference)
G. Make changes to the MapPiece object (modify various values)
H. Mark Map as dirty so the user has to save the scene
I. Ensure objects recorded using RecordObjects are registered as an undoable action.
F. Collapse all undo operation up to group index together into one step
Perhaps I have to add the mapPiece Transform component to the list of objects being recorded?
Edit: nope, I added mapPiece.transform to the array of Objects passed to recordObject() but I still get the same result.
However, I want to unpack the prefab instance. What should I do to have the unpacking of the prefab registered with the same undo command along with all the other modifications? Any Ideas?
I feel like my logic is correct and that this is a bug with the Undo system which is why it would be great if a Unity dev can take a look at this and tell me if I did things correctly and this is indeed a bug or I messed up and in this case what are the correct steps to achieve the desired result?
Just to note, I’ve tried passing InteractionMode.AutomatedAction instead which doesn’t break the scene file but I get incorrect behavior now when doing ctrl+z then ctrl+y which is expected since the unpacking of the prefab is not being recorded if I flag the action as “Automated”.
which is slightly different than my version that I see in VS:
public static void UnpackPrefabInstance(GameObject instanceRoot, PrefabUnpackMode unpackMode, InteractionMode action)
{
if (!IsPartOfNonAssetPrefabInstance(instanceRoot))
{
throw new ArgumentException("UnpackPrefabInstance must be called with a Prefab instance.");
}
if (!IsOutermostPrefabInstanceRoot(instanceRoot))
{
throw new ArgumentException("UnpackPrefabInstance must be called with a root Prefab instance GameObject.");
}
if (action == InteractionMode.UserAction)
{
string name = "Unpack Prefab instance";
Undo.RegisterFullObjectHierarchyUndo(instanceRoot, name);
GameObject[] array = UnpackPrefabInstanceAndReturnNewOutermostRoots(instanceRoot, unpackMode);
GameObject[] array2 = array;
foreach (GameObject instanceComponentOrGameObject in array2)
{
UnityEngine.Object prefabInstanceHandle = GetPrefabInstanceHandle(instanceComponentOrGameObject);
if ((bool)prefabInstanceHandle)
{
Undo.RegisterCreatedObjectUndo(prefabInstanceHandle, name);
}
}
}
else
{
UnpackPrefabInstanceAndReturnNewOutermostRoots(instanceRoot, unpackMode);
}
PrefabUtility.prefabInstanceUnpacked?.Invoke(instanceRoot);
}
