Exception when using custom List Add Button

Issue #141 resolved
Johannes P. created an issue

I strongly suspect that the following issue is due to my own lack of experience with Unity's UI system.


Overview

I have a use case where I have a list of structures that each wrap an Asset.

As a simplified example, let's assume it is a Material:

public List<DemoElement> DemoList;

[System.Serializable]
public class DemoElement
{
    public DemoElement(Material mat) { Mat = mat; }
    [HorizontalGroup, HideLabel, DisplayAsString] 
    public string Data = "Some Other Data";
    [HorizontalGroup, HideLabel] 
    public Material Mat;
}

By default, the List + button generated by Odin for this list will construct an instance of this wrapper, but of course the contained Material Asset will be null initially.

Since in my use case, the user will always end up using the ObjectPicker UI to then populate the contained asset, I attempted to create a solution that would replace the default + List button with one that immediately opens the ObjectPicker, and then adds the wrapped asset to the list, but only if the user selected a valid asset.


Problem

The solution I came up with uses [ListDrawerSettings(HideAddButton = true, OnTitleBarGUI = "...")] to render a custom SirenixEditorGUI.ToolbarButton in place of the default one, and opens an Object picker using EditorGUIUtility.ShowObjectPicker. This works exactly as I had hoped.

The problem I am however running into is that I need to add the resulting selection of EditorGUIUtility.GetObjectPickerObject() back to my original list.

I tried doing so in a function marked with OnInspectorGUIAttribute, and while it -does- appear to correctly add the item to my list, I also get several exceptions:

ListAddExceptions.png

You can find the exceptions/warnings here: https://bitbucket.org/snippets/JohannesMP/dEn6zX


Question

Here is the complete code used in the above example: https://bitbucket.org/snippets/JohannesMP/R7n6Kz

Please note that this is a deliberately simplified example. The wrapper element normally would have much more other attributes and UI, but depends for the most part on a reference to an asset, thus my desire to have the + button automatically prompt the user to select one.

While I am certain that the problem is due to me not understanding the UI call structure correctly, I am curious how I could avoid these errors, or better yet, if there is a more elegant solution to this problem.

Comments (8)

  1. Tor Esa Vestergaard

    Welcome to Unity's horrible layouting system! Every time anything happens in the UI, a layouting event precedes all input/command events, and is then followed by a repaint event. The layout structure is built up in the layout event, and must be replicated exactly in all following events. If the layouts don't match that of the initial layouting event, exceptions such as the one you're getting are thrown.

    The problem is that you're adding the list elements at the wrong time. A general rule to keep in mind is that if you're changing something that affects the order of layout elements (such as adding an extra element to a list), do it last in repaint after everything has settled, so that the layout doesn't change until the next layouting call.

    I've fixed up your demo script here to demonstrate a possible solution. The change is quite simple - once you add a value, you slot it into a temporary variable that then triggers the actual list addition in Repaint. We do this sort of stuff everywhere, internally in Odin. Many value changes don't actually happen until repaint.

    using System.Collections;
    using System.Collections.Generic;
    using UnityEngine;
    using UnityEditor;
    using Sirenix.OdinInspector;
    using Sirenix.Utilities.Editor;
    
    public class Demo : SerializedMonoBehaviour
    {
        [ListDrawerSettings(HideAddButton = true, OnTitleBarGUI = "RenderAddButton")]
        public List<DemoElement> DemoList;
    
        [System.NonSerialized] private int _controlID;
        [System.NonSerialized] private bool _pickerActive = false;
        [System.NonSerialized] private DemoElement _toAdd = null;
    
        // Custom List add button that opens object picker
        void RenderAddButton()
        {
            if (SirenixEditorGUI.ToolbarButton(EditorIcons.Checkmark))
            {
                _controlID = GUIUtility.GetControlID(FocusType.Passive);
                _pickerActive = true;
                EditorGUIUtility.ShowObjectPicker<Material>(null, false, "", _controlID);
            }
        }
    
        // Handle if we have the picked object
        [OnInspectorGUI, PropertyOrder(int.MaxValue)]
        void HandlePickedObject()
        {
            if (HavePickedObject())
            {
                Material mat = EditorGUIUtility.GetObjectPickerObject() as Material;
                if (mat == null)
                    Debug.Log("INVALID OBJECT");
                else
                {
                    Debug.Log("VALID OBJECT!");
                    _toAdd = new DemoElement(mat);
                }
            }
    
            if (Event.current.type == EventType.Repaint && _toAdd != null)
            {
                DemoList.Add(_toAdd);
                _toAdd = null;
            }
        }
    
        // Check if we have a valid item from the object picker
        bool HavePickedObject()
        {
            if (Event.current.commandName != "ObjectSelectorClosed") return false;
            if (EditorGUIUtility.GetObjectPickerControlID() != _controlID) return false;
    
            if (!_pickerActive) return false;
            _pickerActive = false;
            return true;
        }
    
        [System.Serializable]
        public class DemoElement
        {
            public DemoElement(Material mat) { Mat = mat; }
            [HorizontalGroup, HideLabel, DisplayAsString] public string Data = "Some Other Data";
            [HorizontalGroup, HideLabel] public Material Mat;
        }
    }
    
  2. Log in to comment