Message.Create convenience methods

[kapitanmliko]

Quick Description

Having Message.Create(tag, serializable) and Message.Create(tag, writer) is imho not enough.

Explanation

Right now we are limited and we have to use Message.Create(tag, writer) like this:

using (var writer = DarkRiftWriter.Create())
{
    writer.Write(messageContents);
    using (var message = Message.Create(tag, writer))
    {
        recipient.SendMessage(message, mode);
    }
}

It would be much more convenient to be able to achieve the same with less amount of code:

    using (var message = Message.Create(tag, contents))
    {
        recipient.SendMessage(message, mode);
    }

We would just need to add more overloads for Message.Create. And I would suggest to have all those that the DarkRiftWriter has.

Thanks for consideration!

[MrBabadook]

I looked at the code you wrote - this is often done by creating messages using structs/classes that extend IDarkRiftSerializble. I am not a huge fan of in-lining message creation like you show as I find it to be error prone. You can find more complete examples of this in both the FPS and RPG tutorials, Is that the type of thing you are looking for? (This is a very simple example, you can have as much/little data as you want, and if you don't want any data, just the tag, CreateEmpty is already supported). Also if you are not a member of the Discord community you should join :) We have a bunch of resources related to this there.

public void OnTryCreateNewCharacter(string name)
{
    using (Message message = Message.Create((ushort)Tags.RegisterNewCharacterRequest, new RegisterNewCharacterRequestData(name)))
    {
        Client.SendMessage(message, SendMode.Reliable);
     }
}


public struct RegisterNewCharacterRequestData : IDarkRiftSerializable
{
    public string CharacterName;

    public RegisterNewCharacterRequestData(string name)
    {
        CharacterName = name;
    }
    public void Deserialize(DeserializeEvent e)
    {
        CharacterName = e.Reader.ReadString();
    }

    public void Serialize(SerializeEvent e)
    {
        e.Writer.Write(CharacterName);
    }
 }

[MrBabadook]

Writing a second message because the code formatting on Github is giving me trouble but I wanted to add that most people also use some sort of helper method like this combined with the approach above so all you have to write in your code is "SendToClient(tag, message)"

public void SendToClient(ushort id, Tags t, IDarkRiftSerializable obj, SendMode mode = SendMode.Reliable)
{
    using (Message m = Message.Create<IDarkRiftSerializable>((ushort)t, obj))
    {
         ConnectedClients[id].Client.SendMessage(m, mode);
    }
}

[kapitanmliko]

Thank you for your comments. I have joined discord yes.

What I did was just match the Message.Create with DarkRiftWriter.Write. Same thing is already there.

What you are saying is I would have to create IDarkriftSerializable instance for each and every message. To me that sounds inefficient as it means an extra allocation even for byte sized messages.

Let's keep at least the Message Create<T>(ushort tag, T[] obj) where T : IDarkRiftSerializable since now we have to either wrap it in another IDarkRiftSerializable or use the DarkRiftWriter again. And I have also included a little bit of cleanup as the Message Create<T>(ushort tag, T obj) shared most of the code with Message Create(ushort tag, DarkRiftWriter writer) If you don't like it at all we can close this.

The purpose of this was to be make it more convenient while preserving original efficiency. I can think of multiple ways to do it.

You could for example have a specialized builder class for each message like so:

using var message = PlayerMovedMessage.Create(playerPose);
recipient.SendMessage(message, mode);

you can go even a bit further with extension methods. Then it could be:

using var message = PlayerMovedMessage.Create(playerPose);
message.SendReliable(recipient);

There is no extra allocation and it is much easier to look at. It just feels like it should be less work.

[JamJar00]

Thanks for the PR and the issue, I appreciate the time you've taken to put this together!

I'm afraid I have to agree with @MrBabadook here. I feel like the vast majority of messages are sent with multiple bits of data in which will always require you to create either a DarkRiftWriter or an IDarkRiftSerializable to do, so these changes will only help with the rarer case where we just want to send a very simple message. Also, this starts to add an inconsistency between how to send a single vs multiple data points in DarkRift which I don't really like - consistency is often better than simplicity.

I do agree however, sending messages in DR is a bit of a chore still. IDarkRiftSerializables are definitely the prefered method these days because you get to hide the complexity behind the object and it forms a semi-framework for message structure. With IDarkRiftSerializable, you can get pretty close to what you have above (and this doesn't allocate if you use a struct)!

using var message = Message.Create(tag, playerPose);
recipient.SendMessage(message, mode);

Just my two-cents

[4real]

Although I like expressive APIs, I don't think this matters too much in practice as the users will just add a method themselves to allow them to one line things, like:

network.SendMessage(recipient, tag, contents, mode);

Because we might be changing the signature of SendMessage methods eventually (e.g. because we are introducing P2P support requiring recipient targeting also clientside), I don't think we should be adding more variants just yet.