Seeking Tips on SkillUse Packet (1105)

[Konvict-]

I am currently working on a server (Patch 5017), and have been doing a lot of research on other sources etc. Every source that I have looked at, they seem to add the list of targets to a dictionary, then using that dictionary, they generate a skill packet. Am I missing something involving garbage collection, or just the performance of buffer.blockcopy, or could you actually do it this way:

        public class SkillUsePacket
        {
            private Writer Writer;
            private int offset = 20;
            private int TargetCount;
            public SkillUsePacket(uint AttackerUID)
            {
                this.Writer = new Writer(628);
                this.Writer.Fill((ushort)1105, 2);
                this.Writer.Fill(AttackerUID, 4);
            }
            public ushort AimX { set { Writer.Fill(value, 8); } }
            public ushort AimY { set { Writer.Fill(value, 10); } }
            public ushort SkillID { set { Writer.Fill(value, 12); } }
            public byte SkillLevel { set { Writer.Fill(value, 14); } }
            public void AddTarget(uint UniqueID, uint Damage)
            {
                Writer.Fill(UniqueID, offset);
                offset += 4;
                Writer.Fill(Damage, offset);
                offset += 8;
                TargetCount++;
            }
            public byte[] ToBytes
            {
                get
                {
                    int Len = 28 + (TargetCount * 12);
                    Writer.Fill((ushort)Len, 0);
                    Writer.Fill(TargetCount, 16);
                    byte[] newData = new byte[Len];
                    Buffer.BlockCopy(Writer.Bytes, 0, newData, 0, Len);
                    return newData;
                }
            }
        }

I just find it hard to believe nobody else has thought of this (at least thats released a source to the public)

[Super Aids]

You could do it that way, if you want. People worry too much about the GC these days. Honestly small things like that won't even matter the slightest unless you executed it a million times in a row. One thing you could do if you're concerned is to clear the buffer from your Writer though.
I'd suggest implementing IDisposable.


Just a heads up, you need to learn some name conventions.


You're naming your property after a method, referring to "ToBytes". Consider rename it "Bytes" or change to it to a method.

I would suggest changing it to a method, because it's not really used as a property.

Leaving you with either:

public byte[] ToBytes()
            {
                    int Len = 28 + (TargetCount * 12);
                    Writer.Fill((ushort)Len, 0);
                    Writer.Fill(TargetCount, 16);
                    byte[] newData = new byte[Len];
                    Buffer.BlockCopy(Writer.Bytes, 0, newData, 0, Len);
                    return newData;
            }

or this

public byte[] Bytes
            {
                get
                {
                    int Len = 28 + (TargetCount * 12);
                    Writer.Fill((ushort)Len, 0);
                    Writer.Fill(TargetCount, 16);
                    byte[] newData = new byte[Len];
                    Buffer.BlockCopy(Writer.Bytes, 0, newData, 0, Len);
                    return newData;
                }
            }

Also is this?

 public void AddTarget(uint UniqueID, uint Damage)
            {
                Writer.Fill(UniqueID, offset);
                offset += 4;
                Writer.Fill(Damage, offset);
                offset += 8;
                TargetCount++;
            }

Is it supposed to add 8 to the offset? Shouldn't it be 4 considering a uint is 4 bytes? Or is it because you're missing some value(s) to the offset(s) after?

[Konvict-]

Originally Posted by Super Aids

You could do it that way, if you want.

Just a heads up, you need to learn some name conventions.


You're naming your property after a method, referring to "ToBytes". Consider rename it "Bytes" or change to it to a method.

Also is this?

 public void AddTarget(uint UniqueID, uint Damage)
            {
                Writer.Fill(UniqueID, offset);
                offset += 4;
                Writer.Fill(Damage, offset);
                offset += 8;
                TargetCount++;
            }

supposed to add 8 to the offset? Shouldn't it be 4 considering a uint is 4 bytes? Or is it because you're missing some value(s) to the offset(s) after?

Its missing a value at the end. Doesnt seem to be useful though. Everything works as is. Thanks though xD

[Super Aids]

Updated my post with some more. Also what doesn't seem useful? The naming convention? You'd be pretty stupid to not follow standard guidelines when using a programming language; especially if you ever want to get into industrial programming and/or work on/release public projects.

[CptSky]

I was doing something like that in COPS v6.

[Konvict-]

Originally Posted by Super Aids

Updated my post with some more. Also what doesn't seem useful? The naming convention? You'd be pretty stupid to not follow standard guidelines when using a programming language; especially if you ever want to get into industrial programming and/or work on/release public projects.

No no no, I completely understand why naming conventions are important. I usually do it properly in PHP, but kind of get lazy in C# as well as dont quite know all the built in methods C# provides. I was meaning the value im missing doesnt seem to be important.

#edit @CptSky ill have to check it out, may be able to improve my own. Thanks.

[Super Aids]

Originally Posted by Konvict-

but kind of get lazy in C# as well.

Why does everybody use this excuse when their "bad" codes gets corrected?

[Y u k i]

It's pretty much useless to implement IDisposable for managed code. Even Microsoft clearly states its meant to be used for code that does open File Handles or other native ressources.

[Konvict-]

Originally Posted by Super Aids

Why does everybody use this excuse when their "bad" codes gets corrected?

lol, if you want to look at the rest of my source be my guest. I have all my private fields start with an underscore followed by lowercase letter, follow all the typical naming conventions except for the ToBytes which you pointed out. Im completely open to criticism, which is what I am actually asking for here. I'm obviously not the best programmer, and realize there are better ones on this forum. Which is why I made this post. To ask BETTER programmers for tips.