Requesting code review

[yoda]

Converted my fishing macro using new parms and stuff, hopeing maybe a couple people can look over it and see if I made any mistakes.

Thanks in advance.

Code: Select all

|***************************************************************************************
| - Fish.mac -                                                                         *
| - By: Yoda                                                                           *
| - v1.1                                                                               *
|                                                                                      *
| Usage:                                                                               *
| To Fish using a Fisherman's Companion.                                               *
| Will Gate, Sit and /afk when you run out of bait.                                    *
|                                                                                      *
|***************************************************************************************

#turbo 

#event BrokenPole "You can't fish without a fishing pole, go buy one." 
#event NoBait "You can't fish without fishing bait, go buy some."
#event Fizzle "Your spell fizzles" 

Sub Main 

   /cleanup 

   :Fish 
	/call CheckPole  
	/doability Fishing 
	/delay 65  
	/doevents

	/newif (${Cursor.Name.Equal[Tattered Cloth Sandal]}) /destroy
	/delay 1
	/newif (${Cursor.Name.Equal[Rusty Dagger]}) /destroy
	/delay 1
	/newif (${Cursor.ID}) /call KeepItem
	/goto :Fish 

/return 

Sub KeepItem

	/newif (${Cursor.Name.NotEqual[Fish Scales]}) /call DoLog "I caught some (${Cursor.Name})"
	/autoinventory

/return

Sub CheckPole 
   
	/newif (${Me.Inventory["primary"].Name.Equal["Brell's Fishin' Pole"]}) /return 
	/newif (${Me.State.Equal["SIT"]}) /stand
	/delay 4
	/cast item "Fisherman's Companion"
	/delay 11s 
	/autoinventory 
	/sit     

/return 

Sub GateMe 

	/delay 6 
	/newif (${Me.State.Equal["SIT"]}) /stand
	/cast "Gate"
	/delay 5
	/doevents
	/delay 6s
	/newif (${Me.State.Equal["STAND"]}) /sit
	/afk
	/endmacro 

/return
    
Sub DoLog(Text)
	
	/mqlog @Text
	/echo @Text

/return

Sub Event_BrokenPole 

	/call CheckPole 

/return 

Sub Event_NoBait 

	/call GateMe 

/return

Sub Event_Fizzle
	
	/call GateMe 

/return

[Lax]

1) All /if or /newif (same thing now) must have () around the conditions, like:
/if (condition) /command

2) /newif ${Cursor} /call KeepItem
This will look like:
/newif Fish Scales /call KeepItem

which the parser cannot figure out where the conditions end and the command begins, and cannot determine whether you want "Fish Scales" to be true or false. What you want is:
/newif (${Cursor.ID}) /call KeepItem

I dont see anything too obvious other than that

[Chaesar]

Shouldn't the two instances of

Code: Select all

/newif (${Me.State.Equal["SIT"]}) /sit

be

Code: Select all

/newif (${Me.State.Equal["SIT"]}) [color=red]/stand[/color]

in order to stand for casting your fishing pole and gate?

[yoda]

/sit /stand does the same thing, so yes, I could change it.

What if all I want to know, is if there is something on the cursor? that is what I was trying to do with this:

Code: Select all

/newif (${Cursor.ID}) /call KeepItem 

also,

Code: Select all

/call DoLog "Caught (${Cursor.Name}) ..."

Will that put the name of what is on the cursor in that statement?

[Lax]

/newif (${Cursor.ID}) /call KeepItem

that's what that does.

[yoda]

Thanks alot Lax, how does it look now?


Will this put the name of the item on the cursor in the Text for the DoLog subroutine, or does it need to be (${Cursor.ID})?

Code: Select all

/newif (${Cursor.Name.NotEqual[Fish Scales]}) /call DoLog "I caught some (${Cursor.Name})"

Sub DoLog(Text) 
    
   /mqlog @Text 
   /echo @Text 

/return

[Lax]

ID is the item number, not the name

You're doing fine. You can also skip the .Name part in ${Cursor} unless you need to do string operations on the name (you dont), because the "To String" of "item" is "Same as Name". No big deal there, just letting you know

[yoda]

Thanks again lax, onto a Logic question, looking for some advice, I am also converting this fish macro to work for a necro that will FD while fishing. Everything is mostly the same, except one problem. my Event_fizzle now could be from either of the 2 spells that I cast, Gate or Feign Death, I need some logic, so that if it is gate that fizzled, it recast gate, but if FD fizzled, i need to cast FD again and continue on with the macro, any help would be great...

Code: Select all

|***************************************************************************************
| - NecroFish.mac -                                                                    *
| - By: Yoda                                                                           *
| - v1.0                                                                               *
|                                                                                      *
| Usage: For Necros                                                                    *
| To Fish using a Fisherman's Companion.                                               *
| Will Gate, Sit and /afk when you run out of bait.                                    *
| Destroys Tattered Cloth Sandals and Rusty Daggers, keeps everything else.            *
|***************************************************************************************

#event BrokenPole "You can't fish without a fishing pole, go buy one." 
#event NoBait "You can't fish without fishing bait, go buy some."
#event Fizzle "Your spell fizzles" 
#event Death "You die"

Sub Main 

   /cleanup 
   /call Feign
   /call Fishin

/endmacro

Sub Fishin

   :Fish 
	/call CheckPole
	/doability Fishing 
	/delay 65  
	/doevents
	/newif (${Cursor.Name.Equal[Tattered Cloth Sandal]}) /destroy
	/delay 1
	/newif (${Cursor.Name.Equal[Rusty Dagger]}) /destroy
	/delay 1
	/newif (${Cursor.ID}) /call KeepItem
	/goto :Fish 

/return 

Sub CheckPole 
   
	/newif (${Me.Inventory["primary"].Name.Equal["Brell's Fishin' Pole"]}) /return 
	/newif (${Me.State.Equal["SIT"]}) /stand 
	/delay 4
	/cast item "Fisherman's Companion"
	/delay 11s 
	/autoinventory 
	/call Feign
	
/return 

Sub Feign

	/newif (${Me.State.Equal["SIT"]}) /stand
	/cast "Feign Death"
	/delay 6s
	/doevents

/return 

Sub KeepItem

	/newif (${Cursor.Name.NotEqual[Fish Scales]}) /call DoLog "Caught (${Cursor}) ..."
	/autoinventory

/return

Sub GateMe 

	/delay 6 
	/sit
	/newif (${Me.State.Equal["SIT"]}) /stand
	/cast "Gate" 
	/delay 5
	/doevents
	/delay 6s
	/newif (${Me.State.Equal["STAND"]}) /sit
	/afk
	/endmacro 
/return
    
Sub DoLog(Text)

	/mqlog @Text
	/echo @Text

/return

Sub Event_BrokenPole 

	/call CheckPole 

/return 

Sub Event_NoBait 

	/call GateMe 

/return

Sub Event_Fizzle
	

/return

Sub Event_Death
 

/return

[newb-n-training]

Just a newb here Yoda so take the logic as you will but could you simply always recast your spell (Gate, FD, whathaveyou) and end the macro on "LOADING, PLEASE WAIT" in the instance of Gate?

One other thing I thought I'd mention is an event of the gate collapsing (no fizzle, just no gate). Might need to account for that depending on your plans.

[yoda]

How about this, anyone see any problems?

Code: Select all

|***************************************************************************************
| - NecroFish.mac -                                                                    *
| - By: Yoda                                                                           *
| - v1.1                                                                               *
|                                                                                      *
| Usage: For Necros                                                                    *
| To Fish using a Fisherman's Companion.                                               *
| Will Gate, Sit and /afk when you run out of bait.                                    *
| Destroys Tattered Cloth Sandals and Rusty Daggers, keeps everything else.            *
|***************************************************************************************

#event BrokenPole "You can't fish without a fishing pole, go buy one." 
#event NoBait "You can't fish without fishing bait, go buy some."
#event Death "You die"
#event Unstable "unstable"
#event Zone "LOADING, PLEASE WAIT"
#Event End "You stop fishing" 
#Event End "Trying to catch land sharks perhaps?" 
#Event End "You must be on dry land to fish."

Sub Main 

   /cleanup 
   /call Feign

/endmacro

Sub Fishin

   :Fish 
	/call CheckPole
	/doability Fishing 
	/delay 65  
	/doevents
	/newif (${Cursor.Name.Equal[Tattered Cloth Sandal]}) /destroy
	/delay 1
	/newif (${Cursor.Name.Equal[Rusty Dagger]}) /destroy
	/delay 1
	/newif (${Cursor.ID}) /call KeepItem
	/goto :Fish 

/return 

Sub CheckPole 
   
	/newif (${Me.Inventory["primary"].Name.Equal["Brell's Fishin' Pole"]}) /return 
	/sit
	/newif (${Me.State.Equal["SIT"]}) /stand 
	/delay 4
	/cast item "Fisherman's Companion"
	/delay 11s 
	/autoinventory 
	/call Feign
	
/return 

Sub Feign

	/newif (${Me.State.Equal["SIT"]}) /stand
   :Death
	/cast "Feign Death"
	/delay 7s
	/doevents
	/goto :Death

/return 

Sub KeepItem

	/newif (${Cursor.Name.NotEqual[Fish Scales]}) /call DoLog "Caught (${Cursor}) ..."
	/autoinventory

/return

Sub GateMe 

	/delay 6 
	/sit
	/newif (${Me.State.Equal["SIT"]}) /stand
   :Gate
	/cast "Gate" 
	/delay 6s
	/doevents
	/goto :Gate
	
/return
    
Sub DoLog(Text)

	/mqlog @Text
	/echo @Text

/return

Sub Event_BrokenPole 

	/call CheckPole 

/return 

Sub Event_NoBait 

	/call GateMe 

/return

Sub Event_Zone
	
	/newif (${Me.State.Equal["STAND"]}) /sit
	/afk
	/endmacro 

/return

Sub Event_Death

	/call Fishin

/return

Sub Event_Unstable
 
	/call GateMe 

/return

Sub Event_End
 
	/endmacro 

/return