UDP Network server/client for gaming using boost.asio

I've designed those classes for use in a multiplayer game with possibly very high number of clients for one server. Is this implementation good, or is there something obvious I overlooked or something to improve? I expect this part to have very high performance, so that I can just focus on other things and not come back to the basics too often.

Here are code listings:

NetworkServer.h

```
#pragma once

#include "Constants.h"
#include "locked_queue.h"

#include
#include
#include
#include

#include
#include

using boost::asio::ip::udp;

typedef boost::bimap ClientList;
typedef ClientList::value_type Client;
typedef std::pair ClientMessage;

class NetworkServer {
public:
NetworkServer(unsigned short local_port);
~NetworkServer();

bool HasMessages();
ClientMessage PopMessage();

void SendToClient(std::string message, unsigned __int64 clientID, bool guaranteed = false);
void SendToAllExcept(std::string message, unsigned __int64 clientID, bool guaranteed = false);
void SendToAll(std::string message, bool guaranteed = false);

inline unsigned __int64 GetStatReceivedMessages() {return receivedMessages;};
inline unsigned __int64 GetStatReceivedBytes() {return receivedBytes;};
inline unsigned __int64 GetStatSentMessages() {return sentMessages;};
inline unsigned __int64 GetStatSentBytes() {return sentBytes;};

private:
// Network send/receive stuff
boost::asio::io_service io_service;
udp::socket socket;
udp::endpoint server_endpoint;
udp::endpoint remote_endpoint;
std::array recv_buffer;
boost::thread service_thread;

void start_receive();
void handle_receive(const boost::system::error_code& error, std::size_t bytes_transferred);
void handle_send(std::string /message/, const boost::system::error_code& /error/, std::size_t /bytes_transferred/) {}
void run_service();
unsigned __int64 get_client_id(udp::endpoint endpoint);

void send(std::string pmessage

Exception safety

Your locked_queue isn't exception safe. In particular:

queue.pop();
    return value;

If the copy (or move) constructor for _T throws, you could have popped the item from the queue, then the constructor throws as you return the value, and the value is lost and can't be recovered. This is exactly why the standard library separates retrieving the value from removing the value from the collection--you copy first, then if (and only if) that succeeds, you remove it from the collection.

If you're sure you'll never use this with a type whose copy/move ctor can throw, what you're doing should be fine though. Unfortunately, you're using it with std::string, which has a copy ctor that can throw.

YAGNI

You have some of what might be termed "YAGNI violations". For example:

void NetworkServer::SendToAll(std::string message, bool guaranteed)
    {
        for (auto client: clients)
            send(message, client.right);
    };

Here you're passing the function a bool, apparently to indicate whether it should attempt to guarantee delivery, but the function completely ignores that value. At least in normal use, such a guarantee basically is specified when you create the initial connection (TCP guarantees delivery, UDP doesn't).

bool parameters

You're using a bool as a parameter in a non-obvious way. I normally recommend against bool parameters in general. There are exceptions, but in this case it's not immediately obvious how SendToAll("whatever", true) and SendToAll("whatever", false) differ, and what the parameter is intended to mean (though, as noted above, in this case it means absolutely nothing).

Assuming you fix the code so the parameter means something, it would probably be better to replace the bool with an enum so the intent is directly visible in the code, something like this:

enum { ATTEMPT, GUARANTEE };

SendToAll("whatever", ATTEMPT);

...or, given C++11, you probably want to use an enum class instead:

enum class delivery { ATTEMPT, GUARANTEE };

SendToAll("whatever", delivery::ATTEMPT);

The same comment applies to SendToAllExcept.

member initializer lists

Prefer member initialization lists for initialization. An obvious example would be:

receivedBytes = sentBytes = receivedMessages = sentMessages = 0;

(inside the body of a ctor). In this case, you're probably better off with something like:

NetworkClient::NetworkClient(std::string host, std::string server_port, unsigned short local_port) : 
    socket(io_service, udp::endpoint(udp::v4(), local_port)), 
    service_thread(std::bind(&NetworkClient::run_service, this)),
    receivedBytes(0),
    sentBytes(0),
    receivedMessages(0),
    sentMessages(0)
{
    udp::resolver resolver(io_service);
    udp::resolver::query query(udp::v4(), host, server_port);
    server_endpoint = *resolver.resolve(query);
    Send("");
}

Alternatively, you might want to at least consider a small type specifically for counters:

class counter {
    size_t count;
public:
    counter &operator=(size_t val) { count = val; return *this; }
    counter(size_t count=0) : count(count) {}
    operator size_t() { return count; }
    count &operator++() { ++count; return *this; }
    count operator++(int) { counter ret(count); ++count; return ret; }
    bool operator==(counter const &other) { return count == other.count; }
    bool operator!=(counter const &other) { return count != other.count; }
};

With this, you simply define sentBytes, receivedBytes and so on as objects of the counter class, and it's impossible to create an object of that type that's not initialized.

prefer portability when possible

For one obvious example, you use unsigned __int64 in a number of places. __int64 is specific to VC++. Lacking a reason to do otherwise, I'd prefer to use unsigned long long, which works just as well on VC++, but also works with any other conforming implementation (of C++11). The counterpoint is that before C++11, some compilers (most notably VC++) did not support long long. If that's a concern, I'd use an intermediate typedef:

#ifdef _MSC_VER
typedef unsigned __int64 ulonglong;
#else
typedef unsigned long long ulonglong;
#endif

...then the rest of the code would use ulonglong. If you need more compiler-specific code than above, you'd still just change it in one place by adding a typedef for that specific compiler, with (hopefully) no changes necessary to the rest of the code.

const correctness

For example, you have four functions:

inline unsigned __int32 GetStatReceivedMessages(){return receivedMessages;};
inline unsigned __int64 GetStatReceivedBytes(){return receivedBytes;};
inline unsigned __int32 GetStatSentMessages(){return sentMessages;};
inline unsigned __int64 GetStatSentBytes(){return sentBytes;};

Since these functions should not modify the state of the object, they should probably be const member functions:

```
inline unsigned

StackExchange Code Review Q#51235, answer score: 12