A Message to all developers

[Ultimation]

I have been taking some time and going over peoples sources.. and peoples modifications..

I would like to point out a few things.

OOP = Object oriented programming..

Quote from wikipedia

Object-oriented programming (OOP) is a programming paradigm based on the concept of "objects", which are data structures that contain data, in the form of fields, often known as attributes; and code, in the form of procedures, often known as methods. A distinguishing feature of objects is that an object's procedures can access and often modify the data fields of the object with which they are associated (objects have a notion of "this"). In object-oriented programming, computer programs are designed by making them out of objects that interact with one another.[1][2] There is significant diversity in object-oriented programming, but most popular languages are class-based, meaning that objects are instances of classes, which typically also determines their type.

This means 2 very big things you guys seem to get the grasp.

use constructors... Why do so many sources use functions as constructors...

Example..

var actionpckt = ActionPacket.Create(); ??????? or
var actionpckt = new ActionPacket().Create(); ??????????

this is just plain ignorance. Get out of your c++/delphi ways

var actionpckt = new ActionPacket(); //Clean and simple

Second thing.

STOP putting all your fucking handlers in 1 class in 1 file....

You have ways todo this.. 1 class handler per packet..

OR

Partial Classes! Take your pick!

Example

Action Handler.cs
public partial class PacketHandler
{
public void ProcessAction(params)
{

}
}

Item Handler.cs
public partial class PacketHandler
{
public void ProcessItem(params)
{

}
}

Third Thing

If else statements...

Dont use them if you dont need to.

doing

If (packet.type == xxxx)
{
}
else
if (packet.type == xxxx)
{
}

is just pure lazy and inefficient.. Use the CASE statement

switch (packet.type)
{
case xxxx:
{
PacketHandler.ProcessAction(packet);
}
case xxxx:
{
PacketHandler.ProcessItemUsage(packet);
}
}

Finally...

USE SEARCH!


STOP BEING SO LAZY PEOPLE AND CODERS MAY HELP YOU! AND THIS COMMUNITY MAY JUST MAY GET BACK ON TRACK!

[.Ocularis]

I see what you're saying about using constructors, however.. some packet archetypes have different fields, like general action and item action. Using a method after the constructor to construct the archetype is a bad practice? If a constructor were made for each individual packet archetype some parameters would make several constructors ambiguous even though each constructor would write those values to different fields..

Edit:
My only other thought to avoid using methods to construct archetypes is to use an abstract structure and inheritance for the individual types, serializing them as the abstract structure in the constructor of the packet.

[Ultimation]

Let me paste my packets for some types.. They may be incomplete but the concept is there

 
public class MessagePacket:IPacket
    {
        public string From="SYSTEM";
        public string To="ALLUSERS";
        public string Message="";
        public ChatID ChatID=ChatID.Talk;
        public int Avatar = 0;
        public int Color = 0xFFFFFF;
        public MessagePacket(string from, string to, string message,ChatID chatid=ChatID.Talk)
        {
            From = from;
            To = to;
            Message = message;
            ChatID = chatid;
        }
        public MessagePacket(string To, string Message, ChatID chatid = ChatID.Talk)
        {
            this.To = To;
            this.Message = Message;
            ChatID = chatid;
        }
        public MessagePacket()
        {
            
        }
        public MessagePacket(string Message, ChatID chatid = ChatID.Talk)
        {
            this.Message = Message;
            ChatID = chatid;
        }

        public int PacketType { get { return 0x3EC; }}
        public byte[] Build()
        {
            using (var wtr = new PacketWriter(0x3EC))
            {
                wtr.Write(Color);
                wtr.Write((int)ChatID);
                wtr.Write(0);
                wtr.Write(0);
                wtr.Write(Avatar);
                wtr.WriteStringList(From,To,string.Empty,Message);
                return wtr.ToArray();
            }
        }

        public void Read(byte[] data)
        {
            using (var br = new BinaryReader(new MemoryStream(data)))
            {
                br.ReadInt32();
                Color = br.ReadInt32();
                ChatID = (ChatID) br.ReadInt32();
                br.ReadInt32();
                br.ReadInt32();
                Avatar = br.ReadInt32();
                var dat= br.ReadStringList();
                From = dat[0];
                To = dat[1];
                Message = dat[3];
            }
        }

public class ItemUsagePacket:IPacket
    {
        public int PacketType { get { return 0x3F1; }}
        public int UID;
        public int dwParam1;
        public ItemUsuageID ID;
        public int TimeStamp;
        public int dwParam2;
        public int dwParam3;
        public int[] ItemIDS = new int[16];

        public ItemUsagePacket() {  }

        public ItemUsagePacket(int PlayerID)
        {
           
            TimeStamp = Environment.TickCount ^ PlayerID;
            UID = PlayerID;
        }
        public byte[] Build()
        {
            using (var wtr = new PacketWriter(0x3F1))
            {

                wtr.Write((int)UID);
                wtr.Write((int)dwParam1);
                wtr.Write((int)ID);
                wtr.Write((int)TimeStamp);
                wtr.Write((int)dwParam2);
                wtr.Write((int)dwParam3);
                wtr.Write((int)0);
                foreach (int id in ItemIDS)
                    wtr.Write((uint)id);
                return wtr.ToArray();
            }
        }

        public void Read(byte[] data)
        {
            using (var rdr = new BinaryReader(new MemoryStream(data)))
            {
                rdr.ReadInt32();
                UID = rdr.ReadInt32();
                dwParam1 = rdr.ReadInt32();
                ID = (ItemUsuageID)rdr.ReadInt32();
                TimeStamp = rdr.ReadInt32();
                dwParam2 = rdr.ReadInt32();
                dwParam3 = rdr.ReadInt32();
            }
        }

Originally Posted by .Ocularis

I see what you're saying about using constructors, however.. some packet archetypes have different fields, like general action and item action. Using a method after the constructor to construct the archetype is a bad practice? If a constructor were made for each individual packet archetype some parameters would make several constructors ambiguous even though each constructor would write those values to different fields..

Regarding bad practise.. there should never be a need to use an Ambiguous function as a constructor for a class

[.Ocularis]

Aha.
Now I see.. my style and perfectionism doesn't like:

new Action(){uid = 1, type = 2, argument = 3};

That's why I'd use methods... but you're right, using methods is totally excessive and retarded.
Will work on it.

[Ultimation]

Originally Posted by .Ocularis

Aha.
Now I see.. my style and perfectionism doesn't like:

new Action(){uid = 1, type = 2, argument = 3};

That's why I'd use methods... but you're right, using methods is totally excessive and retarded.
Will work on it.

Happy to help

[Best Coder 2014]

Originally Posted by Ultimation

Let me paste my packets for some types.. They may be incomplete but the concept is there

 Spoiler

 
public class MessagePacket:IPacket
    {
        public string From="SYSTEM";
        public string To="ALLUSERS";
        public string Message="";
        public ChatID ChatID=ChatID.Talk;
        public int Avatar = 0;
        public int Color = 0xFFFFFF;
        public MessagePacket(string from, string to, string message,ChatID chatid=ChatID.Talk)
        {
            From = from;
            To = to;
            Message = message;
            ChatID = chatid;
        }
        public MessagePacket(string To, string Message, ChatID chatid = ChatID.Talk)
        {
            this.To = To;
            this.Message = Message;
            ChatID = chatid;
        }
        public MessagePacket()
        {
            
        }
        public MessagePacket(string Message, ChatID chatid = ChatID.Talk)
        {
            this.Message = Message;
            ChatID = chatid;
        }

        public int PacketType { get { return 0x3EC; }}
        public byte[] Build()
        {
            using (var wtr = new PacketWriter(0x3EC))
            {
                wtr.Write(Color);
                wtr.Write((int)ChatID);
                wtr.Write(0);
                wtr.Write(0);
                wtr.Write(Avatar);
                wtr.WriteStringList(From,To,string.Empty,Message);
                return wtr.ToArray();
            }
        }

        public void Read(byte[] data)
        {
            using (var br = new BinaryReader(new MemoryStream(data)))
            {
                br.ReadInt32();
                Color = br.ReadInt32();
                ChatID = (ChatID) br.ReadInt32();
                br.ReadInt32();
                br.ReadInt32();
                Avatar = br.ReadInt32();
                var dat= br.ReadStringList();
                From = dat[0];
                To = dat[1];
                Message = dat[3];
            }
        }

 Spoiler

public class ItemUsagePacket:IPacket
    {
        public int PacketType { get { return 0x3F1; }}
        public int UID;
        public int dwParam1;
        public ItemUsuageID ID;
        public int TimeStamp;
        public int dwParam2;
        public int dwParam3;
        public int[] ItemIDS = new int[16];

        public ItemUsagePacket() {  }

        public ItemUsagePacket(int PlayerID)
        {
           
            TimeStamp = Environment.TickCount ^ PlayerID;
            UID = PlayerID;
        }
        public byte[] Build()
        {
            using (var wtr = new PacketWriter(0x3F1))
            {

                wtr.Write((int)UID);
                wtr.Write((int)dwParam1);
                wtr.Write((int)ID);
                wtr.Write((int)TimeStamp);
                wtr.Write((int)dwParam2);
                wtr.Write((int)dwParam3);
                wtr.Write((int)0);
                foreach (int id in ItemIDS)
                    wtr.Write((uint)id);
                return wtr.ToArray();
            }
        }

        public void Read(byte[] data)
        {
            using (var rdr = new BinaryReader(new MemoryStream(data)))
            {
                rdr.ReadInt32();
                UID = rdr.ReadInt32();
                dwParam1 = rdr.ReadInt32();
                ID = (ItemUsuageID)rdr.ReadInt32();
                TimeStamp = rdr.ReadInt32();
                dwParam2 = rdr.ReadInt32();
                dwParam3 = rdr.ReadInt32();
            }
        }

Regarding bad practise.. there should never be a need to use an Ambiguous function as a constructor for a class

You talk about OOP, but you're breaking one of the fundamental priciples of OOP - encapsulation. Your beloved constructor is completely useless since you're publicly exposing all the internal fields of the class anyways.

These "functions as constructors" are actually an OOP concept as well. It's called factory methods. I'm not saying they're used properly in most CO pserver sources, but to say they're not OOP is wrong. An example of good use of a factory method can actually be found in TQ's CNetMsg class:

 Spoiler

    CNetMsg* CNetMsg::CreateMsg(char* pbufMsg, DWORD dwMsgSize)
    {
            if(!pbufMsg || (int)dwMsgSize > GetMaxSize())
                    return NULL;
     
            assert((int)dwMsgSize == CNetMsg::GetSize(pbufMsg, dwMsgSize));
     
            CNetMsg* pMsg   =NULL;
     
            switch(CNetMsg::GetType(pbufMsg,dwMsgSize))
            {
            case _MSG_LOGIN:
                    pMsg    =new CMsgLogin;
                    break;
            case _MSG_TALK:
                    pMsg    =new CMsgTalk;
                    break;
            case _MSG_ACTION:
                    pMsg    =new CMsgAction;
                    break;
            case _MSG_PLAYER:
                    pMsg    =new CMsgPlayer;
                    break;

            // ...

            default:
                    break;
            }
     
            if(!pMsg)
                    return NULL;
     
            if(!pMsg->Create(pbufMsg, dwMsgSize))
            {
                    SAFE_DELETE(pMsg);
                    return NULL;
            }
            else
            {
                    return pMsg;
            }
    }

Not really related to OOP, but:
Speaking of Delphi/Pascal style, your is super frustrating. Sometimes you use PascalCase for parameter names, other times camelCase. What gives? At least be consistent. I know there's no official standard on this, but I don't think you'll ever find any serious people who'll tell you to use PascalCase for parameter names in C#.

[CptSky]

Originally Posted by Best Coder 2014

You talk about OOP, but you're breaking one of the fundamental priciples of OOP - encapsulation. Your beloved constructor is completely useless since you're publicly exposing all the internal fields of the class anyways.

These "functions as constructors" are actually an OOP concept as well. It's called factory methods. I'm not saying they're used properly in most CO pserver sources, but to say they're not OOP is wrong. An example of good use of a factory method can actually be found in TQ's CNetMsg class:

[...]

Not really related to OOP, but:
Speaking of Delphi/Pascal style, your is super frustrating. Sometimes you use PascalCase for parameter names, other times camelCase. What gives? At least be consistent. I know there's no official standard on this, but I don't think you'll ever find any serious people who'll tell you to use PascalCase for parameter names in C#.

Although he shouldn't expose all the fields of the class publicly (it should probably be a structure at this point), his constructor is not useless as some fields are read-only. Anyway, his point on constructors is not wrong. Most public sources don't respect such basic OOP concepts.

And, you should read again about the factory design pattern as it is not the pattern implemented in most public sources. Having a static method in each class to create the object is not a factory design pattern. The only time it could be right to do it, is if the constructor can fail, so you basically have also error checking on a building fonction, but with exceptions in C#, you shouldn't do it either. The factory design pattern is what you shown in the example from the TQ source. It allows to create an object without knowing exactly the return type.

Finally, you're totally right. His naming convention is not consistent at all. And I don't understand why hungarian notation is still used in strongly typed languages like C#.

[Best Coder 2014]

Originally Posted by CptSky

Although he shouldn't expose all the fields of the class publicly (it should probably be a structure at this point), his constructor is not useless as some fields are read-only. Anyway, his point on constructors is not wrong. Most public sources don't respect such basic OOP concepts.

 Spoiler

        public string From="SYSTEM";
        public string To="ALLUSERS";
        public string Message="";
        public ChatID ChatID=ChatID.Talk;
        public int Avatar = 0;
        public int Color = 0xFFFFFF;

        public int PacketType { get { return 0x3F1; }}
        public int UID;
        public int dwParam1;
        public ItemUsuageID ID;
        public int TimeStamp;
        public int dwParam2;
        public int dwParam3;
        public int[] ItemIDS = new int[16];

I don't see any read-only fields. I'm not disagreeing with you or Ultimation, I'm just pointing out some flaws in his example code.

Originally Posted by CptSky

And, you should read again about the factory design pattern as it is not the pattern implemented in most public sources. Having a static method in each class to create the object is not a factory design pattern. The only time it could be right to do it, is if the constructor can fail, so you basically have also error checking on a building fonction, but with exceptions in C#, you shouldn't do it either. The factory design pattern is what you shown in the example from the TQ source. It allows to create an object without knowing exactly the return type.

I know, that's why I said: "I'm not saying they're used properly in most CO pserver sources". My point was just that this concept of "functions as constructors" - factory methods - isn't necessarily a bad thing in OOP.

[CptSky]

Originally Posted by Best Coder 2014

[...]
I don't see any read-only fields. I'm not disagreeing with you or Ultimation, I'm just pointing out some flaws in his example code.

Was checking the ItemUsagePacket (but with a second thoughts, I think PacketType should be a constant, and shouldn't be considered as a read-only field).

Originally Posted by Best Coder 2014

[...]I know, that's why I said: "I'm not saying they're used properly in most CO pserver sources". [...]

Must have skipped this sentence... I agree with you though.

[.John]

To be honest, a switch is pretty shit as well.
It's far cleaner and better to split all your processing into classes / managers.
Ex:

public WarehouseManager : IEnumerable<Items>,  ...
{
       private Player _player;
       public WarehouseManager(Player player)
       {
             _player = player;
            _player.NetworkHandler[PacketType.Warehouse] = OnWarehousePacket;
        }


        private void OnWarehousePacket(Packet p)
        {


         }
}

Then you can easily do something like

foreach(var item in player.Warehouse) 
    //some code

foreach(var item in player.Inventory)
    //some code

or better yet

var questItem = player.Inventory.First(...)
var questItemCount = player.Inventory.Sum(x => x.TypeId == 1337 ? x.StackCount : 0 )

etc

Here is a bit more info that may help everyone:

The biggest thing about coding a game server is know how to properly split everything up so it reduces the amount of time you have to code.

Comments are optional. The purpose of a comment is to explain the idea / purpose of a function (or perhaps an in-depth analysis), not to tell a person "This function sets the stamina". Code should be self explanatory through design. Ex: Client.Inventory.Deposit(item). Pretty obvious what it does, what might not be so obvious is what happens when the item fails to deposit.

Also, LINQ is your biggest friend (if you're using C#). It's the different between 3 lines of code, or 30. Worry about performance, but don't make it your sole priority. We have pretty damn fast CPUs these days, and CO requires quite little processing (in fact, our server, COPS manages to use on average 2-5% CPU with 100+ players, with monster AI being active, events, etc)

Edit:

As for packet processing, here is the latest packet management I added to ConquerPS. An automatic structure parser via Reflect.Emit (meaning it's blazing fast after the first call, no performance issues). It basically packs/unpacks packets via structs/classes. The packet parser it's self is a reflection scanner which scans all classes at start-up for packet handlers ( a bit more complex, but that's the general idea)

   [PacketHandler(PacketType.Guild)]
        private void OnGuildRequestPacket(Packets.Guild data)
        {
            switch (data.Action)
            {
                case GuildAction.ApplyJoin:
                    {
                        var c = Game.Players[data.Data];
                        if (c != null)
                            Game.Guilds.Join(this, c);
                        break;
                    }
                case GuildAction.InviteJoin:
                    {
                        var c = Game.Players[data.Data];
                        if (c != null)
                            Game.Guilds.Recruit(this, c);
                        break;
                    }
                case GuildAction.PromoteMember:
                    {
                        if (data.Strings.Length < 1 || GuildId == 0)
                            return;

                        var name = data.Strings[0];

                        Game.Guilds.Promote(this, name, (GuildMemberRank)data.Data);
                        break;
                    }
                case GuildAction.DonateEMoney:
                    {
                        Game.Guilds.DonateCP(this, data.Data);
                        break;
                    }
                case GuildAction.DischargeMember:
                    if (data.Strings.Length < 1 || GuildId == 0)
                        return;

                    Game.Guilds.Discharge(this, data.Strings[0]);
                    break;

                case GuildAction.DonateMoney:
                    {
                        Game.Guilds.DonateGold(this, data.Data);
                        break;
                    }
                case GuildAction.QuerySyndicateAttribute:
                    SendGuildInfo();
                    break;

                case GuildAction.PromoteInfo:
                    Game.Guilds.SendPromotionData(this);
                    break;

                case GuildAction.QuerySyndicateName:
                    {
                        var g = Game.Guilds[data.Data];
                        if (g != null)
                            Send(CreateStringPacket(StringAction.Guild, g.Id, g.Name));
                        break;
                    }
                case GuildAction.LeaveSyndicate:
                    if (HasGuild)
                        Game.Guilds.Quit(this);
                    break;

                case GuildAction.SetAlly:
                    if (HasGuild && data.Strings.Length > 0)
                        Game.Guilds.AddAlly(this, data.Strings[0]);
                    break;

                case GuildAction.ClearAlly:
                    if (HasGuild && data.Strings.Length > 0)
                            Game.Guilds.RemoveAlly(this, data.Strings[0]);
                    break;

                case GuildAction.SetEnemy:
                    if (HasGuild && data.Strings.Length > 0)
                            Game.Guilds.AddEnemy(this, data.Strings[0]);
                    break;

                case GuildAction.ClearEnemy:
                    if (HasGuild && data.Strings.Length > 0)
                            Game.Guilds.RemoveEnemy(this, data.Strings[0]);
                    break;

                case GuildAction.ApplyAlly:
                    if (HasGuild && data.Strings.Length > 0)
                            Game.Guilds.AddAlly(this, data.Strings[0]);
                    break;
            }
        }

[Best Coder 2014]

Originally Posted by .John

The packet parser it's self is a reflection scanner which scans all classes at start-up for packet handlers ( a bit more complex, but that's the general idea)

 Spoiler

   [PacketHandler(PacketType.Guild)]
        private void OnGuildRequestPacket(Packets.Guild data)
        {
            switch (data.Action)
            {
                case GuildAction.ApplyJoin:
                    {
                        var c = Game.Players[data.Data];
                        if (c != null)
                            Game.Guilds.Join(this, c);
                        break;
                    }
                case GuildAction.InviteJoin:
                    {
                        var c = Game.Players[data.Data];
                        if (c != null)
                            Game.Guilds.Recruit(this, c);
                        break;
                    }
                case GuildAction.PromoteMember:
                    {
                        if (data.Strings.Length < 1 || GuildId == 0)
                            return;

                        var name = data.Strings[0];

                        Game.Guilds.Promote(this, name, (GuildMemberRank)data.Data);
                        break;
                    }
                case GuildAction.DonateEMoney:
                    {
                        Game.Guilds.DonateCP(this, data.Data);
                        break;
                    }
                case GuildAction.DischargeMember:
                    if (data.Strings.Length < 1 || GuildId == 0)
                        return;

                    Game.Guilds.Discharge(this, data.Strings[0]);
                    break;

                case GuildAction.DonateMoney:
                    {
                        Game.Guilds.DonateGold(this, data.Data);
                        break;
                    }
                case GuildAction.QuerySyndicateAttribute:
                    SendGuildInfo();
                    break;

                case GuildAction.PromoteInfo:
                    Game.Guilds.SendPromotionData(this);
                    break;

                case GuildAction.QuerySyndicateName:
                    {
                        var g = Game.Guilds[data.Data];
                        if (g != null)
                            Send(CreateStringPacket(StringAction.Guild, g.Id, g.Name));
                        break;
                    }
                case GuildAction.LeaveSyndicate:
                    if (HasGuild)
                        Game.Guilds.Quit(this);
                    break;

                case GuildAction.SetAlly:
                    if (HasGuild && data.Strings.Length > 0)
                        Game.Guilds.AddAlly(this, data.Strings[0]);
                    break;

                case GuildAction.ClearAlly:
                    if (HasGuild && data.Strings.Length > 0)
                            Game.Guilds.RemoveAlly(this, data.Strings[0]);
                    break;

                case GuildAction.SetEnemy:
                    if (HasGuild && data.Strings.Length > 0)
                            Game.Guilds.AddEnemy(this, data.Strings[0]);
                    break;

                case GuildAction.ClearEnemy:
                    if (HasGuild && data.Strings.Length > 0)
                            Game.Guilds.RemoveEnemy(this, data.Strings[0]);
                    break;

                case GuildAction.ApplyAlly:
                    if (HasGuild && data.Strings.Length > 0)
                            Game.Guilds.AddAlly(this, data.Strings[0]);
                    break;
            }
        }

John's Overly Complicated Annotation/Attribute-based Observer Pattern™.

[Ultimation]

Originally Posted by Best Coder 2014

You talk about OOP, but you're breaking one of the fundamental priciples of OOP - encapsulation. Your beloved constructor is completely useless since you're publicly exposing all the internal fields of the class anyways.

These "functions as constructors" are actually an OOP concept as well. It's called factory methods. I'm not saying they're used properly in most CO pserver sources, but to say they're not OOP is wrong. An example of good use of a factory method can actually be found in TQ's CNetMsg class:

 Spoiler

    CNetMsg* CNetMsg::CreateMsg(char* pbufMsg, DWORD dwMsgSize)
    {
            if(!pbufMsg || (int)dwMsgSize > GetMaxSize())
                    return NULL;
     
            assert((int)dwMsgSize == CNetMsg::GetSize(pbufMsg, dwMsgSize));
     
            CNetMsg* pMsg   =NULL;
     
            switch(CNetMsg::GetType(pbufMsg,dwMsgSize))
            {
            case _MSG_LOGIN:
                    pMsg    =new CMsgLogin;
                    break;
            case _MSG_TALK:
                    pMsg    =new CMsgTalk;
                    break;
            case _MSG_ACTION:
                    pMsg    =new CMsgAction;
                    break;
            case _MSG_PLAYER:
                    pMsg    =new CMsgPlayer;
                    break;

            // ...

            default:
                    break;
            }
     
            if(!pMsg)
                    return NULL;
     
            if(!pMsg->Create(pbufMsg, dwMsgSize))
            {
                    SAFE_DELETE(pMsg);
                    return NULL;
            }
            else
            {
                    return pMsg;
            }
    }

Not really related to OOP, but:
Speaking of Delphi/Pascal style, your is super frustrating. Sometimes you use PascalCase for parameter names, other times camelCase. What gives? At least be consistent. I know there's no official standard on this, but I don't think you'll ever find any serious people who'll tell you to use PascalCase for parameter names in C#.

Originally Posted by Best Coder 2014

 Spoiler

        public string From="SYSTEM";
        public string To="ALLUSERS";
        public string Message="";
        public ChatID ChatID=ChatID.Talk;
        public int Avatar = 0;
        public int Color = 0xFFFFFF;

        public int PacketType { get { return 0x3F1; }}
        public int UID;
        public int dwParam1;
        public ItemUsuageID ID;
        public int TimeStamp;
        public int dwParam2;
        public int dwParam3;
        public int[] ItemIDS = new int[16];

I don't see any read-only fields. I'm not disagreeing with you or Ultimation, I'm just pointing out some flaws in his example code.



I know, that's why I said: "I'm not saying they're used properly in most CO pserver sources". My point was just that this concept of "functions as constructors" - factory methods - isn't necessarily a bad thing in OOP.

it was an example from acidco about 4 years ago. things have changed since.

Now the point about constructors, yes the way i do it isn't perfect but it does keep things tidy for larger projects. why create a new class and then assign each required parameter. over time that may get somewhat annoying?

I personally prefer to do new Class(arguments) than new Class(){arguments}

Originally Posted by CptSky

Was checking the ItemUsagePacket (but with a second thoughts, I think PacketType should be a constant, and shouldn't be considered as a read-only field).

I disagree, its part of the IPacket interface.

[.John]

Originally Posted by Best Coder 2014

John's Overly Complicated Annotation/Attribute-based Observer Pattern™.

Of course

using System.Runtime.InteropServices;

namespace ProCS.Packets
{
    [PacketData]
    [StructLayout(LayoutKind.Sequential)]
    public class Guild
    {
        public GuildAction Action { get; set; }

        public int Data { get; set; }

        public int RequiredLevel { get; set; }

        public int RequiredRebirth { get; set; }

        public int RequiredProfession { get; set; }

        public string[] Strings { get; set; }

        public Guild()
        {
            Action = GuildAction.None;
            Strings = new string[0];
        }
    }
}

[Super Aids]

Originally Posted by .John

As for packet processing, here is the latest packet management I added to ConquerPS. An automatic structure parser via Reflect.Emit (meaning it's blazing fast after the first call, no performance issues). It basically packs/unpacks packets via structs/classes. The packet parser it's self is a reflection scanner which scans all classes at start-up for packet handlers ( a bit more complex, but that's the general idea)

   [PacketHandler(PacketType.Guild)]
        private void OnGuildRequestPacket(Packets.Guild data)
        {
            switch (data.Action)
            {
                case GuildAction.ApplyJoin:
                    {
                        var c = Game.Players[data.Data];
                        if (c != null)
                            Game.Guilds.Join(this, c);
                        break;
                    }
                case GuildAction.InviteJoin:
                    {
                        var c = Game.Players[data.Data];
                        if (c != null)
                            Game.Guilds.Recruit(this, c);
                        break;
                    }
                case GuildAction.PromoteMember:
                    {
                        if (data.Strings.Length < 1 || GuildId == 0)
                            return;

                        var name = data.Strings[0];

                        Game.Guilds.Promote(this, name, (GuildMemberRank)data.Data);
                        break;
                    }
                case GuildAction.DonateEMoney:
                    {
                        Game.Guilds.DonateCP(this, data.Data);
                        break;
                    }
                case GuildAction.DischargeMember:
                    if (data.Strings.Length < 1 || GuildId == 0)
                        return;

                    Game.Guilds.Discharge(this, data.Strings[0]);
                    break;

                case GuildAction.DonateMoney:
                    {
                        Game.Guilds.DonateGold(this, data.Data);
                        break;
                    }
                case GuildAction.QuerySyndicateAttribute:
                    SendGuildInfo();
                    break;

                case GuildAction.PromoteInfo:
                    Game.Guilds.SendPromotionData(this);
                    break;

                case GuildAction.QuerySyndicateName:
                    {
                        var g = Game.Guilds[data.Data];
                        if (g != null)
                            Send(CreateStringPacket(StringAction.Guild, g.Id, g.Name));
                        break;
                    }
                case GuildAction.LeaveSyndicate:
                    if (HasGuild)
                        Game.Guilds.Quit(this);
                    break;

                case GuildAction.SetAlly:
                    if (HasGuild && data.Strings.Length > 0)
                        Game.Guilds.AddAlly(this, data.Strings[0]);
                    break;

                case GuildAction.ClearAlly:
                    if (HasGuild && data.Strings.Length > 0)
                            Game.Guilds.RemoveAlly(this, data.Strings[0]);
                    break;

                case GuildAction.SetEnemy:
                    if (HasGuild && data.Strings.Length > 0)
                            Game.Guilds.AddEnemy(this, data.Strings[0]);
                    break;

                case GuildAction.ClearEnemy:
                    if (HasGuild && data.Strings.Length > 0)
                            Game.Guilds.RemoveEnemy(this, data.Strings[0]);
                    break;

                case GuildAction.ApplyAlly:
                    if (HasGuild && data.Strings.Length > 0)
                            Game.Guilds.AddAlly(this, data.Strings[0]);
                    break;
            }
        }

Implemented something similar, but in D.

import packets;

@Packet(PacketType.test1)
static void handle1(Client client, DataPacket packet) {
	// ... Handle packet 1
}

@Packet(PacketType.test2)
static void handle2(Client client, DataPacket packet) {
	// ... Handle packet 2
}

@Packet(PacketType.test3)
static void handle3(Client client, DataPacket packet) {
	// ... Handle packet 3
}

And the implementation:

enum PacketType {
	test1 = 1003,
	test2 = 1004,
	test3 = 1005
}

struct Packet { // The Packet attribute ...
public:
	PacketType type;
}

All packets has to be imported public like the following:

public {
	import packets.testpackets;
}

And the packet handler is implemented like the following:

void handlePackets(Client client, DataPacket packet) {
	switch (packet.type) {
		mixin(getPacketHandlers);
		
		default: {
			// not handled ...
			break;
		}
	}
}

string getPacketHandlers()
{
	import std.string : format;
	import std.array : join, split;
	import std.conv : to;
	import std.algorithm : startsWith;
	import std.traits;
	
	string[] cases;
	
	foreach (phandler; __traits(allMembers, packets)) {
		static if (mixin ("isCallable!" ~ phandler)) {
			foreach (pattr; mixin (format("__traits(getAttributes, %s)", phandler))) {
				static if (startsWith(pattr.stringof, "Packet")) { // static if (is(pattr == Packet)) does not seem to work :/
					auto fname = split(phandler.stringof, "\"")[1];
					cases ~= format("case %s: %s(client, packet); break;", to!string(cast(ushort)pattr.type), fname);
				}
			}
		}
	}
	return join(cases, "\n");
}