[Question]New way of packet handling?

[stealarcher]

I have been thinking, and to me it seems that a lot of the packet handlers (especially LOTF) are pretty large. Would it take up more resources to do something more dynamic such as the following? This is just an example obviously. But the main concept is there. The void would be your PacketID, the Arg1 would be your data[]. And it simply calls the method immediately rather then going through a case?

    class Program
    {
        static void Main(string[] args)
        {
            string Void = Convert.ToString(Console.ReadLine());
            string Arg1 = Convert.ToString(Console.ReadLine());
            object[] Args = new object[1];
            Args[0] = Arg1;
            try
            {
                typeof(Program).InvokeMember(Void, BindingFlags.InvokeMethod, null, null, Args);
            }
            catch
            {
                Console.WriteLine("Missing Packet Handler " + Void);
            }
            Console.ReadKey();
        }
        public static void Chat(string Arg1)
        {
            Console.WriteLine("Chat Handler " + Arg1);
        }
        public static void General()
        {
            Console.WriteLine("General Data Handler");
        }
    }

[pro4never]

Not thought of that concept before... so basically you'd do something like an enum for packets and then just do a try/catch trying to invoke a member of that name?


IE:

Enum PacketTypes: ushort
{
ItemUsage = 1009,
}

public void ItemUsage(Client Sender, byte[] Data)
{
//handle ItemUsage
}

Then in your receive packet do something like...

ushort Type = ReadUInt16(Data, 2);

Try to invoke the enumerated method for the type read

catch

print unknown packet needing to be handled.

[stealarcher]

yupp pretty much was the idea xD, but i thought it may use too many resources.

#edit, now that i think about it, it would still be the first method it tries to call, so its not really doing much extra searching, so i dont really see it causing too big of an issue. It may actually save some considering it doesnt have to go through a case or if

#edit #2
Further Example. if ur trying it, u have to add the using System.Reflection; at the top to get the bindingflags.invokemethod. Also if you wanted the handler in a different class, you could replace the typeof(program) with something such as typeof(packethandler)

        enum PacketType : ushort
        {
            ItemUsage = 1009,
        }
        public static unsafe void Handle(GameClient GC, byte[] PData)
        {
            int Position = 0;
            while (Position < PData.Length)
            {
                ushort PacketLength = BitConverter.ToUInt16(PData, Position);
                ushort PacketID = BitConverter.ToUInt16(PData, Position + 2);
                byte[] Data = new byte[PacketLength + 8];
                Buffer.BlockCopy(PData, Position, Data, 0, PacketLength + 8);
                PacketType PT = (PacketType)PacketID;
                object[] args = new object[1];
                args[0] = Data;
                try
                {
                    typeof(Program).InvokeMember(PT.ToString(), BindingFlags.InvokeMethod, null, null, args);
                }
                catch
                {
                    Console.WriteLine("Missing Packet Handler " + PacketID);
                }
            }
        }
        public static void ItemUsage(byte[] Data)
        {
            //Handle Item Usage here
        }

[FuriousFang]

Lol, it's better than the traditional LOTF way of handling packets. =]
Switch statements seem to be the way to go atm, but this is another interesting way of doing it. You're right though, it MIGHT take more resources (though I don't see how, you're still calling methods and redirecting packets by some kind of identification).

[Korvacs]

Invoking is slightly more expensive and time consuming than calling it directly.

[stealarcher]

Alrighty, thanks for the feedback guys.

[_Emme_]

This is how I've always done it:

Kernel:

public delegate void PacketHandlerDelegate(BasePacket Packet, GameClient Client);
public static Dictionary<PacketType, PacketHandlerDelegate> PacketHandlerList = new Dictionary<PacketType, PacketHandlerDelegate>();

GameHandler:

RegisterPacketHandler(PacketType.GameConnectPacket, PacketHandler.HandleGameConnectPacket);

RegisterPacketHandler(PacketType.CreateCharacterPacket, PacketHandler.HandleCreateCharacterPacket);

RegisterPacketHandler(PacketType.DataPacket, PacketHandler.HandleDataPacket);


public static void RegisterPacket(PacketType Type, Kernel.PacketHandlerDelegate Handler)
        {
            Kernel.PacketHandlerList.Add(Type, Handler);
        }

GameReceive:

PacketBuffer Buffer = new PacketBuffer(Packet, Packet.Length);
                BasePacket NewBuffer;
                for (UInt16 i = 0; i < Buffer.Length; i += (UInt16)(NewBuffer.PacketLength + 8))
                {
                    UInt16 Length = Buffer.ReadUInt16(i);
                    if (Length <= Packet.Length - i)
                    {
                        NewBuffer = new BasePacket(Buffer.ReadSubPacket(i, Length));
                        PacketType Type = NewBuffer.PacketType;
                        if (Kernel.PacketHandlerList.ContainsKey(Type))
                            Kernel.PacketHandlerList[Type].Invoke(NewBuffer, Client);
                        else
                            IConsole.WriteLine(string.Format(HexDump("[Game] [Unknown Packet] {0}", NewBuffer.Buffer), Type), ConsoleColor.Red);
                    }
                    else
                        return;
                }

Then at the actual functions, all you need to do is:

public static void HandleGameConnectPacket(BasePacket Packet, GameClient Client)
{
}

Hope that helps!

[stealarcher]

nice, it seems to work better 4 u then switches then im assuming, since you have always done it that way. starting my own custom source just debating on the best way to do it. b4 I end up with something that cant hold 10 players lmao

[Korvacs]

Invoking is a more expensive way of doing it theres no two ways about it, the performance gain is virtually none existant unless your switch is going to contain in excess of 60 or 70 statements.

[stealarcher]

how many known packets are there that are received by the server now days, around 30-40?

[_Emme_]

Originally Posted by Korvacs

Invoking is a more expensive way of doing it theres no two ways about it, the performance gain is virtually none existant unless your switch is going to contain in excess of 60 or 70 statements.

Again, if you're not planning on running a CO server for a living, but rather a way of learning how to program in a fun way, you don't always have to use the best way of doing it just for this case (in my opinion that is).

Sure, CO may not exceed ~70 packets, although if you're planning on bigger projects in the future you might want to learn other teqniques that can benefit for all kinds of solutions; because the way it is right now most of us know how to write a simple switch statement, so I defnitly think you should try different teqniques that may not benefit in this case, but will in the future.

[Korvacs]

For testing/experimental/learning purposes try it out yes, but using it for the sake of it is bad practice, your just coding in inefficiencies if you believe that method is the best you clearly have never tested it against a switch of equal size nor read material on tests which demonstrate that it is inefficient when compared to a switch if the number of entries is less than around 60/70.

[ChingChong23]

make an interface with the void handlepacket, create an array store the index as an instance to an implemented interface, call the void from the interface that you get using the packet id as the index.

[Basser]

This is no new concept.

        private delegate void Arguments();
 
        private readonly IDictionary<PacketType, Arguments> Processors;
 
        public Processor()
        {
            Processors = new Dictionary<PacketType, Arguments>
                             {
                                 {PacketType.DataPacket, DataPacket},
                                 {PacketType.GameConnectPacket, GameConnectPacket},
                                  etc..
                             };
        }
 
        public void Process(byte[] _input, Client _client)
        {
            Packet = new PacketBuffer(_input);
            PacketType Type = (PacketType)Packet.Type;
            Arguments Handler;
            Processors.TryGetValue(Type, out Handler);
            if (Handler != null)
                Handler.Invoke();
            else
                Invoke.WriteLine("Designated handler for " + Type + " was not found.");
        }

[unknownone]

Korv is right that there's no performance gain to get from this, but there is absolutely a gain otherwise (in terms of code management).

I'll just make clear on Korv's mention of invoking being expensive. In the case of the OPs example where he is using reflection to invoke a method by name, there is a clear measurable difference in performance - however, in the example of Emme's, where he is invoking a delegate, the performance difference is completely negligible. (There is a difference, but it's a few nanoseconds compared to a direct method call, of no concern at all). A delegate is essentially a function pointer, and has around the same cost as a virtual method call.

The additional performance cost in Emme's example comes from searching the dictionary for a value though. It's a relatively small cost, hardly relevant. The cost of accessing a dictionary is approx O(1). The same isn't true for a switch, as it needs to (potentially) fall through each case, so at worst will be O(n). (irrelevant unless you have thousands of cases still though). The dictionary approach scales better, and even with a small number of cases is very much a non-issue.

On Emme's code being "bad practice" (ignoring the static fields, which IS bad practice), certainly true if you're using it "for the sake of it", but you don't just use it for the sake of it. You use it because it's a proper object oriented approach to the problem which allows you to completely decouple your packet processor from any implementation details - leading to code which is easier to test, maintain and extend. Being bad practice couldn't be further from the truth - loosely coupled code is good practice. (You can potentially decouple a switch from implementation if done right, but I've not seen that around here yet.)

In the "big switch" approach I've seen in many sources on here, everything is tightly coupled, because the packet processing class depends on just about everything else (which is why people end up resorting to using static fields, because their code doesn't follow established OOP practices). It's impossible to unit test the packet processor, because it depends on the entire server, and can only be tested as a whole. That's completely bad practice by any standard, and if your reason for doing it is to shave a fraction of a percent off the performance of your application, I'd question why you're writing in C#.