Fix Several issue in Property Binder (#106)

PaulDemeulenaere · julienf-unity · commit 62d8a606c5dc · 2020-11-16T17:27:49.000+01:00
diff --git a/com.unity.visualeffectgraph/CHANGELOG.md b/com.unity.visualeffectgraph/CHANGELOG.md
@@ -40,6 +40,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - Fix for VisualEffect prefab override window [Case 1242693](https://issuetracker.unity3d.com/product/unity/issues/guid/1242693/)
 - Fix edited operator being collapsed [Case 1270517](https://issuetracker.unity3d.com/product/unity/issues/guid/1270517/)
 - "Create new VisualEffect Graph" creates a graph from the default template [Case 1279999](https://fogbugz.unity3d.com/f/cases/1279999/)
+- Property Binder : Incorrect Destroy called from edit mode. [Case 1274790](https://issuetracker.unity3d.com/product/unity/issues/guid/1274790/)
+- Property Binder : Unexpected null reference exception while using terrain binder. [Case 1247230](https://issuetracker.unity3d.com/product/unity/issues/guid/1247230/)
+- Property Binder : HierarchyRoot null reference exception while using Hierarchy to Attribute Map. [Case 1274788](https://issuetracker.unity3d.com/product/unity/issues/guid/1274788/)
+- Property Binder : Properties window isn't always up to date. [Case 1248711](https://issuetracker.unity3d.com/product/unity/issues/guid/1248711/)
+- Property Binder : Avoid Warning while building on Mobile "Presence of such handlers might impact performance on handheld devices." when building for Android" [Case 1279471](https://issuetracker.unity3d.com/product/unity/issues/guid/1248711/)
 
 ## [8.2.0] - 2020-07-08
 
diff --git a/com.unity.visualeffectgraph/Editor/Utilities/PropertyBinding/VFXPropertyBinderEditor.cs b/com.unity.visualeffectgraph/Editor/Utilities/PropertyBinding/VFXPropertyBinderEditor.cs
@@ -57,6 +57,8 @@ private void OnDisable()
 
         public override void OnInspectorGUI()
         {
+            serializedObject.Update();
+
             EditorGUI.BeginChangeCheck();
             EditorGUILayout.Space();
             EditorGUILayout.PropertyField(m_ExecuteInEditor);
@@ -65,8 +67,10 @@ public override void OnInspectorGUI()
             EditorGUILayout.Space();
             if (EditorGUI.EndChangeCheck())
                 serializedObject.ApplyModifiedProperties();
-            if (m_ElementEditor != null)
+            if (m_ElementEditor != null && m_ElementEditor.target != null && m_ElementEditor.serializedObject.targetObject != null)
             {
+                m_ElementEditor.serializedObject.Update();
+
                 EditorGUI.BeginChangeCheck();
 
                 var fieldAttribute = BindingFlags.Public | BindingFlags.Instance | BindingFlags.NonPublic;
diff --git a/com.unity.visualeffectgraph/Runtime/Utilities/EventBinding/Implementation/VFXMouseEventBinder.cs b/com.unity.visualeffectgraph/Runtime/Utilities/EventBinding/Implementation/VFXMouseEventBinder.cs
@@ -40,35 +40,67 @@ protected override void SetEventAttribute(object[] parameters)
             }
         }
 
-        private void OnMouseDown()
+        private void DoOnMouseDown()
         {
             if (activation == Activation.OnMouseDown) SendEventToVisualEffect();
         }
 
-        private void OnMouseUp()
+        private void DoOnMouseUp()
         {
             if (activation == Activation.OnMouseUp) SendEventToVisualEffect();
         }
 
-        private void OnMouseDrag()
+        private void DoOnMouseDrag()
         {
             if (activation == Activation.OnMouseDrag) SendEventToVisualEffect();
         }
 
-        private void OnMouseOver()
+        private void DoOnMouseOver()
         {
             if (activation == Activation.OnMouseOver) SendEventToVisualEffect();
         }
 
-        private void OnMouseEnter()
+        private void DoOnMouseEnter()
         {
             if (activation == Activation.OnMouseEnter) SendEventToVisualEffect();
         }
 
-        private void OnMouseExit()
+        private void DoOnMouseExit()
         {
             if (activation == Activation.OnMouseExit) SendEventToVisualEffect();
         }
+
+#if !PLATFORM_ANDROID && !PLATFORM_IOS
+        private void OnMouseDown()
+        {
+            DoOnMouseDown();
+        }
+
+        private void OnMouseUp()
+        {
+            DoOnMouseUp();
+        }
+
+        private void OnMouseDrag()
+        {
+            DoOnMouseDrag();
+        }
+
+        private void OnMouseOver()
+        {
+            DoOnMouseOver();
+        }
+
+        private void OnMouseEnter()
+        {
+            DoOnMouseEnter();
+        }
+
+        private void OnMouseExit()
+        {
+            DoOnMouseExit();
+        }
+#endif
     }
 }
 #endif
diff --git a/com.unity.visualeffectgraph/Runtime/Utilities/PropertyBinding/Implementation/VFXHierarchyAttributeMapBinder.cs b/com.unity.visualeffectgraph/Runtime/Utilities/PropertyBinding/Implementation/VFXHierarchyAttributeMapBinder.cs
@@ -25,7 +25,7 @@ public enum RadiusMode
             Interpolate
         }
 
-        public Transform HierarchyRoot = null;
+        public Transform HierarchyRoot;
         public float DefaultRadius = 0.1f;
         public uint MaximumDepth = 3;
         public RadiusMode Radius = RadiusMode.Fixed;
@@ -69,6 +69,9 @@ void UpdateHierarchy()
         List<Bone> ChildrenOf(Transform source, uint depth)
         {
             List<Bone> output = new List<Bone>();
+            if (source == null)
+                return output;
+
             foreach (Transform child in source)
             {
                 output.Add(new Bone()
diff --git a/com.unity.visualeffectgraph/Runtime/Utilities/PropertyBinding/Implementation/VFXInputButtonBinder.cs b/com.unity.visualeffectgraph/Runtime/Utilities/PropertyBinding/Implementation/VFXInputButtonBinder.cs
@@ -20,7 +20,9 @@ class VFXInputButtonBinder : VFXBinderBase
         public float SmoothSpeed = 2.0f;
         public bool UseButtonSmooth = true;
 
+#if ENABLE_LEGACY_INPUT_MANAGER
         float m_CachedSmoothValue = 0.0f;
+#endif
 
         public override bool IsValid(VisualEffect component)
         {
diff --git a/com.unity.visualeffectgraph/Runtime/Utilities/PropertyBinding/Implementation/VFXInputKeyBinder.cs b/com.unity.visualeffectgraph/Runtime/Utilities/PropertyBinding/Implementation/VFXInputKeyBinder.cs
@@ -20,7 +20,9 @@ class VFXInputKeyBinder : VFXBinderBase
         public float SmoothSpeed = 2.0f;
         public bool UseKeySmooth = true;
 
+#if ENABLE_LEGACY_INPUT_MANAGER
         float m_CachedSmoothValue = 0.0f;
+#endif
 
         public override bool IsValid(VisualEffect component)
         {
diff --git a/com.unity.visualeffectgraph/Runtime/Utilities/PropertyBinding/Implementation/VFXTerrainBinder.cs b/com.unity.visualeffectgraph/Runtime/Utilities/PropertyBinding/Implementation/VFXTerrainBinder.cs
@@ -10,7 +10,7 @@ class VFXTerrainBinder : VFXBinderBase
         public string Property { get { return (string)m_Property; } set { m_Property = value; UpdateSubProperties(); } }
 
         [VFXPropertyBinding("UnityEditor.VFX.TerrainType"), UnityEngine.Serialization.FormerlySerializedAs("TerrainParameter")]
-        public ExposedProperty m_Property;
+        public ExposedProperty m_Property = "Terrain";
         public Terrain Terrain = null;
 
         private ExposedProperty Terrain_Bounds_center;
@@ -58,7 +58,7 @@ public override void UpdateBinding(VisualEffect component)
 
         public override string ToString()
         {
-            return string.Format("Sphere : '{0}' -> {1}", m_Property, Terrain == null ? "(null)" : Terrain.name);
+            return string.Format("Terrain : '{0}' -> {1}", m_Property, Terrain == null ? "(null)" : Terrain.name);
         }
     }
 }
diff --git a/com.unity.visualeffectgraph/Runtime/Utilities/PropertyBinding/VFXPropertyBinder.cs b/com.unity.visualeffectgraph/Runtime/Utilities/PropertyBinding/VFXPropertyBinder.cs
@@ -35,6 +35,13 @@ private void OnEnable()
         {
             m_VisualEffect = GetComponent<VisualEffect>();
         }
+        static private void SafeDestroy(Object toDelete)
+        {
+            if (Application.isPlaying)
+                Destroy(toDelete);
+            else
+                DestroyImmediate(toDelete);
+        }
 
         private void Reset()
         {
@@ -92,7 +99,8 @@ public T AddParameterBinder<T>() where T : VFXBinderBase
         public void ClearPropertyBinders()
         {
             var allBinders = GetComponents<VFXBinderBase>();
-            foreach (var binder in allBinders) Destroy(binder);
+            foreach (var binder in allBinders)
+                SafeDestroy(binder);
         }
 
         /// <summary>
@@ -110,7 +118,8 @@ public void ClearParameterBinders()
         /// <param name="binder">The VFXBinderBase to remove</param>
         public void RemovePropertyBinder(VFXBinderBase binder)
         {
-            if (binder.gameObject == this.gameObject) Destroy(binder);
+            if (binder.gameObject == this.gameObject)
+                SafeDestroy(binder);
         }
 
         /// <summary>
@@ -131,7 +140,8 @@ public void RemovePropertyBinders<T>() where T : VFXBinderBase
         {
             var allBinders = GetComponents<VFXBinderBase>();
             foreach (var binder in allBinders)
-                if (binder is T) Destroy(binder);
+                if (binder is T)
+                    SafeDestroy(binder);
         }
 
         /// <summary>

Original file line number	Diff line number	Diff line change
`@@ -40,35 +40,67 @@ protected override void SetEventAttribute(object[] parameters)`
`40`	`40`	`}`
`41`	`41`	`}`
`42`	`42`
`43`		`- private void OnMouseDown()`
	`43`	`+ private void DoOnMouseDown()`
`44`	`44`	`{`
`45`	`45`	`if (activation == Activation.OnMouseDown) SendEventToVisualEffect();`
`46`	`46`	`}`
`47`	`47`
`48`		`- private void OnMouseUp()`
	`48`	`+ private void DoOnMouseUp()`
`49`	`49`	`{`
`50`	`50`	`if (activation == Activation.OnMouseUp) SendEventToVisualEffect();`
`51`	`51`	`}`
`52`	`52`
`53`		`- private void OnMouseDrag()`
	`53`	`+ private void DoOnMouseDrag()`
`54`	`54`	`{`
`55`	`55`	`if (activation == Activation.OnMouseDrag) SendEventToVisualEffect();`
`56`	`56`	`}`
`57`	`57`
`58`		`- private void OnMouseOver()`
	`58`	`+ private void DoOnMouseOver()`
`59`	`59`	`{`
`60`	`60`	`if (activation == Activation.OnMouseOver) SendEventToVisualEffect();`
`61`	`61`	`}`
`62`	`62`
`63`		`- private void OnMouseEnter()`
	`63`	`+ private void DoOnMouseEnter()`
`64`	`64`	`{`
`65`	`65`	`if (activation == Activation.OnMouseEnter) SendEventToVisualEffect();`
`66`	`66`	`}`
`67`	`67`
`68`		`- private void OnMouseExit()`
	`68`	`+ private void DoOnMouseExit()`
`69`	`69`	`{`
`70`	`70`	`if (activation == Activation.OnMouseExit) SendEventToVisualEffect();`
`71`	`71`	`}`
	`72`	`+`
	`73`	`+#if !PLATFORM_ANDROID && !PLATFORM_IOS`
	`74`	`+ private void OnMouseDown()`
	`75`	`+ {`
	`76`	`+ DoOnMouseDown();`
	`77`	`+ }`
	`78`	`+`
	`79`	`+ private void OnMouseUp()`
	`80`	`+ {`
	`81`	`+ DoOnMouseUp();`
	`82`	`+ }`
	`83`	`+`
	`84`	`+ private void OnMouseDrag()`
	`85`	`+ {`
	`86`	`+ DoOnMouseDrag();`
	`87`	`+ }`
	`88`	`+`
	`89`	`+ private void OnMouseOver()`
	`90`	`+ {`
	`91`	`+ DoOnMouseOver();`
	`92`	`+ }`
	`93`	`+`
	`94`	`+ private void OnMouseEnter()`
	`95`	`+ {`
	`96`	`+ DoOnMouseEnter();`
	`97`	`+ }`
	`98`	`+`
	`99`	`+ private void OnMouseExit()`
	`100`	`+ {`
	`101`	`+ DoOnMouseExit();`
	`102`	`+ }`
	`103`	`+#endif`
`72`	`104`	`}`
`73`	`105`	`}`
`74`	`106`	`#endif`
Original file line number	Diff line number	Diff line change
`@@ -25,7 +25,7 @@ public enum RadiusMode`
`25`	`25`	`Interpolate`
`26`	`26`	`}`
`27`	`27`
`28`		`- public Transform HierarchyRoot = null;`
	`28`	`+ public Transform HierarchyRoot;`
`29`	`29`	`public float DefaultRadius = 0.1f;`
`30`	`30`	`public uint MaximumDepth = 3;`
`31`	`31`	`public RadiusMode Radius = RadiusMode.Fixed;`
`@@ -69,6 +69,9 @@ void UpdateHierarchy()`
`69`	`69`	`List<Bone> ChildrenOf(Transform source, uint depth)`
`70`	`70`	`{`
`71`	`71`	`List<Bone> output = new List<Bone>();`
	`72`	`+ if (source == null)`
	`73`	`+ return output;`
	`74`	`+`
`72`	`75`	`foreach (Transform child in source)`
`73`	`76`	`{`
`74`	`77`	`output.Add(new Bone()`
Original file line number	Diff line number	Diff line change
`@@ -20,7 +20,9 @@ class VFXInputButtonBinder : VFXBinderBase`
`20`	`20`	`public float SmoothSpeed = 2.0f;`
`21`	`21`	`public bool UseButtonSmooth = true;`
`22`	`22`
	`23`	`+#if ENABLE_LEGACY_INPUT_MANAGER`
`23`	`24`	`float m_CachedSmoothValue = 0.0f;`
	`25`	`+#endif`
`24`	`26`
`25`	`27`	`public override bool IsValid(VisualEffect component)`
`26`	`28`	`{`
Original file line number	Diff line number	Diff line change
`@@ -20,7 +20,9 @@ class VFXInputKeyBinder : VFXBinderBase`
`20`	`20`	`public float SmoothSpeed = 2.0f;`
`21`	`21`	`public bool UseKeySmooth = true;`
`22`	`22`
	`23`	`+#if ENABLE_LEGACY_INPUT_MANAGER`
`23`	`24`	`float m_CachedSmoothValue = 0.0f;`
	`25`	`+#endif`
`24`	`26`
`25`	`27`	`public override bool IsValid(VisualEffect component)`
`26`	`28`	`{`
Original file line number	Diff line number	Diff line change
`@@ -10,7 +10,7 @@ class VFXTerrainBinder : VFXBinderBase`
`10`	`10`	`public string Property { get { return (string)m_Property; } set { m_Property = value; UpdateSubProperties(); } }`
`11`	`11`
`12`	`12`	`[VFXPropertyBinding("UnityEditor.VFX.TerrainType"), UnityEngine.Serialization.FormerlySerializedAs("TerrainParameter")]`
`13`		`- public ExposedProperty m_Property;`
	`13`	`+ public ExposedProperty m_Property = "Terrain";`
`14`	`14`	`public Terrain Terrain = null;`
`15`	`15`
`16`	`16`	`private ExposedProperty Terrain_Bounds_center;`
`@@ -58,7 +58,7 @@ public override void UpdateBinding(VisualEffect component)`
`58`	`58`
`59`	`59`	`public override string ToString()`
`60`	`60`	`{`
`61`		`- return string.Format("Sphere : '{0}' -> {1}", m_Property, Terrain == null ? "(null)" : Terrain.name);`
	`61`	`+ return string.Format("Terrain : '{0}' -> {1}", m_Property, Terrain == null ? "(null)" : Terrain.name);`
`62`	`62`	`}`
`63`	`63`	`}`
`64`	`64`	`}`
Original file line number	Diff line number	Diff line change
`@@ -35,6 +35,13 @@ private void OnEnable()`
`35`	`35`	`{`
`36`	`36`	`m_VisualEffect = GetComponent<VisualEffect>();`
`37`	`37`	`}`
	`38`	`+ static private void SafeDestroy(Object toDelete)`
	`39`	`+ {`
	`40`	`+ if (Application.isPlaying)`
	`41`	`+ Destroy(toDelete);`
	`42`	`+ else`
	`43`	`+ DestroyImmediate(toDelete);`
	`44`	`+ }`
`38`	`45`
`39`	`46`	`private void Reset()`
`40`	`47`	`{`
`@@ -92,7 +99,8 @@ public T AddParameterBinder<T>() where T : VFXBinderBase`
`92`	`99`	`public void ClearPropertyBinders()`
`93`	`100`	`{`
`94`	`101`	`var allBinders = GetComponents<VFXBinderBase>();`
`95`		`- foreach (var binder in allBinders) Destroy(binder);`
	`102`	`+ foreach (var binder in allBinders)`
	`103`	`+ SafeDestroy(binder);`
`96`	`104`	`}`
`97`	`105`
`98`	`106`	`/// <summary>`
`@@ -110,7 +118,8 @@ public void ClearParameterBinders()`
`110`	`118`	`/// <param name="binder">The VFXBinderBase to remove</param>`
`111`	`119`	`public void RemovePropertyBinder(VFXBinderBase binder)`
`112`	`120`	`{`
`113`		`- if (binder.gameObject == this.gameObject) Destroy(binder);`
	`121`	`+ if (binder.gameObject == this.gameObject)`
	`122`	`+ SafeDestroy(binder);`
`114`	`123`	`}`
`115`	`124`
`116`	`125`	`/// <summary>`
`@@ -131,7 +140,8 @@ public void RemovePropertyBinders<T>() where T : VFXBinderBase`
`131`	`140`	`{`
`132`	`141`	`var allBinders = GetComponents<VFXBinderBase>();`
`133`	`142`	`foreach (var binder in allBinders)`
`134`		`- if (binder is T) Destroy(binder);`
	`143`	`+ if (binder is T)`
	`144`	`+ SafeDestroy(binder);`
`135`	`145`	`}`
`136`	`146`
`137`	`147`	`/// <summary>`