feat(MemberChange): only call change methods when isActiveAndEnabled

For Behaviours it is important to ignore changes while the Behaviour is disabled as it should handle the state in its OnEnable implementation. This new feature ensures the change handler methods are only called for Behaviours that are active and enabled. The change can be done by either using the inspector or calling a property's setter.
ExtendRealityLtd · Mar 4, 2019 · d0b0120 · d0b0120
1 parent dfc8628
commit d0b0120
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 12 deletions.
diff --git a/Sources/FodyRunner.UnityIntegration/InspectorEditor.cs b/Sources/FodyRunner.UnityIntegration/InspectorEditor.cs
@@ -32,19 +32,21 @@ public override void OnInspectorGUI()
                 do
                 {
                     string propertyPath = property.propertyPath;
+                    Object targetObject = property.serializedObject.targetObject;
 
                     using (EditorGUI.ChangeCheckScope changeCheckScope = new EditorGUI.ChangeCheckScope())
                     using (new EditorGUI.DisabledGroupScope(propertyPath == "m_Script"))
                     {
                         DrawProperty(property);
 
-                        if (!changeCheckScope.changed || !Application.isPlaying)
+                        if (!changeCheckScope.changed
+                            || !Application.isPlaying
+                            || targetObject is Behaviour behaviour && !behaviour.isActiveAndEnabled)
                         {
                             continue;
                         }
                     }
 
-                    Object targetObject = property.serializedObject.targetObject;
                     List<MethodInfo> methodInfos = targetObject.GetType()
                         .GetMethods(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance)
                         .Where(

diff --git a/Sources/MemberChangeMethod.Fody/ModuleWeaver.cs b/Sources/MemberChangeMethod.Fody/ModuleWeaver.cs
@@ -12,9 +12,12 @@
     // ReSharper disable once UnusedMember.Global
     public sealed class ModuleWeaver : BaseModuleWeaver
     {
-        private static readonly string _fullAttributeName = typeof(HandlesMemberChangeAttribute).FullName;
+        private static readonly string _fullBaseAttributeName = typeof(HandlesMemberChangeAttribute).FullName;
+        private static readonly string _fullBeforeChangeAttributeName = typeof(CalledBeforeChangeOfAttribute).FullName;
 
         private MethodReference _isApplicationPlayingGetterReference;
+        private MethodReference _isActiveAndEnabledGetterReference;
+        private TypeReference _behaviourReference;
         private bool _isCompilingForEditor;
 
         public override bool ShouldCleanReference =>
@@ -56,16 +59,19 @@ public override IEnumerable<string> GetAssembliesForScanning()
 
         private void FindReferences()
         {
-            _isApplicationPlayingGetterReference = ModuleDefinition.ImportReference(
-                FindType("UnityEngine.Application")
-                    .Properties.Single(definition => definition.Name == "isPlaying")
-                    .GetMethod);
+            MethodReference ImportPropertyGetter(string typeName, string propertyName) =>
+                ModuleDefinition.ImportReference(
+                    FindType(typeName).Properties.Single(definition => definition.Name == propertyName).GetMethod);
+
+            _isApplicationPlayingGetterReference = ImportPropertyGetter("UnityEngine.Application", "isPlaying");
+            _isActiveAndEnabledGetterReference = ImportPropertyGetter("UnityEngine.Behaviour", "isActiveAndEnabled");
+            _behaviourReference = ModuleDefinition.ImportReference(FindType("UnityEngine.Behaviour"));
             _isCompilingForEditor = DefineConstants.Contains("UNITY_EDITOR");
         }
 
         private static IEnumerable<CustomAttribute> FindAttributes(ICustomAttributeProvider methodDefinition) =>
             methodDefinition.CustomAttributes.Where(
-                    attribute => attribute.AttributeType.Resolve().BaseType.FullName == _fullAttributeName)
+                    attribute => attribute.AttributeType.Resolve().BaseType.FullName == _fullBaseAttributeName)
                 .ToList();
 
         private bool FindProperty(
@@ -128,7 +134,7 @@ private void InsertSetMethodCallIntoPropertySetter(
             Instruction targetInstruction;
             int instructionIndex;
             bool needsPlayingCheck = _isCompilingForEditor;
-            if (attribute.AttributeType.FullName == typeof(CalledBeforeChangeOfAttribute).FullName)
+            if (attribute.AttributeType.FullName == _fullBeforeChangeAttributeName)
             {
                 targetInstruction = storeInstruction.Previous.Previous;
                 instructionIndex = instructions.IndexOf(targetInstruction) - 1;
@@ -163,7 +169,7 @@ private void InsertSetMethodCallIntoPropertySetter(
                     && (testInstruction.Operand as MethodReference)?.FullName
                     == _isApplicationPlayingGetterReference.FullName)
                 {
-                    instructionIndex = instructions.IndexOf(testInstruction) + 1;
+                    instructionIndex = instructions.IndexOf(testInstruction) + 4;
                     needsPlayingCheck = false;
                 }
                 else
@@ -172,16 +178,36 @@ private void InsertSetMethodCallIntoPropertySetter(
                 }
             }
 
-            // if (Application.isPlaying) { this.Method(); }
+            /*
+             if (Application.isPlaying)       // Only if compiling for Editor
+             {
+                 if (this.isActiveAndEnabled) // Only if in a Behaviour
+                 {
+                     this.Method();
+                 }
+             }
+             */
 
             if (needsPlayingCheck)
             {
                 // Call Application.isPlaying getter
                 instructions.Insert(
                     ++instructionIndex,
                     Instruction.Create(OpCodes.Call, _isApplicationPlayingGetterReference));
-                // Don't call the method if false
+                // Bail out if false
                 instructions.Insert(++instructionIndex, Instruction.Create(OpCodes.Brfalse, targetInstruction));
+
+                if (methodReference.DeclaringType.Resolve().IsSubclassOf(_behaviourReference))
+                {
+                    // Load this (for getter call)
+                    instructions.Insert(++instructionIndex, Instruction.Create(OpCodes.Ldarg_0));
+                    // Call Behaviour.isActiveAndEnabled getter
+                    instructions.Insert(
+                        ++instructionIndex,
+                        Instruction.Create(OpCodes.Call, _isActiveAndEnabledGetterReference));
+                    // Bail out if false
+                    instructions.Insert(++instructionIndex, Instruction.Create(OpCodes.Brfalse, targetInstruction));
+                }
             }
 
             // Load this (for method call)