feat: Server attribute now throws error

Assets/Mirror/Components/NetworkSceneChecker.cs

@@ -29,7 +29,7 @@ public class NetworkSceneChecker : NetworkVisibility
 
         Scene currentScene;
 
-        [ServerCallback]
+        [Server(error=false)]
         void Awake()
         {
             currentScene = gameObject.scene;
@@ -46,7 +46,7 @@ public void OnStartServer()
             sceneCheckerObjects[currentScene].Add(NetIdentity);
         }
 
-        [ServerCallback]
+        [Server(error=false)]
         void Update()
         {
             if (currentScene == gameObject.scene)

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

@@ -58,9 +58,6 @@ static void ProcessMethods(TypeDefinition td)
                         case "Mirror.ServerAttribute":
                             Weaver.Error($"Server method {md.Name} must be declared inside a NetworkBehaviour", md);
                             break;
-                        case "Mirror.ServerCallbackAttribute":
-                            Weaver.Error($"ServerCallback method {md.Name} must be declared inside a NetworkBehaviour", md);
-                            break;
                         case "Mirror.ClientAttribute":
                             Weaver.Error($"Client method {md.Name} must be declared inside a NetworkBehaviour", md);
                             break;

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

@@ -15,10 +15,7 @@ public static void ProcessMethodAttributes(TypeDefinition td, MethodDefinition m
                 switch (attr.Constructor.DeclaringType.ToString())
                 {
                     case "Mirror.ServerAttribute":
-                        InjectServerGuard(td, md, true);
-                        break;
-                    case "Mirror.ServerCallbackAttribute":
-                        InjectServerGuard(td, md, false);
+                        InjectServerGuard(td, md, attr);
                         break;
                     case "Mirror.ClientAttribute":
                         InjectClientGuard(td, md, true);
@@ -44,8 +41,10 @@ public static void ProcessMethodAttributes(TypeDefinition td, MethodDefinition m
             }
         }
 
-        static void InjectServerGuard(TypeDefinition td, MethodDefinition md, bool logWarning)
+        static void InjectServerGuard(TypeDefinition td, MethodDefinition md, CustomAttribute attribute)
         {
+            bool throwError = attribute.GetField<bool>("error", true);
+
             if (!Weaver.IsNetworkBehaviour(td))
             {
                 Weaver.Error($"Server method {md.Name} must be declared in a NetworkBehaviour", md);
@@ -57,10 +56,11 @@ static void InjectServerGuard(TypeDefinition td, MethodDefinition md, bool logWa
             worker.InsertBefore(top, worker.Create(OpCodes.Ldarg_0));
             worker.InsertBefore(top, worker.Create(OpCodes.Call, Weaver.NetworkBehaviourIsServer));
             worker.InsertBefore(top, worker.Create(OpCodes.Brtrue, top));
-            if (logWarning)
+            if (throwError)
             {
                 worker.InsertBefore(top, worker.Create(OpCodes.Ldstr, "[Server] function '" + md.FullName + "' called on client"));
-                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);
             InjectGuardReturnValue(md, worker, top);

Assets/Mirror/Editor/Weaver/Weaver.cs

@@ -87,6 +87,8 @@ internal static class Weaver
         public static MethodReference NetworkBehaviourHasAuthority;
         public static MethodReference NetworkBehaviourIsLocalPlayer;
 
+        public static MethodReference MethodInvocationExceptionConstructor;
+
         // custom attribute types
         public static TypeReference SyncVarType;
         public static TypeReference CommandType;
@@ -304,6 +306,9 @@ static void SetupTargetTypes()
                 ScriptableObjectType, CurrentAssembly,
                 md => md.Name == "CreateInstance" && md.HasGenericParameters);
 
+            var MethodInvocationException = NetAssembly.MainModule.GetType("Mirror.MethodInvocationException");
+
+            MethodInvocationExceptionConstructor = Resolvers.ResolveMethodWithArg(MethodInvocationException, CurrentAssembly, ".ctor", "System.String");
 
             MessageBaseType = NetAssembly.MainModule.GetType("Mirror.MessageBase");
             IMessageBaseType = NetAssembly.MainModule.GetType("Mirror.IMessageBase");

Assets/Mirror/Examples/AdditiveScenes/Scripts/ShootingTankBehaviour.cs

@@ -12,7 +12,7 @@ public class ShootingTankBehaviour : NetworkBehaviour
 
         NetworkAnimator networkAnimator;
 
-        [ServerCallback]
+        [Server(error=false)]
         void Start()
         {
             networkAnimator = GetComponent<NetworkAnimator>();

Assets/Mirror/Examples/Basic/Scripts/Player.cs

@@ -56,7 +56,7 @@ public void OnStartServer()
         }
 
         // This only runs on the server, called from OnStartServer via InvokeRepeating
-        [ServerCallback]
+        [Server(error=false)]
         void UpdateData()
         {
             playerData = Random.Range(100, 1000);

Assets/Mirror/Examples/Benchmarks/10k/Scripts/Health.cs

@@ -4,7 +4,7 @@ public class Health : NetworkBehaviour
     {
         [SyncVar] public int health = 10;
 
-        [ServerCallback]
+        [Server(error = false)]
         public void Update()
         {
             health = (health + 1) % 10;

Assets/Mirror/Examples/Benchmarks/2k NetworkTransforms/Scripts/MonsterMovement.cs

@@ -17,7 +17,7 @@ public void OnStartServer()
             start = transform.position;
         }
 
-        [ServerCallback]
+        [Server(error = false)]
         void Update()
         {
             if (moving)

Assets/Mirror/Examples/MultipleAdditiveScenes/Scripts/PhysicsCollision.cs

@@ -21,7 +21,7 @@ private void Start()
             rigidbody3D.isKinematic = !IsServer;
         }
 
-        [ServerCallback]
+        [Server(error=false)]
         void OnCollisionStay(Collision other)
         {
             if (other.gameObject.CompareTag("Player"))

Assets/Mirror/Examples/MultipleAdditiveScenes/Scripts/Reward.cs

@@ -17,7 +17,7 @@ void OnValidate()
                 randomColor = GetComponent<RandomColor>();
         }
 
-        [ServerCallback]
+        [Server(error=false)]
         void OnTriggerEnter(Collider other)
         {
             if (other.gameObject.CompareTag("Player"))

Assets/Mirror/Examples/Pong/Scripts/Ball.cs

@@ -33,7 +33,7 @@ float HitFactor(Vector2 ballPos, Vector2 racketPos, float racketHeight)
         }
 
         // only call this on server
-        [ServerCallback]
+        [Server(error=false)]
         void OnCollisionEnter2D(Collision2D col)
         {
             // Note: 'col' holds the collision information. If the

Assets/Mirror/Examples/Tanks/Scripts/Projectile.cs

@@ -36,9 +36,9 @@ void DestroySelf()
             Server.Destroy(gameObject);
         }
 
-        // ServerCallback because we don't want a warning if OnTriggerEnter is
+        // [Server] because we don't want a warning if OnTriggerEnter is
         // called on the client
-        [ServerCallback]
+        [Server(error=false)]
         void OnTriggerEnter(Collider co)
         {
             //Hit another player

Assets/Mirror/Runtime/CustomAttributes.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Runtime.Serialization;
 using UnityEngine;
 
 namespace Mirror
@@ -59,17 +60,46 @@ public class SyncEventAttribute : Attribute
 
     /// <summary>
     /// Prevents clients from running this method.
-    /// <para>Prints a warning if a client tries to execute this method.</para>
     /// </summary>
     [AttributeUsage(AttributeTargets.Method)]
-    public class ServerAttribute : Attribute { }
+    public class ServerAttribute : 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 clients from running this method.
-    /// <para>No warning is thrown.</para>
+    /// Exception thrown if a guarded method is invoked incorrectly
     /// </summary>
-    [AttributeUsage(AttributeTargets.Method)]
-    public class ServerCallbackAttribute : Attribute { }
+    [Serializable]
+    public class MethodInvocationException : Exception
+    {
+        /// <summary>
+        /// Initializes a new instance of the <see cref="T:MethodInvocationException"/> class
+        /// </summary>
+        public MethodInvocationException()
+        {
+        }
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="T:MethodInvocationException"/> class
+        /// </summary>
+        /// <param name="message">A <see cref="T:System.String"/> that describes the exception. </param>
+        public MethodInvocationException(string message) : base(message)
+        {
+        }
+
+        // A constructor is needed for serialization when an
+        // exception propagates from a remoting server to the client.
+        protected MethodInvocationException(SerializationInfo info,StreamingContext context) : base(info, context)
+        {
+        }
+    }
 
     /// <summary>
     /// Prevents the server from running this method.

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

@@ -49,7 +49,7 @@ public void MonoBehaviourServer()
         [Test]
         public void MonoBehaviourServerCallback()
         {
-            Assert.That(weaverErrors, Contains.Item("ServerCallback method ThisCantBeOutsideNetworkBehaviour must be declared inside a NetworkBehaviour (at System.Void WeaverMonoBehaviourTests.MonoBehaviourServerCallback.MonoBehaviourServerCallback::ThisCantBeOutsideNetworkBehaviour())"));
+            Assert.That(weaverErrors, Contains.Item("Server method ThisCantBeOutsideNetworkBehaviour must be declared inside a NetworkBehaviour (at System.Void WeaverMonoBehaviourTests.MonoBehaviourServerCallback.MonoBehaviourServerCallback::ThisCantBeOutsideNetworkBehaviour())"));
         }
 
         [Test]

Assets/Mirror/Tests/Editor/Weaver/WeaverMonoBehaviourTests~/MonoBehaviourServerCallback.cs

@@ -5,7 +5,7 @@ namespace WeaverMonoBehaviourTests.MonoBehaviourServerCallback
 {
     class MonoBehaviourServerCallback : MonoBehaviour
     {
-        [ServerCallback]
+        [Server(error = false)]
         void ThisCantBeOutsideNetworkBehaviour() { }
     }
 }

Assets/Mirror/Tests/Runtime/GuardsTests.cs

@@ -16,7 +16,7 @@ public void CallServerFunction()
             serverFunctionCalled = true;
         }
 
-        [ServerCallback]
+        [Server(error=false)]
         public void CallServerCallbackFunction()
         {
             serverCallbackFunctionCalled = true;
@@ -70,9 +70,10 @@ public void CannotCallClientCallbackFunctionAsServer()
         [Test]
         public void CannotCallServerFunctionAsClient()
         {
-            clientComponent.CallServerFunction();
-            LogAssert.Expect(UnityEngine.LogType.Warning, "[Server] function 'System.Void Mirror.Tests.ExampleGuards::CallServerFunction()' called on client");
-            Assert.That(clientComponent.serverFunctionCalled, Is.False);
+            Assert.Throws<MethodInvocationException>(() =>
+            {
+               clientComponent.CallServerFunction();
+            });
         }
 
         [Test]

doc/Guides/Attributes.md

@@ -8,11 +8,9 @@ These attributes can be used for Unity game loop methods like Start or Update, a
 
 -   **NetworkSettings**  
     This attribute has been deprecated because `channels` were moved to transports (where applicable) and `interval` was moved to an inspector property
--   **Server**  
-    Only a server can call the method (throws a warning or an error when called on a client).
--   **ServerCallback**  
-    Same as **Server** but does not throw warning when called on client.
--   **Client**  
+-   **<xref:Mirror.ServerAttribute>**  
+    Only a server can call the method (throws an error when called on a client unless you specify error = false).
+-   **<xref:Mirror.ClientAttribute>**  
     Only a Client can call the method (throws a warning or an error when called on the server).
 -   **ClientCallback**  
     Same as **Client** but does not throw warning when called on server.

doc/Guides/NetworkBehaviour.md

@@ -68,11 +68,7 @@ Note that in a peer-hosted setup, when one of the clients is acting as both host
 
 ## Server and Client functions
 
-You can tag member functions in NetworkBehaviour scripts with custom attributes to designate them as server-only or client-only functions. `Server` and `ServerCallback` return immediately if the client is not active. Likewise, `Client` and `ClientCallback` return immediately if the server is not active.
-
-The `Server` and `Client` attributes are for your own custom callback functions. They do not generate compile time errors, but they do emit a warning log message if called in the wrong scope.
-
-The `ServerCallback` and `ClientCallback` attributes are for built-in callback functions that are called automatically by Mirror. These attributes do not cause a warning to be generated.
+You can tag member functions in NetworkBehaviour scripts with custom attributes to designate them as server-only or client-only functions. <xref:Mirror.ServerAttribute> will check that the function is called in the server. Likewise, <xref:Mirror.ClientAttribute> will check if the function is called in the client.
 
 For more information, see [Attributes](Attributes.md).