Inconsistent rendering behavior with Properties

Issue #132 resolved
Johannes P. created an issue

Examples

To illustrate the problem it's easiest for me to first show some examples:

Example 1. Expected behaviour with [System.NonSerialized]

Given the following setup:

public class PropertyTest : MonoBehaviour
{
  [System.NonSerialized]
  [HideInInspector]
  private List<string> _listStorage = new List<string> { "test1", "test2" };

  [ShowInInspector]
  public List<string> ListView
  {
    get { return _listStorage; }
    set { _listStorage = value; }
  }

The ListView is correctly rendered in the inspector:

qalGFux.png

This is the expected behaviour.


Example 2. Unexpected behaviour with [SerializeField]

Given the following setup: (Identical except _listStorage is now has the [SerializeField] attribute instead of [System.NonSerialized]):

  public class PropertyTest : MonoBehaviour
  {
    [SerializeField]
    [HideInInspector]
    private List<string> _listStorage = new List<string> { "test1", "test2" };

    [ShowInInspector]
    public List<string> ListView
    {
      get { return _listStorage; }
      set { _listStorage = value; }
    }
  }

This results in the following unexpected behaviour:

SerializedExample.gif

  1. The ListView is now displayed as a reference
  2. More importantly, the content of _listStorage is NOT displayed through ListView, despite the fact that the original list is hard-coded to be initialized with two values. As you can see in the screecapture, it is currently impossible to add, modify or remove elements via this reference.

Example 3. Reference for [SerializedField] without [HideInInspector]

Compare the previous example (A serialized field that is hidden, where the view fails to render) with the following example, which is identical except the [HideInInspector] attribute on _listStorage is removed:

public class PropertyTest : MonoBehaviour
{
  [SerializeField]
  private List<string> _listStorage = new List<string> { "test1", "test2" };

  [ShowInInspector]
  public List<string> ListView
  {
    get { return _listStorage; }
    set { _listStorage = value; }
  }
}

SerializedExample2.gif

As expected, the previously hidden private variable is now visible.

  1. ListView is still displayed as a reference
  2. However this time ListView does show the content of _listStorage correctly.

Issues

So as you can see, there are several inconsistencies:

Issue 1. Is the property shown as a reference?

  • When the property is referencing a non-serialized field it is NOT shown as a reference.
  • When the property is referencing a serialized field it IS shown as a reference.

Issue 2. Is the content of the property shown correctly?

  • When the property is referencing a hidden serialized field, its content is NOT shown correctly.
  • When the property is referencing a non-hidden serialized field, its content IS shown correctly.

Regarding Issue 1 (reference or not), I can understand why seeing the property as a reference makes sense. However I don't understand why it is not shown as a reference when it is non-serialized.

Also, in some cases I would like to be able to deliberately remove (just in terms of Inspector UI) the fact that the property is a reference, since that adds one extra click to be able to edit its content. In other words, it would be nice to force the first behavior, but I do think the second behavior should be the default.


Regarding Issue 2 (content drawn or not), I don't see any reason why the content of the property should ever not be drawn correctly.

I definitely see the behavior in Example 2 above as a bug.

Comments (6)

  1. Tor Esa Vestergaard

    Thanks for an excellently written issue report. We've run into this before, with users, and the next patch actually has some fixes and changes that causes all of these examples to behave as you'd expect.

    I'll explain why things have been acting this way, though, as you may find it enlightening. There are actually perfectly logical explanations for all of it, and we've had to be a bit hacky to get things to act in a " UX-logical" way rather than "technically-logical" way.

    • [NonSerialized] causes Odin to not include the member in the property tree at all, IE, its existence is utterly ignored - unless it is also marked with [ShowInInspector]. This is why there are no references to it - the property tree isn't even looking at them or including their values for consideration.
    • [HideInInspector] does not affect whether the member is included in the property tree - it just triggers a drawer with a very high priority, that doesn't draw anything at all. This is so that properties that are hidden, but serialized, can still be affected by the property logic for, say, prefab modifications.

    Also, in some cases I would like to be able to deliberately remove (just in terms of Inspector UI) the fact that the property is a reference, since that adds one extra click to be able to edit its content. In other words, it would be nice to force the first behavior, but I do think the second behavior should be the default.

    I can see why you'd want this, but it's sadly not something that we are likely to do. The current behaviour acts as protection against infinite draw loops - where-in values contain references to themselves. If we just drew references directly without such a cut-off, we'd crash with a stack overflow. This is why - for now at least - we always show references this way, closed by default. There are some things we could do to ease this - detecting cyclic references and only forcing the behaviour in such cases, and so on, but for now, this is how it is, as we're focusing our attention elsewhere :)

    Hope that explained things a bit. As noted, these issues should be resolved in the next patch, with all of your examples acting as you expect.

  2. Log in to comment