spatialos / gdk-for-unity

SpatialOS GDK for Unity
https://gdk.improbable.io/spatialos-gdk-unity
MIT License
355 stars 77 forks source link

NullReferenceException errors within the ComponentUpdated scope give useless stacktraces. #468

Closed polartron closed 6 years ago

polartron commented 6 years ago

Description

When a regular null reference exception occurs while inside a ComponentUpdated callback, the errors given by the unity console does not point towards where the nullreference exception happened.

Expected behaviour

I should be able to view the stacktrace and see in which file and on which line it happened. I should also be able to double click on the error to view in code where it happened, just like how it works normally.

Current behaviour

A nullreferenceexception outputs this and double clicking on it brings me to the LogEvent struct in the Gdk.Core

NullReferenceException: Object reference not set to an instance of an object
Improbable.Gdk.Core.LogEvent.WithField (System.String key, System.Object value) (at Packages/com.improbable.gdk.core/Logging/LogEvent.cs:52)
Improbable.Gdk.Core.ForwardingDispatcher.HandleLog (UnityEngine.LogType type, Improbable.Gdk.Core.LogEvent logEvent) (at Packages/com.improbable.gdk.core/Logging/ForwardingDispatcher.cs:68)
Improbable.Gdk.GameObjectRepresentation.ReaderWriterBase`2[TSpatialComponentData,TComponentUpdate].OnComponentUpdate (TComponentUpdate update) (at Packages/com.improbable.gdk.core/GameObjectRepresentation/ReadersWriters/ReaderWriterBase.cs:216)
Improbable.Transform.TransformInternal+GameObjectComponentDispatcher.InvokeOnComponentUpdateCallbacks (System.Collections.Generic.Dictionary`2[TKey,TValue] entityToInjectableStore) (at Assets/Generated/Source/improbable/transform/TransformInternalGameObjectComponentDispatcher.cs:172)
Improbable.Gdk.GameObjectRepresentation.GameObjectDispatcherSystem.UpdateMonoBehaviours () (at Packages/com.improbable.gdk.core/GameObjectRepresentation/GameObjectDispatcherSystem.cs:207)
Improbable.Gdk.GameObjectRepresentation.GameObjectDispatcherSystem.OnUpdate () (at Packages/com.improbable.gdk.core/GameObjectRepresentation/GameObjectDispatcherSystem.cs:99)
Unity.Entities.ComponentSystem.InternalUpdate () (at C:/Users/proxy_j9kjt6u/AppData/Local/Unity/cache/packages/staging-packages.unity.com/com.unity.entities@0.0.12-preview.13/Unity.Entities/ComponentSystem.cs:324)
Unity.Entities.ScriptBehaviourManager.Update () (at C:/Users/proxy_j9kjt6u/AppData/Local/Unity/cache/packages/staging-packages.unity.com/com.unity.entities@0.0.12-preview.13/Unity.Entities/ScriptBehaviourManager.cs:77)
Unity.Entities.ScriptBehaviourUpdateOrder+DummyDelagateWrapper.TriggerUpdate () (at C:/Users/proxy_j9kjt6u/AppData/Local/Unity/cache/packages/staging-packages.unity.com/com.unity.entities@0.0.12-preview.13/Unity.Entities/ScriptBehaviourUpdateOrder.cs:703)

Steps to reproduce

Add the following script to the Character prefab in the Playground project. The reference test should be null.

public class ErrorTest : MonoBehaviour
{
    [Require] TransformInternal.Requirable.Reader transformInternalReader;
    public Image test;

    void OnEnable()
    {
        transformInternalReader.ComponentUpdated += OnComponentUpdated;
    }

    void OnDisable()
    {
        transformInternalReader.ComponentUpdated -= OnComponentUpdated;
    }

    private void OnComponentUpdated(TransformInternal.Update updatedata)
    {
        test.sprite = null;
    }

    void Update()
    {
        if (Input.GetKeyDown(KeyCode.Z))
        {
            test.sprite = null;
        }
    }
}

Environment

Unity 2018.2.10f1 Personal

tenevdev commented 6 years ago

Investigated yesterday. Turns out this is an actual bug in the logger because a log event is implemented as a struct and a field of a reference type can be used without being initialised.

jamiebrynes7 commented 6 years ago

Thanks for the report, we are tracking this internally as UTY-1323 👍

tenevdev commented 6 years ago

This will be fixed by https://github.com/spatialos/gdk-for-unity/pull/509 at least temporarily until we make improvements to how events are logged.

tenevdev commented 6 years ago

@polartron I'm closing this as done since https://github.com/spatialos/gdk-for-unity/pull/509 was accepted and merged.