7
\$\begingroup\$

Writing a MMORPG server in nodejs, I have to handle packets. These packets have a structure of

<length> <id> <data>

So what I did was use a node package called packet

And first get the packet id:

client.on('data', function(data) {
  parser.extract("l16 => length, l16 => id", function (packet) {
    if (packet.id === 300) {

    }
    if (packet.id === 301) {

    }
    if (packet.id === 302) {

    }
    if (packet.id === 303) {

    }
  });

  // parse the packet id to handle it accordingly
  parser.parse(data);
});

There are packet Id's upto 1000+ which would make my if statements very long. But each packet id contains different data and different ways to handle it.

For example packet 300 which is the handshake packet. It sends the client version to the server and the servers checks it and returns if it is correct or not, if not then client will error wrong version.

  if (packet.id === 300) {
    console.log('[recieved] packet:handshake');

    parser.extract("handshake", function (packet) {
      if (packet.gameClientVer === config.gameClientVer) {
        // do stuff
      } else {
        // do stuff 
      }
    });

    // parse structure for packet 300
    parser.parse(data);
  }
});

the handshake packet is defined as:

serializer.packet("handshake", "l16 => length, l16 => id, l16 => gameClientVer, l16 => gameUpdateVer, l16 => gameDateVer");

My question is, how can I improve this?

\$\endgroup\$
4
\$\begingroup\$

I would have a function per packet.id:

function handleHandShake(packet, data)
{
  //Do not use console.log for mmorpg's, write to a text file if you must
  //console.log('[recieved] packet:handshake');

  parser.extract("handshake", function (packet) {
    if (packet.gameClientVer === config.gameClientVer) {
      // do stuff
    } else {
      // do stuff 
    }
  });

  //parse structure for packet 300
  parser.parse(data);
}

I would go for a packet.id based routing table:

var routes = [
  '300' : handleHandShake,
  '301' : addExperience,
  etc. etc.
]

Then, you can extract the packet.id and execute the proper function

client.on('data', function(data) {
  parser.extract("l16 => length, l16 => id", function (packet) {
    if( routes[packet.id] ){
      routes[packet.id](packet, data);
    } else {
      //Do something ( logging? ) if we get unknown packet id's
    }
  });

  // parse the packet id to handle it accordingly
  parser.parse(data);
});

It was fun learning about the package module, great question.

\$\endgroup\$
  • \$\begingroup\$ This seems like a good approach. Care to explain why I shouldn't be console.logging on MMORPG? Does this have something to do with performance? \$\endgroup\$ – majidarif Apr 2 '14 at 15:07
  • \$\begingroup\$ Yes indeed, console.log is synchronous and will murder performance \$\endgroup\$ – konijn Apr 2 '14 at 15:10
  • \$\begingroup\$ Oh, right. I should just make those logs available for debugging. Thanks \$\endgroup\$ – majidarif Apr 2 '14 at 15:25
4
\$\begingroup\$

You should compartmentalize your "handlers" for each of the packets. Each handler should contain a canHandle method that determines whether it can process the packet and a process method that does the work. Example:

var packet300Handler = {
    canHandle: function(packet) {
        return packet.id === 300;
    },
    process: function(packet) {
        // Do work here.
    }
};

var handlers = [packet300Handler, packet301Handler, packet302Handler];

client.on('data', function(data) {
    parser.extract('DATA', function(packet) {
        for (var i = 0; i < handlers.length; i++) {
            var handler = handlers[i];

            if (handler.canHandle(packet)) {
                handler.process(packet);
                break; // Leave loop now that something has handled the packet.
            }
        }
    });
});
\$\endgroup\$
  • \$\begingroup\$ Although the cost probably wouldn't be too bad, I don't see why you'd loop through an array looking for an element that can handle a given packet rather than just having an associative array keyed on packet.id and referring directly to the process function (as suggested in the other answer). \$\endgroup\$ – PeterT Apr 2 '14 at 15:23

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.