slavniyteo / one-line

One line property drawer for Unity3d
MIT License
146 stars 12 forks source link

Ability to use OneLine with CustomPropertyDrawer? #15

Closed KonH closed 6 years ago

KonH commented 6 years ago

I'm interested in using your utility, but found one issue and now don't know can it be resolved in basics. In my project unity3d-class-type-reference is used like this:

[Serializable]
public class LogNode {
    [ClassImplements(typeof(ILogContext))]
    public ClassTypeReference Context;
    public bool               Enabled;
}
...
public List<LogNode> Nodes = new List<LogNode>();

In standard cases ClassTypeReference instance is shown as drop-down, which allow you to select class with given constraints. ClassTypeReference used their own CustomPropertyDrawer to do it.

Problem is here: when [OneLine] attribute is used on Nodes it shown as just editable string, which is not properly usable.

It is any proposals how this issue can be resolved with/without changes in utility code?

slavniyteo commented 6 years ago

Thank you for this case. It is similar to #13 with a difference in one detail: [Range] is not a CustomPropertyDrawer.

Right now OneLine doesn't draw custom property drawers but it is the next task in my TODO list.

But you can add this behaviour very simply here.

private void DrawProperty(Rect rect, SerializedProperty property){
    bool isCustomPropertyDrawer = IsThisPropertyMustBeDrawnWithYourPropertyDrawer(property);
    if (isCustomPropertyDrawer){
        EditorGUI.PropertyField(rect, property, GUIContent.none);
    }
    else {
        typeof(EditorGUI)
            .GetMethod("DefaultPropertyField", BindingFlags.NonPublic | BindingFlags.Static)
            .Invoke(null, new object[]{rect, property, GUIContent.none});
    }
}

It will draw your fields in whole cycle (with all decorators and custom drawers). But see at the comment here

/*

So whole drawing cycle produces some artifacts with property decorators like [Header] or [Space].

You can avoid it with drawing this way:

private void DrawProperty(Rect rect, SerializedProperty property){
    bool isCustomPropertyDrawer = IsThisPropertyMustBeDrawnWithYourPropertyDrawer(property);
    if (isCustomPropertyDrawer){
        // EditorGUI.PropertyField(rect, property, GUIContent.none);
        var drawer = new YourCustomPropertyDrawer(); // Cache it, pleace
        drawer.OnGUI(rect, property, GUIContent.none);
    }
    else {
        typeof(EditorGUI)
            .GetMethod("DefaultPropertyField", BindingFlags.NonPublic | BindingFlags.Static)
            .Invoke(null, new object[]{rect, property, GUIContent.none});
    }
}

Sorry, I can't check the code above, so it is just a pseudocode =)

I wish it is useful for you.

KonH commented 6 years ago

Thanks, I get it. Now needs to go inside third party customPropertyDrawer code, because this is not so simple to combine with it. If you interested in this case - https://github.com/rotorz/unity3d-class-type-reference/blob/master/assets/Editor/ClassTypeReferencePropertyDrawer.cs

slavniyteo commented 6 years ago

Sorry, I might write code of magic method IsThisPropertyMustBeDrawnWithYourPropertyDrawer. Full code is below (I can't verify it with compiler or Unity, so this is just a concept).

private void DrawProperty(Rect rect, SerializedProperty property){
    bool isClass = IsThisPropertyMustBeDrawnWithYourPropertyDrawer(property);
    if (isCustomPropertyDrawer){
        var drawer = new ClassTypeReferencePropertyDrawer(); // Cache it, pleace
        drawer.OnGUI(rect, property, GUIContent.none);
    }
    else {
        typeof(EditorGUI)
            .GetMethod("DefaultPropertyField", BindingFlags.NonPublic | BindingFlags.Static)
            .Invoke(null, new object[]{rect, property, GUIContent.none});
    }
}

private static bool IsThisPropertyMustBeDrawnWithYourPropertyDrawer(property) {
    bool fieldType = GetPropertyType(property);
    return fieldType == typeof(ClassTypeReference);
}

private static Type GetPropertyType(SerializedProperty property){
    string[] path = property.propertyPath.Split('.');

    bool isFailed = false;
    Type type = property.serializedObject.targetObject.GetType();
    FieldInfo field = null;
    for (int i = 0; i < path.Length; i++) {
        field = type.GetField(path[i], BindingFlags.Public 
                                        | BindingFlags.NonPublic
                                        | BindingFlags.Instance);

        if (field != null) {
            type = field.FieldType;
        }
        else {
            isFailed = true;
        }

        int next = i + 1;
        if (next < path.Length && path[next] == "Array") {
            i += 2;
            if (type.IsArray) {
                type = type.GetElementType();
            }
            else {
                type = type.GetGenericArguments()[0];
            }
        }
    }

    return isFailed ? type : null;
}

I think, it can be optimized, but this is just proof concept. Maybe property.type may be used, but I can't check any code now.

Pleace don't close the issue, I'm going to add an ability to draw any custom property drawers some day. I think, OneLine v0.3 will draw CustomPropertyDrawers, which GetProeprtyHeight() return one line height.

slavniyteo commented 6 years ago

I did it today. But need some time to test and optimize a little. Now works with CustomPropertyDrawer either for class directly and for attribute.

You can take it at custom-drawers branch.

Can you test it with your property drawer?

I need to implement a few cases. For example, when [CustomPropertyDrawer] is used with useForChildren flag. It is something like a beta version.

KonH commented 6 years ago

Almost working for my case. You can look example here - https://github.com/KonH/UDBaseExample/commit/a13e9ff3b4af2971e7a502d62d391fb8f039fbad

Property was drawn properly, but interaction broken with exception on custom property drawer side:

NullReferenceException: Object reference not set to an instance of an object
Rotorz.Games.Reflection.ClassTypeReferencePropertyDrawer.DrawTypeSelectionControl (UnityEngine.Rect position, UnityEngine.GUIContent label, System.String classRef, Rotorz.Games.Reflection.ClassTypeConstraintAttribute filter) (at Assets/UDBase/Extensions/ClassTypeReference/Editor/ClassTypeReferencePropertyDrawer.cs:180)
Rotorz.Games.Reflection.ClassTypeReferencePropertyDrawer.DrawTypeSelectionControl (UnityEngine.Rect position, UnityEditor.SerializedProperty property, UnityEngine.GUIContent label, Rotorz.Games.Reflection.ClassTypeConstraintAttribute filter) (at Assets/UDBase/Extensions/ClassTypeReference/Editor/ClassTypeReferencePropertyDrawer.cs:192)
Rotorz.Games.Reflection.ClassTypeReferencePropertyDrawer.OnGUI (UnityEngine.Rect position, UnityEditor.SerializedProperty property, UnityEngine.GUIContent label) (at Assets/UDBase/Extensions/ClassTypeReference/Editor/ClassTypeReferencePropertyDrawer.cs:278)
OneLine.CustomDrawer.DrawProperty (UnityEngine.Rect rect, UnityEditor.SerializedProperty property) (at Assets/UDBase/Extensions/OneLine/OneLine/Editor/Drawers/CustomDrawer.cs:27)
OneLine.CustomDrawer.Draw (UnityEngine.Rect rect, UnityEditor.SerializedProperty property) (at Assets/UDBase/Extensions/OneLine/OneLine/Editor/Drawers/CustomDrawer.cs:21)
OneLine.SimpleFieldDrawer+<AddSlices>c__AnonStorey0.<>m__0 (UnityEngine.Rect rect) (at Assets/UDBase/Extensions/OneLine/OneLine/Editor/Drawers/SimpleFieldDrawer.cs:42)
OneLine.Slice.Draw (UnityEngine.Rect rect) (at Assets/UDBase/Extensions/OneLine/OneLine/Editor/Slices/Slice.cs:29)
OneLine.OneLinePropertyDrawer.<OnGUI>m__0 (OneLine.Slice slice, UnityEngine.Rect rect) (at Assets/UDBase/Extensions/OneLine/OneLine/Editor/OneLinePropertyDrawer.cs:110)
OneLine.OneLinePropertyDrawer.DrawLine (UnityEngine.Rect position, UnityEditor.SerializedProperty property, System.Action`2[T1,T2] draw) (at Assets/UDBase/Extensions/OneLine/OneLine/Editor/OneLinePropertyDrawer.cs:136)
OneLine.OneLinePropertyDrawer.OnGUI (UnityEngine.Rect position, UnityEditor.SerializedProperty property, UnityEngine.GUIContent label) (at Assets/UDBase/Extensions/OneLine/OneLine/Editor/OneLinePropertyDrawer.cs:110)
UnityEditor.PropertyDrawer.OnGUISafe (UnityEngine.Rect position, UnityEditor.SerializedProperty property, UnityEngine.GUIContent label) (at /Users/builduser/buildslave/unity/build/Editor/Mono/ScriptAttributeGUI/PropertyDrawer.cs:22)
UnityEditor.PropertyHandler.OnGUI (UnityEngine.Rect position, UnityEditor.SerializedProperty property, UnityEngine.GUIContent label, System.Boolean includeChildren) (at /Users/builduser/buildslave/unity/build/Editor/Mono/ScriptAttributeGUI/PropertyHandler.cs:142)
UnityEditor.EditorGUI.PropertyFieldInternal (UnityEngine.Rect position, UnityEditor.SerializedProperty property, UnityEngine.GUIContent label, System.Boolean includeChildren) (at /Users/builduser/buildslave/unity/build/Editor/Mono/EditorGUI.cs:5671)
UnityEditor.EditorGUI.PropertyField (UnityEngine.Rect position, UnityEditor.SerializedProperty property, System.Boolean includeChildren) (at /Users/builduser/buildslave/unity/build/artifacts/generated/common/editor/EditorGUIBindings.gen.cs:949)
UnityEditor.EditorGUI.PropertyField (UnityEngine.Rect position, UnityEditor.SerializedProperty property) (at /Users/builduser/buildslave/unity/build/artifacts/generated/common/editor/EditorGUIBindings.gen.cs:944)
UnityEditor.Editor.OptimizedInspectorGUIImplementation (UnityEngine.Rect contentRect) (at /Users/builduser/buildslave/unity/build/artifacts/generated/common/editor/EditorBindings.gen.cs:289)
UnityEditor.GenericInspector.OnOptimizedInspectorGUI (UnityEngine.Rect contentRect) (at /Users/builduser/buildslave/unity/build/Editor/Mono/Inspector/GenericInspector.cs:32)
UnityEditor.InspectorWindow.DrawEditor (UnityEditor.Editor[] editors, System.Int32 editorIndex, System.Boolean rebuildOptimizedGUIBlock, System.Boolean& showImportedObjectBarNext, UnityEngine.Rect& importedObjectBarRect) (at /Users/builduser/buildslave/unity/build/Editor/Mono/Inspector/InspectorWindow.cs:1225)
UnityEditor.InspectorWindow.DrawEditors (UnityEditor.Editor[] editors) (at /Users/builduser/buildslave/unity/build/Editor/Mono/Inspector/InspectorWindow.cs:1022)
UnityEditor.InspectorWindow.OnGUI () (at /Users/builduser/buildslave/unity/build/Editor/Mono/Inspector/InspectorWindow.cs:361)
System.Reflection.MonoMethod.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) (at /Users/builduser/buildslave/mono/build/mcs/class/corlib/System.Reflection/MonoMethod.cs:305)
Rethrow as TargetInvocationException: Exception has been thrown by the target of an invocation.
System.Reflection.MonoMethod.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) (at /Users/builduser/buildslave/mono/build/mcs/class/corlib/System.Reflection/MonoMethod.cs:313)
System.Reflection.MethodBase.Invoke (System.Object obj, System.Object[] parameters) (at /Users/builduser/buildslave/mono/build/mcs/class/referencesource/mscorlib/system/reflection/methodbase.cs:229)
UnityEditor.HostView.Invoke (System.String methodName, System.Object obj) (at /Users/builduser/buildslave/unity/build/Editor/Mono/HostView.cs:295)
UnityEditor.HostView.Invoke (System.String methodName) (at /Users/builduser/buildslave/unity/build/Editor/Mono/HostView.cs:288)
UnityEditor.HostView.InvokeOnGUI (UnityEngine.Rect onGUIPosition) (at /Users/builduser/buildslave/unity/build/Editor/Mono/HostView.cs:261)
UnityEditor.DockArea.OldOnGUI () (at /Users/builduser/buildslave/unity/build/Editor/Mono/GUI/DockArea.cs:387)
UnityEngine.Experimental.UIElements.IMGUIContainer.DoOnGUI (UnityEngine.Event evt) (at /Users/builduser/buildslave/unity/build/Runtime/UIElements/Managed/IMGUIContainer.cs:195)
UnityEngine.Experimental.UIElements.IMGUIContainer.HandleIMGUIEvent (UnityEngine.Event e) (at /Users/builduser/buildslave/unity/build/Runtime/UIElements/Managed/IMGUIContainer.cs:333)
UnityEngine.Experimental.UIElements.IMGUIContainer.HandleEvent (UnityEngine.Experimental.UIElements.EventBase evt) (at /Users/builduser/buildslave/unity/build/Runtime/UIElements/Managed/IMGUIContainer.cs:317)
UnityEngine.Experimental.UIElements.EventDispatcher.PropagateEvent (UnityEngine.Experimental.UIElements.EventBase evt) (at /Users/builduser/buildslave/unity/build/Runtime/UIElements/Managed/EventDispatcher.cs:459)
UnityEngine.Experimental.UIElements.EventDispatcher.DispatchEvent (UnityEngine.Experimental.UIElements.EventBase evt, UnityEngine.Experimental.UIElements.IPanel panel) (at /Users/builduser/buildslave/unity/build/Runtime/UIElements/Managed/EventDispatcher.cs:340)
UnityEngine.Experimental.UIElements.UIElementsUtility.DoDispatch (UnityEngine.Experimental.UIElements.BaseVisualElementPanel panel) (at /Users/builduser/buildslave/unity/build/Runtime/UIElements/Managed/UIElementsUtility.cs:251)
UnityEngine.Experimental.UIElements.UIElementsUtility.ProcessEvent (System.Int32 instanceID, System.IntPtr nativeEventPtr) (at /Users/builduser/buildslave/unity/build/Runtime/UIElements/Managed/UIElementsUtility.cs:78)
UnityEngine.GUIUtility.ProcessEvent (System.Int32 instanceID, System.IntPtr nativeEventPtr) (at /Users/builduser/buildslave/unity/build/Runtime/IMGUI/Managed/GUIUtility.cs:175)
slavniyteo commented 6 years ago

I see, that at calling

public override void OnGUI(Rect position, SerializedProperty property, GUIContent label)
{
    DrawTypeSelectionControl(position, property.FindPropertyRelative("classRef"), label, attribute as ClassTypeConstraintAttribute);
}

attribute as ClassTypeConstraintAttribute returns null.

Thank you for efficient testing. I'll fix it soon.

slavniyteo commented 6 years ago

I made two more steps to resolve this issue.

There were two problems:

Now OneLine looks for custom drawers with prediction that they are always useWithChildren=true. I'll fix it soon. But you already can try actual version in your project.

OneLine needs some optimizations (so much reflections, it iterates through all types in searching for custom drawers; it searches for Custom Attributes, etc). It will be done too.

Additionally, I add an example to monitor this feature's progress here. Produses the following result: image

slavniyteo commented 6 years ago

Done with #17.

If you will notice any artefact or bug, reopen please.

KonH commented 6 years ago

Works great, thank you! But one more thing - https://imgur.com/bAjMNGX, warnings in example code looks not cool as anything else)

slavniyteo commented 6 years ago

My bad.

Just smelly example. Fixed with 138b6e12258dc8f32339541f633ceb1c4c9f2b87