[ValueDropdown] on a property calls set{ } twice when choosing a value through the dropdown list

Issue #783 invalid
Linard Hug created an issue
  1. When [ValueDropdown] is put on a property and a value is selected from the dropdown, the set { } of the property is called twice in quick succession, which can lead to undesired side effects.
  2. Add the provided MonoBehaviour component on any gameobject and set a value of either Foo or Bar in the inspector through their respective value dropdown. Notice the Debug.Log() that is called twice. Alternatively, add a breakpoint to the set { } of a property and do the above action in debug-mode.

Through some preliminary testing I found out this occurs if the value returned by get { } does not equal the value passed to set { }from the value dropdown. This is the case if for example the passed value is modified inside the set{ } before being saved into the backing field of the property such as adding 1 to a passed int value or instantiating a prefab from a passed prefab asset.
This also occurs in the simple case where the property always returns a constant value such as get => 0; and the passed value is not 0. This behaviour does not occur when setting the value through the displayed field (when using AppendNextDrawer = true).
3. N/A
4. 2020.3.4f1
5. v3.0.4.0
6. No
7. Windows 10 20H2 (19042.928)

Could be a total shot in the dark but it appears to me as if Odin makes sure the value is set correctly but notices that the value it gets from get { } is still not the value the user selected so it tries to set it again.

Comments (3)

  1. Tor Esa Vestergaard

    This is not, strictly speaking, an error. Currently the value dropdown, like the enum, changes the value as an item is selected in the dropdown, and also when something triggers the dropdown to close.

    Typically this wouldn't cause several sets, as it would see that the value hasn't changed since last time and so there is no need to set the same value again. However, when the property is inconsistent, then you get behaviour like this.

    I don't think we will treat this as a bug at all, because our API does not make this promise of doing a specific number of sets with regards to properties - this is not the only case where such behaviour would be exhibited. A lot of Odin is predicated on doing value comparisons and checking that stuff is up to date. So rather, I would say we consider such properties as the example to be very bad practice, and code like this should not be expected to work.

  2. Linard Hug reporter

    I would say we consider such properties as the example to be very bad practice, and code like this should not be expected to work.

    While I see that it will be the case most of the time, this does affect imo valid pratices as mentioned in the issue such as using the value dropdown to give the user a list of prefab assets to choose from which is then instantiated and set to the backing field of the property. Or is there another suggested workflow to create this functionality?

  3. Log in to comment