Deadcows / MyBox

MyBox is a set of attributes, tools and extensions for Unity
http://deadcow.ru
MIT License
1.94k stars 244 forks source link

Conditional attributes don't work with properties #188

Closed Tommigun1980 closed 1 year ago

Tommigun1980 commented 2 years ago

Hello and thanks for a great library!

However I found a slight issue (or lack of feature), namely that the conditional attributes (and possibly some other attributes, didn't check) only seem to work with fields and not with properties. So this won't work:

            [SerializeField]
            private bool someBool;
            public bool SomeBool
            {
                get { return this.someBool; }
            }

            [SerializeField]
            [ConditionalField(nameof(SomeClass.SomeBool))]
            private SomeInnerClassData someInnerClass;
            public SomeInnerClassData SomeInnerClass
            {
                get { return this.someInnerClass; }
            }

It does work if someBool is made public and bound to directly, but that breaks data encapsulation. It is quite convenient to have fields that are only editable through data.

So please consider adding support to ConditionalField attributes to also check for properties and not just fields!

Thank you so much.

snappedToGrid commented 1 year ago

Hey there! I just tried adding AttributeTargets.Property to the ConditionalFieldAttribute class definition, and it actually seems to be working 😮

This is the class definition

[AttributeUsage(AttributeTargets.Property | AttributeTargets.Field)]
    public class ConditionalFieldAttribute : PropertyAttribute

And this is how I use it with an actual property

[field:SerializeField]
        [field:ConditionalField(true, "IsCool")]
        public Object MyObj { get; private set; }

        bool IsCool()
        {
            return NameProperty== "Coolio";
        }

As you can see, I did have to resort to using a function to check instead of just a simple field check, because my other field was also a property. But it seems to do the job!

I haven't tested it thoroughly, but I'll issue a pull request and maybe author would like to use it. Pull request: https://github.com/Deadcows/MyBox/pull/216

Deadcows commented 1 year ago

Merged the fix. Thanks @snappedToGrid!