is that what we call BAD CODING?

01/08/2011 01:31 { Angelius }#1

Well....
Lately i have been asking Questions about the BroadCast thingy but im getting NO Answers so i made up my mind coding a new BC system But... when its done an i got it all working i though about WOW It Looks LIKE SHIT
But it Dosent Matter As long as Its working But ....Yet i Really Wanna Know How Bad It is i Mean My Coding Shit

Cus i dont want to know that my own shit is slowing down my work lol
so the following void is gonna start working every time i/they send a broadcast and its gonna count all the way to the number 100
Is That bad ?

PHP Code:


			
public static void GoSend()
        {
            MySqlCommand cmd = new MySqlCommand(MySqlCommandType.SELECT);
            cmd.Select("broadcast").Where("BroadCastUID", 1);
            MySqlReader r = new MySqlReader(cmd);
            while (r.Read())
            {
                BroadCastStruct B = new BroadCastStruct();
                B.CharID = r.ReadUInt16("CharID");
                B.Message = r.ReadString("Message");
                B.Name = r.ReadString("Name");
                B.Cps = r.ReadInt16("Cps");
                B.BroadCastUID = r.ReadInt16("BroadCastUID");
                World.SendMsgToAll(B.Name, B.Message, 2500, 0, System.Drawing.Color.Yellow);
                MySqlCommand Cmd2 = new MySqlCommand(MySqlCommandType.DELETE);
                Cmd2.Delete("broadcast", "BroadCastUID", 1).Execute();
                WorldTables.BroadCastUID -= 1;
                MySqlCommand Cmd = new MySqlCommand(MySqlCommandType.UPDATE);
                Cmd.Update("broadcast");
                Cmd
                .Set("BroadCastUID", 1);
                Cmd.Where("BroadCastUID", 2);
                Cmd.Execute();
                MySqlCommand Cmd0 = new MySqlCommand(MySqlCommandType.UPDATE);
                Cmd0.Update("broadcast");
                Cmd0
                .Set("BroadCastUID", 2);
                Cmd0.Where("BroadCastUID", 3);
                Cmd0.Execute();
                MySqlCommand Cmd1 = new MySqlCommand(MySqlCommandType.UPDATE);
                Cmd1.Update("broadcast");
                Cmd1
                .Set("BroadCastUID", 3);
                Cmd1.Where("BroadCastUID", 4);
                Cmd1.Execute();
                MySqlCommand Cmd22 = new MySqlCommand(MySqlCommandType.UPDATE);
                Cmd22.Update("broadcast");
                Cmd22
                .Set("BroadCastUID", 4);
                Cmd22.Where("BroadCastUID", 5);
                Cmd22.Execute();
                MySqlCommand Cmd3 = new MySqlCommand(MySqlCommandType.UPDATE);
                Cmd3.Update("broadcast");
                Cmd3
                .Set("BroadCastUID", 5);
                Cmd3.Where("BroadCastUID", 6);
                Cmd3.Execute();
                MySqlCommand Cmd4 = new MySqlCommand(MySqlCommandType.UPDATE);
                Cmd4.Update("broadcast");
                Cmd4
                .Set("BroadCastUID", 6);
                Cmd4.Where("BroadCastUID", 7);
                Cmd4.Execute();
                MySqlCommand Cmd5 = new MySqlCommand(MySqlCommandType.UPDATE);
                Cmd5.Update("broadcast");
                Cmd5
                .Set("BroadCastUID", 7);
                Cmd5.Where("BroadCastUID", 8);
                Cmd5.Execute();
                MySqlCommand Cmd6 = new MySqlCommand(MySqlCommandType.UPDATE);
                Cmd6.Update("broadcast");
                Cmd6
                .Set("BroadCastUID", 8);
                Cmd6.Where("BroadCastUID", 9);
                Cmd6.Execute();
                MySqlCommand Cmd7 = new MySqlCommand(MySqlCommandType.UPDATE);
                Cmd7.Update("broadcast");
                Cmd7
                .Set("BroadCastUID", 9);
                Cmd7.Where("BroadCastUID", 10);
                Cmd7.Execute();
                MySqlCommand CMd8 = new MySqlCommand(MySqlCommandType.UPDATE);
                CMd8.Update("broadcast");
                CMd8
                .Set("BroadCastUID", 10);
                CMd8.Where("BroadCastUID", 11);
                CMd8.Execute();
                MySqlCommand CMd9 = new MySqlCommand(MySqlCommandType.UPDATE);
                CMd9.Update("broadcast");
                CMd9
                .Set("BroadCastUID", 11);
                CMd9.Where("BroadCastUID", 12);
                CMd9.Execute();
                MySqlCommand CMd10 = new MySqlCommand(MySqlCommandType.UPDATE);
                CMd10.Update("broadcast");
                CMd10
                .Set("BroadCastUID", 12);
                CMd10.Where("BroadCastUID", 13);
                CMd10.Execute();

EDIT.... im using a table named BroadCast to save/delete the players messages

01/08/2011 01:50 Syst3m_W1z4rd#2

Code:

byte ii = 1;//Because it is always 1 > i
for (byte i = 0; i < 100; i++)
{
    ii++;
    MySqlCommand Cmd = new MySqlCommand(MySqlCommandType.UPDATE);
    Cmd.Update("broadcast");
    Cmd.Set("BroadCastUID", i);
    CMd.Where("BroadCastUID", ii);
    CMd.Execute();
}

Don't know if it will work using a for loop.

01/08/2011 02:38 { Angelius }#3

Quote:

Originally Posted by Syst3m_W1z4rd

Code:

byte ii = 1;//Because it is always 1 > i
for (byte i = 0; i < 100; i++)
{
    ii++;
    MySqlCommand Cmd = new MySqlCommand(MySqlCommandType.UPDATE);
    Cmd.Update("broadcast");
    Cmd.Set("BroadCastUID", i);
    CMd.Where("BroadCastUID", ii);
    CMd.Execute();
}

Don't know if it will work using a for loop.

Hey Smart ass it works :P
but ii had to = 0 as i have the first broad cast id starts with 0
oh well lets face it i dident even think about it .... how silly lmao

and now

Code:

for (byte Thanks = 0; Thanks < 100; Thanks++)
{
Thanks++
Thanks++
Thanks++
Thanks++
Thanks++
Thanks++
lol
    MySqlCommand Cmd = new MySqlCommand(MySqlCommandType.UPDATE);
    Cmd.Update("Syst3m_W1z4rd");
    Cmd.Set("Received Thanks", Thanks);
    CMd.Where("Thanks Number", 122);
    CMd.Execute();
}

01/08/2011 04:00 samehvan#4

First of all it is a very good thing that u got the guts and made ur mind to start coding that is really awesome

allow me to wounder why u save to ur MySql data base every single BC msg i think that would cause some troubles with DB connections and slowing down ur server a little bit

if u don't use a MySql DB table and u just use a virtual Data Table to save them that would be ok

Another way to do it
trying to save them in a Data memory store instead , something like Dictionaries/arrays/HashTables

Dictionary<uint,BCMsg> BroadMessages=new Dictionary<uint,BCMsg>();

while the BCMsg Class would be something like

class BCMsg
{
public uint Id;
public uint SenderId;
public string SenderName;
public byte CPs;
public DateTime SendTime;
public string Msg;
}

Oh Sorry ddnt notice ur BC structure it is the same i think

and when u got a message

BCMsg BC=new BCMsg()
BC.SendTime=DateTime.Now;
BC.SenderId=Client.Id;
BC.SenderName=Client.Name;
BC.Msg=(Message);
BC.Id=BroadMessages.Count+1;
while(BroadMessages.ContainsKey(BC.Id)) BC.Id++;
BroadMessages.Add(BC.Id,BC);

then if u wanted to save the BCMsgs to your Data Base for any reason u can simply call the save func

foreach(BCMsg in BroadMessages.values)
{
// ur sql update/insert commands here
}

also when sending the Msgs to a client lets say the last 100 Msgs

uint LastMsgId=(uint)BroadMessages.Count;
uint StartMsg=(uint)(LastMsgId-100);
for(uint x=StartMsg;x<LastMsgId+1;x++)
{
if(BroadMessages.ContainsKey(x))
{
BCMsg B=BroadMessages[x];
World.SendMsgToAll(B.Name, B.Message, 2500, 0, System.Drawing.Color.Yellow);
}
}

Note :- this is a fast thinking code may be it will need some edits to be a complete system , yet i hope this might help u in anyway

Finally :- once again trying to code is a very good thinking Good luck and keep up the good work

01/08/2011 05:01 bone-you#5

Quote:

Originally Posted by Syst3m_W1z4rd

Code:

byte ii = 1;//Because it is always 1 > i
for (byte i = 0; i < 100; i++)
{
    ii++;
    MySqlCommand Cmd = new MySqlCommand(MySqlCommandType.UPDATE);
    Cmd.Update("broadcast");
    Cmd.Set("BroadCastUID", i);
    CMd.Where("BroadCastUID", ii);
    CMd.Execute();
}

just do i+1 instead of having another variable.

edit: by the way... if you already execute a mysql query, don't make an other var, just new over the old one. New vars adds to the stack which slows down the function/block.

I would also recommend an internal broadcast list. Using the database for every broadcast call is inefficient when you're not trying to save the messages for the long term.

01/08/2011 10:17 .Kinshi#6

You could throw BC's in a queue, would be the best way to handle them.

01/08/2011 13:23 Basser#7

Quote:

Originally Posted by Syst3m_W1z4rd

Code:

for (byte i = 0; i < 100; i++)
{
    MySqlCommand Cmd = new MySqlCommand(MySqlCommandType.UPDATE);
    Cmd.Update("broadcast");
    Cmd.Set("BroadCastUID", i);
    CMd.Where("BroadCastUID", i +1);
    CMd.Execute();
}

Looks better imo.
I find the way you do broadcast right now very strange.
You change the unique ID of an object? WHY????
Also, you go throw 99 queries for updating those UIDs. WHY????
Why would you even store these values?
I'd say this is terrible coding, but we all do terrible coding at first.

01/08/2011 13:55 { Angelius }#8

Quote:

Originally Posted by samehvan

First of all it is a very good thing that u got the guts and made ur mind to start coding that is really awesome

allow me to wounder why u save to ur MySql data base every single BC msg i think that would cause some troubles with DB connections and slowing down ur server a little bit

if u don't use a MySql DB table and u just use a virtual Data Table to save them that would be ok

Another way to do it
trying to save them in a Data memory store instead , something like Dictionaries/arrays/HashTables

Dictionary<uint,BCMsg> BroadMessages=new Dictionary<uint,BCMsg>();

while the BCMsg Class would be something like

class BCMsg
{
public uint Id;
public uint SenderId;
public string SenderName;
public byte CPs;
public DateTime SendTime;
public string Msg;
}

Oh Sorry ddnt notice ur BC structure it is the same i think

and when u got a message

BCMsg BC=new BCMsg()
BC.SendTime=DateTime.Now;
BC.SenderId=Client.Id;
BC.SenderName=Client.Name;
BC.Msg=(Message);
BC.Id=BroadMessages.Count+1;
while(BroadMessages.ContainsKey(BC.Id)) BC.Id++;
BroadMessages.Add(BC.Id,BC);

then if u wanted to save the BCMsgs to your Data Base for any reason u can simply call the save func

foreach(BCMsg in BroadMessages.values)
{
// ur sql update/insert commands here
}

also when sending the Msgs to a client lets say the last 100 Msgs

uint LastMsgId=(uint)BroadMessages.Count;
uint StartMsg=(uint)(LastMsgId-100);
for(uint x=StartMsg;x<LastMsgId+1;x++)
{
if(BroadMessages.ContainsKey(x))
{
BCMsg B=BroadMessages[x];
World.SendMsgToAll(B.Name, B.Message, 2500, 0, System.Drawing.Color.Yellow);
}
}

Note :- this is a fast thinking code may be it will need some edits to be a complete system , yet i hope this might help u in anyway

Finally :- once again trying to code is a very good thinking Good luck and keep up the good work

Awesome and i wold say thank you so much an i Really Really Appreciate it but...
still learning :xD

another but...

what if i wanted to fix the whole upgrading message thing ?
What if i wanted to see other ppl messages when i open the all info awaiting ?
none of the examples up there well make it an option am i right ?
lets see
take a look at this vid please

thats for the sending a broadcast thing now that im done with that part its time to check the upgrade message thingy which i was trying to fix for ages using the Dictionary/we but never had the chance to say F** yeah i Made it .... but Oh WELL im welling to create a complete BC'ing system or im not gonna do it at all according to what i have learned till now if im gonna start it im gonna do it the right way and thats why im typing atm :D
sooo..... now that i have the DataBase Saving/Loading Messages i can Update/Renumber the BC's ID's depending on the Mysql connection with the DataBase an after all the suffering with the silly Packets(THE WORST PART ) i Had The Following results
Please watch the vid

so now that i have the upgrade message thingy works fine i can Send/Upgrade/See all Other Awaiting to be sent BroadCasts or else enjoy the last results as a learning shit lol hell yeah we can always learn ...
However i was wondering is that an option to do all that stuff or in other words to Create a BroadCast system to match the real one lets say like the one that is in RealCO Using VDT Such as Dictionaries/arrays/HashTables thing ?

Ps. the last results is a fully working BC system but not sure about the Consequences of using the sql shit for the BC .and thanks everyone happy to know that there is someone that is welling to support us ...BEGINNERS

Quote:

Originally Posted by .Kinshi

You could throw BC's in a queue, would be the best way to handle them.

an example wold be more helpful ..Thank you

Quote:

Originally Posted by Basser

Quote:

Looks better imo.
I find the way you do broadcast right now very strange.
You change the unique ID of an object? WHY????
Also, you go throw 99 queries for updating those UIDs. WHY????
Why would you even store these values?
I'd say this is terrible coding, but we all do terrible coding at first.

i LOL'ed at that red line
and than i was like o_O he is right but i dont know what to answer him :P
all i can say is (I Can Be a Good student) and that was the only way i knew to code that shit so dont expect alot :P

01/08/2011 14:17 Korvacs#9

The thing you need to remember is:

Less = More

In almost all cases of programming this is true, you need to figure out how to do something, in the smallest amount of code possible, while of course keeping that code efficient, in most cases less code is normally more efficient than lots of code.

Therefore throwing in a loop to perform work a great number of times, will be more efficient than doing all of those things one after anouther, this applies to almost all cases.

01/08/2011 14:57 { Angelius }#10

Quote:

Originally Posted by Korvacs

The thing you need to remember is:

Less = More

In almost all cases of programming this is true, you need to figure out how to do something, in the smallest amount of code possible, while of course keeping that code efficient, in most cases less code is normally more efficient than lots of code.

Therefore throwing in a loop to perform work a great number of times, will be more efficient than doing all of those things one after anouther, this applies to almost all cases.

Hell yeah .... ok ok i'll be honest what you just said is all true an i learned it the hard way LOL...
that code ! the one i started the thread with i coded the whole shit all the way to the 100 OMFG took me more than 6 hours to get it done an have it working but.... when (system wizard) posted that looping code i was like o_O i spent all that time coding shit an i never though about how to less = more :P
i removed more than 800 line of codes to replace it with 8 lines that dose the same 800 line job uhhh .

01/08/2011 19:51 bone-you#11

[Only registered and activated users can see links. Click Here To Register...]

xsco mark II.

Search for "broadcast". The code looks bad, but it works perfectly (for that patch). It includes bumping messages up, message queuing, and internal list, everything. It has all the IDs sent from the client related to broadcast.

As far as actual broadcast management, I'm pretty satisfied with my broadcast class. I do think it was the first fully functional broadcast as far as pservers go.

Code:

#pragma once

#define DEF_MAX_BROADCASTS 100


typedef struct tagBROADCAST {
	int time;//time until expire
	int id;//total id
	int charid;//char id
	char * text;//message
	char * name;//char name
	int spentcps;
} BROADCAST;

class CBroadcast
{
public:
	CBroadcast()
	{
		for (int i = 0; i < DEF_MAX_BROADCASTS; i++)
			m_stBroadcastList[i] = 0;
		m_stBroadcastCurrent = 0;
		m_iBroadcastcount = 0;
		lock = false;
	}
	//~CBroadcast() {}

	BROADCAST * m_stBroadcastList[DEF_MAX_BROADCASTS];
	BROADCAST * m_stBroadcastCurrent;

	int m_iBroadcastcount;
	int m_iBroadcastleft;
	bool lock;

	short sBroadcastAdd(int charid, char * charname, char * message, int size)
	{
		while (lock)
			Sleep(1);
		lock = true;
		if (!m_stBroadcastCurrent)
		{
			m_stBroadcastCurrent = new BROADCAST;
			m_stBroadcastCurrent->id = m_iBroadcastcount;
			m_stBroadcastCurrent->spentcps = 0;
			m_stBroadcastCurrent->name = new char[strlen(charname)+1];
			m_stBroadcastCurrent->name[strlen(charname)] = 0;
			memcpy(m_stBroadcastCurrent->name, charname, strlen(charname));

			m_stBroadcastCurrent->text = new char[size+1];
			m_stBroadcastCurrent->text[strlen(message)] = 0;
			memcpy(m_stBroadcastCurrent->text, message, size);

			m_stBroadcastCurrent->charid = charid;
			m_stBroadcastCurrent->time = timeGetTime()+60000;
			m_iBroadcastcount++;
			lock = false;
			return 0;
		}
		for (int i = 0; i < DEF_MAX_BROADCASTS; i++)
		{
			if (!m_stBroadcastList[i])
			{
				m_stBroadcastList[i] = new BROADCAST;
				m_stBroadcastList[i]->id = m_iBroadcastcount;
				m_stBroadcastList[i]->spentcps = 0;
				m_stBroadcastList[i]->name = new char[strlen(charname)+1];
				m_stBroadcastList[i]->name[strlen(charname)] = 0;
				memcpy(m_stBroadcastList[i]->name, charname, strlen(charname));

				m_stBroadcastList[i]->text = new char[size+1];
				m_stBroadcastList[i]->text[strlen(message)] = 0;
				memcpy(m_stBroadcastList[i]->text, message, size);

				m_stBroadcastList[i]->charid = charid;
				if (i > 0)
					m_stBroadcastList[i]->time = m_stBroadcastList[i-1]->time+60000;
				else
					m_stBroadcastList[i]->time = m_stBroadcastCurrent->time+60000;
				m_iBroadcastcount++;
				lock = false;
				return i;
			}
		}
		lock = false;
		return 0;
	};
	void __inline BroadcastDelete()
	{
		while (lock)
			Sleep(1);
		lock = true;
		if (m_stBroadcastCurrent && m_stBroadcastList[0])
		{
			m_stBroadcastCurrent->id = 0;
			delete m_stBroadcastCurrent->name;
			m_stBroadcastCurrent->name = 0;
			delete m_stBroadcastCurrent->text;
			m_stBroadcastCurrent->text = 0;
			m_stBroadcastCurrent->charid = 0;
			m_stBroadcastCurrent->time = 0;
			delete m_stBroadcastCurrent;
			m_stBroadcastCurrent = m_stBroadcastList[0];
			for (int i = 0; i < DEF_MAX_BROADCASTS-1; i++)
				m_stBroadcastList[i] = m_stBroadcastList[i+1];
			m_stBroadcastList[DEF_MAX_BROADCASTS-1] = NULL;
			m_iBroadcastcount--;
		}
		lock = false;
	};
	void __inline BroadcastClear()
	{
		while (lock)
			Sleep(1);
		lock = true;
		m_iBroadcastcount = 0;
		if (m_stBroadcastCurrent)
		{
			m_stBroadcastCurrent->id = 0;
			delete m_stBroadcastCurrent->name;
			m_stBroadcastCurrent->name = 0;
			delete m_stBroadcastCurrent->text;
			m_stBroadcastCurrent->text = 0;
			m_stBroadcastCurrent->charid = 0;
			m_stBroadcastCurrent->time = 0;
			delete m_stBroadcastCurrent;
			m_stBroadcastCurrent = 0;
		}
		for (int i = 0; i < DEF_MAX_BROADCASTS; i++)
		{
			if (m_stBroadcastList[i])
			{
				m_stBroadcastList[i]->id = 0;
				delete m_stBroadcastList[i]->name;
				m_stBroadcastList[i]->name = 0;
				delete m_stBroadcastList[i]->text;
				m_stBroadcastList[i]->text = 0;
				m_stBroadcastList[i]->charid = 0;
				m_stBroadcastList[i]->time = 0;
				delete m_stBroadcastList[i];
				m_stBroadcastList[i] = 0;
			}
		}
		lock = false;
	};
	bool __inline bBroadcastCheckTime()
	{
		if (timeGetTime() > m_stBroadcastCurrent->time)
			return true;
	};
};

don't want to spam this thread with pics, so I'll just include links.

[Only registered and activated users can see links. Click Here To Register...]
[Only registered and activated users can see links. Click Here To Register...]
[Only registered and activated users can see links. Click Here To Register...]
[Only registered and activated users can see links. Click Here To Register...]
[Only registered and activated users can see links. Click Here To Register...]