[C#]Working with received packets

[shout]

I have been working on how to.. uh... do stuff... with the packets I receive. This was my idea...

Code Select Expand


      private void BNETConnection_Received(object sender, byte[] packet)
      {
         byte[] ServerToken;
         string CRValueString;
         if (packet[1] = 0x50)
            Shout.BnetPacketDisassembler.P0x50(packet, out ServerToken, out CRValueString);
         if (packet[1] = 0x25)
         {
            byte[] PingValue = Shout.BnetPacketDisassembler.P0x25(packet);
            BnetPacket PingPacket = new BnetPacket(0x25);
            PingPacket.InsertDWORD(PingValue);
            base.Send(PingPacket.Data);
         }
      }


Edit: Spotted bug in code :/

I know I only have two packets, its not done yet. Any suggustions or anything?

[iGotPropz]

Well, from some experiencefrom viewing random code here, I am not 100% sure and I am not sure if your code is working for you.

But , when I see..

Code Select Expand

packet[1]

..it makes me wonder if that truely will pick out the packet ID after 0xff in the packet header.

And aslo, I suggest a switch case not if() statements

Code Select Expand


switch(packetID)
{
      case 0x50;
      //Packet stuff
      break;

      case 0x25;
      //Packet Stuff
      break;
}


But then again, that is my preference. I use page switches in PHP (almost same thing) and it is a lot cleaner, and just like using IF's but to me it seems easier to manage.

You would, above the switch case, find out what the packet ID is then create that as the variable "packetID" which will execute the switch case.

That's my input..I'm just now also entering the receivng of packets with my bot.

EDIT: Typo errors... --___--

[shout]

Just so you know, its

Code Select Expand

case 0x50: instead of

Code Select Expand

case 0x50;

So... this might work a little better...

Code Select Expand


     private void BNETConnection_Received(object sender, byte[] packet)
     {
        if (packet[0] = 0xff)
        {
           byte packetID = packet[1];
           PacketStuff(packet, packetID);
        }
     }

     private void PacketStuff(byte[] packet, byte packetID)
     {
        byte[] ServerToken;
        string CRValueString;
        switch (packetID)
        {
           case 0x50:
           {
              Shout.BnetPacketDisassembler.P0x50(packet, out ServerToken, out CRValueString);
              break;
           }
           case 0x25:
           {
              byte[] PingValue = Shout.BnetPacketDisassembler.P0x25(packet);
              BnetPacket PingPacket = new BnetPacket(0x25);
              PingPacket.InsertDWORD(PingValue);
              base.Send(PingPacket.Data);
              break;
           }
        }
     }


Anyone got an idea for a better name then PacketStuff? I just woke up and cant think of a better name @.@

[MyndFyre: combined two posts into one.]

[MyndFyre]

This is a good start.  However, you might consider parsing your packet in another method than the Socket_Received method.  Here's why.

When you have a lot of local variables in a function (and you will), they take time to initialize.  When you parse all of your data within a function that is called as an event, you run the risk of having two threads operate on data at the same time.  As I recall, in .NET, the way events handle multithreading the same method can cause locals to be overwritten.  If you start an independent method, though, you might be better off.  That's how my bot is structured, and I've never had a problem with it; I'm not sure about the corrupt locals though.  If you get weird data, I'd look into it.

Second, you might want to look at this C# interface that I wrote, or this VB 6 class that does pretty much the same thing.  It acts like a stream on a byte array, and then lets you read data as digestible chunks, and then advances where you currently are.  So, if you read a DWORD from the stream, it will advance by four bytes.

I agree with iGotPropz's assessment that you should use the switch/case statement.  It will end up with the same IL (and machine code) in the end, but the code will be infinitely clearer.

Also, checking to make sure that (packet[0] == 0xff) -- oooh look at your code!  That couldn't compile -- if has to have a boolean expression.  You're making an assignment of 0xff to packet[0]!  bad!  Anyway, you're not providing the infrastructure to combine split packets nor split combined packets.  Make sure you check to make sure that the given length is the actual length of the packet you receive.

Ewww shout, these assignments are all over your code.  Equality testing is done using the operator ==, not =.  One = is assignment.

[shout]

Ooooooops...

Correct me if I'm wrong but all of the battle.net packets only have 0xff in the header so...

Code Select Expand


      private void BNETConnection_Received(object sender, byte[] packet)
      {
         int PacketLength;
         if (packet[0] == 0xff)
         {
            PacketLength = (int)packet[2] * 256 + (int)packet[3];
            if (PacketLength > packet.Length)
            {
               IncompletePackets.Add(packet);
            }
            else if (PacketLength < packet.Length)
            {
               FusedPackets.Add(packet);
            }
            else
            {
               PacketStuff(packet, packet[1]);
            }
         }
         else
         {
            IncompletePackets.Add(packet);
         }
      }


IncompletePackets and FusedPackets are ArrayLists, I am going to change that sometime in the near future.

I am currently working on a better way to handle the good packets.

Also, I wrote a class called PacketDebuffer, and it seems to work.

[K]

Code Select Expand


// on receive:
// where b is the array of bytes received
ProcRaw(b);


Code Select Expand


     private List<byte> _bBuffer = new List<byte>();

     private void ProcRaw(byte[] b)
     {
           _bBuffer.AddRange(b);

           while (_bBuffer.Count  >= 4)
           {
                 ushort len = BitConverter.ToUInt16(_bBuffer.GetRange(2, 2).ToArray(), 0);
                 if (_bBuffer.Count < len)
                       break;

                 BncsPacket p = BncsPacket.FromBytes(_bBuffer.GetRange(0, len).ToArray());
                 _bBuffer.RemoveRange(0, len);

                 CaughtPacket(p);
           }
     }


Code Select Expand


      protected void CaughtPacket(BncsPacket p)
       {
#if _DEBUG
         Console.WriteLine("RECV: 0x{0:X}", p.ID);
         Console.WriteLine(p.ToHexString());
#endif
         switch (p.ID)
           {
               case BncsPacketID.AuthCheck:
                   mOnAuthCheck(p);
                   break;

               case BncsPacketID.AuthInfo:
                   mOnAuthInfo(p);
                   break;

               case BncsPacketID.ChangePassword:
                   mOnChangePassword(p);
                   break;

               case BncsPacketID.ChatEvent:
                   mOnChatEvent(p);
                   break;

               case BncsPacketID.CreateAccount:
                   mOnCreateAccount(p);
                   break;

               case BncsPacketID.EnterChat:
                   mOnEnterChat(p);
                   break;

               // etcetera:

               default:
                   mOnUnhandled(p);
                   break;
           }
       }



Edit: You may have noticed that I'm using Generics; they're available in the .NET 2.0 beta.

[MyndFyre]

I love generics....