Page 2 of 4

Posted: Thu Dec 08, 2005 1:36 pm
by toomanynames
I wasn't trying to "start anything" either. It was more of an open ended question. Fearless and AD00 have helped me a lot towards coding so in a way I "lookup" to what they have developed.

In most of my posts that I've placed code (what I'll call enhancements), I very rarely get feedback, so when I hear someone say they like X better then Y and Y is something I made, I'd like to know why to better myself/code.

In the end I don't care what someone uses as everything has a place of sorts, my point was more along the lines of what those two felt was "wrong" with the code

It's all good

Posted: Thu Dec 08, 2005 1:38 pm
by mystikule
/concur

If someone so desired they could slice and dice to copy/paste the best of both these incs to get the best of both worlds.

But this in nonvip here hehe :wink:

Posted: Thu Dec 08, 2005 1:46 pm
by fearless
I was causing trouble. I admit it.

Posted: Thu Dec 08, 2005 1:48 pm
by A_Druid_00
Damn, pwnt by IRC. That doesn't mean I won't need to do some serious work on this one too, it's just that advloot has a lot of features that I don't need with autobot.

Some suggestions if you want to make this a little better:
Get rid of the /varsets in presetup. No need to set every string in an array to NULL. Plus, since you appear to be storing the corpse's ID # (Which is the irght way to do it), it can be an array of integers instead, which will take up a good bit less memory (around half the same size array as a string). Same goes for TempCorpse, playercorpse, and any other string you're using integers in. I'm amazed numeric comparisons work with strings the way you're doing them as is.
Instead of setting up 10 timers (which is also pretty memory intense), declare another array of integers, and set the number to ${MacroQuest.Running}+TimeYouWantToWaitInSeconds.
Alteratively, you can use a 2d (/declare CorpseList[10,2]) array which contains the corpse ID as well as the timestamp of when the timer should expire. You can find a good example of populating/removing values from a 2d array in both AdvBot's MDL routine(Where I stole it) and AutoBot's Add/Remove NPCs and Add/RemoveFromQueue.
If you use this approach you can safely up the size of the array to double what it is now using the same amount of memory.

Get rid of the /doevents in the beginning of the macro. I wouldn't want another routine screwing with my events. I call them when it's convenient to call them.

While it's only 2 aliases, if you plan on adding more in the future, it's best to put a NLVersion variable out there and using that as a decision to write aliases or not. When you're 6 boxing on one PC, 6 macros writing 2 aliases at the same time can put a crunch on your hard drive.

Organize your ini file aplhabetically instead of by zone. There are a number of zones that share the same drops. For example, Phosphorus Powder and just about every Planes of Power zone among others. No sense recycling the same ini entry for 50 different zones and setting the detroy/loot/etc flag for every one of them. Just put an entry based on Left[1] or something.

Make better use of /for loops where possible. Example:

Code: Select all

  /varset Corpseskip[10] ${Corpseskip[9]} 
  /varset Corpseskip[9] ${Corpseskip[8]} 
  /varset Corpseskip[8] ${Corpseskip[7]} 
  /varset Corpseskip[7] ${Corpseskip[6]} 
  /varset Corpseskip[6] ${Corpseskip[5]} 
  /varset Corpseskip[5] ${Corpseskip[4]} 
  /varset Corpseskip[4] ${Corpseskip[3]} 
  /varset Corpseskip[3] ${Corpseskip[2]} 
  /varset Corpseskip[2] ${Corpseskip[1]} 
  /varset Corpseskip[1] ${Target.ID} 
Can easily become:

Code: Select all

  /declare i int local
  /for i 10 downto 2
  /varset Corpseskip[${i}] ${Corpseskip[${Math.Calc[${i}-1]}]} 
  /next i
  /varset Corpseskip[1] ${Target.ID} 
There's more, but I'm probably making your brain hurt as is.

Posted: Thu Dec 08, 2005 1:54 pm
by toomanynames
O noes, AD00 offered advice for Myst when I was the one asking LOL.

That is one of those puts foot in mouth things, yes?

/hugs AD00

Posted: Thu Dec 08, 2005 2:00 pm
by mystikule
Love all the tips, that which does not kill you only makes you stronger. 2d arrays, I need to play with these.

Posted: Thu Dec 08, 2005 2:01 pm
by A_Druid_00
Your inc is pretty clean toomanynames, don't get me wrong. I plan on taking a look at yours more in depth, in a clever ruse to avoid updating my healing code.

Posted: Thu Dec 08, 2005 2:23 pm
by A_Druid_00
I should expand more on the ini thing. Instead of using ${Zone.ID} for your section name, make it ${Invslot[Blah].Item.Name.Left[1]}

Posted: Sun Dec 11, 2005 7:47 am
by mystikule
Cleaned up some of the coding and reorg'd the ini. Haven't yet looked into 2d arrays. Thanks for the all the suggestions AD00, and as always more critism/suggestions are welcome from anyone.

Posted: Sun Dec 11, 2005 1:06 pm
by A_Druid_00
The next thing to adress is the /gotos. The general concensus is that /goto is pretty slow, and in most cases is best avoided at all costs. You can usually get around it by changing your /goto section into a whole other sub, example:

Code: Select all

  |- Check version in ini and skip aliases if current 
  /if (${Ini[${NLLootINIFile},Loot,Version].Equal[${NLVersion}]}) /goto :NLEndAlias 

  /ini "${NLLootINIFile}" Loot Version "${NLVersion}" 
  /squelch /alias /autoloot /echo SetAutoLoot 
  /squelch /alias /lootstatus /echo Autoloot Status 

  :NLEndAlias 
can just as easily be:

Code: Select all

Sub NLPresetup
Stuff
/call NLCheckVersion
More Stuff
/return

Sub NLCheckVersion
/if (${Ini[${NLLootINIFile},Loot,Version].Equal[${NLVersion}]}) /return
/ini "${NLLootINIFile}" Loot Version "${NLVersion}"
/squelch /alias /autoloot /echo SetAutoLoot
/squelch /alias /lootstatus /echo Autoloot Status
/return
The auto building of the ini sections is probably a good idea, so you have the sections guaranteed in alpha order. I'll play with it a bit and see how it goes.

Edit:
I'm pretty sure this section won't work as intended:

Code: Select all

Sub Event_SetAutoLoot(string Line)
|- Set the new loot distance, if supplied
/if (${Line.Arg[4].Length}) /varset NLlootdist ${Line.Arg[4]}
|- Set the mode
/if (${Line.Arg[3].Equal[ALL]}) {
  /varset NLlootmode ALL
  /echo ** AutoLoot ON - Will loot ALL items
  /echo ****CAUTION**** Turning off requestor for NODROP items!!!  ****CAUTION****
  /echo ** AutoLoot distance is ${NLlootdist} ft.
  /docommand /lootnodrop never
} else /if (${Line.Arg[3].Equal[DROP]}) {
  /varset NLlootmode DROP
  /echo ** AutoLoot ON - Will loot only DROPABLE items
  /echo ** AutoLoot distance is ${NLlootdist} ft.
  /docommand /lootnodrop always
} else /if (${Line.Arg[3].Equal[NDITEM]}) {
  /varset NLlootmode NDITEM
  /echo ** AutoLoot ON - Will loot only SPECIFIED NO-DROP items
  /echo ** AutoLoot distance is ${NLlootdist} ft.
  /docommand /lootnodrop always
} else /if (${Line.Arg[3].Equal[OFF]}) {
  /varset NLlootmode OFF
  /echo ** AutoLoot OFF - Will NOT loot any items
} else {
  /echo Syntax:
  /echo /autoloot <all|drop|nditem|off> <radius#>
  /echo Defaulting to OFF
  /varset NLlootmode OFF
}
/ini "${NLLootINIFile}" "Loot" "AutoLootMode" "${NLlootmode}"
/ini "${NLLootINIFile}" "Loot" "AutoLootDistance" "${NLlootdist}"
/return
MQ only checks the first } else in the line. It's meant for statements like /if THIS, /do this, ELSE /do this. No big deal though, as each of those extra /if statements will return FALSE if one of them is TRUE. I could be wrong though, but that's my experience

Posted: Wed Dec 14, 2005 6:27 pm
by mystikule
Posted a new version. Cleaned up some of the /goto commands and got rid of timers. Tested for approx two hours, seems everything is functioning as intended. Post any suggestions or requests as always. I've had a ball learning better ways to get things coded so if you see something that can be improved letme know.

Posted: Tue Jun 27, 2006 1:23 pm
by aChallenged1
For my next trick, I'm going to see about converting this into IS format.

That should be fun and educational. Someone please send Lax a padded desk cover so he doesn't kill himself when I start asking more questions. :wink:

Posted: Mon Aug 21, 2006 8:14 pm
by Kiniktoo
Request this as a plugin~

Posted: Sun Dec 28, 2008 4:56 pm
by BlackOp
I hate to bump such an old thread but I have a small problem with looting.

Everything works exactly as it should while using ninjaloot.inc except for when the NPC corpse being looted contains a stack of items. For example, if a snake drops 2 snake eggs, the 2 eggs are stacked in the loot window. The Everquest UI pops up a little box asking how many you want to loot.

The popup box is the problem.

The popup is causing the script to pause because it does not click the accept button. I looked through the Everquest options but do not see a way to shut off this popup box.

The command to loot in ninjaloot.inc is just the normal eq command /loot.

Thank you for any suggestions. I just came back to EQ and MQ2 recently so I am pretty out of date on everything.

Posted: Sun Dec 28, 2008 5:17 pm
by ieatacid
Can't you make it hold down the shift key to take the whole stack (I think that's the right key). This would also not affect the looting of non-stacked items.