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?

asked Apr 2, 2014 at 7:29
\$\endgroup\$

2 Answers 2

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.

answered Apr 2, 2014 at 14:41
\$\endgroup\$
3
  • \$\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\$ Commented Apr 2, 2014 at 15:07
  • \$\begingroup\$ Yes indeed, console.log is synchronous and will murder performance \$\endgroup\$ Commented Apr 2, 2014 at 15:10
  • \$\begingroup\$ Oh, right. I should just make those logs available for debugging. Thanks \$\endgroup\$ Commented Apr 2, 2014 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.
 }
 }
 });
});
konijn
34.2k5 gold badges70 silver badges267 bronze badges
answered Apr 2, 2014 at 12:38
\$\endgroup\$
1
  • \$\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\$ Commented Apr 2, 2014 at 15:23

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.