Using Addressables Asset References List causes Unity Editor Corruption

Issue #620 closed
Raghavendra Upadhya created an issue

Hi,

As you can check the below given image. As soon as I try to access the AssetReferences List (Addressables) in the inspector. Many of the Editor UI text starts overlapping each other and stays the same way until i restart Unity, but this happens again as soon i open the AssetReferences List in the Inspector.

Comments (13)

  1. Tor Esa Vestergaard

    This has been reported before. As far as we've been able to determine, this is an internal bug in Unity's own drawers for AssetReferences list, that we cannot do much about. You would be better served to report this issue to Unity.

  2. Raghavendra Upadhya reporter

    Thanks for replying, but it seems to work fine when used with Odin Inspector? Anyway, I’ll wait for further updates on Addressables from Unity to see if this issue gets fixed till then.

  3. Raghavendra Upadhya reporter

    Okay, this bug has been fixed in the latest version of Unity 2018 LTS. Please close this bug.

  4. Tor Esa Vestergaard

    Thanks for the heads-up! - can you link to a forum thread or patch note where this is reflected, just for our peace of mind? 🙂

  5. Raghavendra Upadhya reporter

    Sorry, I just updated my Unity from 2018.4.5f1 to 2018.4.15f1 and it got fixed. So, i am not sure in which version it was fixed. If i come across any posts or any notes related to it then I’ll surely post it here.

  6. Tobias Mollstam

    I’m seeing this exact issue in 2018.4.15, 2018.4.17 (latest 2018 LTS), 2019.2 and 2019.3.

    Will dig around some more and get back to you, but would be nice to know what you know. Also, is there a way to disable the Odin list drawer, in worst case, as it is better than letting this “Element #” label surface everywhere. 😱

    Cheers

  7. Tobias Mollstam

    All right I’ve zoned in a bit more on it. Unity is being quirky (surprise) but I believe the error is in Odin.

    The quirk in Unity is that AssetReferenceDrawer.OnGUI(...) mutates the provided label:

    // com.unity.addressables@1.6.0/Editor/GUI/AssetReferenceDrawer.cs
    
    public override void OnGUI(Rect position, SerializedProperty property, GUIContent label)
    {
        if (property == null || label == null)
        {
            Debug.LogError("Error rendering drawer for AssetReference property.");
            return;
        }
    
        string labelText = label.text;
        m_AssetRefObject = property.GetActualObjectForSerializedProperty<AssetReference>(fieldInfo, ref labelText);
        label.text = labelText;
    
        // ...
    

    Further up the call stack, in Odin’s AbstractTypeUnityPropertyDrawer<>.DrawPropertyLayout(...) there is code using GUIContent.none as a fallback label

    // Odin Inspector/Source/Sirenix.OdinInspector.Editor/Drawers/Value Drawers/AbstractTypeUnityPropertyDrawer.cs
    
    label = label ?? GUIContent.none;
    

    So the AssetReference drawer mutates GUIContent.none to have a text level and presto, it is everywhere.

    This is obviously smelly from the Addressable package’s part, but maybe it was never built to have GUIContent.none to be provided as a label, I’m not quite sure.

  8. Tobias Mollstam

    For the record keeping the label as null in AbstractTypeUnityPropertyDrawer does not cause any null pointer exceptions or other bad stuff. It Just Works™

  9. Tor Esa Vestergaard

    Hmm. That is very good information to have! - this isn’t what I remember being the issue (though I was running a fever when I last investigated this a while back, probably I was simply mistaken at the time), but it does make it fixable on our end. *Many* Unity drawers (and custom drawers written by users) do complain about being passed a null label, it should be said, so we’ll have to make an edge case for this in particular.

  10. Tor Esa Vestergaard

    Alright I have a potential fixed build that should handle this edge case. If anybody is up for testing it and verifying that it works, please shoot me a message on Discord.

  11. Tobias Mollstam

    I’ve just removed the none backup in our local code, will see when it explodes. 😬

    Would be great to not have to allocate a blank GUIStyle for each OnGUI, but I guess that's a worst case scenario 🤔

  12. Log in to comment