Automatic Promotions?
 

When I use my clan GUI, when I promote someone, it automatically promotes them over and over to the max rank! x.x Can someone tell me how to fix this?
BTW: its the if(params[0] == "demote") and the if(params[0] == "promote") where im stuck...

You need to use else if instead of just if because you are using a sleep after promotion (which I don't really see why), and then since you don't have else if, the compiler reads the next line of code which tells the system to repromote the player again and it will continue this process all the way until there are no more ranks. Also, you should get rid of the sleeps.

Say the player's position is "Recruit". When the code gets to this line:

NPC Code:

---

        if (pl.clientr.position == "Recruit"){ 

---

the result prove true so it continues inside the conditional block and sets the player's position to warrior, your chat tells you it has become warrior and then the script sleeps for a second.

After that is done it comes upon this line:

NPC Code:

---

         if (pl.clientr.position == "Warrior"){ 

---

Because you just set pl.clientr.position to Warrior, it is of course, true so the statements inside the conditional block are also read.

You are wondering, why does it read this line too? Well, why wouldn't it? You haven't put anything into the code to stop it from continuing with the following statements.

One solution is to make all the if statements into else if except for the top one (the Recruit one). The else if means that the statement will only be read if the conditional statement before was not true. If it was true, all the following else if statements will be ignored.

Another solution is to use a switch block like:

NPC Code:

---

switch (pl.clientr.position) {

  case "Recruit":

    //set the positon to the new one

    //and set your chat

  break;

  case "Warrior":

    //set the positon to the new one

    //and set your chat

  break;

}

---

Important thing to remember with switchs is that the break; statement causes you to escape the entire switch once the stuff before it has been carried out. The code will automatically skip to which ever case that the variable in the switch is equal to. If you don't have the break before the next case, the code will do the cases after the one it skipped to even if they aren't true.

And yes, you really don't need those sleeps, though I'm assuming you just had them to figure out the problem with the code.

Please see this for your GUIs.

Also, I know it's personal preference, but it really would be nice on our (mine, at least) eyes if you would add line breaks every now and then.

Your code is also hard to follow in some places because you do not consistently indent.

It's a lot easier to follow if you always indent when using braces.

At the very first few lines, you can do this:

which will end the function, rather then adding a big check which just turns out to be confusing.

Around line 6, you're doing (and also near the 'update' command)

which should be (GS2):
or even
You can also name your parameters for easier recognition by yourself/others.

Instead of doing

you should use multi-dimensional arrays to avoid some kind of synchronization error.

You should remove the sleeps, they serve absolutely no purpose that I can see besides potentially messing up the script (sleeps tend to break things).

For promoting/demoting, it would be a lot simpler to do something like

Demote would be something like

Also, make sure you don't do this

because 'pl' is a player object. In GS2, when you attempt to print an object, it outputs 'object.name', which happens to be the players account name. Instead you should do something like