feat: display user-friendly log with an unexpected messages. (#417)

display user-friendly log with an unexpected messages.  

### Before:
> Unknown message ID 1234 connection(...). May be due to no existing RegisterHandler for this message.

### After:
> "Unexpected message Mirror.SceneMessage received in connection(...). Did you register a handler for it?"

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

@@ -1,5 +1,6 @@
 // finds all readers and writers and register them
 using System;
+using System.Collections.Generic;
 using System.Linq;
 using Mono.Cecil;
 using Mono.Cecil.Cil;
@@ -9,15 +10,29 @@
 
 namespace Mirror.Weaver
 {
+    class TypeReferenceComparer : IEqualityComparer<TypeReference>
+    {
+        public bool Equals(TypeReference x, TypeReference y)
+        {
+            return x.FullName == y.FullName;
+        }
+
+        public int GetHashCode(TypeReference obj)
+        {
+            return obj.FullName.GetHashCode();
+        }
+    }
     public static class ReaderWriterProcessor
     {
+        private readonly static HashSet<TypeReference> messages = new HashSet<TypeReference>(new TypeReferenceComparer());
 
         public static void Process(AssemblyDefinition CurrentAssembly, Assembly unityAssembly)
         {
             // darn global state causing bugs
             Weaver.WeaveLists.generateContainerClass = null;
             Readers.Init();
             Writers.Init();
+            messages.Clear();
             foreach (Assembly unityAsm in unityAssembly.assemblyReferences)
             {
                 if (unityAsm.name == "Mirror")
@@ -87,12 +102,10 @@ private static void GenerateReadersWriters(ModuleDefinition module, MethodRefere
             if (!method.IsGenericInstance)
                 return;
 
-            bool generate =
+            bool isMessage =
                 method.Is(typeof(MessagePacker), nameof(MessagePacker.Pack)) ||
                 method.Is(typeof(MessagePacker), nameof(MessagePacker.GetId)) ||
                 method.Is(typeof(MessagePacker), nameof(MessagePacker.Unpack)) ||
-                method.Is<NetworkWriter>(nameof(NetworkWriter.Write)) ||
-                method.Is<NetworkReader>(nameof(NetworkReader.Read)) ||
                 method.Is<IMessageHandler>(nameof(IMessageHandler.Send)) ||
                 method.Is<IMessageHandler>(nameof(IMessageHandler.SendAsync)) ||
                 method.Is<IMessageHandler>(nameof(IMessageHandler.RegisterHandler)) ||
@@ -110,11 +123,21 @@ private static void GenerateReadersWriters(ModuleDefinition module, MethodRefere
                 method.Is<NetworkServer>(nameof(NetworkServer.SendToClientOfPlayer)) ||
                 method.Is<INetworkServer>(nameof(INetworkServer.SendToAll));
 
+            bool generate = isMessage ||
+                method.Is<NetworkWriter>(nameof(NetworkWriter.Write)) ||
+                method.Is<NetworkReader>(nameof(NetworkReader.Read));
+
             if (generate)
             {
                 var instanceMethod = (GenericInstanceMethod)method;
                 TypeReference parameterType = instanceMethod.GenericArguments[0];
+
+                if (parameterType.IsGenericParameter)
+                    return;
+
                 GenerateReadersWriters(module, parameterType);
+                if (isMessage)
+                    messages.Add(parameterType);
             }
         }
 
@@ -210,6 +233,8 @@ public static void InitializeReaderAndWriters(AssemblyDefinition currentAssembly
             var rwInitializer = new MethodDefinition("InitReadWriters", MethodAttributes.Public |
                     MethodAttributes.Static,
                     WeaverTypes.Import(typeof(void)));
+            TypeDefinition generateClass = Weaver.WeaveLists.generateContainerClass;
+            generateClass.Methods.Add(rwInitializer);
 
             System.Reflection.ConstructorInfo attributeconstructor = typeof(RuntimeInitializeOnLoadMethodAttribute).GetConstructor(new [] { typeof(RuntimeInitializeLoadType)});
 
@@ -230,12 +255,25 @@ public static void InitializeReaderAndWriters(AssemblyDefinition currentAssembly
             Writers.InitializeWriters(worker);
             Readers.InitializeReaders(worker);
 
+            RegisterMessages(currentAssembly.MainModule, worker);
+
             worker.Append(worker.Create(OpCodes.Ret));
 
             Weaver.WeaveLists.ConfirmGeneratedCodeClass();
-            TypeDefinition generateClass = Weaver.WeaveLists.generateContainerClass;
 
-            generateClass.Methods.Add(rwInitializer);
+        }
+
+        private static void RegisterMessages(ModuleDefinition module, ILProcessor worker)
+        {
+            System.Reflection.MethodInfo method = typeof(MessagePacker).GetMethod(nameof(MessagePacker.RegisterMessage));
+            MethodReference registerMethod = module.ImportReference(method);
+
+            foreach (TypeReference message in messages)
+            {
+                var genericMethodCall = new GenericInstanceMethod(registerMethod);
+                genericMethodCall.GenericArguments.Add(module.ImportReference(message));
+                worker.Append(worker.Create(OpCodes.Call, genericMethodCall));
+            }
         }
     }
 }

Assets/Mirror/Runtime/MessagePacker.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Generic;
 using System.ComponentModel;
 
 namespace Mirror
@@ -16,6 +17,25 @@ namespace Mirror
     //    (probably even shorter)
     public static class MessagePacker
     {
+        /// <summary>
+        /// Map of Message Id => Type
+        /// When we receive a message, we can lookup here to find out what type
+        /// it was.  This is populated by the weaver.
+        /// </summary>
+        private static readonly Dictionary<int, Type> messageTypes = new Dictionary<int, Type>();
+
+        public static Type GetMessageType(int id) => messageTypes[id];
+
+        public static void RegisterMessage<T>()
+        {
+            int id = GetId<T>();
+
+            if (messageTypes.TryGetValue(id, out Type type) && type != typeof(T)) {
+                throw new ArgumentException($"Message {typeof(T)} and {messageTypes[id]} have the same ID. Change the name of one of those messages");
+            }
+            messageTypes[id] = typeof(T);
+        }
+
         public static int GetId<T>()
         {
             return GetId(typeof(T));

Assets/Mirror/Runtime/NetworkConnection.cs

@@ -264,7 +264,18 @@ internal void InvokeHandler(int msgType, NetworkReader reader, int channelId)
             {
                 msgDelegate(this, reader, channelId);
             }
-            else if (Debug.isDebugBuild) logger.Log("Unknown message ID " + msgType + " " + this + ". May be due to no existing RegisterHandler for this message.");
+            else
+            {
+                try
+                {
+                    Type type = MessagePacker.GetMessageType(msgType);
+                    throw new InvalidDataException($"Unexpected message {type} received in {this}. Did you register a handler for it?");
+                }
+                catch (KeyNotFoundException)
+                {
+                    throw new InvalidDataException($"Unexpected message ID {msgType} received in {this}. May be due to no existing RegisterHandler for this message.");
+                }
+            }
         }
 
         // note: original HLAPI HandleBytes function handled >1 message in a while loop, but this wasn't necessary
@@ -289,6 +300,10 @@ internal void TransportReceive(ArraySegment<byte> buffer, int channelId)
                     // try to invoke the handler for that message
                     InvokeHandler(msgType, networkReader, channelId);
                 }
+                catch (InvalidDataException ex)
+                {
+                    logger.Log(ex.ToString());
+                }
                 catch (Exception ex)
                 {
                     logger.LogError("Closed connection: " + this + ". Invalid message " + ex);

Assets/Tests/Editor/MessagePackerTest.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Generic;
 using System.IO;
 using NUnit.Framework;
 
@@ -90,5 +91,54 @@ public void UnpackInvalidMessage()
                 int msgType2 = MessagePacker.UnpackId(reader2);
             });
         }
+
+        struct SomeRandomMessage { };
+
+        [Test]
+        public void RegisterMessage()
+        {
+            MessagePacker.RegisterMessage<SomeRandomMessage>();
+
+            int id = MessagePacker.GetId<SomeRandomMessage>();
+
+            Type type = MessagePacker.GetMessageType(id);
+
+            Assert.That(type, Is.EqualTo(typeof(SomeRandomMessage)));
+        }
+
+        // these 2 messages have a colliding message id
+        struct SomeRandomMessage2121143 { };
+        struct SomeRandomMessage2133122 { };
+
+        [Test]
+        public void RegisterMessage2()
+        {
+            MessagePacker.RegisterMessage<SomeRandomMessage2121143>();
+            Assert.Throws<ArgumentException>(() =>
+            {
+                MessagePacker.RegisterMessage<SomeRandomMessage2133122>();
+            });
+        }
+
+        [Test]
+        public void FindSystemMessage()
+        {
+            int id = MessagePacker.GetId<SceneMessage>();
+            Type type = MessagePacker.GetMessageType(id);
+            Assert.That(type, Is.EqualTo(typeof(SceneMessage)));
+        }
+
+        struct SomeRandomMessageNotRegistered { };
+        [Test]
+        public void FindUnknownMessage()
+        {
+            // note that GetId<> will cause the weaver to register it
+            // but GetId() will not
+            int id = MessagePacker.GetId(typeof(SomeRandomMessageNotRegistered));
+            Assert.Throws<KeyNotFoundException>(() =>
+            {
+                Type type = MessagePacker.GetMessageType(id);
+            });
+        }
     }
 }

Assets/Tests/Editor/NetworkConnectionTest.cs

@@ -0,0 +1,41 @@
+using System.IO;
+using NSubstitute;
+using NUnit.Framework;
+using UnityEngine;
+
+namespace Mirror.Tests
+{
+    public class NetworkConnectionTest : MonoBehaviour
+    {
+        [Test]
+        public void NoHandler()
+        {
+            var connection = new NetworkConnection(Substitute.For<IConnection>());
+
+            int messageId = MessagePacker.GetId<SceneMessage>();
+            var reader = new NetworkReader(new byte[] { 1, 2, 3, 4 });
+            InvalidDataException exception = Assert.Throws<InvalidDataException>(() =>
+            {
+                connection.InvokeHandler(messageId, reader, 0);
+            });
+
+            Assert.That(exception.Message, Does.StartWith("Unexpected message Mirror.SceneMessage received"));
+        }
+
+        [Test]
+        public void UnknownMessage()
+        {
+            var connection = new NetworkConnection(Substitute.For<IConnection>());
+
+            int messageId = MessagePacker.GetId<SceneMessage>();
+            var reader = new NetworkReader(new byte[] { 1, 2, 3, 4 });
+            InvalidDataException exception = Assert.Throws<InvalidDataException>(() =>
+            {
+                // some random id with no message
+                connection.InvokeHandler(1234, reader, 0);
+            });
+
+            Assert.That(exception.Message, Does.StartWith("Unexpected message ID 1234 received"));
+        }
+    }
+}