Add factory method to create a Message by from tag and IMessageBuffer

[tedbarth]

Currently the only way to get pre-serialized Data (byte[]) into a Message is by either using DarkRift.DarkRiftWriter.WriteRaw and passing the Writer to Message factory method or by passing an object implementing IDarkRiftSerializable.

Both ways will cause an unnecessary memory copy of the data.

Luckily IMessageBuffer is public, so we could write this:

public class ByteArrayMappingMessageBuffer : IMessageBuffer {
  public ByteArrayMappingMessageBuffer(byte[] buffer) {
    Buffer = buffer;
  }

  public void Dispose() {
    throw new System.NotImplementedException();
  }

  public IMessageBuffer Clone() {
    throw new System.NotImplementedException();
  }

  public byte[] Buffer { get; }

  public int Count {
    get => Buffer.Length;
    set => throw new System.NotImplementedException();
  }

  public int Offset {
    get => 0;
    set => throw new System.NotImplementedException();
  }
}

The PR adds a new method to Message allowing to not only pass a DarkRiftWriter but an IMessageBuffer directly. Passing the one above will allow us to create and send a Message using pre-serialized data and also helps separating Serialization from Networking.

byte[] serializedData = ...;
var buffer = new ByteArrayMappingMessageBuffer(serializedData);
Message message = Message.Create(message.Tag, buffer);
client.SendMessage(message, sendMode);

[JamJar00]

Thanks for the PR! I really like the idea of separating out the serialization from the messages. It will help facilitate using other serializers a lot and will make some usecases a lot simpler without needing writers!

I'm not totally convinced exposing an interface for an IMessageBuffer is the right option here though. DarkRift's message buffers are a core part of its memory management and there are some nuances that go with that, for example you can't just throw an exception on Clone, that has very specific and important meaning to DarkRift's memory management!

Secondly I think we need a corresponding read method to get the data out the other end again and that can definitely not be as an IMessageBuffer (for many reasons)!

I think there's still value in the prinicple here though. So I wonder if creating void Create(ushort, byte[]) and byte[] Deserialize() would be useful. DarkRift actually has a class very similar to your ByteArrayMappingMemoryBuffer already which could be used behind the scenes of Create, however Deserialize will almost always need a memory copy still because the byte array in the message buffer will be managed by DarkRift so shouldn't be exposed to user code.

[tedbarth]

Thanks for the fast reaction! I totally understand your points and have replaced the new method (now private) with:

public static Message Create(ushort tag, byte[] data)

I currently can't see, why Deserialize() would be necessary, though. There already is this direct way, which I think is enough, unless you want it as helper method:

Message message = e.GetMessage();
DarkRiftReader darkRiftReader = message.GetReader();
byte[] data = darkRiftReader.ReadRaw(darkRiftReader.Length);

[tedbarth]

Offtopic: I often get confused by method semantic and now understood why: Your Get Methods often create new objects with separate state. For instance Message.GetReader() does create a new reader with a position set to 0. Calling it again will give you another with its own position. What confuses me here is that due to the Get prefix of the method I would expect an idempotent behavior, meaning every time I either get the same reader or a new one that is immutable or a new one that shares the same state with other readers. Instead the returned reader right now is mutable with its own state. I would expect this behavior from a method with name Message.CreateReader().

[tedbarth]

Oh damn, I somehow closed this issue. :D