diff --git a/Assets/Mirror/Editor/Weaver/Processors/SyncVarAttributeProcessor.cs b/Assets/Mirror/Editor/Weaver/Processors/SyncVarAttributeProcessor.cs index 76e3ac7ed5c..86446b28f6c 100644 --- a/Assets/Mirror/Editor/Weaver/Processors/SyncVarAttributeProcessor.cs +++ b/Assets/Mirror/Editor/Weaver/Processors/SyncVarAttributeProcessor.cs @@ -20,7 +20,7 @@ public class SyncVarAttributeProcessor Logger Log; string HookParameterMessage(string hookName, TypeReference ValueType) => - $"void {hookName}({ValueType} oldValue, {ValueType} newValue)"; + $"void {hookName}({ValueType} oldValue, {ValueType} newValue), void {hookName}({ValueType} oldValue) or void {hookName}()"; public SyncVarAttributeProcessor(AssemblyDefinition assembly, WeaverTypes weaverTypes, SyncVarAccessLists syncVarAccessLists, Logger Log) { @@ -55,12 +55,49 @@ public FieldDefinition CreateNewActionFieldDefinitionFromHookMethod(FieldDefinit return new FieldDefinition(syncVarHookDelegateFieldName, FieldAttributes.Public, syncVarHookActionDelegateType); } + MethodDefinition CreateHookShim(FieldDefinition syncVar, MethodDefinition hookMethod) + { + if (hookMethod.Parameters.Count > 1) + throw new Exception($"Hook {hookMethod.FullName} has {hookMethod.Parameters.Count} params - can only generate shims for 0 or 1 params"); + + MethodAttributes methodAttributes = MethodAttributes.Public | MethodAttributes.HideBySig; + + if (hookMethod.IsStatic) + methodAttributes |= MethodAttributes.Static; + + MethodDefinition shim = new MethodDefinition($"_Mirror_HookShim_{syncVar.Name}_{hookMethod.Name}", methodAttributes, hookMethod.Module.TypeSystem.Void); + + shim.Parameters.Add(new ParameterDefinition("oldValue", ParameterAttributes.None, syncVar.FieldType)); + shim.Parameters.Add(new ParameterDefinition("newValue", ParameterAttributes.None, syncVar.FieldType)); + + syncVar.DeclaringType.Methods.Add(shim); + + ILProcessor il = shim.Body.GetILProcessor(); + + // for instance hooks: load `this` for the call + if (!hookMethod.IsStatic) + il.Emit(OpCodes.Ldarg_0); + + if (hookMethod.Parameters.Count == 1) il.Emit(OpCodes.Ldarg_1); + + // call virtual correctly (so overrides still work) + il.Emit(hookMethod.IsVirtual ? OpCodes.Callvirt : OpCodes.Call, hookMethod); + + il.Emit(OpCodes.Ret); + + return shim; + } + // push hook from GetHookMethod() onto the stack as a new Action. // allows for reuse without handling static/virtual cases every time. // perf warning: it is recommended to use this method only when generating IL to create a new Action() in order to store it into a field // avoid using this to emit IL to instantiate a new action instance every single time one is needed for the same method public void GenerateNewActionFromHookMethod(FieldDefinition syncVar, ILProcessor worker, MethodDefinition hookMethod) { + // If hook has 0 or 1 parameters, use a generated shim that matches Action + if (hookMethod.Parameters.Count < 2) + hookMethod = CreateHookShim(syncVar, hookMethod); + // IL_000a: ldarg.0 // IL_000b: ldftn instance void Mirror.Examples.Tanks.Tank::ExampleHook(int32, int32) // IL_0011: newobj instance void class [netstandard]System.Action`2::.ctor(object, native int) @@ -127,9 +164,14 @@ MethodDefinition FindHookMethod(TypeDefinition td, FieldDefinition syncVar, stri { List methods = td.GetMethods(hookFunctionName); - List methodsWith2Param = new List(methods.Where(m => m.Parameters.Count == 2)); + List methodsWithRightParamCount = new List( + methods + .Where(m => m.Parameters.Count <= 2) + // Prefer methods with more parameters + .OrderByDescending(m => m.Parameters.Count) + ); - if (methodsWith2Param.Count == 0) + if (methodsWithRightParamCount.Count == 0) { Log.Error($"Could not find hook for '{syncVar.Name}', hook name '{hookFunctionName}'. " + $"Method signature should be {HookParameterMessage(hookFunctionName, syncVar.FieldType)}", @@ -139,13 +181,9 @@ MethodDefinition FindHookMethod(TypeDefinition td, FieldDefinition syncVar, stri return null; } - foreach (MethodDefinition method in methodsWith2Param) - { + foreach (MethodDefinition method in methodsWithRightParamCount) if (MatchesParameters(syncVar, method)) - { return method; - } - } Log.Error($"Wrong type for Parameter in hook for '{syncVar.Name}', hook name '{hookFunctionName}'. " + $"Method signature should be {HookParameterMessage(hookFunctionName, syncVar.FieldType)}", @@ -157,9 +195,12 @@ MethodDefinition FindHookMethod(TypeDefinition td, FieldDefinition syncVar, stri bool MatchesParameters(FieldDefinition syncVar, MethodDefinition method) { - // matches void onValueChange(T oldValue, T newValue) - return method.Parameters[0].ParameterType.FullName == syncVar.FieldType.FullName && - method.Parameters[1].ParameterType.FullName == syncVar.FieldType.FullName; + // matches void onValueChange(T oldValue, T newValue), void onValueChange(T oldValue) or void onValueChange() + foreach (var parameter in method.Parameters) + if (parameter.ParameterType.FullName != syncVar.FieldType.FullName) + return false; + + return true; } public MethodDefinition GenerateSyncVarGetter(FieldDefinition fd, string originalName, FieldDefinition netFieldId) diff --git a/Assets/Mirror/Tests/Editor/SyncVars/SyncVarAttributeHookTest.cs b/Assets/Mirror/Tests/Editor/SyncVars/SyncVarAttributeHookTest.cs index 4fef2c9bd2a..ed0b49db5bf 100644 --- a/Assets/Mirror/Tests/Editor/SyncVars/SyncVarAttributeHookTest.cs +++ b/Assets/Mirror/Tests/Editor/SyncVars/SyncVarAttributeHookTest.cs @@ -16,6 +16,56 @@ void OnValueChanged(int oldValue, int newValue) HookCalled.Invoke(oldValue, newValue); } } + class HookBehaviourOneParam : NetworkBehaviour + { + [SyncVar(hook = nameof(OnValueChanged))] + public int value = 0; + + public event Action HookCalled; + + void OnValueChanged(int oldValue) + { + HookCalled.Invoke(oldValue); + } + } + + class HookBehaviourNoParams : NetworkBehaviour + { + [SyncVar(hook = nameof(OnValueChanged))] + public int value = 0; + + public event Action HookCalled; + + void OnValueChanged() + { + HookCalled.Invoke(); + } + } + + class HookBehaviourPrecedence : NetworkBehaviour + { + [SyncVar(hook = nameof(OnValueChanged))] + public int value = 0; + + public event Action HookCalled; + + + void OnValueChanged(int oldValue) + { + HookCalled.Invoke(1); + } + + void OnValueChanged(int oldValue, int newValue) + { + HookCalled.Invoke(2); + } + + void OnValueChanged() + { + HookCalled.Invoke(0); + } + + } class GameObjectHookBehaviour : NetworkBehaviour { @@ -247,6 +297,81 @@ public void Hook_OnlyCalledOnClientd() Assert.That(serverCalled, Is.EqualTo(0)); } + + + [Test] + public void HookOneParam_CalledWhenSyncingChangedValued() + { + CreateNetworkedAndSpawn( + out _, out _, out HookBehaviourOneParam serverObject, + out _, out _, out HookBehaviourOneParam clientObject); + + const int serverValue = 24; + + // change it on server + serverObject.value = serverValue; + + // hook should change it on client + int callCount = 0; + clientObject.HookCalled += (oldValue) => + { + callCount++; + Assert.That(oldValue, Is.EqualTo(0)); + }; + + ProcessMessages(); + Assert.That(callCount, Is.EqualTo(1)); + } + + + [Test] + public void HookNoParams_CalledWhenSyncingChangedValued() + { + CreateNetworkedAndSpawn( + out _, out _, out HookBehaviourNoParams serverObject, + out _, out _, out HookBehaviourNoParams clientObject); + + const int serverValue = 24; + + // change it on server + serverObject.value = serverValue; + + // hook should change it on client + int callCount = 0; + clientObject.HookCalled += () => + { + callCount++; + }; + + ProcessMessages(); + Assert.That(callCount, Is.EqualTo(1)); + } + + [Test] + public void HookPrecedence_CalledWhenSyncingChangedValued() + { + CreateNetworkedAndSpawn( + out _, out _, out HookBehaviourPrecedence serverObject, + out _, out _, out HookBehaviourPrecedence clientObject); + + const int serverValue = 24; + + // change it on server + serverObject.value = serverValue; + + // hook should change it on client + int callCount = 0; + clientObject.HookCalled += (paramCount) => + { + callCount++; + // method with 2 params has the highest precedence + Assert.AreEqual(2, paramCount); + }; + + ProcessMessages(); + Assert.That(callCount, Is.EqualTo(1)); + } + [Test] public void StaticMethod_HookCalledWhenSyncingChangedValued() { diff --git a/Assets/Mirror/Tests/Editor/Weaver/WeaverSyncVarAttributeHookTests.cs b/Assets/Mirror/Tests/Editor/Weaver/WeaverSyncVarAttributeHookTests.cs index 95f3c8a7486..afe7f69bf25 100644 --- a/Assets/Mirror/Tests/Editor/Weaver/WeaverSyncVarAttributeHookTests.cs +++ b/Assets/Mirror/Tests/Editor/Weaver/WeaverSyncVarAttributeHookTests.cs @@ -6,7 +6,7 @@ public class WeaverSyncVarAttributeHookTests : WeaverTestsBuildFromTestName { static string OldNewMethodFormat(string hookName, string ValueType) { - return string.Format("void {0}({1} oldValue, {1} newValue)", hookName, ValueType); + return string.Format("void {0}({1} oldValue, {1} newValue), void {0}({1} oldValue) or void {0}()", hookName, ValueType); } [Test] diff --git a/Assets/Mirror/Tests/Editor/Weaver/WeaverSyncVarAttributeHookTests_IsSuccess/FindsHookWithNoParams.cs b/Assets/Mirror/Tests/Editor/Weaver/WeaverSyncVarAttributeHookTests_IsSuccess/FindsHookWithNoParams.cs new file mode 100644 index 00000000000..06a9d53048d --- /dev/null +++ b/Assets/Mirror/Tests/Editor/Weaver/WeaverSyncVarAttributeHookTests_IsSuccess/FindsHookWithNoParams.cs @@ -0,0 +1,16 @@ +using Mirror; +using UnityEngine; + +namespace WeaverSyncVarHookTests.FindsHookWithOtherOverloadsInOrder +{ + class FindsHookWithNoParams : NetworkBehaviour + { + [SyncVar(hook = nameof(onChangeHealth))] + int health; + + void onChangeHealth() + { + + } + } +} diff --git a/Assets/Mirror/Tests/Editor/Weaver/WeaverSyncVarAttributeHookTests_IsSuccess/FindsHookWithNoParams.cs.meta b/Assets/Mirror/Tests/Editor/Weaver/WeaverSyncVarAttributeHookTests_IsSuccess/FindsHookWithNoParams.cs.meta new file mode 100644 index 00000000000..db6d66fb3c1 --- /dev/null +++ b/Assets/Mirror/Tests/Editor/Weaver/WeaverSyncVarAttributeHookTests_IsSuccess/FindsHookWithNoParams.cs.meta @@ -0,0 +1,3 @@ +fileFormatVersion: 2 +guid: fcdd7591784c4a5e83ea275b155e924a +timeCreated: 1767697570 \ No newline at end of file diff --git a/Assets/Mirror/Tests/Editor/Weaver/WeaverSyncVarAttributeHookTests_IsSuccess/FindsHookWithOneParam.cs b/Assets/Mirror/Tests/Editor/Weaver/WeaverSyncVarAttributeHookTests_IsSuccess/FindsHookWithOneParam.cs new file mode 100644 index 00000000000..bae30c30e53 --- /dev/null +++ b/Assets/Mirror/Tests/Editor/Weaver/WeaverSyncVarAttributeHookTests_IsSuccess/FindsHookWithOneParam.cs @@ -0,0 +1,16 @@ +using Mirror; +using UnityEngine; + +namespace WeaverSyncVarHookTests.FindsHookWithOtherOverloadsInOrder +{ + class FindsHookWithOneParam : NetworkBehaviour + { + [SyncVar(hook = nameof(onChangeHealth))] + int health; + + void onChangeHealth(int oldValue) + { + + } + } +} diff --git a/Assets/Mirror/Tests/Editor/Weaver/WeaverSyncVarAttributeHookTests_IsSuccess/FindsHookWithOneParam.cs.meta b/Assets/Mirror/Tests/Editor/Weaver/WeaverSyncVarAttributeHookTests_IsSuccess/FindsHookWithOneParam.cs.meta new file mode 100644 index 00000000000..d2599fbf2d0 --- /dev/null +++ b/Assets/Mirror/Tests/Editor/Weaver/WeaverSyncVarAttributeHookTests_IsSuccess/FindsHookWithOneParam.cs.meta @@ -0,0 +1,3 @@ +fileFormatVersion: 2 +guid: 8789eff36ef34233989182f291e8727c +timeCreated: 1767697556 \ No newline at end of file diff --git a/Assets/Mirror/Tests/Editor/Weaver/WeaverSyncVarAttributeHookTests_IsSuccess/FindsHookWithTwoParams.cs b/Assets/Mirror/Tests/Editor/Weaver/WeaverSyncVarAttributeHookTests_IsSuccess/FindsHookWithTwoParams.cs new file mode 100644 index 00000000000..95b400d5ec3 --- /dev/null +++ b/Assets/Mirror/Tests/Editor/Weaver/WeaverSyncVarAttributeHookTests_IsSuccess/FindsHookWithTwoParams.cs @@ -0,0 +1,16 @@ +using Mirror; +using UnityEngine; + +namespace WeaverSyncVarHookTests.FindsHookWithOtherOverloadsInOrder +{ + class FindsHookWithTwoParams : NetworkBehaviour + { + [SyncVar(hook = nameof(onChangeHealth))] + int health; + + void onChangeHealth(int oldValue, int newValue) + { + + } + } +} diff --git a/Assets/Mirror/Tests/Editor/Weaver/WeaverSyncVarAttributeHookTests_IsSuccess/FindsHookWithTwoParams.cs.meta b/Assets/Mirror/Tests/Editor/Weaver/WeaverSyncVarAttributeHookTests_IsSuccess/FindsHookWithTwoParams.cs.meta new file mode 100644 index 00000000000..8e8e5ae9a4a --- /dev/null +++ b/Assets/Mirror/Tests/Editor/Weaver/WeaverSyncVarAttributeHookTests_IsSuccess/FindsHookWithTwoParams.cs.meta @@ -0,0 +1,3 @@ +fileFormatVersion: 2 +guid: 4ee680a10b3b406da65a9c63144bce5b +timeCreated: 1767697539 \ No newline at end of file diff --git a/Assets/Mirror/Tests/Editor/Weaver/WeaverSyncVarAttributeHookTests~/ErrorWhenNoHookWithCorrectParametersFound.cs b/Assets/Mirror/Tests/Editor/Weaver/WeaverSyncVarAttributeHookTests~/ErrorWhenNoHookWithCorrectParametersFound.cs index 74592f05339..06e85637b60 100644 --- a/Assets/Mirror/Tests/Editor/Weaver/WeaverSyncVarAttributeHookTests~/ErrorWhenNoHookWithCorrectParametersFound.cs +++ b/Assets/Mirror/Tests/Editor/Weaver/WeaverSyncVarAttributeHookTests~/ErrorWhenNoHookWithCorrectParametersFound.cs @@ -7,12 +7,12 @@ class ErrorWhenNoHookWithCorrectParametersFound : NetworkBehaviour [SyncVar(hook = nameof(onChangeHealth))] int health; - void onChangeHealth(int someOtherValue) + void onChangeHealth(int someOtherValue, int moreValue, bool anotherValue) { } - void onChangeHealth(int someOtherValue, int moreValue, bool anotherValue) + void onChangeHealth(int someOtherValue, int moreValue, int anotherValue, int moreValues) { }