OnValidate on MonoBehaviours or ScriptableObjects does not get called for T fields rendered by OdinValueDrawer<T>

[Jeffrey Campbell]

Is there anything additional that a developer would need to do for a class implementing OdinValueDrawer<T> to properly trigger OnValidate for MonoBehaviours or ScriptableObjects that have a field for T. We seem to have found a bug where in Unity 2018.3.3f1 using OdinInspector 2.0.5, OnValidate doesn't get called for fields being rendered by an OdinValueDrawer.

I've included a barebones example below that should help a developer reproduce this issue.

using System;
using UnityEngine;

namespace _Tests
{
    [CreateAssetMenu]
    public class TestScriptableObject : ScriptableObject
    {
        public TestSerializedObject objOne;

        [SerializeField]
        private TestSerializedObject objTwo;

        private void OnValidate()
        {
            Debug.Log("TestScriptableObject.OnValidate fired");
        }
    }


    [Serializable]
    public class TestSerializedObject
    {
        public string objName;
    }
}

using UnityEngine;

namespace _Tests
{
    public class TestSerializedMonoBehaviour : MonoBehaviour
    {
        public TestSerializedObject objOne;

        [SerializeField]
        private TestSerializedObject objTwo;

        private void OnValidate()
        {
            Debug.Log("TestSerializedMonoBehaviour.OnValidate fired");
        }
    }
}

using Sirenix.OdinInspector.Editor;
using UnityEditor;
using UnityEngine;

namespace _Tests.Editor
{
    public sealed class TestSerializedObjectDrawer : OdinValueDrawer<TestSerializedObject>
    {
        protected override void DrawPropertyLayout(GUIContent label)
        {
            ValueEntry.SmartValue.objName = EditorGUILayout.TextField(ValueEntry.SmartValue.objName);
        }
    }
}

[Jeffrey Campbell]

It’s bad when I re-google for a solution/workaround and find a bug report I made more than a year ago with nary a response

[Jeffrey Campbell]

Still finding this to be an issue on Odin 2.1.13 on Unity 2019.3.0f6

[Tor Esa Vestergaard]

Sorry for missing this issue the first time around. This isn't a bug, but expected behaviour. The way that the example drawer is implemented bypasses Odin's property system entirely and so Odin will never know that anything changed and stuff like OnValidate and OnValueChanged will not be called.

In essence, what it's doing wrong is that it's getting a reference value out of a property, and then it's changing a value within that reference. Generally you should be getting the child property for the value you want to change, and then change the value through that. See this video.

However, this is a pattern that is explicitly discouraged by Odin - generally no drawer should directly control the drawing of sub-values of the thing it is drawing. Instead you should be getting the child property and calling .Draw() on it, and modifying its appearance with attributes or other custom drawers.

And if a drawer does follow the bad “god drawer” pattern, it is itself responsible for making sure that everything works as expected. You can do this by calling Property.ValueEntry.WeakValues.ForceMarkDirty(); after a value has been changed "invisibly".

[Jeffrey Campbell]

Thanks for the response, I have a couple of questions.

However, this is a pattern that is explicitly discouraged by Odin - generally no drawer should directly control the drawing of sub-values of the thing it is drawing.

I’m confused at what the proper way is for an OdinValueDrawer to be used; in the example above for TestSerializedObjectDrawer is drawing only the fields of TestSerializedObject. Is there a different drawer type that should be used? Or are you saying that each individual property of something like TestSerializedObject should be drawn with its own drawer (or indirectly drawn) and that multiple fields shouldn’t be drawn by a single drawer? I’m confused at how I’d be able to use multiple fields on a class drawn by an OdinDrawer together to be able to create the final UI.

For example, a common use case I have right now is that I have a POCO class IDReference that contains two strings for IDs for something. I have a drawer created that enables a developer to select and assign the value of ParentID from a dropdown before being able to select ChildID, and once ParentID is assigned a similar dropdown appears for a developer to assign the value of ChildID. What would be the recommended Odin way to create a custom inspector GUI for something like this (via drawers, attributes, etc…)?

public class IDReference
{
  public string ParentID;
  public string ChildID;
}  

‌