0

I have a Netty-based game server implementation that handles 40 or so distinct packets with their own serialization structure, for brevity I'll refer to them as FooPacket, BarPacket, ... These packet types have been assigned consecutive numerical "packet types", which are unsigned bytes. For example, FooPacket is type 0x01, BarPacket is type 0x02, etc...

All of these packets extend an abstract class, MyGameNetworkPacket.

My current approach with Netty is to use an initial decoding layer to convert a TCP stream into a series of unparsed packet objects, which are defined as consisting of a packet type, length, and buffer containing the actual packet contents. (no byte stuffing is performed). I then feed this packet down the pipeline to one of a few "deserializing processors", each of which handles a set of packet types that are semantically related to each other (for example one such processor might handle connection state messages, while another may handle packets related to a certain aspect of the game itself). As per the design of Netty's MessageToMessageDecoder usage, my decoder is implemented as:

protected void decode (ChannelHandlerContext ctx, UnparsedPacket msg, List<Object> out)
        throws Exception {
    switch (msg.command) {
        case FooPacket.COMMAND_ID:
            FooPacket fooPacket = new FooPacket(msg.frame);
            out.add(fooPacket);
            break;
        case BarPacket.COMMAND_ID:
            BarPacket barPacket = new BarPacket(msg.frame);
            out.add(barPacket);
            break;
    }

}

Once I get a structure decided upon I can easily create this with a code generator.

In addition, I override public boolean acceptInboundMessage(Object) to specify if an unparsed packet should be handled by this decoder.

This is attaining a bit of what appears to be a code-smell for me, so I was looking into alternative solutions, such as:

  • Having an array whose (integer keys) map to values of a lambda that takes a RawPacket and returns the parsed packet. This seems like a somewhat weird way to use an array and requires that I manually maintain the definition of that array so that its index-element mapping exactly matches the actual packet type to packet structure mapping already in use.
  • Having a map structure that does the same thing as the array. Similar pitfalls, but somewhat more maintainable.

Are these any more novel ways to approach this that I haven't thought of yet?

1

Ideally, you would override the decode method for each packet type.

But the decode method isn't part of the specific subclass. That is why the switch, or a map that returns the packet-specific factory is justified. It smells because you have to repeat the same code over and over and the method has dependencies on all 40 subclasses. But if the code is generated, I don't see a problem there.

If you want more independence and avoid repeating the same code for each and every subclass, you could map your integer to the packet-specific Class, create an instance from the class and ask the instance to initialize itself from the frame.

You would have something like:

static Class<? extends MyGameNetworkPacket>[] packetClasses;
static {
    packetClasses = new Class[50];
    packetClasses[1] = FooPacket.class;
    packetClasses[2] = BarPacket.class;
    packetClasses[5] = GimmeFivePacket.class;
}

And in the decode method:

Class<? extends MyGameNetworkPacket> packetClass = packetClasses[msg.command];
if( packetClass != null ){
    MyGameNetworkPacket pkt;
    try {
        pkt = packetClass.newInstance();
    } catch( InstantiationException | IllegalAccessException e ){
        throw new RuntimeException("Failed to create instance.", e);
    }
    pkt.fillFrom(msg.frame);
    out.add(pkt);
} else {
    throw new RuntimeException("Invalid command ID: " + msg.command + ".");
}

It doesn't look very elegant because of the try/catch for errors even though they cannot happen if the classes were written properly. But it reduces the dependency on the subclasses to the reference to their class. You could even initialize the array from an external configuration file that contains the class name for each command ID.

  • My concern with the reflective option is to put reflection into a potentially-critical section of code. I could easily see large amounts of data being sent (for example during initialization of a session over a LAN). I'm going to try some benchmarks and see what comes out. – ζ-- Aug 30 '14 at 19:28
4

What you have described, in one way or another, is exactly how anyone would do it and I don't see any problem with this method.

On a side note: you don't have to use lambda functions in your map/array. A simple interface bound method would be cleaner.

  • "What you have described" refers to which of my approaches? The switch-statement-based one, or the lambda/interface-bound-method based one? – ζ-- Aug 29 '14 at 21:52
  • Any combination thereof. Using the generic packet format and relay it down the chain either directly, as events, or as part of a map. All the idea mentioned are valid what you choose to use is up to you based on how you want to manage the code. – lots of crisps Aug 29 '14 at 21:54

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.