Not so much a problem but more of an coding execution standpoint.

I am in the middle of programming a game server using C++ & Winsock. All good so far. 8)

I am making a network class & I have the app at the stage now where the client will accept and disconnect multiple clients gracefully and bug free (as far as I can tell).

I am now stuck on how I shoud handle the recieve code.

The incoming data will be of a variable size with the first byte (or two) being the identifier of the incoming packet type.

[Byte1 (Idenifier][Byte2... ByteN (Data)] // Data length will be constant depending on what the identifier is.

The problem is where I should handle the recieved data. If I was going to have the class reconstuct the incoming data and check for packet boundaries, the class would have to know about all of the data types, which would then decrease re-usablility (as the class would then be 'custom written' for the application.

However, if I get the class to just fill a 'client buffer' for each incoming socket connection, I could get the application to reconstruct the data (and not the class). But, then this semi-defeats the purpose of having the network class.

Any ideas on how I can get the class to handle the incoming data better?

Thanks in advance.

Actually, I may have just brainstormed a method of handling this issue.

I'll get back to you shortly (coding it now). :)

You could have the network class provide a safe and controlled way for each class to read from the network socket, according to a well-defined protocol. That way only the packet identifier is read, and the network class then delegates the work of parsing the remaining data to whatever class should be parsing it, elegantly. And you can still stream from the socket, instead of having to grab the entire packet at once. In this sense, the network class provides a level of abstraction, which is what you wanted.

If you are concerned about reusability, you should cleanly separate parsing (reading data from the packet) and actual work done. Ideally you should be able to simply strip out the parsing code and still have a working class. I don't think creating a dedicated network/class interface for each class is really going to make things better, it's probably overkill (do you really want to deal with EnemyParser, TerrainPacket, and so on?!)

Optimization generally means punching through layers and putting knowledge in places where it doesn't normally belong.

You have to make some trade-offs. Is it worth it saving a few bytes, and having to put knowledge of the protocol into the network class? If not, then you should length-prefix each packet, and you'll be fine. Have the sender put in length, type, payload, and you're good. The network class can then de-frame packets to the type,payload level, and pass that on to the registered handler for each type.

class PacketHandler {
public:
  virtual void HandlePacket(size_t size, void const *data) = 0;
};
 
class Network {
public:
  void AddPacketHandler(int type, PacketHandler *handler);
private:
  size_t CheckBuffer(size_t size, void const *data);
};
 
size_t Network::CheckBuffer(size_t size, void const *data) {
  size_t packetsize = 0;
  size_t offset = 0;
  if (!TryDecodeSize(size, data, &offset, &packetsize)) {
    return 0;
  }
  if (size - offset < packetsize) {
    return 0;
  }
  int type = 0;
  size_t typesize;
  if (!TryDecodeType(size-offset, (char *)data + offset, &typesize, &type)) {
    return packetsize + offset; // bad packet, skip past it
  }
  PacketHandler *handler = LookupTypeHandler(type);
  if (!handler) {
    return packetsize + offset; // unknown packet, skip it
  }
  handler->HandlePacket(packetsize - typesize, (char *)data + offset + typesize);
  return packetsize + offset;
}

If saving that length byte (or two?) matters, then you have to do something like registering type-specific protocol decoders with the networking subsystem. You'd then find the decoder class for the given type code, after you've decoded the type code, and ask that class whether there's enough data to actually dispatch the packet. This means the decoder code may run several times for a given packet before you have enough data to actually dispatch the packet.

class PacketDecoder {
public:
  virtual bool CanDecodePacket(size_t available, void const *start, size_t *o_size) = 0;
};
 
class PacketHandler {
public:
  virtual void HandlePacket(size_t size, void const *data) = 0;
};
 
class Network {
public:
  void AddPacketDecoder(int type, PacketDecoder *decoder);
  void AddPacketHandler(int type, PacketHandler *handler);
private:
  bool CheckBuffer(size_t size, void const *data) = 0;
};
 

size_t Network::CheckBuffer(size_t size, void const *data) {
  int type = 0;
  size_t typesize;
  if (!TryDecodeType(size, (char *)data, &typesize, &type)) {
    return 0; // don't have type yet
  }
  PacketDecoder *decoder = LookupTypeDecoder(type);
  if (!decoder) {
    exit(1); // the connection is in a bad state and there is no way to recover
  }
  size_t packetsize = 0;
  if (!decoder->CanDecodePacket(size - typesize, (char *)data + typesize), &packetsize) {
    return 0; // can't decode packet yet
  }
  PacketHandler *handler = LookupTypeHandler(type);
  if (!handler) {
    exit(1); // the network system is mis-configured and can't proceed
  }
  handler->HandlePacket(packetsize, (char *)data + typesize);
  return packetsize + typesize; // packet consumed
}

 

Thanks for your tips guys 8)

@hplus0603 - I didn't even think of sending the size first. That would probably solve my issue.

I could do something like this;

[Byte 1-2 (Size)][Byte 3-4 (Identifier)][Byte 5-n (Payload)]

This way the network class can reconstruct the data and pass it on to the main app (without the network class having to know what the data stream is).

The way I am planning this game to go, network traffic will be minimal (possibly once per second at peak). So, 'trigger speeds' will not be an issue.

Unless your going to have more than 255 messages I think the message identifier/type only needs to be one byte. The size could be a 16-bit integer.