[Kristi]

Skyld threw in his critques but ill put mine in anyway

PHP Code:

//Point and Copy: Cloning Device
//By: Gambet
//Notes by Hell Raven

//#CLIENTSIDE
function onKeyPressed(keycode) {
  if (keycode == 305) {
    // If possible, we do not want to test unneccessary conditions
    // !true = false, and !false = true, so this will make if the opposite
    // of what it is.
    this.cloner_on = !this.cloner_on;

    // Now we are setting the player's chat to Outfit Cloner: Off or On
    // We have an array containing {"Off","On"}, where off is
    // element 0, and on is element 1. False = 0 and True = 1,
    // so this works.
    player.chat = "Outfit Cloner:" SPC ({"Off","On"})[this.cloner_on];
  }
}

function onMouseDown(button) {
  // Condition really belongs on one line
  // Check the most likely false condition first
  // To optimize resources.
  if (button == "double" && this.cloner_on) {

    // Cut short if we have not "cooled down" yet.
    if(timevar2 < this.cooldown) {
      player.chat = "You must wait " @ abs(int(timevar2 - this.cooldown)) @ " more seconds!";
      return; //Exit function
    }
    
    // You originally had the npc loop nested inside the player loop.
    // That means you checked all the npcs again everytime
    // you switched to a different player. That is bad :x
    
    // See if any player is in clone range
    for (p: players)
      if (mousex in |p.x,p.x+1.5| && mousey in |p.y,p.y+2|) {
        cloneobj(player,p);
        this.cooldown = timevar2 + 10;
        player.chat = "Cloning Successful!";
        return; // We have cloned so we do not need to look anymore.
      }
    
    // See if any npc is in clone range 
    // note, we check for ani so we do not clone npcs that aren't showchars.   
    for (n: npcs)
      if (n.ani != null && mousex in |n.x,n.x+1.5| && mousey in |n.y,n.y+2|) {
        cloneobj(player,n);
        this.cooldown = timevar2 + 10;
        player.chat = "Cloning Successful!" @ n.ani;
        return; // We have cloned so we do not need to look anymore
      }
  }
}

function cloneObj(copycat,original) {
  // makes copycat look like original :D

  // the colors array cannot be set at once, for some reason
  for(temp.i = 0; temp.i <=5; temp.i++)
    copycat.colors[temp.i] = original.colors[temp.i];

  copycat.head   = original.head;
  copycat.sword  = original.sword;
  copycat.body   = original.body;
  copycat.shield = original.shield;
} 


You should make functions for certain things, like a player clone. honestly it should be a class attacked to a player, to do player.clone(obj), but we are keeping it in one weapon.

The nested loop was something i pointed out to skyld. isntead of
for(players) {
for(npcs) {
}
}
we want
for (players) {
}
for (npcs) {
}

We want to return when we have achieved our goal, so we are not achieving it multiple times (in my opinion).

Also, i threw in a check to make sure the npc was a character.

The rest of my notes are in the script.