[Bug][Expandable] Will make other expandable element which in front of this Attirbute not update

[wojiuxuihuan]

Here's the code that reproduce the bug:

    public class ExpandableTest : MonoBehaviour
    {
        public UnityEvent onClick;//Bug: the List can't be modify
        [ResizableTextArea]
        public string multiLine;//Bug: the content can't be save after hit 'Enter'
        [Expandable]
        public ScriptableObject obj0;
    }

When you edit the onClick/multiLine field in Inspector, you will find that their value will not get save:

But if you move the obj0 on top of the onClick/multiLine field, they will work again.

Unity Version: 2021.2.7f1c1

[rhys-vdw]

More generally this affects any field that precedes a field decorated with ExpandableAttribute, including int etc, regardless of whether they have NA decorators. The field can be edited but will revert to its previous value when blurred.

Collapsing/expanding the field or removing its asset reference makes no difference.

This is true in two of my projects using NaughtyAttributes, both using Unity 2020.3.21.

[rhys-vdw]

This issue is present in 2.1.1 only. Rolling back to 2.1.0 resolves the problem (but presumably will then suffer from #232). @dbrizov it seems that the issue was introduced in e4edd04d78aeb27cb99134b8fc84f521485de6de.

See reproduction here: https://github.com/rhys-vdw/NaughtyAttributes/commit/77b2b0e2e287c7ead3c5c1a9eee4a8e621711e05 (branch https://github.com/rhys-vdw/naughtyattributes/tree/294-reproduction)

[rhys-vdw]

I have discovered that if you remove this line the issue goes away.

https://github.com/dbrizov/NaughtyAttributes/blob/178959b4f9ded9aead6cd1f9a80decf41b9b6812/Assets/NaughtyAttributes/Scripts/Editor/PropertyDrawers/ExpandablePropertyDrawer.cs#L74

My guess is that when the drawer is called on the blur event this code will somehow reset the value on the main inspector object before it has a chance to commit the changes via the final call to ApplyModifiedProperties here:

https://github.com/dbrizov/NaughtyAttributes/blob/178959b4f9ded9aead6cd1f9a80decf41b9b6812/Assets/NaughtyAttributes/Scripts/Editor/NaughtyInspector.cs#L133

Why this is affecting the main object is not clear to me though, presumably some subtlety of other changes in the problematic commit.

Actually I see no issue with line removed. I assume the purpose of this line is to ensure that the nested editor is in sync with the asset. I tested this by opening a second inspector and modifying the nested asset there. Regardless of the presence of this line the asset will update correctly as soon as the mouse enters the inspector of the object with the Expandable attribute.

Looking at the docs we see:

If you keep a reference to a SerializedObject instance for more than one frame, you must make sure to manually call its SerializedObject.Update method before you read any data from it, as one or more target objects may have been modified elsewhere, such as from a separate SerializedObject stream.

source: https://docs.unity3d.com/ScriptReference/SerializedObject.html

This suggests to me that this line is not required as the SerializedProperty reference appears to be recreated on every invocation of OnInspectorGUI.

I will open a PR with this simple change.

[rhys-vdw]

Why was this closed? #295 is still not merged, was there another fix?

[wojiuxuihuan]

Why was this closed? #295 is still not merged, was there another fix?

I see the author had reply your pull request, and I think it might not well for two issues link to the same problem so this one is closed.

[rhys-vdw]

There is no other issue @wojiuxuihuan. This should stay open until the PR is merged, at which point it will automatically be closed by GitHub.

[wojiuxuihuan]

There is no other issue @wojiuxuihuan. This should stay open until the PR is merged, at which point it will automatically be closed by GitHub.

I see, thanks for notify!

[dbrizov]

295 PR merged into the v2 branch 7cddf69bca9d09eb8205498ebaf6bdc50f7641fd