Breaking change in StateUpdaters in case of multiple attributes in 3.0

Issue #726 open
Eran Yaacobi created an issue

1. What happened?

On updating from Odin to version 3.0.2 (from 2.1.13), the behaviour of ShowIf/HideIf changes when having multiple of these attributes on the same field.

Previously, in the case of:

[ShowIf(nameof(HasValue))]
[ShowIf(nameof(FloatValue), 0f)]
[HideIf(nameof(Basic))]
public Int32 IntegerValue;

[ShowIf(nameof(HasValue))]
[ShowIf(nameof(IntegerValue), 0)]
[HideIf(nameof(Basic))]
public Single FloatValue;

public Boolean HasValue;

public Boolean Basic;

Then IntegerValue would be shown only if HasValue is true, FloatValue is not zero, and Basic is false.

In 3.0.3, the first two ‘ShowIf’ attribute (being first) have no effect, and so only if Basic is true will IntegerValue be hidden.

2. How can we reproduce it

Use the above code and test it in version 3.0.2 compared to 2.1.13.

4. What version of Unity are you using

2020.1.0

5. What version of Odin are you using?

3.0.2

6. Do you have Editor Only mode enabled?

Yes

7. What operating system are you on?

Windows 10

Comments (8)

  1. Tor Esa Vestergaard
    • changed status to open

    Interesting - we'll have to consider how to solve this problem without introducing more problems than solving it, well, solves. Meanwhile, as a stopgap, I'd suggested using an expression to combine all the conditions into one attribute.

  2. Eran Yaacobi reporter

    The situation above is extremely common in my project (making fixing it to be with expressions quite time consuming), and when using ‘nameof’ (which is a lot safer) writing expressions is extremely inconvenient because string interpolation is not supported in attribute constructors, so instead I’ve overridden ShowIf and HideIf with my own implementations that handle this scenario.

    Notice though that there are two things to address - one is the issue with the “default” implementation, and the other is how OnStateUpdate works. The problem I faced is that the state is retained between calls, and so I cannot “know” whether the current value is from another state updater that is before me in the list (in which case I shouldn’t override it if the value is false), or from the previous iteration (in which case I need to ignore it).

    What I did (in a very hackish manner) to make my ‘ShowIf/HideIf’ implementations work, which I think you can do cleanly without complicating things, is to reset the Visible state to its default value (‘true’) before calling to OnStateUpdate of the state updaters. This way, the default implementation can use “&=” instead of “=” and it’ll work. It does mean that the value from the last iteration is ignored in this case, but that currently happens anyhow (and you also already store it in a separate variable in case it’s needed from what I’ve seen).

    In order to make the behaviour more generic, you can also extend the ‘persistent’ field of states to be an enum with three values instead of a boolean (Persistent, “SemiPersistent” or some other name for the current behaviour, and Volatile for the proposed behaviour), and have Visible be Volatile. If you want to further extend this so it’ll be possible to implement ‘or’ behaviour between state-updaters and not just ‘and’, you could even have the Visible state be a ‘Boolean?’ field (and later when queried to decide whether to show a field or not it’ll result to the default ‘true’ value if it is none).

  3. Log in to comment