feat: HasAuthority attribute now throws error (#276)

* feat: HasAuthority attribute now throws error

If you want to get an error you do this:
```cs
[HasAuthority]
public void ClientFunctionOnly() {}
```

If you don't want to get an error, you do this:
```cs
[HasAuthority(error = false)]
public void ClientFunctionOnly() {}
```

BREAKING CHANGE: [HasAuthorityCallback] is now [HasAuthority(error = false)]

* fix test

Co-authored-by: Paul Pacheco <paul.pacheco@aa.com>

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

@@ -24,10 +24,7 @@ public static void ProcessMethodAttributes(TypeDefinition td, MethodDefinition m
                         InjectClientGuard(td, md, false);
                         break;
                     case "Mirror.HasAuthorityAttribute":
-                        InjectHasAuthorityGuard(td, md, true);
-                        break;
-                    case "Mirror.HasAuthorityCallbackAttribute":
-                        InjectHasAuthorityGuard(td, md, false);
+                        InjectHasAuthorityGuard(td, md, attr);
                         break;
                     case "Mirror.LocalPlayerAttribute":
                         InjectLocalPlayerGuard(td, md, true);
@@ -91,8 +88,10 @@ static void InjectClientGuard(TypeDefinition td, MethodDefinition md, bool logWa
             worker.InsertBefore(top, worker.Create(OpCodes.Ret));
         }
 
-        static void InjectHasAuthorityGuard(TypeDefinition td, MethodDefinition md, bool logWarning)
+        static void InjectHasAuthorityGuard(TypeDefinition td, MethodDefinition md, CustomAttribute attribute)
         {
+            bool throwError = attribute.GetField<bool>("error", true);
+
             if (!Weaver.IsNetworkBehaviour(td))
             {
                 Weaver.Error($"Has Authority method {md.Name} must be declared in a NetworkBehaviour", md);
@@ -104,10 +103,11 @@ static void InjectHasAuthorityGuard(TypeDefinition td, MethodDefinition md, bool
             worker.InsertBefore(top, worker.Create(OpCodes.Ldarg_0));
             worker.InsertBefore(top, worker.Create(OpCodes.Call, Weaver.NetworkBehaviourHasAuthority));
             worker.InsertBefore(top, worker.Create(OpCodes.Brtrue, top));
-            if (logWarning)
+            if (throwError)
             {
                 worker.InsertBefore(top, worker.Create(OpCodes.Ldstr, "[Has Authority] function '" + md.FullName + "' called on player without authority"));
-                worker.InsertBefore(top, worker.Create(OpCodes.Call, Weaver.logWarningReference));
+                worker.InsertBefore(top, worker.Create(OpCodes.Newobj, Weaver.MethodInvocationExceptionConstructor));
+                worker.InsertBefore(top, worker.Create(OpCodes.Throw));
             }
 
             InjectGuardParameters(md, worker, top);

Assets/Mirror/Runtime/CustomAttributes.cs

@@ -117,17 +117,17 @@ public class ClientCallbackAttribute : Attribute { }
 
     /// <summary>
     /// Prevents players without authority from running this method.
-    /// <para>Prints a warning if a player without authority tries to execute this method.</para>
     /// </summary>
     [AttributeUsage(AttributeTargets.Method)]
-    public class HasAuthorityAttribute : Attribute { }
-
-    /// <summary>
-    /// Prevents players without authority from running this method.
-    /// <para>No warning is printed.</para>
-    /// </summary>
-    [AttributeUsage(AttributeTargets.Method)]
-    public class HasAuthorityCallbackAttribute : Attribute { }
+    public class HasAuthorityAttribute : Attribute 
+    { 
+        /// <summary>
+        /// If true,  when the method is called from a client, it throws an error
+        /// If false, no error is thrown, but the method won't execute
+        /// useful for unity built in methods such as Await, Update, Start, etc.
+        /// </summary>
+        public bool error = true;
+    }
 
     /// <summary>
     /// Prevents nonlocal players from running this method.

Assets/Mirror/Tests/Editor/Weaver/WeaverClientServerAttributeTests.cs

@@ -23,6 +23,13 @@ public void NetworkBehaviourClient()
             CheckAddedCode(Weaver.NetworkBehaviourIsClient, "WeaverClientServerAttributeTests.NetworkBehaviourClient.NetworkBehaviourClient", "ClientOnlyMethod");
         }
 
+        [Test]
+        public void NetworkBehaviourHasAuthority()
+        {
+            Assert.That(weaverErrors, Is.Empty);
+            CheckAddedCode(Weaver.NetworkBehaviourHasAuthority, "WeaverClientServerAttributeTests.NetworkBehaviourHasAuthority.NetworkBehaviourHasAuthority", "HasAuthorityMethod");
+        }
+
         /// <summary>
         /// Checks that first Instructions in MethodBody is addedString
         /// </summary>

Assets/Mirror/Tests/Editor/Weaver/WeaverClientServerAttributeTests~/NetworkBehaviourHasAuthority.cs

@@ -0,0 +1,13 @@
+using Mirror;
+
+namespace WeaverClientServerAttributeTests.NetworkBehaviourHasAuthority
+{
+    class NetworkBehaviourHasAuthority : NetworkBehaviour
+    {
+        [HasAuthority]
+        void HasAuthorityMethod()
+        {
+            // test method
+        }
+    }
+}

Assets/Mirror/Tests/Runtime/GuardsTests.cs

@@ -1,5 +1,6 @@
 using NUnit.Framework;
 using UnityEngine.TestTools;
+using UnityEngine;
 
 namespace Mirror.Tests
 {
@@ -9,6 +10,8 @@ public class ExampleGuards : NetworkBehaviour
         public bool serverCallbackFunctionCalled;
         public bool clientFunctionCalled;
         public bool clientCallbackFunctionCalled;
+        public bool hasAuthorityCalled;
+        public bool hasAuthorityNoErrorCalled;
 
         [Server]
         public void CallServerFunction()
@@ -33,6 +36,18 @@ public void CallClientCallbackFunction()
         {
             clientCallbackFunctionCalled = true;
         }
+
+        [HasAuthority]
+        public void CallAuthorityFunction() 
+        {
+            hasAuthorityCalled = true;
+        }
+
+        [HasAuthority(error = false)]
+        public void CallAuthorityNoErrorFunction() 
+        {
+            hasAuthorityNoErrorCalled = true;
+        }
     }
 
     public class GuardsTests : ClientServerSetup<ExampleGuards>
@@ -97,5 +112,44 @@ public void CanCallClientCallbackFunctionAsClient()
             Assert.That(clientComponent.clientCallbackFunctionCalled, Is.True);
         }
 
+        [Test]
+        public void CanCallHasAuthorityFunctionAsClient()
+        {
+            clientComponent.CallAuthorityFunction();
+            Assert.That(clientComponent.hasAuthorityCalled, Is.True);
+        }
+
+        [Test]
+        public void CanCallHasAuthorityCallbackFunctionAsClient()
+        {
+            clientComponent.CallAuthorityNoErrorFunction();
+            Assert.That(clientComponent.hasAuthorityNoErrorCalled, Is.True);
+        }
+
+        [Test]
+        public void GuardHasAuthorityError()
+        {
+            var obj = new GameObject("randomObject", typeof(NetworkIdentity), typeof(ExampleGuards));
+            ExampleGuards guardedComponent = obj.GetComponent<ExampleGuards>();
+
+            Assert.Throws<MethodInvocationException>( () => 
+            {
+                guardedComponent.CallAuthorityFunction();
+            });
+
+            Object.Destroy(obj);
+        }
+
+        [Test]
+        public void GuardHasAuthorityNoError()
+        {
+            var obj = new GameObject("randomObject", typeof(NetworkIdentity), typeof(ExampleGuards));
+            ExampleGuards guardedComponent = obj.GetComponent<ExampleGuards>();
+
+            guardedComponent.CallAuthorityNoErrorFunction();
+            Assert.That(guardedComponent.hasAuthorityNoErrorCalled, Is.False);
+
+            Object.Destroy(obj);
+        }
     }
 }