garettbass / UnityExtensions.SelectionHistory

Adds navigation menu items in Unity Editor: "Edit/Selection/Back", "Edit/Selection/Forward"
MIT License
27 stars 3 forks source link

SelectionHistory overwrites GUIUtility.processEvent return value #2

Open leohilbert opened 1 year ago

leohilbert commented 1 year ago

Hi, first up: Thanks for the plugin! I had something similar, but making the history serialized is really useful!

However after adding SelectionHistory to my project a lot of my shortcuts were not working anymore (especially in ShaderGraph). Additionally there was some other weird behaviour with inputs being executed in wrong context etc. After some digging I found the issue here:

https://github.com/garettbass/UnityExtensions.SelectionHistory/blob/11b69815f7156577d41c4d39551001aa304a8588/Editor/SelectionHistory.cs#L139C47-L139C47

When using a MulticastDelegate like the one created with Delegate.Combine(processEvent, value); the last return value get's returned to the method invoker. That means that after adding this class GUIUtility.processEvent will always return false. This is of course not intended, as we only want to intercept the event and not hinder any additional processes from happening. As an easy fix you could just reverse the parameters

Delegate.Combine(value, processEvent); This way our method gets called, but does not change the output of the original method.

For myself I've switched it to a more explicit implementation that does not rely on delegates, but just wraps the listener and always calls the original handler unless we are intercepting a Mouse4 oder Mouse5 event. It's a lot cleaner imo, but also a bit more code.


    public class GUIUtilityProcessEventInterceptor
    {
        private static readonly FieldInfo GUIUtilityProcessEvent = typeof(GUIUtility).GetField(
            "processEvent",
            BindingFlags.Static |
            BindingFlags.Public |
            BindingFlags.NonPublic
        );

        private static readonly Func<int, IntPtr, bool> OriginalHandler;

        public static event Action OnMouse4Clicked;
        public static event Action OnMouse5Clicked;

        static GUIUtilityProcessEventInterceptor()
        {
            OriginalHandler = (Func<int, IntPtr, bool>)GUIUtilityProcessEvent.GetValue(null);

            Func<int, IntPtr, bool> onProcessEvent = OnProcessEvent;
            GUIUtilityProcessEvent.SetValue(null, onProcessEvent);
        }

        private static bool OnProcessEvent(
            int instanceID,
            IntPtr nativeEventPtr
        )
        {
            var currentEvent = Event.current;
            if (
                currentEvent.type == EventType.MouseUp &&
                currentEvent.clickCount == 1
            )
            {
                switch (currentEvent.button)
                {
                    case 3:
                        OnMouse4Clicked?.Invoke();
                        Event.current.Use();
                        return true;
                    case 4:
                        OnMouse5Clicked?.Invoke();
                        Event.current.Use();
                        return true;
                }
            }

            return OriginalHandler(instanceID, nativeEventPtr);
        }
    }
garettbass commented 1 year ago

Thanks, we recently fixed this at work too, but I haven’t updated this repository.

hannesdelbeke commented 1 week ago

hi, any chance we could get an update in this repo 🙏

garettbass commented 1 week ago

Your PR is welcome