jacobdufault / fullinspector

Full Inspector supercharges Unity's inspector
MIT License
111 stars 27 forks source link

Fix editor callbacks adding registrations every time they're called #187

Closed hymerman closed 7 years ago

hymerman commented 7 years ago

Fixes #185

SugoiDev commented 7 years ago

Interesting issue.

There's a problem with using ifdef in this part of the code. It won't support DLL builds.

I don't really know how to workaround this while still retaining your improvements.

Any ideas @jacobdufault?

I recon there's a similar situation (with UNITY_EDITOR ifdef block) in tk.HorizontalGroup.

jacobdufault commented 7 years ago

Ah, this is an interesting problem. fiLateBindings exists because it is not possible to have ifdefs for UNITY_EDITOR inside of non-editor DLLs, as they will get compiled out of DLL based builds.

What about having fiLateBindings register a single EditorApplication.update callback; that callback will just iterate through a list of callbacks that fiLateBindings stores.

SugoiDev commented 7 years ago

I did something similar for my EditorBindings. It registers itself and iterates over a list to run the callbacks.

I use a class CallbackWithContext that stores a "context" object for the callback. I use this to remove any callbacks that have null contexts.

Note that, in order for this solution to be available to non-editor scritpts, it is not placed in the editor assembly. This is the reason for the #if usage.

Below is the minimal implementation.

#if UNITY_EDITOR

namespace LostAlloy.Modules {
    using System;
    using System.Collections.Generic;
    using UnityEditor;
    using UnityEngine;

    /// <summary>
    ///     Use this class to safely add editor bindings.
    /// </summary>
    [InitializeOnLoad]
    public class LAEditorBindings {
        [SerializeField]
        private static readonly List<CallbackWithContext> _CallbacksWithContext;

        static LAEditorBindings() {
            _CallbacksWithContext = new List<CallbackWithContext>();

            EditorApplication.update -= Update;
            EditorApplication.update += Update;
            Debug.Log("LAEditorBindings initialized.");
        }

        public static void RemUpdateFunc(object context, Action callback) {
            if (context == null) {
                return;
            }

            var c = new CallbackWithContext(context, callback);
            _CallbacksWithContext.RemoveAll(o => Equals(o, c));
        }

        /// <summary>
        ///     No need to check if already added.
        /// </summary>
        /// <param name="context"></param>
        /// <param name="callback"></param>
        public static void AddUpdateFunc(object context, Action callback, bool executeInPlaymode = false) {
            if (context == null) {
                return;
            }

            var c = new CallbackWithContext(context, callback, executeInPlaymode);
            if (_CallbacksWithContext.Contains(c)) {
                return;
            }

            _CallbacksWithContext.Add(c);
        }

        private static void Update() {
            var total = _CallbacksWithContext.Count;

            //loop backwards since we're removing elements from the list
            for (var i = total - 1; i >= 0; i--) {
                var c = _CallbacksWithContext[i];
                if (c.Context != null) {
                    if (!Application.isPlaying || c.ExecuteInPlayMode) {
                        c.Callback();
                    }
                } else {
                    //can't even touch the Context object here, as it will make unity spew errors all over when it was destroyed
                    Debug.Log("LAEditorBindings: removing a null entry from the editor updates callback");
                    _CallbacksWithContext.RemoveAt(i);
                }
            }
        }

        [Serializable]
        private class CallbackWithContext {

            public CallbackWithContext(object context, Action callback, bool executeInPlaymode = false) {
                Context = context;
                Callback = callback;
                ExecuteInPlayMode = executeInPlaymode;
            }

            public object Context { get; }

            public Action Callback { get; }

            public bool ExecuteInPlayMode { get; }

            public override bool Equals(object o) {
                var other = o as CallbackWithContext;
                if (other == null) {
                    return false;
                }

                return (other.Context == Context) && (other.Callback == Callback);
            }

            public override int GetHashCode() {
                //prime number to reduce diagonal collisions
                return Context.GetHashCode() * 7 + Callback.GetHashCode();
            }

            public override string ToString() {
                return GetHashCode().ToString();
            }
        }

    }
}

#endif
jacobdufault commented 7 years ago

I've merged #194 which should address this issue. Thanks for the patches!