check my code please (1 Viewer)

[motorsgt]

smacked up revision of noob's code from way back. Some subs left out due to the fact that i want to make sure i am on the right track to start with.

|-------Events go here--------------------------------------|
#Event Slowed                "#*# slows down."
#Event Prepare               "#*# gain party experience"
|-------end events------------------------------------------|



|-------Declare weapons and self buff items here------------|
   /declare slow_weapon        string outer    *Itemname*
   /declare dps_mainhand       string outer    *Itemname*
   /declare dps_offhand        string outer    *Itemname*
   /declare haste_item         string outer    *Itemname*
   /declare hp_buff            string outer    *Itemname*
   /declare ac_buff            string outer    *Itemname*
|--------end weapon and item declares------------------------|


|--------other declares--------------------------------------|
   /declare slowonce           bool outer TRUE
   /declare dotonce            bool outer TRUE
   /declare tankname           string outer      *TANKNAME*
   /declare healername         string outer      *HEALERNAME*
   /declare myzone	       string outer      ${Zone.ID}
|--------end other delclares---------------------------------|


|--------mob array-------------------------------------------|
/declare MobArray[5] string outer
   /varset MobArray[1] "Nghala"
   /varset MobArray[2] "Tetor"
   /varset MobArray[3] "Talliz"
   /varset MobArray[4] "Kalliz"
   /varset MobArray[5] "Dreamthief"
|--------end mob array---------------------------------------|


|--------PC alerts-------------------------------------------|
/Alert add pc ${healername}
/Alert add pc ${tankname}
|-------end pc alerts----------------------------------------|


|-------Main Sub---------------------------------------------|
Sub Main
/call playercheck
/call GMcheck
/call buffcheck
/call mobcheck
/return
|-------end main sub----------------------------------------|


|-------check for npc as target-----------------------------|

Sub mobcheck

/for MobCheck 1 to ${MobArray.Size}
            /if (!${Target.Type.Equal["NPC"]}) 
         /next MobCheck    
/if ((${Target.Type.Equal["NPC"]})&&(${Target.Distance}>20)) / Call combat
/return

|-------end mob check---------------------------------------|


|-------Combat sub------------------------------------------|
Sub Combat

/if (${Target.Distance}>20) /warp target

  :combat loop
    /doevents
    /attack on
    /stick 13 moveback
    /if (${Target.Type.NotEqual["NPC"]} || !${Target.ID}) /return {
|------Nukes and DoTs to decrease mobs HPS
    /if ((${Me.PctHPs}>60)&&(${Target.PctHPs}<50)&&(${dotonce})) {
	/tell ${HealerName} dot now biatch
        /varset dotonce FALSE
	}
    /if ((${Me.PctHPs}>60)&&(${Target.PctHPs}<90)&&(${nukeonce})) {
        /tell ${HealerName} nuke now biatch
        /varset nukeonce FALSE
	}
  /goto :combat loop 
/return
|-------end combat sub--------------------------------------|

|-------player check----------------------------------------|
Sub playercheck
  /if (${Spawn[pc noalert 2 radius 500].ID}) { 
	/warp succor
	/mqlog Player near
	/quit
	/endmacro
   /return
|-------end player check------------------------------------|


|-------gm check--------------------------------------------|
Sub GMCheck
/if (${Zone.ID}!=${myzone}) {
	/quit
	/endmacro
	}	
   /if (${Spawn[gm].ID}) {

      /mqlog GM ENTERED ZONE!!!

      /keypress forward
      /keypress back

      /quit
      /endmacro
      /unload
   }
   
/return
|------end gmcheck------------------------------------------|


Sub slowed
 /exchange *Itemname* mainhand
 /exchange *Itemname* offhand
 /return

Sub prepare
 /unequip offhand
 /exchange item ${slow_weapon} mainhand
 /return

 

[Tone]

You need to put a loop into your Sub Main, otherwise it's only going to call those checks once. Basically your macro will do the checks, and then after the first mob is down it will reach the end of the macro. This macro is also over commented which gets rather annoying, but that's not really an issue code wise just personal preference.

I also noticed that your combat loop is named "combat loop" when naming variables, events, subs, and loops avoid using spacing. It makes coding much easier and faster aswell as smoother. I'd suggest renaming it to combatloop or just simply combat.

/if ((${Target.Type.Equal["NPC"]})&&(${Target.Distance}>20)) / Call combat

The spacing here might just be the forums, but that call won't fire. Also change it to /call Combat or rename your Sub Combat to Sub combat. Case matters in macros :P

Your array and declared variables all seem to be used correctly through out the rest of the macro. It's a good start post or pm the finish product if you want me to go through it some more with you.