Help me find the leak

[Wing-Span]

I've been struggling with one problem: after updating the map, sometimes all players—or a single player—get disconnected after 10–20 minutes of gameplay. The suspicious part is in the trigger below, where I added a 0.5-second safety window during which the tower selltime count will not increase.


My question isn’t even about the trigger itself — it works correctly. I'm asking about a potential leak that I might be missing due to my lack of experience. Tell me, is there really no consequence to using a global timer without clearing it in the hashtable? Or is there an issue, and the desyncs are happening because of this?

           boolean array udg_SellSafeActive
           timer array udg_SellSafeTimer

        function SellSafeExpire takes nothing returns nothing
            local timer t = GetExpiredTimer()
            local integer regionIndex = LoadInteger(TimerUtils_2_hasht, 1, GetHandleId(t))
            set udg_SellSafeActive[regionIndex] = false
        endfunction


function s_2_Sell_End_Sell takes nothing returns nothing
    local timer t
    local integer s
    local real tX
    local real tY
    local string name
    local integer sellgold
    local integer twrRegion
    local integer twrOwn
    local integer regionIndex

    set t = GetExpiredTimer()
    set s = LoadInteger(TimerUtils_2_hasht,0,GetHandleId(t))
    set tX = s_2_TowerUnit_x[s_2_Sell_tower[s]]
    set tY = s_2_TowerUnit_y[s_2_Sell_tower[s]]
    set name = GetUnitName(s_2_TowerUnit_Tower[s_2_Sell_tower[s]])
    set twrRegion = s_2_TowerUnit_containRegion[s_2_Sell_tower[s]]
    set twrOwn = s_2_TowerUnit_TowerOwn[s_2_Sell_tower[s]]

    if s_2_Sell_stopped[s] then
        call sc_2_Sell_release(s)
        return
    endif

    call sc_2_TowerUnit_remove(s_2_Sell_tower[s])
    set sellgold = R2I(s_2_TowerUnit_SellAmount[s_2_Sell_tower[s]])
    call DestroyEffect(AddSpecialEffect("Abilities\\Spells\\Other\\Transmute\\PileofGold.mdl",tX,tY))
    call DisplayTextToPlayer(s_2_PlayerData_player[twrOwn],0,0,"Received |cffFF8C00"+I2S(sellgold)+"|r gold for selling |cffFF8C00"+name+"|r.")
    call SetPlayerState(s_2_PlayerData_player[twrOwn],PLAYER_STATE_RESOURCE_GOLD,GetPlayerState(s_2_PlayerData_player[twrOwn],PLAYER_STATE_RESOURCE_GOLD)+sellgold)

    call PauseTimer(s_2_PlayerData_selltimer[PDArr[twrRegion]])

    if s_2_PlayerData_regioncreepcount[PDArr[twrRegion]] > 0 then
        set regionIndex = PDArr[twrRegion]


//this fragment:
        if not udg_SellSafeActive[regionIndex] then
            set udg_SellSafeActive[regionIndex] = true
            set s_2_PlayerData_selltime[regionIndex] = s_2_PlayerData_selltime[regionIndex] + 1

            if udg_SellSafeTimer[regionIndex] == null then
                set udg_SellSafeTimer[regionIndex] = CreateTimer()
            endif

            call SaveInteger(TimerUtils_2_hasht,1,GetHandleId(udg_SellSafeTimer[regionIndex]),regionIndex)
            call TimerStart(udg_SellSafeTimer[regionIndex],0.5,false,function SellSafeExpire)
        endif
//end fragment.


        call TimerStart(s_2_PlayerData_selltimer[regionIndex],sellDecrement,false,function sc_2_PlayerData_selldecrement)
    endif

    call ForGroup(s_2_PlayerData_regionCreeps[PDArr[twrRegion]],function RepathUnits_2_creepEnum)
    call sc_2_Sell_release(s)
endfunction

 

[Uncle]

You leak timer "t" in both functions. You need to add this before the end of each function:

set t = null

Non-primitive local variables need to be nulled inside of the function that created them. But if they're used as an argument in another function then they do NOT need to be nulled in that other function. In other words: only null them in the function that initially created them.

But that shouldn't cause a crash or desync.

This seems a little odd to me, why are you declaring GUI variables in your code?

        boolean array udg_SellSafeActive
        timer array udg_SellSafeTimer

udg_ implies it's a variable created in the Trigger Editor. Maybe this is intended but I'm just not used to it.

Here's some cleaned up code using methods I'm more familiar with:

library ExampleCode

    globals
        boolean array SellSafeActive
        timer array SellSafeTimer
    endglobals

    function SellSafeExpire takes nothing returns nothing
        local timer t = GetExpiredTimer()
        local integer regionIndex = LoadInteger(TimerUtils_2_hasht, 1, GetHandleId(t))
        set SellSafeActive[regionIndex] = false
        set t = null
    endfunction
 
    function s_2_Sell_End_Sell takes nothing returns nothing
        local timer t = GetExpiredTimer()
        local integer s = LoadInteger(TimerUtils_2_hasht,0,GetHandleId(t))
        local real tX
        local real tY
        local string name
        local integer sellGold
        local integer twrRegion
        local integer twrOwn
        local integer regionIndex
        set t = null
 
        // Exit early if need be
        if s_2_Sell_stopped[s] then
            call sc_2_Sell_release(s)
            return
        endif

        set tX = s_2_TowerUnit_x[s_2_Sell_tower[s]]
        set tY = s_2_TowerUnit_y[s_2_Sell_tower[s]]
        set name = GetUnitName(s_2_TowerUnit_Tower[s_2_Sell_tower[s]])
        set twrRegion = s_2_TowerUnit_containRegion[s_2_Sell_tower[s]]
        set twrOwn = s_2_TowerUnit_TowerOwn[s_2_Sell_tower[s]]
        set sellGold = R2I(s_2_TowerUnit_SellAmount[s_2_Sell_tower[s]])
       
        // You may want to move this chunk of code to the very bottom
        call PauseTimer(s_2_PlayerData_selltimer[PDArr[twrRegion]])
        call DestroyEffect(AddSpecialEffect("Abilities\\Spells\\Other\\Transmute\\PileofGold.mdl",tX,tY))
        call DisplayTextToPlayer(s_2_PlayerData_player[twrOwn],0,0,"Received |cffFF8C00"+I2S(sellGold)+"|r gold for selling |cffFF8C00"+name+"|r.")
        call SetPlayerState(s_2_PlayerData_player[twrOwn],PLAYER_STATE_RESOURCE_GOLD,GetPlayerState(s_2_PlayerData_player[twrOwn],PLAYER_STATE_RESOURCE_GOLD)+sellGold)
        call sc_2_TowerUnit_remove(s_2_Sell_tower[s])
 
        if s_2_PlayerData_regioncreepcount[PDArr[twrRegion]] > 0 then
            set regionIndex = PDArr[twrRegion]
            if not SellSafeActive[regionIndex] then
                set SellSafeActive[regionIndex] = true
                set s_2_PlayerData_selltime[regionIndex] = s_2_PlayerData_selltime[regionIndex] + 1
                if SellSafeTimer[regionIndex] == null then
                    set SellSafeTimer[regionIndex] = CreateTimer()
                endif
                call SaveInteger(TimerUtils_2_hasht,1,GetHandleId(SellSafeTimer[regionIndex]),regionIndex)
                call TimerStart(SellSafeTimer[regionIndex],0.5,false,function SellSafeExpire)
            endif
            call TimerStart(s_2_PlayerData_selltimer[regionIndex],sellDecrement,false,function sc_2_PlayerData_selldecrement)
        endif
 
        call ForGroup(s_2_PlayerData_regionCreeps[PDArr[twrRegion]],function RepathUnits_2_creepEnum)
        call sc_2_Sell_release(s)
    endfunction
 
endlibrary

 

[Wing-Span]

You leak timer "t" in both functions. You need to add this before the end of each function:

set t = null

Non-primitive local variables need to be nulled inside of the function that created them. But if they're used as an argument in another function then they do NOT need to be nulled in that other function. In other words: only null them in the function that initially created them.

But that shouldn't cause a crash or desync.

This seems a little odd to me, why are you declaring GUI variables in your code?

        boolean array udg_SellSafeActive
        timer array udg_SellSafeTimer

udg_ implies it's a variable created in the Trigger Editor. Maybe this is intended but I'm just not used to it.

Here's some cleaned up code using methods I'm more familiar with:

library ExampleCode

    globals
        boolean array SellSafeActive
        timer array SellSafeTimer
    endglobals

    function SellSafeExpire takes nothing returns nothing
        local timer t = GetExpiredTimer()
        local integer regionIndex = LoadInteger(TimerUtils_2_hasht, 1, GetHandleId(t))
        set SellSafeActive[regionIndex] = false
        set t = null
    endfunction
 
    function s_2_Sell_End_Sell takes nothing returns nothing
        local timer t = GetExpiredTimer()
        local integer s = LoadInteger(TimerUtils_2_hasht,0,GetHandleId(t))
        local real tX
        local real tY
        local string name
        local integer sellGold
        local integer twrRegion
        local integer twrOwn
        local integer regionIndex
        set t = null
 
        // Exit early if need be
        if s_2_Sell_stopped[s] then
            call sc_2_Sell_release(s)
            return
        endif

        set tX = s_2_TowerUnit_x[s_2_Sell_tower[s]]
        set tY = s_2_TowerUnit_y[s_2_Sell_tower[s]]
        set name = GetUnitName(s_2_TowerUnit_Tower[s_2_Sell_tower[s]])
        set twrRegion = s_2_TowerUnit_containRegion[s_2_Sell_tower[s]]
        set twrOwn = s_2_TowerUnit_TowerOwn[s_2_Sell_tower[s]]
        set sellGold = R2I(s_2_TowerUnit_SellAmount[s_2_Sell_tower[s]])
      
        // You may want to move this chunk of code to the very bottom
        call PauseTimer(s_2_PlayerData_selltimer[PDArr[twrRegion]])
        call DestroyEffect(AddSpecialEffect("Abilities\\Spells\\Other\\Transmute\\PileofGold.mdl",tX,tY))
        call DisplayTextToPlayer(s_2_PlayerData_player[twrOwn],0,0,"Received |cffFF8C00"+I2S(sellGold)+"|r gold for selling |cffFF8C00"+name+"|r.")
        call SetPlayerState(s_2_PlayerData_player[twrOwn],PLAYER_STATE_RESOURCE_GOLD,GetPlayerState(s_2_PlayerData_player[twrOwn],PLAYER_STATE_RESOURCE_GOLD)+sellGold)
        call sc_2_TowerUnit_remove(s_2_Sell_tower[s])
 
        if s_2_PlayerData_regioncreepcount[PDArr[twrRegion]] > 0 then
            set regionIndex = PDArr[twrRegion]
            if not SellSafeActive[regionIndex] then
                set SellSafeActive[regionIndex] = true
                set s_2_PlayerData_selltime[regionIndex] = s_2_PlayerData_selltime[regionIndex] + 1
                if SellSafeTimer[regionIndex] == null then
                    set SellSafeTimer[regionIndex] = CreateTimer()
                endif
                call SaveInteger(TimerUtils_2_hasht,1,GetHandleId(SellSafeTimer[regionIndex]),regionIndex)
                call TimerStart(SellSafeTimer[regionIndex],0.5,false,function SellSafeExpire)
            endif
            call TimerStart(s_2_PlayerData_selltimer[regionIndex],sellDecrement,false,function sc_2_PlayerData_selldecrement)
        endif
 
        call ForGroup(s_2_PlayerData_regionCreeps[PDArr[twrRegion]],function RepathUnits_2_creepEnum)
        call sc_2_Sell_release(s)
    endfunction
 
endlibrary

You are amazing! How did I not notice this...