HomeUser Control Panel (unavailable in archive)ForumsTutorialsArt GalleryResourcesMaps

Suggestions on code

03-13-2007, 05:35 AM#1
Fluxx
Moving my way into jass, but I want to build good habits before the bad ones set in. Would any kind souls take a look at this trigger I wrote and mention any bad habits or bad coding they might see? Thanks!

Collapse JASS:
function TwistBack_Filter0 takes nothing returns boolean
     return(UnitHasBuffBJ(GetFilterUnit(),'B000') == true)
endfunction

function TwistBack_Filter1 takes nothing returns boolean
     return(UnitHasBuffBJ(GetFilterUnit(),'B001') == true)
endfunction

function TwistBack_Filter2 takes nothing returns boolean
     return(UnitHasBuffBJ(GetFilterUnit(),'B002') == true)
endfunction

function TwistBack_Filter3 takes nothing returns boolean
     return(UnitHasBuffBJ(GetFilterUnit(),'B003') == true)
endfunction

function TwistBack_Filter4 takes nothing returns boolean
     return(UnitHasBuffBJ(GetFilterUnit(),'B004') == true)
endfunction

function Trig_TwistBackFunc_Actions takes nothing returns nothing

local unit u
local group array g
local player array a
local boolexpr array p
local rect loc = GetPlayableMapRect()
local integer i = 0
local integer array b
local integer array c

set p[0] = Condition(function TwistBack_Filter0)
set p[1] = Condition(function TwistBack_Filter1)
set p[2] = Condition(function TwistBack_Filter2)
set p[3] = Condition(function TwistBack_Filter3)
set p[4] = Condition(function TwistBack_Filter4)

set c[0] = 'A00H'
set c[1] = 'A00D' 
set c[2] = 'A00E' 
set c[3] = 'A00F' 
set c[4] = 'A00G' 
 
set b[0] = 'B000'
set b[1] = 'B001' 
set b[2] = 'B002' 
set b[3] = 'B003' 
set b[4] = 'B004' 
 
loop
 exitwhen i > 4
 set g[i] = CreateGroup()
 set a[i] = Player(i)
 call GroupEnumUnitsInRect(g[i], loc, p[i])
 set i = i+1
endloop 

set i = 0 

loop
 exitwhen i > 4 
  loop 
   set u = FirstOfGroup(g[i])
   exitwhen u == null
    if UnitHasBuffBJ(u,b[i]) then
     if GetRandomInt(1,10) == 1 then
      call UnitRemoveAbility(u,c[i])
      call SetUnitOwner(u,a[i],true)
      call GroupRemoveUnit(g[i],u)
     else
      call GroupRemoveUnit(g[i],u) 
     endif 
    endif
  endloop
   set i = i+1
endloop    

set i = 0

loop
 exitwhen i > 4
 call DestroyGroup(g[i])
 call DestroyBoolExpr(p[i])
 set g[i] = null
 set p[i] = null
 set a[i] = null
 set i = i+1
endloop   

set loc = null

endfunction
 
      
function Trig_TwistBack_Actions takes nothing returns nothing

local timer t = CreateTimer()
call TimerStart(t,1,true,function Trig_TwistBackFunc_Actions) 
set t = null

endfunction

//===========================================================================
function InitTrig_TwistBack takes nothing returns nothing
    set gg_trg_TwistBack = CreateTrigger(  )
    call TriggerRegisterTimerEventSingle( gg_trg_TwistBack, 5 ) 
    call TriggerAddAction(gg_trg_TwistBack, function Trig_TwistBack_Actions )
endfunction

My concern is about the speed that buffs are applied. Should I insted check the level of the ability that gives the buff?

-Fluxx
03-13-2007, 06:09 AM#2
Earth-Fury
#1: Use [jass] and [ijass] and [trigger] tags.
#2: Your local variable names dont make that much sense. "p" for a boolexpr, while "a" for player?.. descriptive variable names are a must. (usually you can get away with 'p' for player, 'i' for an iterator integer and the like without being confusing. especially if used as function arguments.)
#3:
Collapse JASS:
function Trig_TwistBack_Actions takes nothing returns nothing

local timer t = CreateTimer()
call TimerStart(t,1,true,function Trig_TwistBackFunc_Actions) 
set t = null

endfunction

http://wc3campaigns.net/showthread.php?t=81872
Quote:
Timers: Attempting to set timers to null will eventually result in catastrophe.
Please read ;)




Besides that, and from a rather brief lookover of your code (you REALLY need better variable names. hurts my head to remember what "c" is...) its rather good. Especially for a beginner.
03-13-2007, 06:19 AM#3
Fluxx
Hmmm,
Read and re read the tutorial on timers. I am going to have to check it out later, as it is not making too much sense at the moment (the reusable timer stack seems to be a good way to do things, but I cannot think of how to incorporate it in, yet).