Any way to make this item randomizer not lag?

[gr8dayseth]

ok so long story short I needed an addon event that someone made for me that could randomize a weapon w/ a blacklist and give it to the player, problem is it lags for like a second or two unlike addrandomitem(in event_setrandom if you don't know) no matter how many things on the blacklist I put on there, and with many people using this it's very noticeable and annoying.
I dunno much of what I'm looking at so by any chance is there a way to make it more efficient?

Code: [Select]

registerOutputEvent(Player, "addRandomItemBlacklisted", "");
$Pref::Server::AddRandomItemBlacklist = "NONE" TAB "Wrench" TAB "Printer" TAB "Fillcan" ;

function checkIfToolBlacklisted(%uiname)
{
for( %i = 0 ; %i < getFieldCount( %bl = $Pref::Server::AddRandomItemBlacklist ) ; %i++ )
if( %uiName $= getField( %bl , %i ) ) { return 0; }

return 1;
}

function Player::addRandomItemBlacklisted(%player,%client)
{
   for(%i = 0; %i < DataBlockGroup.getCount(); %i++)
   {
      %obj = DataBlockGroup.getObject(%i);
      if(%obj.getClassName() $= "ItemData" && (%uiname = %obj.uiName) !$= "" && checkIfToolBlacklisted(%uiname)) //parser rape
         %item[%itemCount++] = %obj;
   }
   %randomitem = getRandom(0,%itemCount);
   if(%player.tool[0] $= %item[%randomitem] || %player.tool[1] $= %item[%randomitem] || %player.tool[2] $= %item[%randomitem] || %player.tool[3] $= %item[%randomitem] || %player.tool[4] $= %item[%randomitem])
   {
      return;
   }
   for(%i=0;%i<5;%i++)
   {
      %tool = %player.tool[%i];
      if(%tool == 0)
      {
         %player.tool[%i] = %item[%randomitem];
         %player.weaponCount++;
         messageClient(%client,'MsgItemPickup','',%i,%item[%randomitem]);
         break;
      }
   }
}

[Ad Bot]

[otto-san]

don't re-build the list every time. find a way to store it (probably in a global 'array'), or build it manually so you have better control.

also, in loops, it's best to store the values from functions like getCount and getFieldCount in variables rather than having them in the conditional, otherwise the function is called every iteration

e.g.
%ct = %butts.getCount();
for(%i = 0; %i < %ct; %i++)
   ...

rather than

for(%i = 0; %i < %butts.getCount(); %i++)
   ...

[Kyuande]

Here's what I used for adding items.
You can use the item cache object's size and randomize it.
What findItemByName does is creates a cache, and doesn't rescan unless the cache got deleted somehow. You can find item names much easier with it, but currently I forgot that it has to be exact with some stuff, "hammer" will not work, but "Hammer" will. Can anyone fix that?

%player.addNewItem(ItemCache.item[getRandom(0, ItemCache.itemCount-1)]);

function findItemByName(%item, %val)
{
   if(isObject(%item))
      return %item.getName();

   if(!isObject(ItemCache)) new ScriptObject(ItemCache);
   if(ItemCache.itemCount <= 0 || %val) //We don't need to cause lag everytime we try to find an item
   {
      ItemCache.itemCount = 0;
      for(%i=0;%i<DatablockGroup.getCount();%i++)
      {
         %obj = DatablockGroup.getObject(%i);
         if(%obj.getClassName() $= "ItemData" && strLen(%obj.uiName) > 0)
         {
            ItemCache.item[ItemCache.itemCount] = %obj;
            ItemCache.itemCount++;
         }
      }
   }

   //First let's see if we find something to be exact
   if(ItemCache.itemCount > 0)
   {
      for(%a=0;%a<ItemCache.itemCount;%a++)
      {
         %objA = ItemCache.item[%a];
         if(%objA.getClassName() $= "ItemData")
            if(%objA.uiName $= %item)
               return %objA.getName();
      }
   }

   //Okay, we found nothing, let's see if we can find it.
   if(ItemCache.itemCount > 0)
   {
      for(%a=0;%a<ItemCache.itemCount;%a++)
      {
         %objA = ItemCache.item[%a];
         if(%objA.getClassName() $= "ItemData")
            if(strPos(%objA.uiName, %item) == 0)
               return %objA.getName();
      }
   }
   return -1;
}

function Player::addNewItem(%player, %item)
{
   %client = %player.client;
   if(isObject(%item))
   {
      if(%item.getClassName() !$= "ItemData") return false;
      %item = %item.getName();
   }
   else
      %item = findItemByName(%item);
   if(!isObject(%item)) return -1;
   %item = nameToID(%item);
   for(%i = 0; %i < %player.getDatablock().maxTools; %i++)
   {
      %tool = %player.tool[%i];
      if(!isObject(%tool))
      {
         %player.tool[%i] = %item;
         %player.weaponCount++;
         messageClient(%client,'MsgItemPickup','',%i,%item);
         return true;
      }
   }
   
   return false;
}
//registerOutputEvent(Player,addNewItem,"string 50 50");

[otto-san]

i'm pretty sure (italics to indicate unsureness) that there's no real benefit to using an object here, and that it's more efficient to just use global values if all you're doing is storing a list. either way works and would solve the problem, tho

also that could be way simplified by just having a list of potential items and adding a random entry from that list (ie addItem(list[getRandom(length - 1)])