InspectorProperty.PopDraw() Causes Exception

Issue #498 resolved
Tim Williams created an issue

It appears InspectorProperty.PopDraw() does not check if the drawCountStack has any elements. This leads to an exception being thrown if it tries to pop from an empty stack.

I was able to cause an exception like this by creating a button and inside the button function calling

EditorUtility.DisplayCancelableProgressBar()

Calling that produced the following exception:

InvalidOperationException: Stack empty.
System.Collections.Generic.Stack`1[T].ThrowForEmptyStack () (at <343bdfbb3c2d4d21b745a87548c1a7cd>:0)
System.Collections.Generic.Stack`1[T].Pop () (at <343bdfbb3c2d4d21b745a87548c1a7cd>:0)
Sirenix.OdinInspector.Editor.InspectorProperty.PopDraw () (at D:/Sirenix/Sirenix Solution/Sirenix.OdinInspector.Editor/Examples/Attributes/TitleExamples.cs:49)
Sirenix.OdinInspector.Editor.InspectorProperty.Draw (UnityEngine.GUIContent defaultLabel) (at D:/Sirenix/Sirenix Solution/Sirenix.OdinInspector.Editor/Examples/Attributes/TableMatrixExamples.cs:43)
Sirenix.OdinInspector.Editor.InspectorUtilities.DrawPropertiesInTree (Sirenix.OdinInspector.Editor.PropertyTree tree) (at D:/Sirenix/Sirenix Solution/Sirenix.OdinInspector.Editor/Utilities/PersistentContext/PersistentContextCache.cs:209)
Rethrow as OdinPropertyException: This error occurred while being drawn by Odin.
Odin Property Path: TryRunWizard
Odin Drawer Chain: DisableInPlayModeAttributeDrawer, EnableIfAttributeDrawer, DefaultMethodDrawer.
UnityEngine.Debug:LogException(Exception)
Sirenix.OdinInspector.Editor.InspectorUtilities:DrawPropertiesInTree(PropertyTree) (at D:/Sirenix/Sirenix Solution/Sirenix.OdinInspector.Editor/Utilities/PersistentContext/PersistentContextCache.cs:209)
Sirenix.OdinInspector.Editor.PropertyTree:Draw(Boolean) (at D:/Sirenix/Sirenix Solution/Sirenix.OdinInspector.Editor/Drawers/Group Drawers/ResponsiveButtonGroupAttributeDrawer.cs:32)
Sirenix.OdinInspector.Editor.OdinEditorWindow:DrawEditor(Int32)
Sirenix.OdinInspector.Editor.OdinEditorWindow:DrawEditors() (at D:/Sirenix/Sirenix Solution/Sirenix.OdinInspector.Editor/Core/Value Entries/PropertyListValueEntryChanger.cs:133)
Sirenix.OdinInspector.Editor.OdinEditorWindow:OnGUI() (at D:/Sirenix/Sirenix Solution/Sirenix.OdinInspector.Editor/Core/Value Entries/PropertyListValueEntryChanger.cs:118)
UnityEngine.GUIUtility:ProcessEvent(Int32, IntPtr)

Comments (8)

  1. Bjarke Elias
    • changed status to open

    Thanks!

    Will give it a go tomorrow with the DisplayCancelableProgressbar approach, and see if I can reproduce and do something about.

    Are there other places where you've experienced something like this happen? @daschatten How can I reproduce your case?

  2. Tim Williams reporter

    It tends to happen any time a function in an OdinEditorWindow with the [Button()] attribute makes an extended running call, whether that's importing assets or other longer running functions. The errors are thrown once the button function finishes and exits.

  3. Tim Williams reporter

    Putting the following code in an OdinEditorWindow class will cause the error:

    [Button("SomeTestButton")]
            private void CauseError()
            {
                if (EditorUtility.DisplayCancelableProgressBar("Odin Test", "This will cause an error", 0))
                    return;
    
                EditorUtility.ClearProgressBar();
            }
    
  4. Tor Esa Vestergaard

    This isn’t really an error in Odin, so much as it is an unavoidable consequence of how Unity’s IMGUI system functions. There are a variety of “solutions” available to us, but this is more of a design thing than a bug-fixing thing. Fixing this will break other things, and so forth.

    Currently Odin executes button logic immediately when the click registers, as part of the GUI code. IE, the button method runs during one of the invoked IMGUI events. Doing it this way is neat for a lot of reasons, and a pain for a lot of other reasons. Particularly, for this issue, calling a method during this event such as BuildPipeline.BuildPlayer() will irrevocably mess up the entire GUI context, and so, when the IMGUI event leaves the button method and tries to continue drawing, everything is now broken, stacks have been reset, and so forth.

    Thankfully, there are two simple ways to avoid the issue altogether:

    1. Whenever you write button code/IMGUI code that is going to mess up the IMGUI context, end the code with a call to GUIUtility.ExitGUI() or by throwing an ExitGUIException(). This lets the event know that everything is broken now, and it needs to stop drawing and start over properly.
    2. Delay the actual method call by running the method contents in a delayed context, such as EditorApplication.delayCall += () => { do the actual thing }, so it runs outside of the IMGUI context entirely.

  5. Tim Williams reporter

    Calling “GUIUtility.ExitGUI()” causes more exceptions to be logged in the console. Having to do a delaycall every time you use the function sucks, but will likely work going forward since this is a Unity bug

  6. Log in to comment