Rework NotifyUpdater

monkeymanboy · Feb 23, 2024 · 34c0a7a · 34c0a7a
1 parent fad9c72
commit 34c0a7a
Show file tree

Hide file tree

Showing 5 changed files with 73 additions and 77 deletions.
diff --git a/BeatSaberMarkupLanguage/BSMLParser.cs b/BeatSaberMarkupLanguage/BSMLParser.cs
@@ -118,10 +118,7 @@ public BSMLParserParams Parse(string content, GameObject parent, object host = n
 
         public BSMLParserParams Parse(XmlNode parentNode, GameObject parent, object host = null)
         {
-            BSMLParserParams parserParams = new()
-            {
-                host = host,
-            };
+            BSMLParserParams parserParams = new(host);
 
             FieldAccessOption fieldAccessOptions = FieldAccessOption.Auto;
             PropertyAccessOption propertyAccessOptions = PropertyAccessOption.Auto;

diff --git a/BeatSaberMarkupLanguage/Components/NotifyUpdater.cs b/BeatSaberMarkupLanguage/Components/NotifyUpdater.cs
@@ -2,73 +2,89 @@
 using System.Collections.Generic;
 using System.ComponentModel;
 using System.Reflection;
-using UnityEngine;
 
 namespace BeatSaberMarkupLanguage.Components
 {
-    public class NotifyUpdater : MonoBehaviour
+    internal class NotifyUpdater
     {
-        private INotifyPropertyChanged notifyHost;
+        private readonly Dictionary<string, PropertyAction> actionDict = new();
+        private readonly INotifyPropertyChanged notifyHost;
 
-        public INotifyPropertyChanged NotifyHost
+        internal NotifyUpdater(INotifyPropertyChanged notifyHost)
         {
-            get => notifyHost;
-            set
+            this.notifyHost = notifyHost;
+            this.notifyHost.PropertyChanged += NotifyHost_PropertyChanged;
+        }
+
+        internal bool AddAction(string propertyName, Action<object> action)
+        {
+            if (actionDict.TryGetValue(propertyName, out PropertyAction notify))
             {
-                if (notifyHost != null)
+                notify.AddAction(action);
+            }
+            else
+            {
+                PropertyInfo prop = notifyHost.GetType().GetProperty(propertyName, BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic);
+
+                if (prop == null)
                 {
-                    notifyHost.PropertyChanged -= NotifyHost_PropertyChanged;
+                    Logger.Log.Error($"No property '{propertyName}' on object of type '{notifyHost.GetType().FullName}'");
+                    return false;
                 }
 
-                notifyHost = value;
-
-                if (notifyHost != null)
+                if (prop.GetMethod == null)
                 {
-                    notifyHost.PropertyChanged -= NotifyHost_PropertyChanged;
-                    notifyHost.PropertyChanged += NotifyHost_PropertyChanged;
+                    Logger.Log.Error($"Property '{propertyName}' on object of type '{notifyHost.GetType().FullName}' does not have a getter");
+                    return false;
                 }
-            }
-        }
 
-        private Dictionary<string, Action<object>> ActionDict { get; set; } = new Dictionary<string, Action<object>>();
+                actionDict.Add(propertyName, new PropertyAction(prop, action));
+            }
 
-        public bool AddAction(string propertyName, Action<object> action)
-        {
-            ActionDict.Add(propertyName, action);
             return true;
         }
 
-        public bool RemoveAction(string propertyName)
+        private void NotifyHost_PropertyChanged(object sender, PropertyChangedEventArgs e)
         {
-            if (ActionDict != null)
+            // https://learn.microsoft.com/en-us/dotnet/api/system.componentmodel.propertychangedeventargs.propertyname?view=netframework-4.7.2#remarks
+            if (string.IsNullOrEmpty(e.PropertyName))
             {
-                return ActionDict.Remove(propertyName);
+                foreach (PropertyAction propertyAction in actionDict.Values)
+                {
+                    propertyAction.Invoke(sender);
+                }
+            }
+            else if (actionDict.TryGetValue(e.PropertyName, out PropertyAction propertyAction))
+            {
+                propertyAction.Invoke(sender);
+            }
+            else
+            {
+                Logger.Log.Warn($"PropertyChanged invoked for '{e.PropertyName}' on object of type '{sender.GetType().FullName}' but no such property is registered!");
             }
-
-            return false;
         }
 
-        private void NotifyHost_PropertyChanged(object sender, PropertyChangedEventArgs e)
+        private class PropertyAction
         {
-            // OnDestroy is not called for disabled objects so this will make sure it is called if it gets called while destroyed
-            if (this == null)
+            private readonly PropertyInfo property;
+
+            private Action<object> action;
+
+            internal PropertyAction(PropertyInfo property, Action<object> action)
             {
-                OnDestroy();
-                return;
+                this.property = property;
+                this.action = action;
             }
 
-            PropertyInfo prop = sender.GetType().GetProperty(e.PropertyName, BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic);
-
-            if (ActionDict.TryGetValue(e.PropertyName, out Action<object> action))
+            internal void AddAction(Action<object> newAction)
             {
-                action?.Invoke(prop.GetValue(sender));
+                action += newAction;
             }
-        }
 
-        private void OnDestroy()
-        {
-            ActionDict.Clear();
-            NotifyHost = null;
+            internal void Invoke(object sender)
+            {
+                action.Invoke(property.GetValue(sender));
+            }
         }
     }
 }
diff --git a/BeatSaberMarkupLanguage/Parser/BSMLParserParams.cs b/BeatSaberMarkupLanguage/Parser/BSMLParserParams.cs
@@ -1,5 +1,7 @@
 using System;
 using System.Collections.Generic;
+using System.ComponentModel;
+using BeatSaberMarkupLanguage.Components;
 using UnityEngine;
 
 namespace BeatSaberMarkupLanguage.Parser
@@ -13,6 +15,14 @@ public class BSMLParserParams
         private readonly Dictionary<string, Action> events = new();
         private readonly Dictionary<string, List<GameObject>> objectsWithTag = new();
 
+        internal BSMLParserParams(object host)
+        {
+            this.host = host;
+            this.NotifyUpdater = host is INotifyPropertyChanged notifyHost ? new NotifyUpdater(notifyHost) : null;
+        }
+
+        internal NotifyUpdater NotifyUpdater { get; }
+
         public void AddEvent(string ids, Action action)
         {
             foreach (string id in ids.Split(','))

diff --git a/BeatSaberMarkupLanguage/TypeHandlers/FormattableTextHandler.cs b/BeatSaberMarkupLanguage/TypeHandlers/FormattableTextHandler.cs
@@ -24,13 +24,13 @@ public override void HandleType(BSMLParser.ComponentTypeWithData componentType,
         {
             base.HandleType(componentType, parserParams);
             FormattableText formattableText = componentType.component as FormattableText;
-            NotifyUpdater updater = null;
+
             if (componentType.data.TryGetValue("data", out string dataStr))
             {
                 if (parserParams.values.TryGetValue(dataStr, out BSMLValue dataValue))
                 {
                     formattableText.Data = dataValue.GetValue();
-                    BindValue(componentType, parserParams, dataValue, val => formattableText.Data = val, updater);
+                    BindValue(componentType, parserParams, dataValue, val => formattableText.Data = val);
                 }
                 else
                 {
@@ -43,7 +43,7 @@ public override void HandleType(BSMLParser.ComponentTypeWithData componentType,
                 if (parserParams.values.TryGetValue(formatterStr, out BSMLValue formatterValue))
                 {
                     formattableText.SetFormatter(formatterValue.GetValue());
-                    updater = BindValue(componentType, parserParams, formatterValue, val => formattableText.SetFormatter(val), updater);
+                    BindValue(componentType, parserParams, formatterValue, val => formattableText.SetFormatter(val));
                 }
                 else
                 {

diff --git a/BeatSaberMarkupLanguage/TypeHandlers/TypeHandler.cs b/BeatSaberMarkupLanguage/TypeHandlers/TypeHandler.cs
@@ -39,55 +39,28 @@ public override void HandleType(ComponentTypeWithData componentType, BSMLParserP
         {
             if (componentType.component is T obj)
             {
-                NotifyUpdater updater = null;
                 foreach (KeyValuePair<string, string> pair in componentType.data)
                 {
                     if (CachedSetters.TryGetValue(pair.Key, out Action<T, string> action))
                     {
                         action.Invoke(obj, pair.Value);
                         if (componentType.valueMap.TryGetValue(pair.Key, out BSMLValue value))
                         {
-                            updater = BindValue(componentType, parserParams, value, val => action.Invoke(obj, val.InvariantToString()), updater);
+                            BindValue(componentType, parserParams, value, val => action.Invoke(obj, val.InvariantToString()));
                         }
                     }
                 }
             }
         }
 
-        protected static NotifyUpdater GetOrCreateNotifyUpdater(ComponentTypeWithData componentType, BSMLParserParams parserParams)
+        protected static void BindValue(ComponentTypeWithData componentType, BSMLParserParams parserParams, BSMLValue value, Action<object> onChange)
         {
-            NotifyUpdater updater = null;
+            NotifyUpdater notifyUpdater = parserParams.NotifyUpdater;
 
-            if (parserParams.host is System.ComponentModel.INotifyPropertyChanged notifyHost && !componentType.component.gameObject.TryGetComponent(out updater))
+            if (notifyUpdater != null && value is BSMLPropertyValue prop)
             {
-                updater = componentType.component.gameObject.AddComponent<NotifyUpdater>();
-                updater.NotifyHost = notifyHost;
+                notifyUpdater.AddAction(prop.PropertyInfo.Name, onChange);
             }
-
-            return updater;
-        }
-
-        protected static NotifyUpdater BindValue(ComponentTypeWithData componentType, BSMLParserParams parserParams, BSMLValue value, Action<object> onChange, NotifyUpdater notifyUpdater = null)
-        {
-            if (value == null)
-            {
-                return notifyUpdater;
-            }
-
-            if (value is BSMLPropertyValue prop)
-            {
-                if (notifyUpdater == null)
-                {
-                    notifyUpdater = GetOrCreateNotifyUpdater(componentType, parserParams);
-                }
-
-                if (notifyUpdater != null)
-                {
-                    notifyUpdater.AddAction(prop.PropertyInfo.Name, onChange);
-                }
-            }
-
-            return notifyUpdater;
         }
     }