Packing messages for UDP networking

Question

I just came across the following method in a game engine i'm currently working on:

bool addmsg(int type, const char *fmt, ...) {
    if (!connected) return false;
    static uchar buf[MAXTRANS];
    ucharbuf p(buf, sizeof (buf));
    putint(p, type);
    int numi = 1, numf = 0, nums = 0, mcn = -1;
    bool reliable = false;
    if (fmt)
    {
        va_list args;
        va_start(args, fmt);
        while (*fmt) switch (*fmt++)
            {
            case 'r': reliable = true; break;
            case 'c':
                {
                    gameent *d = va_arg(args, gameent *);
                    mcn = !d || d == player1 ? -1 : d->clientnum;
                    break;
                }
            case 'v':
                {
                    int n = va_arg(args, int);
                    int *v = va_arg(args, int *);
                    loopi(n) putint(p, v[i]);
                    numi += n;
                    break;
                }

            case 'i':
                {
                    int n = isdigit(*fmt) ? *fmt++ - '0' : 1;
                    loopi(n) putint(p, va_arg(args, int));
                    numi += n;
                    break;
                }
            case 'f':
                {
                    int n = isdigit(*fmt) ? *fmt++ - '0' : 1;
                    loopi(n) putfloat(p, (float)va_arg(args, double));
                    numf += n;
                    break;
                }
            case 's': sendstring(va_arg(args, const char *), p); nums++; break;
            }
        va_end(args);
    }
    int num = nums || numf ? 0 : numi, msgsize = server::msgsizelookup(type);
    if (msgsize && num != msgsize) { fatal("inconsistent msg size for %d (%d != %d)", type, num, msgsize); }
    if (reliable) messagereliable = true;
    if (mcn != messagecn)
    {
        static uchar mbuf[16];
        ucharbuf m(mbuf, sizeof (mbuf));
        putint(m, N_FROMAI);
        putint(m, mcn);
        messages.put(mbuf, m.length());
        messagecn = mcn;
    }
    messages.put(buf, p.length());

    return true;
}

This is hideous. People actually thought it was a good idea to send messages across the network like this:

addmsg(MY_ACTION, "rii3ii5", 1, 42, 42, 42, e.type, e.attr1, e.attr2, e.attr3, e.attr4, e.attr5)

Oh yes! Super-readable. Now, i'm trying to be positive and slowly refactor this. I'm thinking about using a template to dynamically interpret the types in the message and encode properly. Does anyone have a suggestion on how to even start with this crap? Thanks!

Answer 1

There's the Qt way, with data streams.

class DataStream {
    /* has an internal buffer, can use std::string, std::vector<char> or even uchar [] */

    DataStream &operator << (int i) {
        putint(internal_buffer, i);
        return this;
    }
    /* ... */

    /* Multiple serialization */
    void pack() {
        return;
    }

    template <typename T, typename ...Params>
    void pack(const T &item, Params&&...params) {
        (*this) << item;
        pack(std::forward<Params>(params)...);
    }

    const char* buffer() const {
        return internal_buffer.data();
    }
};

That way, you'd have:

template<typename ...Params>
bool addmsg(int type, Params &&... params) { 
    DataStream stream;
    stream << type;

    ...

    stream.pack(std::forward<Params>(params)...);

    //Get stream buffer and use that in msg
}

This kind of thing is dangerous, you have to make sure that ints are the same on both platforms. Using types like uint32_t instead of int (when calling the addmsg function) might be wiser, unless you limit your DataStream to only one type of integer.

You can extend DataStream to support other types like custom structures:

struct MyStruct {
    int a,b,c;
    float f;
};

DataStream &operator << (DataStream &stream, const MyStruct &s) {
    stream << s.a << s.b << s.c << s.f;
    return stream;
}

That way instead of 5 arguments for members of e, e could directly be passed to the addmsg function:

addmsg(MY_ACTION, 1, 42, 42, 42, e);

Answer 2

You can just use function overloading. Don't really need templates.

#include <string>

constexpr size_t MAXTRANS = 128;

enum reliable_t { reliable };

class message
{
public:
    void send() { }

    message& operator<<(reliable_t)
    {
        m_reliable = true;
        return *this;
    }

    message& operator<<(int x)
    {
        // add int to message
        return *this;
    }

    message& operator<<(float x)
    {
        // add float to message
        return *this;
    }

    message& operator<<(const std::string& x)
    {
        // add string to message
        return *this;
    }

private:
    unsigned char m_buf[MAXTRANS];
    bool m_reliable = false;
};

int main()
{
    message msg;
    msg << reliable << 42 << 42 << "ultimate question";
    msg.send();
    return 0;
}

Here is link to coliru example.