fix: fixing generic syncvar hooks

Assets/Mirage/Weaver/Processors/SyncVarProcessor.cs

@@ -420,25 +420,38 @@ void WriteEndFunctionCall()
 
         void WriteCallHookEvent(ILProcessor worker, EventDefinition @event, VariableDefinition oldValue, FoundSyncVar syncVarField)
         {
-            FieldReference eventField = @event.DeclaringType.GetField(@event.Name);
-            Instruction nop = worker.Create(OpCodes.Nop);
+            // get backing field for event, and sure it is generic instance (eg MyType<T>.myEvent
+            FieldReference eventField = @event.DeclaringType.GetField(@event.Name).MakeHostGenericIfNeeded();
+
+            // get Invoke method and make it correct type
+            MethodReference invokeNonGeneric = @event.Module.ImportReference(typeof(Action<,>).GetMethod("Invoke"));
+            MethodReference invoke = invokeNonGeneric.MakeHostInstanceGeneric((GenericInstanceType)@event.EventType);
+
+            Instruction nopEvent = worker.Create(OpCodes.Nop);
+            Instruction nopEnd = worker.Create(OpCodes.Nop);
+
+            // **null check**
             worker.Append(worker.Create(OpCodes.Ldarg_0));
             worker.Append(worker.Create(OpCodes.Ldfld, eventField));
+            // dup so we dont need to load field twice
+            worker.Append(worker.Create(OpCodes.Dup));
+
             // jump to nop if null
-            worker.Append(worker.Create(OpCodes.Brfalse, nop));
+            worker.Append(worker.Create(OpCodes.Brtrue, nopEvent));
+            // pop because we didn't use field on if it was null
+            worker.Append(worker.Create(OpCodes.Pop));
+            worker.Append(worker.Create(OpCodes.Br, nopEnd));
+
+            // **call invoke**
+            worker.Append(nopEvent);
 
-            worker.Append(worker.Create(OpCodes.Ldarg_0));
-            worker.Append(worker.Create(OpCodes.Ldfld, eventField));
             WriteOldValue();
             WriteNewValue();
 
-            MethodReference invokeNonGeneric = @event.Module.ImportReference(typeof(Action<,>).GetMethod("Invoke"));
-            MethodReference invoke = invokeNonGeneric.MakeHostInstanceGeneric((GenericInstanceType)@event.EventType);
             worker.Append(worker.Create(OpCodes.Call, invoke));
 
             // after if (event!=null)
-            worker.Append(nop);
-
+            worker.Append(nopEnd);
 
 
             // *** Local functions used to write OpCodes ***

Assets/Tests/Runtime/ClientServer/Generics/WithGenericSyncVarEvent.cs

@@ -0,0 +1,74 @@
+using System;
+using System.Collections;
+using NSubstitute;
+using NUnit.Framework;
+using UnityEngine.TestTools;
+
+namespace Mirage.Tests.Runtime.ClientServer.Generics
+{
+    public class WithGenericSyncVarEvent_behaviour<T> : NetworkBehaviour
+    {
+        public event Action<T, T> hook;
+
+        [SyncVar(hook = nameof(hook))]
+        public T value;
+    }
+
+    public class WithGenericSyncVarEvent_behaviourInt : WithGenericSyncVarEvent_behaviour<int>
+    {
+    }
+    public class WithGenericSyncVarEvent_behaviourObject : WithGenericSyncVarEvent_behaviour<MyClass>
+    {
+    }
+
+    public class WithGenericSyncVarEventInt : ClientServerSetup<WithGenericSyncVarEvent_behaviourInt>
+    {
+        [Test]
+        public void DoesNotError()
+        {
+            // passes setup without errors
+            Assert.Pass();
+        }
+
+        [UnityTest]
+        public IEnumerator HookEventCalled()
+        {
+            const int num = 32;
+            Action<int, int> hook = Substitute.For<Action<int, int>>();
+            clientComponent.hook += hook;
+            serverComponent.value = num;
+
+            yield return null;
+            yield return null;
+
+            hook.Received(1).Invoke(0, num);
+        }
+    }
+    public class WithGenericSyncVarEventObject : ClientServerSetup<WithGenericSyncVarEvent_behaviourObject>
+    {
+        [Test]
+        public void DoesNotError()
+        {
+            // passes setup without errors
+            Assert.Pass();
+        }
+
+        [UnityTest]
+        public IEnumerator HookEventCalled()
+        {
+            const int num = 32;
+            Action<MyClass, MyClass> hook = Substitute.For<Action<MyClass, MyClass>>();
+
+            clientComponent.hook += hook;
+            serverComponent.value = new MyClass { Value = num };
+
+            yield return null;
+            yield return null;
+
+            hook.Received(1).Invoke(
+                Arg.Is<MyClass>(x => x == null),
+                Arg.Is<MyClass>(x => x != null && x.Value == num)
+            );
+        }
+    }
+}

Assets/Tests/Runtime/ClientServer/Generics/WithGenericSyncVarHook.cs

@@ -11,17 +11,12 @@ public class WithGenericSyncVarHook_behaviour<T> : NetworkBehaviour
         public event Action<T, T> hookCalled;
 
         [SyncVar(hook = nameof(onValueChanged))]
-        public T valueWithHook;
+        public T value;
 
         void onValueChanged(T oldValue, T newValue)
         {
-            hookCalled.Invoke(oldValue, newValue);
+            hookCalled?.Invoke(oldValue, newValue);
         }
-
-        public event Action<T, T> eventHook;
-
-        [SyncVar(hook = nameof(eventHook))]
-        public T valueWithEvent;
     }
 
     public class WithGenericSyncVarHook_behaviourInt : WithGenericSyncVarHook_behaviour<int>
@@ -46,28 +41,15 @@ public IEnumerator HookMethodCalled()
             const int num = 32;
             Action<int, int> hook = Substitute.For<Action<int, int>>();
             clientComponent.hookCalled += hook;
-            serverComponent.valueWithHook = num;
-
-            yield return null;
-            yield return null;
-
-            hook.Received(1).Invoke(default, num);
-        }
-
-        [UnityTest]
-        public IEnumerator HookEventCalled()
-        {
-            const int num = 32;
-            Action<int, int> hook = Substitute.For<Action<int, int>>();
-            clientComponent.eventHook += hook;
-            serverComponent.valueWithEvent = num;
+            serverComponent.value = num;
 
             yield return null;
             yield return null;
 
-            hook.Received(1).Invoke(default, num);
+            hook.Received(1).Invoke(0, num);
         }
     }
+
     public class WithGenericSyncVarHookObject : ClientServerSetup<WithGenericSyncVarHook_behaviourObject>
     {
         [Test]
@@ -83,27 +65,15 @@ public IEnumerator HookMethodCalled()
             const int num = 32;
             Action<MyClass, MyClass> hook = Substitute.For<Action<MyClass, MyClass>>();
             clientComponent.hookCalled += hook;
-            serverComponent.valueWithHook = new MyClass { Value = num };
-
-            yield return null;
-            yield return null;
-
-            hook.Received(1).Invoke(default, Arg.Is<MyClass>(x => x != null && x.Value == num));
-        }
-
-        [UnityTest]
-        public IEnumerator HookEventCalled()
-        {
-            const int num = 32;
-            Action<MyClass, MyClass> hook = Substitute.For<Action<MyClass, MyClass>>();
-
-            clientComponent.eventHook += hook;
-            serverComponent.valueWithEvent = new MyClass { Value = num };
+            serverComponent.value = new MyClass { Value = num };
 
             yield return null;
             yield return null;
 
-            hook.Received(1).Invoke(default, Arg.Is<MyClass>(x => x != null && x.Value == num));
+            hook.Received(1).Invoke(
+                Arg.Is<MyClass>(x => x == null),
+                Arg.Is<MyClass>(x => x != null && x.Value == num)
+            );
         }
     }
 }

Assets/Tests/Runtime/ClientServer/Generics/WithGenericSyncVarHookBig.cs

@@ -0,0 +1,133 @@
+using System;
+using System.Collections;
+using Mirage.Serialization;
+using NSubstitute;
+using NUnit.Framework;
+using UnityEngine.TestTools;
+
+namespace Mirage.Tests.Runtime.ClientServer.Generics
+{
+    public class WithGenericSyncVarBig_behaviour<T> : NetworkBehaviour
+    {
+        public event Action<T, T> hookMethod;
+
+        [SyncVar(hook = nameof(onValueChanged))]
+        public T value1;
+
+        void onValueChanged(T oldValue, T newValue)
+        {
+            hookMethod?.Invoke(oldValue, newValue);
+        }
+
+        public event Action<T, T> hookEvent;
+
+        [SyncVar(hook = nameof(hookEvent))]
+        public T value2;
+        [SyncVar]
+        public int value3;
+        [SyncVar]
+        public T value4;
+
+        public T valueNotVar;
+    }
+
+    public class WithGenericSyncVarBig_behaviourInt : WithGenericSyncVarBig_behaviour<int>
+    {
+    }
+    public class WithGenericSyncVarBig_behaviourObject : WithGenericSyncVarBig_behaviour<MyClass>
+    {
+    }
+
+    public class WithGenericSyncVarBigInt : ClientServerSetup<WithGenericSyncVarBig_behaviourInt>
+    {
+        [Test]
+        public void DoesNotError()
+        {
+            // passes setup without errors
+            Assert.Pass();
+        }
+
+        [UnityTest]
+        public IEnumerator HookEventCalled()
+        {
+            const int num1 = 32;
+            const int num2 = 132;
+            const int num3 = 232;
+            const int num4 = 332;
+            Action<int, int> hook1 = Substitute.For<Action<int, int>>();
+            Action<int, int> hook2 = Substitute.For<Action<int, int>>();
+            clientComponent.hookMethod += hook1;
+            clientComponent.hookEvent += hook2;
+
+            serverComponent.value1 = num1;
+            serverComponent.value2 = num2;
+            serverComponent.value3 = num3;
+            serverComponent.value4 = num4;
+
+
+            var writer = new NetworkWriter(500);
+            serverComponent.SerializeSyncVars(writer, false);
+            var reader = new NetworkReader();
+            reader.Reset(writer.ToArraySegment());
+
+            clientComponent.DeserializeSyncVars(reader, false);
+
+            hook1.Received(1).Invoke(0, num1);
+            //hook2.Received(1).Invoke(0, num2);
+            Assert.That(clientComponent.value1, Is.EqualTo(num1));
+            Assert.That(clientComponent.value2, Is.EqualTo(num2));
+            Assert.That(clientComponent.value3, Is.EqualTo(num3));
+            Assert.That(clientComponent.value4, Is.EqualTo(num4));
+
+            yield return null;
+            yield return null;
+        }
+    }
+    public class WithGenericSyncVarBigObject : ClientServerSetup<WithGenericSyncVarBig_behaviourObject>
+    {
+        [Test]
+        public void DoesNotError()
+        {
+            // passes setup without errors
+            Assert.Pass();
+        }
+
+        [UnityTest]
+        public IEnumerator HookEventCalled()
+        {
+            const int num1 = 32;
+            const int num2 = 132;
+            const int num3 = 232;
+            const int num4 = 332;
+            Action<MyClass, MyClass> hook1 = Substitute.For<Action<MyClass, MyClass>>();
+            Action<MyClass, MyClass> hook2 = Substitute.For<Action<MyClass, MyClass>>();
+            clientComponent.hookMethod += hook1;
+            clientComponent.hookEvent += hook2;
+
+            serverComponent.value1 = new MyClass { Value = num1 };
+            serverComponent.value2 = new MyClass { Value = num2 };
+            serverComponent.value3 = num3;
+            serverComponent.value4 = new MyClass { Value = num4 };
+
+            yield return null;
+            yield return null;
+
+            hook1.Received(1).Invoke(
+                Arg.Is<MyClass>(x => x == null),
+                Arg.Is<MyClass>(x => x != null && x.Value == num1)
+            );
+            hook2.Received(1).Invoke(
+                Arg.Is<MyClass>(x => x == null),
+                Arg.Is<MyClass>(x => x != null && x.Value == num2)
+            );
+            Assert.That(clientComponent.value1, Is.Not.Null);
+            Assert.That(clientComponent.value2, Is.Not.Null);
+            Assert.That(clientComponent.value3, Is.Not.Null);
+            Assert.That(clientComponent.value4, Is.Not.Null);
+            Assert.That(clientComponent.value1.Value, Is.EqualTo(num1));
+            Assert.That(clientComponent.value2.Value, Is.EqualTo(num2));
+            Assert.That(clientComponent.value3, Is.EqualTo(num3));
+            Assert.That(clientComponent.value4.Value, Is.EqualTo(num4));
+        }
+    }
+}