Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ConditionalFieldAttribute is rather slow #240

Open
SmikeSix opened this issue Sep 25, 2023 · 3 comments
Open

ConditionalFieldAttribute is rather slow #240

SmikeSix opened this issue Sep 25, 2023 · 3 comments

Comments

@SmikeSix
Copy link

Hey i just wrote a complex editor script with lots of lists and other things and the ConditionalFieldAttribute is really slow.

I guess there cant be much done about it, since unitys editor script is slow.

So basically.. we need to get rid of unitys editor and switch to a other solution.

@Deadcows
Copy link
Owner

I feel a rebellious spirit in you 😁
Yeah, I don't like much how the editor has evolved over the years.
But with all my love for the editor extensions, I still have not invested my time into learning the UI Toolkit that came to improve the situation with the "slow IMGUI". So, maybe the answer to this issue lies in this domain and I just need to rewrite it, if it's even possible 🤔

Btw, what's the other solution you have in mind? We're also searching for alternatives in our studio, although will probably stick with the 2023 version of Unity for a few more years

@SmikeSix
Copy link
Author

Hehe. I guess last weeks made Unity shortcomings even more annoying :)

Hmm.. i was thinking about switching to Odin inspector, even though it may end up costing more in the future. But they plan to release a Standalone Version and also Godot Version of their editor.
So yeah im also not sure. But i digged in and did some performance test on the editor and its just seems to be poorly optimized overall. So it probably is worth the time to switch.


I hacked something together (really just try and error) and used the DrawIf Script i found on the Unity Forum and merged it with your scripts. It seems to work on complex structures.
It only looks for Enums & booleans and caches the SerializedProperty. Its at least in my use case (~1000x executed over lots of elements) fast enough so it does not annoy me anymore.
But.. only tested in 2020 LTS and try on your own risk (if anyone ever copies this).

Only part were a workaround is needed (i think same in the original):

[System.Serializable] public class ListOfRectAreas { [ParentListProvider("RectAreas", ParentListProviderAttribute.Type.Integer, true, true)] public int[] areas; }
Basically if you have costum property drawers on list you need to encapsle them, like above.
[DrawIf("animationMotionOverwriteTargetRandomly", true)] public ListOfRectAreas animationMotionOverwriteToTargetList;

If you check on multiple enums ->
[DrawIf("targetBy", DrawIf.Conditions.Is, DecisionPoolSelection.ContinueMainAnimation, DecisionPoolSelection.ContinueMainAnimationOrSpecialState_Weakened)] public float delay;

I think in your script _toShow = ConditionalUtility.IsPropertyConditionMatch(property, conditional.Data); makes it slow. Since Unitys FindProperty is just bad. I cache those properties on first selection of the GameObject and then clear the dictionaries afterwards. Btw this feels more like putting duct tape on a crashed car, since rest of unitys code is slow.

`[CustomPropertyDrawer(typeof(DrawIfAttribute))]
public class DrawIfPropertyDrawer : PropertyDrawer {
#region Fields
// Reference to the attribute on the property.
DrawIfAttribute drawIf;
// Field that is being compared.
SerializedProperty comparedField;
private bool _initialized;
private PropertyDrawer _customPropertyDrawer;

#endregion

private bool _toShow = true;
public override float GetPropertyHeight(SerializedProperty property, GUIContent label) {
    if (!(attribute is DrawIfAttribute conditional)) return EditorGUI.GetPropertyHeight(property);

    var targetObject = property.serializedObject.targetObject;

    int objId = -1;
    if (targetObject != null) {
        objId = targetObject.GetInstanceID();
    }

    drawIf = conditional;
    CachePropertyDrawer(property);

    _toShow = ShowMe(property, conditional, objId);

    if (!_toShow && drawIf.disablingType == DrawIfAttribute.DisablingType.DontDraw) {
        return -2f;
    }

    if (_customPropertyDrawer != null) return _customPropertyDrawer.GetPropertyHeight(property, label);

    return EditorGUI.GetPropertyHeight(property);
}
private void CachePropertyDrawer(SerializedProperty property) {
    if (_initialized) return;
    _initialized = true;
    if (fieldInfo == null) return;

    var customDrawer = CustomDrawerUtility.GetPropertyDrawerForProperty(property, fieldInfo, attribute);

    if (customDrawer == null) customDrawer = TryCreateAttributeDrawer();

    _customPropertyDrawer = customDrawer;


    // Try to get drawer for any other Attribute on the field
    PropertyDrawer TryCreateAttributeDrawer() {
        var secondAttribute = TryGetSecondAttribute();
        if (secondAttribute == null) return null;

        var attributeType = secondAttribute.GetType();
        var customDrawerType = CustomDrawerUtility.GetPropertyDrawerTypeForFieldType(attributeType);
        if (customDrawerType == null) return null;

        return CustomDrawerUtility.InstantiatePropertyDrawer(customDrawerType, fieldInfo, secondAttribute);


        //Get second attribute if any
        Attribute TryGetSecondAttribute() {
            return (PropertyAttribute)fieldInfo.GetCustomAttributes(typeof(PropertyAttribute), false)
                .FirstOrDefault(a => !(a is DrawIfAttribute));
        }
    }
}
public static Regex ARRAY_DATA_REGEX = new Regex(@"\.Array\.data\[\d+\]$");

private bool ShowMe(SerializedProperty property, DrawIfAttribute drawIf, int objId) {
    var propertyPath = property.propertyPath;

    if (propertyPath[propertyPath.Length - 1] == ']') {
        var alternative = DrawIf.hashing.TryGetValue(propertyPath, out var newPropertyPath);

        if (!alternative) {
            if (ARRAY_DATA_REGEX.IsMatch(property.propertyPath)) {
                var propertyPathTmp = (ARRAY_DATA_REGEX.Replace(propertyPath, string.Empty));
                string final = propertyPathTmp.Contains(".") ? System.IO.Path.ChangeExtension(propertyPathTmp, drawIf.comparedPropertyName) : drawIf.comparedPropertyName;
                DrawIf.hashing.Add(propertyPath, final);

                propertyPath = final;
            }
        } else {
            propertyPath = newPropertyPath;
        }
    } else {
        var alternative = DrawIf.hashing.TryGetValue(propertyPath, out var newPropertyPath);
        if (!alternative) {
            string final = propertyPath.Contains(".") ? System.IO.Path.ChangeExtension(propertyPath, drawIf.comparedPropertyName) : drawIf.comparedPropertyName;


            DrawIf.hashing.Add(propertyPath, final);

            propertyPath = final;
        } else {

            propertyPath = newPropertyPath;

        }

    }

    string path = propertyPath;// propertyPath.Contains(".") ? System.IO.Path.ChangeExtension(propertyPath, drawIf.comparedPropertyName) : drawIf.comparedPropertyName;
    string cacheInd = ZString.Concat(objId, path);
    DrawIf.hashingProperties.TryGetValue(cacheInd, out SerializedProperty sp);
    if (sp != null) {
        comparedField = sp;
    } else {
        comparedField = property.serializedObject.FindProperty(path);
        DrawIf.hashingProperties.Add(cacheInd, comparedField);
    }
    if (comparedField == null) {
        Debug.LogError("Cannot find property with name: " + path);
        return true;
    }
    if (drawIf.compareValues == null) {
        // get the value & compare based on types
        return Compare(path, drawIf.comparedValue);
    } else {

        var v = drawIf.compareValues;

        bool anyTrue = false; //default is or not and
        for (int i = 0; i < v.Length; i++) {
            bool compared = Compare(path, v[i]);
            if (compared) { //if any is false, we return false. true&&false == false, else
                anyTrue = true;
            }
        }
        return anyTrue;
    }

}

private bool Compare(string path, object comparedValue) {
    switch (comparedField.type) { // Possible extend cases to support your own type
        case "bool":
            return ValidateExpression(drawIf, comparedField.boolValue.Equals(comparedValue));
        case "Enum":
            //Debug.Log("ISx " + (comparedField.enumValueIndex == (int)drawIf.comparedValue));
            return ValidateExpression(drawIf, (comparedField.enumValueIndex == (int)comparedValue));
        default:
            Debug.LogError("Error: " + comparedField.type + " is not supported of " + path);
            return true;
    }
}

public bool ValidateExpression(DrawIfAttribute attribute, bool b) {
    switch (attribute.condition) {
        case DrawIf.Conditions.Is:
            return b;
        case DrawIf.Conditions.IsNot:
            return !b;
    }

    return false;
}

public override void OnGUI(Rect position, SerializedProperty property, GUIContent label) {
    // If the condition is met, simply draw the field.

    if (_toShow) {
        if (!CustomDrawerUsed()) EditorGUI.PropertyField(position, property, label, true);


        bool CustomDrawerUsed() {
            if (_customPropertyDrawer == null) return false;

            try {
                _customPropertyDrawer.OnGUI(position, property, label);
                return true;
            } catch (Exception e) {
                Debug.LogWarning(
                    "Unable to use CustomDrawer of type " + _customPropertyDrawer.GetType() + ": " + e,
                    property.serializedObject.targetObject);

                return false;
            }
        }

        //EditorGUI.PropertyField
    } //...check if the disabling type is read only. If it is, draw it disabled
    else if (drawIf.disablingType == DrawIfAttribute.DisablingType.ReadOnly) {
        // GUI.enabled = false;
        //  EditorGUI.PropertyField(position, property);
        //  GUI.enabled = true;
    }
    //none
}

}`

It also has a static DrawIf class (just so naming is convinient), that holds the caches

`#if UNITY_EDITOR
[InitializeOnLoad]
#endif
public static class DrawIf {
#if UNITY_EDITOR
public static Dictionary<string, string> hashing = new Dictionary<string, string>();
public static Dictionary<string, SerializedProperty> hashingProperties = new Dictionary<string, SerializedProperty>();

static DrawIf() {
    // Subscribe to the domain reload event
    UnityEditor.AssemblyReloadEvents.beforeAssemblyReload += OnBeforeAssemblyReload;
}

private static void OnBeforeAssemblyReload() {
    // This method will be called before scripts are recompiled
    // Add your code here to execute after scripts have been recompiled

    Debug.Log("Scripts have been recompiled. Performing post-recompile actions...");
    hashing.Clear();
    hashingProperties.Clear();
    // Example: You can call methods or perform any other actions here
    // For example, resetting some script states or updating references
}

#endif
public enum Conditions {
Is, IsNot
}
}`

The Attribute:

`[AttributeUsage(AttributeTargets.Property | AttributeTargets.Field)]
public class DrawIfAttribute : PropertyAttribute {

#region Fields

public string comparedPropertyName { get; private set; }
public object comparedValue { get; private set; }
public object[] compareValues { get; private set; }


public DisablingType disablingType { get; private set; }
public DrawIf.Conditions condition { get; private set; }

/// <summary>
/// Types of comperisons.
/// </summary>
public enum DisablingType {
    ReadOnly = 2,
    DontDraw = 3
}

#endregion

/// <summary>
/// Only draws the field only if a condition is met. Supports enum and bools.
/// </summary>
/// <param name="comparedPropertyName">The name of the property that is being compared (case sensitive).</param>
/// <param name="comparedValue">The value the property is being compared to.</param>
/// <param name="disablingType">The type of disabling that should happen if the condition is NOT met. Defaulted to DisablingType.DontDraw.</param>
public DrawIfAttribute(string comparedPropertyName, object comparedValue,
    DrawIf.Conditions condition = DrawIf.Conditions.Is,
    DisablingType disablingType = DisablingType.DontDraw) {
    this.comparedPropertyName = comparedPropertyName;
    this.comparedValue = comparedValue;
    this.disablingType = disablingType;
    this.condition = condition;
}
public DrawIfAttribute(string comparedPropertyName, DrawIf.Conditions condition, params object[] compareValues) {
    this.comparedPropertyName = comparedPropertyName;
    this.compareValues = compareValues;
    this.disablingType = DrawIfAttribute.DisablingType.DontDraw;
    this.condition = condition;
}

}`
And it needs to clear the old caches after we deselect the gameobject in the editor. I marked it with a interface, but could be just always cleaned on selection change.

public interface ICleanup_HasDrawIfs{ }

`[InitializeOnLoad]
public static class SmikeEditorUtils
{

static SmikeEditorUtils()
{

    Selection.selectionChanged -= OnSelectionChanged;
    Selection.selectionChanged += OnSelectionChanged;
}

private static void OnSelectionChanged()
{
    var objs = Selection.gameObjects;
    if (objs != null) {
        for (int i = 0; i < objs.Length; i++) {
            MonoBehaviour has = (MonoBehaviour)objs[i].GetComponent<ICleanup_HasDrawIfs>();
            if(has != null) {
                DrawIf.hashingProperties.Clear();
                DrawIf.hashingArrayTitles.Clear();
            }  
            
        }
    }

}

}`

@Deadcows
Copy link
Owner

Yeah, ConditionalField was pretty simple and fast once, but now there is a lot going on under the hood. I got to make lots of tweaks to support nested inspectors, inheritance, and a bunch of other edge cases. Now it is a really clumsy bunch of code and I had no chance to dig into optimization 🤔
I'll check DrawIf code for insights though, thank you for sharing 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants