fix: better error for Command, ClientRpc and TargetRpc marked as abst…

…ract (#1947)

* weaver test for virtual and abstract commands

* adding error for abstract methods

* adding base class for command tests and renaming file

* adding tests for virutal commands

* renaming classes to make more sense

* removing whitespace

* adding tests for Client Target Rpc too

* fixing compile error

* fixing message and typo

* removing line

* renaming namespace and base class to be used by Rpc

* tests for calling Rpc

* tests for virtual rpc

* fixing typo

Assets/Mirror/Editor/Weaver/Processors/NetworkBehaviourProcessor.cs

@@ -924,6 +924,12 @@ void ProcessMethods()
 
         void ProcessClientRpc(HashSet<string> names, MethodDefinition md, CustomAttribute clientRpcAttr)
         {
+            if (md.IsAbstract)
+            {
+                Weaver.Error("Abstract ClientRpc are currently not supported, use virtual method instead", md);
+                return;
+            }
+
             if (!RpcProcessor.ProcessMethodsValidateRpc(md))
             {
                 return;
@@ -948,6 +954,12 @@ void ProcessClientRpc(HashSet<string> names, MethodDefinition md, CustomAttribut
 
         void ProcessTargetRpc(HashSet<string> names, MethodDefinition md, CustomAttribute targetRpcAttr)
         {
+            if (md.IsAbstract)
+            {
+                Weaver.Error("Abstract TargetRpc are currently not supported, use virtual method instead", md);
+                return;
+            }
+
             if (!TargetRpcProcessor.ProcessMethodsValidateTargetRpc(md))
                 return;
 
@@ -970,6 +982,12 @@ void ProcessTargetRpc(HashSet<string> names, MethodDefinition md, CustomAttribut
 
         void ProcessCommand(HashSet<string> names, MethodDefinition md, CustomAttribute commandAttr)
         {
+            if (md.IsAbstract)
+            {
+                Weaver.Error("Abstract Commands are currently not supported, use virtual method instead", md);
+                return;
+            }
+
             if (!CommandProcessor.ProcessMethodsValidateCommand(md))
                 return;
 

Assets/Mirror/Tests/Editor/ClientRpcOverrideTest.cs

@@ -0,0 +1,75 @@
+using System;
+using NUnit.Framework;
+
+namespace Mirror.Tests.RemoteAttrributeTest
+{
+    class VirtualClientRpc : NetworkBehaviour
+    {
+        public event Action<int> onVirtualSendInt;
+
+        [ClientRpc]
+        public virtual void RpcSendInt(int someInt)
+        {
+            onVirtualSendInt?.Invoke(someInt);
+        }
+    }
+
+    class VirtualOverrideClientRpc : VirtualClientRpc
+    {
+        public event Action<int> onOverrideSendInt;
+
+        [ClientRpc]
+        public override void RpcSendInt(int someInt)
+        {
+            onOverrideSendInt?.Invoke(someInt);
+        }
+    }
+
+    public class ClientRpcOverrideTest : RemoteTestBase
+    {
+        [Test]
+        public void VirtualRpcIsCalled()
+        {
+            VirtualClientRpc hostBehaviour = CreateHostObject<VirtualClientRpc>(true);
+
+            const int someInt = 20;
+
+            int virtualCallCount = 0;
+            hostBehaviour.onVirtualSendInt += incomingInt =>
+            {
+                virtualCallCount++;
+                Assert.That(incomingInt, Is.EqualTo(someInt));
+            };
+
+            hostBehaviour.RpcSendInt(someInt);
+            ProcessMessages();
+            Assert.That(virtualCallCount, Is.EqualTo(1));
+        }
+
+
+        [Test]
+        public void OverrideVirtualRpcIsCalled()
+        {
+            VirtualOverrideClientRpc hostBehaviour = CreateHostObject<VirtualOverrideClientRpc>(true);
+
+            const int someInt = 20;
+
+            int virtualCallCount = 0;
+            int overrideCallCount = 0;
+            hostBehaviour.onVirtualSendInt += incomingInt =>
+            {
+                virtualCallCount++;
+            };
+            hostBehaviour.onOverrideSendInt += incomingInt =>
+            {
+                overrideCallCount++;
+                Assert.That(incomingInt, Is.EqualTo(someInt));
+            };
+
+            hostBehaviour.RpcSendInt(someInt);
+            ProcessMessages();
+            Assert.That(virtualCallCount, Is.EqualTo(0));
+            Assert.That(overrideCallCount, Is.EqualTo(1));
+        }
+    }
+}

Assets/Mirror/Tests/Editor/ClientRpcTest.cs

@@ -0,0 +1,37 @@
+using System;
+using NUnit.Framework;
+
+namespace Mirror.Tests.RemoteAttrributeTest
+{
+    class ClientRpcBehaviour : NetworkBehaviour
+    {
+        public event Action<int> onSendInt;
+
+        [ClientRpc]
+        public void RpcSendInt(int someInt)
+        {
+            onSendInt?.Invoke(someInt);
+        }
+    }
+
+    public class ClientRpcTest : RemoteTestBase
+    {
+        [Test]
+        public void RpcIsCalled()
+        {
+            ClientRpcBehaviour hostBehaviour = CreateHostObject<ClientRpcBehaviour>(true);
+
+            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));
+        }
+    }
+}

Assets/Mirror/Tests/Editor/CommandOverrideTest.cs

@@ -0,0 +1,75 @@
+using System;
+using NUnit.Framework;
+
+namespace Mirror.Tests.RemoteAttrributeTest
+{
+    class VirtualCommand : NetworkBehaviour
+    {
+        public event Action<int> onVirtualSendInt;
+
+        [Command]
+        public virtual void CmdSendInt(int someInt)
+        {
+            onVirtualSendInt?.Invoke(someInt);
+        }
+    }
+
+    class VirtualOverrideCommand : VirtualCommand
+    {
+        public event Action<int> onOverrideSendInt;
+
+        [Command]
+        public override void CmdSendInt(int someInt)
+        {
+            onOverrideSendInt?.Invoke(someInt);
+        }
+    }
+
+    public class CommandOverrideTest : RemoteTestBase
+    {
+        [Test]
+        public void VirtualCommandIsCalled()
+        {
+            VirtualCommand hostBehaviour = CreateHostObject<VirtualCommand>(true);
+
+            const int someInt = 20;
+
+            int virtualCallCount = 0;
+            hostBehaviour.onVirtualSendInt += incomingInt =>
+            {
+                virtualCallCount++;
+                Assert.That(incomingInt, Is.EqualTo(someInt));
+            };
+
+            hostBehaviour.CmdSendInt(someInt);
+            ProcessMessages();
+            Assert.That(virtualCallCount, Is.EqualTo(1));
+        }
+
+
+        [Test]
+        public void OverrideVirtualCommandIsCalled()
+        {
+            VirtualOverrideCommand hostBehaviour = CreateHostObject<VirtualOverrideCommand>(true);
+
+            const int someInt = 20;
+
+            int virtualCallCount = 0;
+            int overrideCallCount = 0;
+            hostBehaviour.onVirtualSendInt += incomingInt =>
+            {
+                virtualCallCount++;
+            };
+            hostBehaviour.onOverrideSendInt += incomingInt =>
+            {
+                overrideCallCount++;
+                Assert.That(incomingInt, Is.EqualTo(someInt));
+            };
+
+            hostBehaviour.CmdSendInt(someInt);
+            ProcessMessages();
+            Assert.That(virtualCallCount, Is.EqualTo(0));
+            Assert.That(overrideCallCount, Is.EqualTo(1));
+        }
+    }
+}

Assets/Mirror/Tests/Editor/CommandAttrributeTest.cs → Assets/Mirror/Tests/Editor/CommandTest.cs

@@ -1,10 +1,9 @@
 using System;
-using System.Collections.Generic;
 using NUnit.Framework;
 using UnityEngine;
 using UnityEngine.TestTools;
 
-namespace Mirror.Tests.CommandAttrributeTest
+namespace Mirror.Tests.RemoteAttrributeTest
 {
     class AuthorityBehaviour : NetworkBehaviour
     {
@@ -28,88 +27,8 @@ public void CmdSendInt(int someInt)
         }
     }
 
-    public class CommandTest
+    public class CommandTest : RemoteTestBase
     {
-        private List<GameObject> spawned = new List<GameObject>();
-
-        [SetUp]
-        public void Setup()
-        {
-            Transport.activeTransport = new GameObject().AddComponent<MemoryTransport>();
-
-            // start server/client
-            NetworkServer.Listen(1);
-            NetworkClient.ConnectHost();
-            NetworkServer.SpawnObjects();
-            NetworkServer.ActivateHostScene();
-            NetworkClient.ConnectLocalServer();
-
-            NetworkServer.localConnection.isAuthenticated = true;
-            NetworkClient.connection.isAuthenticated = true;
-
-            ClientScene.Ready(NetworkClient.connection);
-        }
-
-        [TearDown]
-        public void TearDown()
-        {
-            // stop server/client
-            NetworkClient.DisconnectLocalServer();
-
-            NetworkClient.Disconnect();
-            NetworkClient.Shutdown();
-
-            NetworkServer.Shutdown();
-
-            // destroy left over objects
-            foreach (GameObject item in spawned)
-            {
-                if (item != null)
-                {
-                    GameObject.DestroyImmediate(item);
-                }
-            }
-
-            spawned.Clear();
-
-            NetworkIdentity.spawned.Clear();
-
-            GameObject.DestroyImmediate(Transport.activeTransport.gameObject);
-        }
-
-
-        T CreateHostObject<T>(bool spawnWithAuthority) where T : NetworkBehaviour
-        {
-            GameObject gameObject = new GameObject();
-            spawned.Add(gameObject);
-
-            gameObject.AddComponent<NetworkIdentity>();
-
-            T behaviour = gameObject.AddComponent<T>();
-
-            // spawn outwith authority
-            if (spawnWithAuthority)
-            {
-                NetworkServer.Spawn(gameObject, NetworkServer.localConnection);
-            }
-            else
-            {
-                NetworkServer.Spawn(gameObject);
-            }
-            ProcessMessages();
-
-            Debug.Assert(behaviour.hasAuthority == spawnWithAuthority, $"Behaviour Had Wrong Authority when spawned, This means that the test is broken and will give the wrong results");
-
-            return behaviour;
-        }
-
-        private static void ProcessMessages()
-        {
-            // run update so message are processed
-            NetworkServer.Update();
-            NetworkClient.Update();
-        }
-
         [Test]
         public void CommandIsSentWithAuthority()
         {