1
\$\begingroup\$

I'm writing a server for an MMO game using boost::asio. I would like to know, are there any design or other issues in my code? And what should I improve in it? Thanks in advance.

BaseServer.h:

#ifndef BASE_SERVER_H
#define BASE_SERVER_H

#include <asio.hpp>
#include "MessageProcessor.h"

class BaseServer
{
public:
    BaseServer(asio::io_context& ioContext, unsigned short port)
        : socket(ioContext, asio::ip::udp::endpoint(asio::ip::udp::v4(), port))
    {
        receivePacket();
    }

protected:
    asio::ip::udp::socket socket;

    virtual void handlePacket() = 0;

private:
    void receivePacket()
    {
        socket.async_receive(asio::null_buffers(), [this](std::error_code ec, std::size_t bytes_recvd)
        {
            if (ec == asio::error::operation_aborted) return;
            handlePacket();
            receivePacket();
        });
    }
};

#endif

GameServer.h:

#ifndef GAME_SERVER_H
#define GAME_SERVER_H

#include <Net/BaseServer.h>
#include <Net/MessageProcessor.h>
#include <Utils/BitStream.h>

class GameServer : public BaseServer
{
public:
    GameServer(asio::io_context& ioContext, unsigned short port);

protected:
    MessageProcessor<BitStream&, asio::ip::udp::endpoint> messageProcessor;

    void asyncParsePacket(unsigned char* buffer, unsigned short packetSize, asio::ip::udp::endpoint senderEndpoint);
    virtual void handlePacket() override;
};

#endif

GameServer.cpp:

#include "GameServer.h"

#include <iostream>
#include "Messages/Client/TestMessage.h"

GameServer::GameServer(asio::io_context& ioContext, unsigned short port)
    : BaseServer(ioContext, port)
{
    messageProcessor.registerHandler(0x01, [](BitStream& stream, asio::ip::udp::endpoint endpoint)
    {
        TestMessage mes;
        mes.deserialize(stream);

        std::cout << "Test message received! A = " << mes.a << ", B = " << mes.b << std::endl;
    });
}

void GameServer::asyncParsePacket(unsigned char* buffer, unsigned short packetSize, asio::ip::udp::endpoint senderEndpoint)
{
    BitStream stream(buffer, packetSize);
    delete[] buffer;
    unsigned char messageId;
    stream >> messageId;

    auto handler = messageProcessor.getHandler(messageId);
    if (handler) handler(stream, senderEndpoint);
}

void GameServer::handlePacket()
{
    unsigned int available = socket.available();
    unsigned char* buffer = new unsigned char[available];
    asio::ip::udp::endpoint senderEndpoint;
    std::error_code ec;
    unsigned short packetSize = socket.receive_from(asio::buffer(buffer, available), senderEndpoint, 0, ec);
    socket.get_io_service().post(std::bind(&AuthServer::asyncParsePacket, this, buffer, packetSize, senderEndpoint));
}

BaseMessage.h:

#ifndef BASE_MESSAGE_H
#define BASE_MESSAGE_H

#include "../Utils/BitStream.h"

class BaseMessage
{
protected:
    unsigned short id;

public:
    BaseMessage(unsigned short messageId)
        : id(messageId) {}
    virtual ~BaseMessage() = default;

    unsigned short getId() const { return this->id; }

    virtual void serialize(BitStream& stream) const = 0;
    virtual void deserialize(BitStream& stream) = 0;
};

#endif

MessageProcessor.h

#ifndef MESSAGE_PROCESSOR_H
#define MESSAGE_PROCESSOR_H

#include <vector>
#include <functional>

class BitStream;

template <typename ... HandlerArgs>
class MessageProcessor
{
protected:
    using MessageHandler = std::function<void (HandlerArgs ...)>;
    std::vector<MessageHandler> messageHandlers;

public:
    void registerHandler(unsigned short id, MessageHandler handler)
    {
        if (messageHandlers.size() <= id) messageHandlers.resize(id);
        messageHandlers.insert(messageHandlers.begin() + id, handler);
    }

    MessageHandler getHandler(unsigned short id) const
    {
        return id < messageHandlers.size() ? messageHandlers[id] : 0;
    }
};

#endif
\$\endgroup\$
  • \$\begingroup\$ Can you tell us more about the current functionality and why you're doing what you're doing? For example, what's the purpose of handlePacket() and why is receivePacket() called recursively? \$\endgroup\$ – Mast Jan 11 at 16:08
  • \$\begingroup\$ @Mast receivePacket() is needed to encapsulate async_read function and avoid boilerplate code in derived classes. handlePacket() is like an onMessageReceived() callback. receivePacket() is called recursively because I need the server to receive messages in a loop. \$\endgroup\$ – Alex Kuzub Jan 11 at 16:22
  • \$\begingroup\$ Are you aware there's such a thing as recursion depth and exceeding the maximum depth will result in a stack overflow? What is your maximum stack size and how long have you tried running your server? It should crash eventually if I read this right. \$\endgroup\$ – Mast Jan 11 at 16:27
  • \$\begingroup\$ @Mast This is not a recursion. async_receive is an asynchronous function, so it just adds new async request to the I/O queue and exits immediately. \$\endgroup\$ – Alex Kuzub Jan 11 at 16:33
1
\$\begingroup\$

You can make the call to recieve_from async by creating a temp struct with the variables you need to keep alive and the buffer. Then you can put it in a shared_ptr (to account for the potential copies and capture that shared_ptr in the lambda:

void GameServer::handlePacket()
{
    unsigned int available = socket.available();
    struct rec_data{
        std::vector<unsigned char> buffer;
        asio::ip::udp::endpoint senderEndpoint;
    }

    std::shared_ptr<rec_data> data = std::make_shared<rec_data>();
    data->buffer.resize(available);

    socket.receive_from(asio::buffer(data ->buffer.data(), available), 
         data ->senderEndpoint, 0, 
         [data](const std::error_code& error, 
                std::size_t bytes_transferred)
         {
             if(!error)
                 asyncParsePacket(data->buffer.data(), bytes_transferred, data->senderEndpoint);
         });
}

The int you use for registerHandler is a magic number. Make it an enum and give each message type a name. Make sure to share the header between the sender and receiver.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.