Cammin / LDtkToUnity

Unity importer for the LDtk level editor
https://cammin.github.io/LDtkToUnity
MIT License
388 stars 39 forks source link

Conflict with Odin Inspector when drawing arrays #14

Closed Sookhaal closed 3 years ago

Sookhaal commented 3 years ago

Whenever Odin is active and an array with the LDtkField attribute is drawn on the inspector, a weird graphic bug happens. The ldtk icon d_SimpleIcon.png gets drawn everywhere and every tooltips get messed up.

With Odin active and no arrays opened, everything is working as expected: odin and arrays not opened

But as soon as you open an array: odin and array opened

And that icon is drawn every where in Unity: Screenshot 2021-05-13 144344

Also the tooltip gets drawn at the bottom of the inspector (it was working well before opening up the array): tooltip position

I did a temporary fix to quickly solve the issue on my side, but I'd rather have a proper fix if possible:

// Inside LDtkUnity/Editor/LDtkFieldAttributeDrawer.cs
public override void OnGUI(Rect position, SerializedProperty property, GUIContent label)
{
#if ODIN_INSPECTOR
    if (property.name != "data") {
        label.image = LDtkIconLoader.LoadSimpleIcon();
        label.tooltip = "'This field is sed by LDtk";
     }
#else
      label.image = LDtkIconLoader.LoadSimpleIcon();
      label.tooltip = "'This field is sed by LDtk";
#endif
    EditorGUI.PropertyField(position, property, label);
}
Cammin commented 3 years ago

Very interesting bug. I'll address it as soon as I can and push a hotfix.

I believe that a proper fix would be to construct a new GUIContent instead of using the existing one passed in the function. What's likely happening is this GUIContent is passed around for the rest of the drawers in Unity/Odin. My mistake 😅

Also, I noticed that the tooltip has a couple typos in it so that will be fixed too.

Thanks for reaching out to me on this! This is well documented. 🙂

Cammin commented 3 years ago

Fix the property drawer LDtkAttribute for Odin

Cammin commented 3 years ago

I tested with Odin and can verify it's fixed. It's in the package update 1.3.9. 👍 Here was my solution: 9d2d3d8

Sookhaal commented 3 years ago

Awesome, thanks for the fix! Great job with that package :)