feat: Add excludeOwner option to ClientRpc (#1954)

* WIP * WIP * WIP * WIP * feat: Add excludeOwner option to ClientRpc * removed default value * Fixed tests * Update Assets/Mirror/Editor/Weaver/Processors/RpcProcessor.cs Co-authored-by: James Frowen <jamesfrowendev@gmail.com> * adding weaver test for exlude owner * adding test for call to exclude owner * adding comment back * removing ClientRpcInfo we don't need ClientRpcInfo as client rpc is only called server side so we don't need to double check exlucde owner, weaver can handle it * removing extra register * fixing test compile errors * Update Assets/Mirror/Tests/Editor/Weaver/WeaverClientRpcTests~/ClientRpcThatExcludesOwner.cs * doc: fixed typos in readme * doc: Updated ChangeLog * doc: Updated docs for Command ignoreAuthority option * doc: Updated ChangeLog * fixing typo in comment (#1963) * Removed YouTube link. Link is changing soon. (#1962) * Removed YouTube link. Link is changing soon. (#1961) * Removed YouTube link. Link is changing soon. (#1960) * Removed YouTube link. Link is changing soon. (#1959) * Removed YouTube link. Links are changing. (#1958) * Removed YouTube link. Links are changing. (#1957) * Update NetworkBehavior.md (#1956) * fixed comment typo * doc: Updated Migration guide example Co-authored-by: James Frowen <jamesfrowendev@gmail.com> Co-authored-by: FirstGearGames <firstgeargames@gmail.com>
MirageNet · Jun 5, 2020 · 864fdd5 · 864fdd5
1 parent b4d0296
commit 864fdd5
Show file tree

Hide file tree

Showing 10 changed files with 108 additions and 14 deletions.
diff --git a/Assets/Mirror/Editor/Weaver/Processors/NetworkBehaviourProcessor.cs b/Assets/Mirror/Editor/Weaver/Processors/NetworkBehaviourProcessor.cs
@@ -21,7 +21,7 @@ class NetworkBehaviourProcessor
         // <SyncVarField,NetIdField>
         readonly Dictionary<FieldDefinition, FieldDefinition> syncVarNetIds = new Dictionary<FieldDefinition, FieldDefinition>();
         readonly List<CmdResult> commands = new List<CmdResult>();
-        readonly List<MethodDefinition> clientRpcs = new List<MethodDefinition>();
+        readonly List<ClientRpcResult> clientRpcs = new List<ClientRpcResult>();
         readonly List<MethodDefinition> targetRpcs = new List<MethodDefinition>();
         readonly List<EventDefinition> eventRpcs = new List<EventDefinition>();
         readonly List<MethodDefinition> commandInvocationFuncs = new List<MethodDefinition>();
@@ -37,6 +37,12 @@ public struct CmdResult
             public bool ignoreAuthority;
         }
 
+        public struct ClientRpcResult
+        {
+            public MethodDefinition method;
+            public bool excludeOwner;
+        }
+
         public NetworkBehaviourProcessor(TypeDefinition td)
         {
             Weaver.DLog(td, "NetworkBehaviourProcessor");
@@ -259,7 +265,8 @@ void GenerateConstants()
 
             for (int i = 0; i < clientRpcs.Count; ++i)
             {
-                GenerateRegisterRemoteDelegate(cctorWorker, Weaver.registerRpcDelegateReference, clientRpcInvocationFuncs[i], clientRpcs[i].Name);
+                ClientRpcResult clientRpcResult = clientRpcs[i];
+                GenerateRegisterRemoteDelegate(cctorWorker, Weaver.registerRpcDelegateReference, clientRpcInvocationFuncs[i], clientRpcResult.method.Name);
             }
 
             for (int i = 0; i < targetRpcs.Count; ++i)
@@ -939,8 +946,15 @@ void ProcessClientRpc(HashSet<string> names, MethodDefinition md, CustomAttribut
                 Weaver.Error($"Duplicate ClientRpc name {md.Name}", md);
                 return;
             }
+
+            bool excludeOwner = clientRpcAttr.GetField("excludeOwner", false);
+
             names.Add(md.Name);
-            clientRpcs.Add(md);
+            clientRpcs.Add(new ClientRpcResult
+            {
+                method = md,
+                excludeOwner = excludeOwner
+            });
 
             MethodDefinition rpcCallFunc = RpcProcessor.ProcessRpcCall(netBehaviourSubclass, md, clientRpcAttr);
 

diff --git a/Assets/Mirror/Editor/Weaver/Processors/RpcProcessor.cs b/Assets/Mirror/Editor/Weaver/Processors/RpcProcessor.cs
@@ -68,6 +68,12 @@ public static MethodDefinition ProcessRpcCall(TypeDefinition td, MethodDefinitio
 
             NetworkBehaviourProcessor.WriteSetupLocals(worker);
 
+            if (Weaver.GenerateLogErrors)
+            {
+                worker.Append(worker.Create(OpCodes.Ldstr, "Call ClientRpc function " + md.Name));
+                worker.Append(worker.Create(OpCodes.Call, Weaver.logErrorReference));
+            }
+
             NetworkBehaviourProcessor.WriteCreateWriter(worker);
 
             // write all the arguments that the user passed to the Rpc call
@@ -81,6 +87,9 @@ public static MethodDefinition ProcessRpcCall(TypeDefinition td, MethodDefinitio
                 rpcName = rpcName.Substring(RpcPrefix.Length);
             }
 
+            int channel = clientRpcAttr.GetField("channel", 0);
+            bool excludeOwner = clientRpcAttr.GetField("excludeOwner", false);
+
             // invoke SendInternal and return
             // this
             worker.Append(worker.Create(OpCodes.Ldarg_0));
@@ -90,7 +99,8 @@ public static MethodDefinition ProcessRpcCall(TypeDefinition td, MethodDefinitio
             worker.Append(worker.Create(OpCodes.Ldstr, rpcName));
             // writer
             worker.Append(worker.Create(OpCodes.Ldloc_0));
-            worker.Append(worker.Create(OpCodes.Ldc_I4, clientRpcAttr.GetField("channel", 0)));
+            worker.Append(worker.Create(OpCodes.Ldc_I4, channel));
+            worker.Append(worker.Create(excludeOwner ? OpCodes.Ldc_I4_1 : OpCodes.Ldc_I4_0));
             worker.Append(worker.Create(OpCodes.Callvirt, Weaver.sendRpcInternal));
 
             NetworkBehaviourProcessor.WriteRecycleWriter(worker);

diff --git a/Assets/Mirror/Runtime/CustomAttributes.cs b/Assets/Mirror/Runtime/CustomAttributes.cs
@@ -33,6 +33,7 @@ public class ClientRpcAttribute : Attribute
     {
         // this is zero
         public int channel = Channels.DefaultReliable;
+        public bool excludeOwner = false;
     }
 
     /// <summary>

diff --git a/Assets/Mirror/Runtime/NetworkBehaviour.cs b/Assets/Mirror/Runtime/NetworkBehaviour.cs
@@ -232,7 +232,7 @@ public virtual bool InvokeCommand(int cmdHash, NetworkReader reader)
 
         #region Client RPCs
         [EditorBrowsable(EditorBrowsableState.Never)]
-        protected void SendRPCInternal(Type invokeClass, string rpcName, NetworkWriter writer, int channelId)
+        protected void SendRPCInternal(Type invokeClass, string rpcName, NetworkWriter writer, int channelId, bool excludeOwner)
         {
             // this was in Weaver before
             if (!NetworkServer.active)
@@ -258,7 +258,10 @@ protected void SendRPCInternal(Type invokeClass, string rpcName, NetworkWriter w
                 payload = writer.ToArraySegment()
             };
 
-            NetworkServer.SendToReady(netIdentity, message, channelId);
+            // The public facing parameter is excludeOwner in [ClientRpc]
+            // so we negate it here to logically align with SendToReady.
+            bool includeOwner = !excludeOwner;
+            NetworkServer.SendToReady(netIdentity, message, includeOwner, channelId);
         }
 
         [EditorBrowsable(EditorBrowsableState.Never)]

diff --git a/Assets/Mirror/Tests/Editor/ClientRpcTest.cs b/Assets/Mirror/Tests/Editor/ClientRpcTest.cs
@@ -14,6 +14,17 @@ public void RpcSendInt(int someInt)
         }
     }
 
+    class ExcludeOwnerBehaviour : NetworkBehaviour
+    {
+        public event Action<int> onSendInt;
+
+        [ClientRpc(excludeOwner = true)]
+        public void RpcSendInt(int someInt)
+        {
+            onSendInt?.Invoke(someInt);
+        }
+    }
+
     public class ClientRpcTest : RemoteTestBase
     {
         [Test]
@@ -33,5 +44,43 @@ public void RpcIsCalled()
             ProcessMessages();
             Assert.That(callCount, Is.EqualTo(1));
         }
+
+        [Test]
+        public void RpcIsCalledForNotOwnerd()
+        {
+            bool owner = false;
+            ExcludeOwnerBehaviour hostBehaviour = CreateHostObject<ExcludeOwnerBehaviour>(owner);
+
+            const int someInt = 20;
+
+            int callCount = 0;
+            hostBehaviour.onSendInt += incomingInt =>
+            {
+                callCount++;
+                Assert.That(incomingInt, Is.EqualTo(someInt));
+            };
+            hostBehaviour.RpcSendInt(someInt);
+            ProcessMessages();
+            Assert.That(callCount, Is.EqualTo(1));
+        }
+
+        [Test]
+        public void RpcNotCalledForOwnerd()
+        {
+            bool owner = true;
+            ExcludeOwnerBehaviour hostBehaviour = CreateHostObject<ExcludeOwnerBehaviour>(owner);
+
+            const int someInt = 20;
+
+            int callCount = 0;
+            hostBehaviour.onSendInt += incomingInt =>
+            {
+                callCount++;
+                Assert.That(incomingInt, Is.EqualTo(someInt));
+            };
+            hostBehaviour.RpcSendInt(someInt);
+            ProcessMessages();
+            Assert.That(callCount, Is.EqualTo(0));
+        }
     }
 }
diff --git a/Assets/Mirror/Tests/Editor/NetworkBehaviourTests.cs b/Assets/Mirror/Tests/Editor/NetworkBehaviourTests.cs
@@ -4,9 +4,7 @@
 
 namespace Mirror.Tests
 {
-    class EmptyBehaviour : NetworkBehaviour
-    {
-    }
+    class EmptyBehaviour : NetworkBehaviour { }
 
     class SyncVarGameObjectEqualExposedBehaviour : NetworkBehaviour
     {
@@ -50,7 +48,7 @@ public class NetworkBehaviourSendRPCInternalComponent : NetworkBehaviour
         // counter to make sure that it's called exactly once
         public int called;
 
-        // weaver generates this from [Command]
+        // weaver generates this from [ClientRpc]
         // but for tests we need to add it manually
         public static void RPCGenerated(NetworkBehaviour comp, NetworkReader reader)
         {
@@ -60,7 +58,7 @@ public static void RPCGenerated(NetworkBehaviour comp, NetworkReader reader)
         // SendCommandInternal is protected. let's expose it so we can test it.
         public void CallSendRPCInternal()
         {
-            SendRPCInternal(GetType(), nameof(RPCGenerated), new NetworkWriter(), 0);
+            SendRPCInternal(GetType(), nameof(RPCGenerated), new NetworkWriter(), 0, false);
         }
     }
 
@@ -70,7 +68,7 @@ public class NetworkBehaviourSendTargetRPCInternalComponent : NetworkBehaviour
         // counter to make sure that it's called exactly once
         public int called;
 
-        // weaver generates this from [Command]
+        // weaver generates this from [TargetRpc]
         // but for tests we need to add it manually
         public static void TargetRPCGenerated(NetworkBehaviour comp, NetworkReader reader)
         {
@@ -90,7 +88,7 @@ public class NetworkBehaviourSendEventInternalComponent : NetworkBehaviour
         // counter to make sure that it's called exactly once
         public int called;
 
-        // weaver generates this from [Command]
+        // weaver generates this from [SyncEvent]
         // but for tests we need to add it manually
         public static void EventGenerated(NetworkBehaviour comp, NetworkReader reader)
         {

diff --git a/Assets/Mirror/Tests/Editor/Weaver/.WeaverTests.csproj b/Assets/Mirror/Tests/Editor/Weaver/.WeaverTests.csproj
@@ -83,6 +83,7 @@
     <Compile Include="WeaverClientRpcTests~\AbstractClientRpc.cs" />
     <Compile Include="WeaverClientRpcTests~\ClientRpcCantBeStatic.cs" />
     <Compile Include="WeaverClientRpcTests~\ClientRpcStartsWithRpc.cs" />
+    <Compile Include="WeaverClientRpcTests~\ClientRpcThatExcludesOwner.cs" />
     <Compile Include="WeaverClientRpcTests~\ClientRpcValid.cs" />
     <Compile Include="WeaverClientRpcTests~\OverrideAbstractClientRpc.cs" />
     <Compile Include="WeaverClientRpcTests~\OverrideVirtualClientRpc.cs" />

diff --git a/Assets/Mirror/Tests/Editor/Weaver/WeaverClientRpcTests.cs b/Assets/Mirror/Tests/Editor/Weaver/WeaverClientRpcTests.cs
@@ -45,5 +45,11 @@ public void OverrideAbstractClientRpc()
         {
             Assert.That(weaverErrors, Contains.Item("Abstract ClientRpc are currently not supported, use virtual method instead (at System.Void WeaverClientRpcTests.OverrideAbstractClientRpc.BaseBehaviour::RpcDoSomething())"));
         }
+
+        [Test]
+        public void ClientRpcThatExcludesOwner()
+        {
+            Assert.That(weaverErrors, Is.Empty);
+        }
     }
 }
diff --git a/Assets/Mirror/Tests/Editor/Weaver/WeaverClientRpcTests~/AbstractClientRpc.cs b/Assets/Mirror/Tests/Editor/Weaver/WeaverClientRpcTests~/AbstractClientRpc.cs
@@ -1,6 +1,5 @@
 using Mirror;
 
-
 namespace WeaverClientRpcTests.AbstractClientRpc
 {
     abstract class AbstractClientRpc : NetworkBehaviour

diff --git a/Assets/Mirror/Tests/Editor/Weaver/WeaverClientRpcTests~/ClientRpcThatExcludesOwner.cs b/Assets/Mirror/Tests/Editor/Weaver/WeaverClientRpcTests~/ClientRpcThatExcludesOwner.cs
@@ -0,0 +1,13 @@
+using Mirror;
+
+namespace WeaverClientRpcTests.ClientRpcThatExcludesOwner
+{
+    class ClientRpcThatExcludesOwner : NetworkBehaviour
+    {
+        [ClientRpc(excludeOwner = true)]
+        void RpcDoSomething()
+        {
+            // do something
+        }
+    }
+}