[JASS] Leaks / Efficiency double-check

[DistantHaze]

This trigger runs about every second, so it needs to be as slick as possible. I guess I just wanted a second set (or multiple sets) of eyes to look it over for any inefficiencies and leaks:

function Every1Seconds_Action01 takes nothing returns nothing
    local multiboard Mb          = udg_Multiboard
    local multiboarditem Mbi
    local integer q              = 1
    local integer q_1120         = bj_randDistID [1120]
    local integer n_TC           = udg_TimerCount + 1
    local unit misc_unit
    local real misc_unitL
    local real misc_unitLx
    local real misc_unitX
    local real misc_unitY
    local real m
    local rect r
    local group g
//
    if q_1120 > 0 then
        if q_1120 > 1 then
            set bj_randDistID [1120] = q_1120 - 1
        else
            set bj_randDistID [1120] = 0
            call ReviveHero (gg_unit_Ulic_0023, 5488.000, 9296.000, true)
        endif
    endif
    loop
        exitwhen q > 11
        set q_1120 = bj_randDistID [q + 1120]
        if q_1120 > 0 then
            if q_1120 > 1 then
                set bj_randDistID [q + 1120] = q_1120 - 1
                set Mbi = MultiboardGetItem (Mb, q - 1, 0)
                call MultiboardSetItemValue (Mbi, udg_PlayerString [q + 40] + " (" + I2S (q_1120 - 1) + ")...|r")
            else
                set bj_randDistID [q + 1120] = 0
                set misc_unit = bj_ghoul [q + 1120]
                set bj_ghoul [q + 1120] = null
                if bj_randDistID [q + 1900] == 0 then
                    set misc_unitX = 5488.000
                    set misc_unitY = 9296.000
                else
                    set misc_unitX = GetUnitX (misc_unit)
                    set misc_unitY = GetUnitY (misc_unit)
                endif
                call RemoveUnit (misc_unit)
                if GetPlayerSlotState (Player (q)) == PLAYER_SLOT_STATE_LEFT then
                    set Mb = udg_Multiboard
                    set Mbi = MultiboardGetItem (Mb, q - 1, 0)
                    call MultiboardSetItemValue (Mbi, "|cffef453ac: Player " + I2S (q) + "|r")
                    set Mbi = MultiboardGetItem (Mb, q - 1, 1)
                    call MultiboardSetItemValue (Mbi, "|cffef453aleft|r")
                    set Mbi = MultiboardGetItem (Mb, q - 1, 2)
                    call MultiboardSetItemValue (Mbi, "|cffef453a" + I2S (bj_randDistID [q + 1300]) + "|r")
                    set Mb = null
                    set Mbi = null
                else
                    call ReviveHero (bj_ghoul [q + 1000], misc_unitX, misc_unitY, false)
                endif
            endif
        endif
        set q = q + 1
    endloop
    set misc_unit = null
    set Mb = null
    set Mbi = null
    set udg_TimerCount = n_TC
    if n_TC - ((n_TC / 2) * 2) == 0 then
        call TriggerExecute (gg_trg_Every3Seconds)
    elseif n_TC == 7 or n_TC == 15 then
        set r = gg_rct_NeutralRegion
        set g = CreateGroup ()
        call GroupEnumUnitsInRect (g, r, null)
        loop
            set misc_unit = FirstOfGroup (g)
            exitwhen misc_unit == null
            call GroupRemoveUnit (g, misc_unit)
            if GetUnitAbilityLevel (misc_unit, 'BOhx') == 0 then
                set misc_unitL = GetUnitState (misc_unit, UNIT_STATE_LIFE)
                set misc_unitLx = GetUnitState (misc_unit, UNIT_STATE_MAX_LIFE)
                if misc_unitL < misc_unitLx then
                    set m = (misc_unitLx - misc_unitL) * 0.20
                    call SetUnitState (misc_unit, UNIT_STATE_LIFE, misc_unitL + m)
                    call DestroyEffect (AddSpecialEffectTarget ("Abilities\\Spells\\Undead\\VampiricAura\\VampiricAuraTarget.mdl", misc_unit, "origin"))
                endif
            endif
        endloop
        call DestroyGroup (g)
        set g = null
        set r = null
    elseif n_TC > 17 then
        set udg_TimerCount = 0
        call TriggerExecute (gg_trg_Every15Seconds)
    endif
endfunction

function InitTrig_Every1Seconds takes nothing returns nothing
    set gg_trg_Every1Seconds = CreateTrigger ()
    call DisableTrigger (gg_trg_Every1Seconds)
    call TriggerRegisterTimerEvent (gg_trg_Every1Seconds, 1.20, true)
    call TriggerAddAction (gg_trg_Every1Seconds, function Every1Seconds_Action01)
endfunction

 

[overcold_ice]

more efficient (vJASS), use this instead of that InitTrig_ thing
-> libraries/scopes + library/scope initializer / mod+/struct onInit . . .

and i'm confused reading it, what it does exactly?

 

[DistantHaze]

more efficient (vJASS), use this instead of that InitTrig_ thing
-> libraries/scopes + library/scope initializer / mod+/struct onInit . . .

and i'm confused reading it, what it does exactly?

Thanks, but I really don't know anything about vJass (hence the "Jass" listing in my title). I'm just trying to tidy up my map so I can post it and be done with it.

 

[WaterKnight]

MultiboardGetItem creates a new object that wants to be destroyed via MultiboardReleaseItem or be cached.

SetUnitState(unit, UNIT_STATE_LIFE, real) can be replaced by SetWidgetLife(widget, real) and analog for the getter.

Keep the group globally.

Instead of checking for the buff, you could catch when the buff is gained/lost and stuff those units in the group, so you do not need to pick everytime.

Do not use shitty bj variables please.

 

[DistantHaze]

Do not use shitty bj variables please.

Do you mean replacing the bj_randDistID [q + 1900] (and others) with arrays like udg_array [q + 5], etc.?

 

[WaterKnight]

Yes, and with proper names. It has to do with independency, security and semantics that you don't just abuse other stuff. Nor should you inline most constant numbers.

 

[mckill2009]

dont use local multiboard, just reference all of it to your global one...
instead of making many MultiboardSetItemValue, you could create a function
that way so that you dont need to create locals for Mb and Mbi...

function MBSetItemValue takes integer row, integer column, string s returns nothing
    call MultiboardSetItemValue(MultiboardGetItem(udg_Multiboard, row, column), s) 
endfunction

and call it like this...
call MBSetItemValue(q - 1, 0, udg_PlayerString [q + 40] + " (" + I2S (q_1120 - 1) + ")...|r")

 

[DistantHaze]

Granted, I still have some bj's to edit out, but does this trigger look like I'm heading in the right direction?

function Every1Seconds_Filter01 takes nothing returns nothing
    local unit enum_unit         = GetEnumUnit ()
    local real misc_unitL
    local real misc_unitLx
    local real m
//
    if GetUnitAbilityLevel (enum_unit, 'BOhx') == 0 then
        set misc_unitL = GetWidgetLife (enum_unit)
        set misc_unitLx = GetUnitState (enum_unit, UNIT_STATE_MAX_LIFE)
        if misc_unitL < misc_unitLx then
            set m = (misc_unitLx - misc_unitL) * 0.20
            call SetWidgetLife (enum_unit, misc_unitL + m)
            call DestroyEffect (AddSpecialEffectTarget ("Abilities\\Spells\\Undead\\VampiricAura\\VampiricAuraTarget.mdl", enum_unit, "origin"))
        else
            call GroupRemoveUnit (udg_InTownUnits, enum_unit)
        endif
    endif
    set enum_unit = null    
endfunction

function Every1Seconds_Action01 takes nothing returns nothing
    local integer q              = 1
    local integer n_PRW          = udg_PlayerRezWait [0]
    local integer n_TC           = udg_TimerCount + 1
    local unit misc_unit
    local real misc_unitX
    local real misc_unitY
//
    if n_PRW > 0 then
        set udg_PlayerRezWait [q] = n_PRW - 1
        if n_PRW == 1 then
            set udg_PlayerRezWait [0] = 0
            call ReviveHero (gg_unit_Ulic_0023, 5488.000, 9296.000, true)
        endif
    endif
    loop
        exitwhen q > 11
        set n_PRW = udg_PlayerRezWait [q]
        if n_PRW > 0 then
            if n_PRW > 1 then
                set udg_PlayerRezWait [q] = n_PRW - 1
                call MultiboardSetItemValue (MultiboardGetItem (udg_Multiboard, q - 1, 0), udg_PlayerString [q + 40] + " (" + I2S (n_PRW - 1) + ")...|r")
            else
                set udg_PlayerRezWait [q] = 0
                set misc_unit = bj_ghoul [q + 1120]
                set bj_ghoul [q + 1120] = null
                if bj_randDistID [q + 1900] == 0 then
                    set misc_unitX = 5488.000
                    set misc_unitY = 9296.000
                else
                    set misc_unitX = GetUnitX (misc_unit)
                    set misc_unitY = GetUnitY (misc_unit)
                endif
                call RemoveUnit (misc_unit)
                if GetPlayerSlotState (Player (q)) == PLAYER_SLOT_STATE_LEFT then
                    call MultiboardSetItemValue (MultiboardGetItem (udg_Multiboard, q - 1, 0), "|cffef453ac: Player " + I2S (q) + "|r")
                    call MultiboardSetItemValue (MultiboardGetItem (udg_Multiboard, q - 1, 1), "|cffef453aleft|r")
                    call MultiboardSetItemValue (MultiboardGetItem (udg_Multiboard, q - 1, 2), "|cffef453a" + I2S (bj_randDistID [q + 1300]) + "|r")
                else
                    call ReviveHero (udg_PlayerHero [q], misc_unitX, misc_unitY, false)
                endif
            endif
        endif
        set q = q + 1
    endloop 
    set misc_unit = null
    set udg_TimerCount = n_TC
    if n_TC - ((n_TC / 2) * 2) == 0 then
        call TriggerExecute (gg_trg_Every3Seconds)
    elseif n_TC == 7 or n_TC == 15 then
        call ForGroup (udg_InTownUnits, function Every1Seconds_Filter01)
    elseif n_TC > 17 then
        set udg_TimerCount = 0
        call TriggerExecute (gg_trg_Every15Seconds)
    endif
endfunction

function InitTrig_Every1Seconds takes nothing returns nothing
    set gg_trg_Every1Seconds = CreateTrigger ()
    call DisableTrigger (gg_trg_Every1Seconds)
    call TriggerRegisterTimerEvent (gg_trg_Every1Seconds, 1.20, true)
    call TriggerAddAction (gg_trg_Every1Seconds, function Every1Seconds_Action01)
endfunction

 

[WaterKnight]

I repeat, MultiboardGetItem leaks. Also implement white lines and such, just that it looks neatly organized. Yeah, learn vJass.

 

[DistantHaze]

If the MB function leaks, feel free to include an example of how not to make it leak. (I seem to be receiving contradictory advice.)

function MBSetItemValue takes integer row, integer column, string s returns nothing
    call MultiboardSetItemValue (MultiboardGetItem (udg_Multiboard, row, column), s) 
endfunction

function Every1Seconds_Filter01 takes nothing returns nothing
    local unit enum_unit         = GetEnumUnit ()
    local real misc_unitL
    local real misc_unitLx
    local real m
//
    if GetUnitAbilityLevel (enum_unit, 'BOhx') == 0 then
        set misc_unitL = GetWidgetLife (enum_unit)
        set misc_unitLx = GetUnitState (enum_unit, UNIT_STATE_MAX_LIFE)
        if misc_unitL < misc_unitLx then
            set m = (misc_unitLx - misc_unitL) * 0.20
            call SetWidgetLife (enum_unit, misc_unitL + m)
            call DestroyEffect (AddSpecialEffectTarget ("Abilities\\Spells\\Undead\\VampiricAura\\VampiricAuraTarget.mdl", enum_unit, "origin"))
        else
            call GroupRemoveUnit (udg_InTownUnits, enum_unit)
        endif
    endif
    set enum_unit = null    
endfunction

function Every1Seconds_Action01 takes nothing returns nothing
    local integer q              = 1
    local integer n_PRW          = udg_PlayerRezWait [0]
    local integer n_TC           = udg_TimerCount + 1
    local unit misc_unit
    local real misc_unitX
    local real misc_unitY
//
    if n_PRW > 0 then
        set udg_PlayerRezWait [q] = n_PRW - 1
        if n_PRW == 1 then
            set udg_PlayerRezWait [0] = 0
            call ReviveHero (gg_unit_Ulic_0023, 5488.000, 9296.000, true)
        endif
    endif
//
    loop
        exitwhen q > 11
        set n_PRW = udg_PlayerRezWait [q]
        if n_PRW > 0 then
            if n_PRW > 1 then
                set udg_PlayerRezWait [q] = n_PRW - 1
                call MBSetItemValue (q - 1, 0, udg_PlayerString [q + 40] + " (" + I2S (n_PRW - 1) + ")...|r")
            else
                set udg_PlayerRezWait [q] = 0
                set misc_unit = bj_ghoul [q + 1120]
                set bj_ghoul [q + 1120] = null
                if bj_randDistID [q + 1900] == 0 then
                    set misc_unitX = 5488.000
                    set misc_unitY = 9296.000
                else
                    set misc_unitX = GetUnitX (misc_unit)
                    set misc_unitY = GetUnitY (misc_unit)
                endif
                call RemoveUnit (misc_unit)
                if GetPlayerSlotState (Player (q)) == PLAYER_SLOT_STATE_LEFT then
                    call MBSetItemValue (q - 1, 0, "|cffef453ac: Player " + I2S (q) + "|r")
                    call MBSetItemValue (q - 1, 1, "|cffef453aleft|r")
                    call MBSetItemValue (q - 1, 2, "|cffef453a" + I2S (bj_randDistID [q + 1300]) + "|r")
                else
                    call ReviveHero (udg_PlayerHero [q], misc_unitX, misc_unitY, false)
                endif
            endif
        endif
        set q = q + 1
    endloop 
    set misc_unit = null
//
    set udg_TimerCount = n_TC
    if n_TC - ((n_TC / 2) * 2) == 0 then
        call TriggerExecute (gg_trg_Every3Seconds)
    elseif n_TC == 7 or n_TC == 15 then
        call ForGroup (udg_InTownUnits, function Every1Seconds_Filter01)
    elseif n_TC > 17 then
        set udg_TimerCount = 0
        call TriggerExecute (gg_trg_Every15Seconds)
    endif
endfunction

function InitTrig_Every1Seconds takes nothing returns nothing
    set gg_trg_Every1Seconds = CreateTrigger ()
    call DisableTrigger (gg_trg_Every1Seconds)
    call TriggerRegisterTimerEvent (gg_trg_Every1Seconds, 1.20, true)
    call TriggerAddAction (gg_trg_Every1Seconds, function Every1Seconds_Action01)
endfunction

 

[WaterKnight]

MultiboardGetItem creates a new object that wants to be destroyed via MultiboardReleaseItem or be cached.

It can easily be proven by displaying the handle id of multiboarditems pointing to the same cell multiple times.

call BJDebugMsg(I2S(GetHandleId(MultiboardGetItem(multiboard, row, column))))
call BJDebugMsg(I2S(GetHandleId(MultiboardGetItem(multiboard, row, column))))
call BJDebugMsg(I2S(GetHandleId(MultiboardGetItem(multiboard, row, column))))

 

[mckill2009]

Honestly, I really do not know of multiboarditem can be used as a handle, and thus it leaks, perhaps coz I dont use them always...

well here's the fixed one...

function MBSetItemValue takes integer row, integer column, string s returns nothing
    local multiboarditem mbi = MultiboardGetItem (udg_Multiboard, row, column)
    call MultiboardSetItemValue (mbi, s) 
    call MultiboardReleaseItem(mbi)
    set mbi = null
endfunction

 

[WaterKnight]

Of course they are handles, every type besides the primitives is handle. It would be alright if MultiboardGetItem would always return the same value for a cell but it does not. Strange thing is, although they have no gameplay relevance, they count to agents and have therefore to be synced.

 

[DistantHaze]

I don't think this trigger can get much leaner (although I'm not sure what is meant by "white lines"). Thanks for all the help!

function Every1Seconds_MultiboardFunction takes integer row, integer column, string s returns nothing
    local multiboarditem Mbi     = MultiboardGetItem (udg_Multiboard, row, column)
//
    call MultiboardSetItemValue (Mbi, s) 
    call MultiboardReleaseItem (Mbi)
    set Mbi = null
endfunction

function Every1Seconds_Filter01 takes nothing returns nothing
    local unit enum_unit         = GetEnumUnit ()
    local real enum_unitL
    local real enum_unitLx
    local real m
//
    if GetUnitAbilityLevel (enum_unit, 'BOhx') == 0 then
        set enum_unitL = GetWidgetLife (enum_unit)
        set enum_unitLx = GetUnitState (enum_unit, UNIT_STATE_MAX_LIFE)
        if enum_unitL < enum_unitLx then
            set m = (enum_unitLx - enum_unitL) * 0.20
            call SetWidgetLife (enum_unit, enum_unitL + m)
            call DestroyEffect (AddSpecialEffectTarget ("Abilities\\Spells\\Undead\\VampiricAura\\VampiricAuraTarget.mdl", enum_unit, "origin"))
        else
            call GroupRemoveUnit (udg_InTownUnits, enum_unit)
        endif
    endif
    set enum_unit = null    
endfunction

function Every1Seconds_Action01 takes nothing returns nothing
    local integer q              = 1
    local integer n_PRW          = udg_PlayerRezWait [0]
    local integer n_TC           = udg_TimerCount + 1
//
    if n_PRW > 0 then
        set udg_PlayerRezWait [q] = n_PRW - 1
        if n_PRW == 1 then
            set udg_PlayerRezWait [0] = 0
            call ReviveHero (gg_unit_Ulic_0023, 5488.000, 9296.000, true)
        endif
    endif
//
    loop
        exitwhen q > 11
        set n_PRW = udg_PlayerRezWait [q]
        if n_PRW > 0 then
            if n_PRW > 1 then
                set udg_PlayerRezWait [q] = n_PRW - 1
                call Every1Seconds_MultiboardFunction (q - 1, 0, udg_PlayerString [q + 40] + " (" + I2S (n_PRW - 1) + ")...|r")
            else
                set udg_PlayerRezWait [q] = 0
                if GetPlayerSlotState (Player (q)) == PLAYER_SLOT_STATE_LEFT then
                    call Every1Seconds_MultiboardFunction (q - 1, 0, "|cffef453ac: Player " + I2S (q) + "|r")
                    call Every1Seconds_MultiboardFunction (q - 1, 1, "|cffef453aleft|r")
                    call Every1Seconds_MultiboardFunction (q - 1, 2, "|cffef453a" + I2S (bj_randDistID [q + 1300]) + "|r")
                else
                    call ReviveHero (udg_PlayerHero [q], 5488.000, 9296.000, false)
                endif
            endif
        endif
        set q = q + 1
    endloop 
//
    set udg_TimerCount = n_TC
    if n_TC - ((n_TC / 2) * 2) == 0 then
        call TriggerExecute (gg_trg_Every3Seconds)
    elseif n_TC == 7 or n_TC == 15 then
        call ForGroup (udg_InTownUnits, function Every1Seconds_Filter01)
    elseif n_TC > 17 then
        set udg_TimerCount = 0
        call TriggerExecute (gg_trg_Every15Seconds)
    endif
endfunction

function InitTrig_Every1Seconds takes nothing returns nothing
    set gg_trg_Every1Seconds = CreateTrigger ()
    call DisableTrigger (gg_trg_Every1Seconds)
    call TriggerRegisterTimerEvent (gg_trg_Every1Seconds, 1.20, true)
    call TriggerAddAction (gg_trg_Every1Seconds, function Every1Seconds_Action01)
endfunction