[JASS] Do these triggers leak?

[Warseeker]

Hi, I wanted to optimize the old creep respawn system in my custom campaign so I asked ChatGPT to help me with that and it said that I should replace the Wait with a Timer instead to improve performance. Here are my triggers:

Creep Setup

function InitCreepRespawn takes nothing returns nothing
    local group g = CreateGroup()
    local unit u
    local integer id

    call GroupEnumUnitsOfPlayer(g, Player(PLAYER_NEUTRAL_AGGRESSIVE), null)

    loop
        set u = FirstOfGroup(g)
        exitwhen u == null
        call GroupRemoveUnit(g, u)

        set udg_CreepIndex = udg_CreepIndex + 1
        set id = udg_CreepIndex

        call SetUnitUserData(u, id)

        set udg_CreepX[id] = GetUnitX(u)
        set udg_CreepY[id] = GetUnitY(u)

        set udg_CreepType[id] = GetUnitTypeId(u)
        set udg_CreepTimer[id] = CreateTimer()
    endloop

    call DestroyGroup(g)
endfunction
//===========================================================================
function InitTrig_Respawn_Setup takes nothing returns nothing
    set gg_trg_Respawn_Setup = CreateTrigger(  )
    call TriggerAddAction( gg_trg_Respawn_Setup, function InitCreepRespawn )
endfunction

Creep Respawn

function RespawnCheck takes nothing returns nothing
    local timer t = GetExpiredTimer()
    local integer id = 1
    local group g
    local boolean nearby = false
    local unit u

    loop
        exitwhen id > udg_CreepIndex
        if udg_CreepTimer[id] == t then

            set g = CreateGroup()
            call GroupEnumUnitsOfPlayer(g, Player(0), null)

            loop
                set u = FirstOfGroup(g)
                exitwhen u == null
                call GroupRemoveUnit(g, u)

                if IsUnitAliveBJ(u) and IsUnitInRangeXY(u, udg_CreepX[id], udg_CreepY[id], 1400) then
                    set nearby = true
                    exitwhen true
                endif
            endloop

            call DestroyGroup(g)

            // Delay respawn if player nearby
            if nearby then
                call TimerStart(t, 5.0, false, function RespawnCheck)
                return
            endif

            // Respawn creep
            call SetUnitUserData(CreateUnit(Player(PLAYER_NEUTRAL_AGGRESSIVE),udg_CreepType[id],udg_CreepX[id],udg_CreepY[id],GetRandomReal(0,360)),id)

            return
        endif
        set id = id + 1
    endloop
endfunction

function CreepDies takes nothing returns nothing
    local unit u = GetTriggerUnit()
    local integer lvl = GetUnitLevel(u)
    local integer id = GetUnitUserData(u)
    local integer low
    local integer high
    local integer chance

    // Item Table
    if lvl <= 3 then
       set low    = 1
       set high   = 3
       set chance = 15
    elseif lvl <= 6 then
       set low    = 4
       set high   = 6
       set chance = 20
    else
       set low    = 7
       set high   = 9
       set chance = 25
    endif

    if GetRandomInt(1, 100) <= chance then
       call CreateItem( udg_CreepTable[GetRandomInt(low,high)], GetUnitX(u),GetUnitY(u) )
    endif

    // Respawn Timer
    if id > 0 then
        call TimerStart(udg_CreepTimer[id], 15.0, false, function RespawnCheck)
    endif

    set u = null
endfunction
//===========================================================================
function InitTrig_Respawn_Creep takes nothing returns nothing
    set gg_trg_Respawn_Creep = CreateTrigger(  )
    call TriggerRegisterPlayerUnitEventSimple( gg_trg_Respawn_Creep, Player(PLAYER_NEUTRAL_AGGRESSIVE), EVENT_PLAYER_UNIT_DEATH )
    call TriggerAddAction( gg_trg_Respawn_Creep, function CreepDies )
endfunction

Creep Wander

function FilterNeutralHostile takes nothing returns boolean
    return GetOwningPlayer(GetFilterUnit()) == Player(PLAYER_NEUTRAL_AGGRESSIVE)
endfunction

function CreepWanderLoop takes nothing returns nothing
    local integer i = 1
    local unit h
    local group g
    local unit u
    local integer id
    local real hx
    local real hy
    local real angle
    local real dist
    local real tx
    local real ty

    loop
        exitwhen i > udg_NHeroes

        set h = udg_Hero[i]

        if h != null and IsUnitAliveBJ(h) then

            set hx = GetUnitX(h)
            set hy = GetUnitY(h)

            set g = CreateGroup()

            call GroupEnumUnitsInRange(g,hx,hy,1000,Condition(function FilterNeutralHostile))

            loop
                set u = FirstOfGroup(g)
                exitwhen u == null
                call GroupRemoveUnit(g, u)

                if IsUnitAliveBJ(u) and GetUnitCurrentOrder(u) == 0 and GetUnitLevel(u) < 6 then

                    set id = GetUnitUserData(u)

                    // random point inside leash radius
                    set angle = GetRandomReal(0, 6.283)
                    set dist = GetRandomReal(100, 400)

                    set tx = udg_CreepX[id] + dist * Cos(angle)
                    set ty = udg_CreepY[id] + dist * Sin(angle)

                    if not IsTerrainPathable(tx,ty,PATHING_TYPE_WALKABILITY) and GetRandomInt(1, 100) <= 40 then
                    call IssuePointOrder(u, "attack", tx, ty)
                    endif

                endif

            endloop

            call DestroyGroup(g)

        endif

        set i = i + 1
    endloop

    set h = null
    set u = null
endfunction
//===========================================================================
function InitTrig_Respawn_Roam takes nothing returns nothing
    set gg_trg_Respawn_Roam = CreateTrigger(  )
    call TriggerRegisterTimerEvent( gg_trg_Respawn_Roam, 3.0, true )
    call TriggerAddAction( gg_trg_Respawn_Roam, function CreepWanderLoop )
endfunction

Creep Table

function Respawn_Table takes nothing returns nothing
    // Runes
    set udg_CreepTable[1] = 'rhe2' //Rune of Healing
    set udg_CreepTable[2] = 'rman' //Rune of Mana
    set udg_CreepTable[3] = 'rspd' //Rune of Speed
    // Scrolls
    set udg_CreepTable[4] = 'shea' //Scroll of Healing
    set udg_CreepTable[5] = 'sman' //Scroll of Mana
    set udg_CreepTable[6] = 'spro' //Scroll of Protection
    // Tomes
    set udg_CreepTable[7] = 'tdex' //Tome of Agility
    set udg_CreepTable[8] = 'tstr' //Tome of Strength
    set udg_CreepTable[9] = 'tint' //Tome of Intelligence
endfunction
//===========================================================================
function InitTrig_Respawn_Table takes nothing returns nothing
    set gg_trg_Respawn_Table = CreateTrigger(  )
    call TriggerRegisterTimerEvent( gg_trg_Respawn_Table, 0.0, false )
    call TriggerAddAction( gg_trg_Respawn_Table, function Respawn_Table )
endfunction

Everything seem to work as intended during my tests but I'm not sure if these triggers are leaking or cause issues on the long run. Can someone tell me if these triggers are okay? Thanks.

 

[MyPad]

These are some functions of interest when verifying for leaks (from what I can glance at):

• function InitCreepRespawn:
  • The group variable "g" is not dereferenced (nulled) properly, resulting in a reference leak.
• function RespawnCheck:
  • The timer variable "t" is not dereferenced properly, resulting in a reference leak.
  • The group variable "g" is not dereferenced properly, resulting in a reference leak.
  • The unit variable "u" is not dereferenced properly, resulting in a reference leak.
• function CreepWanderLoop:
  • The group variable "g" is not dereferenced properly, resulting in a reference leak.

The numerous variables above are already good to go when it comes to creation and destruction. However, they're still susceptible to the reference counter leak as a result of not being dereferenced properly.

 

[the.Axolotl]

One thing I noticed- RespawnCheck only checks Player 0 for nearby units. On a map with multiple players, creeps might respawn even if a nearby player is there. You’d want to loop through all active players for MP. Otherwise, the logic looks solid.
Also, on a larger-scale map with many creeps, having a separate timer per creep could add up over time. I saw you mentioned it was tested on a small scale, just giving a heads up.

 

[Warseeker]

These are some functions of interest when verifying for leaks (from what I can glance at):

• function InitCreepRespawn:
  • The group variable "g" is not dereferenced (nulled) properly, resulting in a reference leak.
• function RespawnCheck:
  • The timer variable "t" is not dereferenced properly, resulting in a reference leak.
  • The group variable "g" is not dereferenced properly, resulting in a reference leak.
  • The unit variable "u" is not dereferenced properly, resulting in a reference leak.
• function CreepWanderLoop:
  • The group variable "g" is not dereferenced properly, resulting in a reference leak.

The numerous variables above are already good to go when it comes to creation and destruction. However, they're still susceptible to the reference counter leak as a result of not being dereferenced properly.

Thanks! I'm still new to JASS and I didn't know about those reference leaks.

One thing I noticed- RespawnCheck only checks Player 0 for nearby units. On a map with multiple players, creeps might respawn even if a nearby player is there. You’d want to loop through all active players for MP. Otherwise, the logic looks solid.

Yes, this is a single player campaign so I only need to check one player.

Also, on a larger-scale map with many creeps, having a separate timer per creep could add up over time. I saw you mentioned it was tested on a small scale, just giving a heads up.

What would be the alternative?

 

[the.Axolotl]

Thanks! I'm still new to JASS and I didn't know about those reference leaks.

Yes, this is a single player campaign so I only need to check one player.

What would be the alternative?

you can use a single periodic timer and track respawn times in an array. for example, on death you store something like RespawnTime[id] = current time + delay, and then have one global loop (every 1 second) that checks all creeps and respawns the ones whose time has run out, but this also changes the logic a bit

 

[Warseeker]

you can use a single periodic timer and track respawn times in an array. for example, on death you store something like RespawnTime[id] = current time + delay, and then have one global loop (every 1 second) that checks all creeps and respawns the ones whose time has run out, but this also changes the logic a bit

Okay, I'll try that. So the logic would be something like this?

Every 1 second

Loop through each RespawnTimer[id] and decrease its value by 1

Once it reaches 0 respawn the creep

 

[the.Axolotl]

Decrease RespawnTimer[id] every second until it reaches 0

This would turn you system into a bunch of countdowns, also maybe less precise if loop not equal exactly 1s.

Instead, store the exact time when the creep should respawn.

So on death you do something like:
RespawnTime[id] = current game time + delay

Then in the loop you just check:
if current time >= RespawnTime[id] → respawn

---

PS I'm also pretty new to JASS, but this approach with comparing game time i did many times in GUI via custom scripts, that's why that instantly came to my mind, I call this Timestamp model lol

Spoiler: code

function GlobalRespawnLoop takes nothing returns nothing
    local integer id = 1
    local real currentTime = TimerGetElapsed(udg_GameTimer)
    local integer p
    local group g
    local unit u
    local boolean nearby


    loop
        exitwhen id > udg_CreepIndex


        // check if this creep is scheduled to respawn
        if udg_CreepRespawnTime[id] > 0.0 and currentTime >= udg_CreepRespawnTime[id] then


            set nearby = false
            set p = 0


            // loop through all players
            loop
                exitwhen p > bj_MAX_PLAYERS or nearby


                if GetPlayerSlotState(Player(p)) == PLAYER_SLOT_STATE_PLAYING then
                    set g = CreateGroup()
                    call GroupEnumUnitsOfPlayer(g, Player(p), null)


                    loop
                        set u = FirstOfGroup(g)
                        exitwhen u == null
                        call GroupRemoveUnit(g, u)


                        if IsUnitAliveBJ(u) and IsUnitInRangeXY(u, udg_CreepX[id], udg_CreepY[id], 1400.0) then
                            set nearby = true
                            exitwhen true
                        endif
                    endloop


                    call DestroyGroup(g)
                    set g = null
                    set u = null
                endif


                set p = p + 1
            endloop


            // if player nearby → delay respawn slightly
            if nearby then
                set udg_CreepRespawnTime[id] = currentTime + 5.0
            else
                // respawn creep
                call SetUnitUserData(CreateUnit(
                    Player(PLAYER_NEUTRAL_AGGRESSIVE),
                    udg_CreepType[id],
                    udg_CreepX[id],
                    udg_CreepY[id],
                    GetRandomReal(0, 360)
                ), id)


                // reset timer
                set udg_CreepRespawnTime[id] = 0.0
            endif


        endif


        set id = id + 1
    endloop
endfunction

---

So basically:

• Check all players( if you want to be extra precise: if IsUnitAliveBJ(u) and GetOwningPlayer(u) != Player(PLAYER_NEUTRAL_AGGRESSIVE) then ; Or even restrict to playing players if needed).
• If someone is nearby push respawn forward by 5s
• Else respawn immediately.

And as for timer, I've seen it as common practice:
call TimerStart(udg_GameTimer, 1000000.0, true, null)

---

Just more thoughts: set g = null and u = null after the loop if you want to follow strict nulling

 

[Warseeker]

Decrease RespawnTimer[id] every second until it reaches 0

This would turn you system into a bunch of countdowns, also maybe less precise if loop not equal exactly 1s.

Instead, store the exact time when the creep should respawn.

So on death you do something like:
RespawnTime[id] = current game time + delay

Then in the loop you just check:
if current time >= RespawnTime[id] → respawn

---

PS I'm also pretty new to JASS, but this approach with comparing game time i did many times in GUI via custom scripts, that's why that instantly came to my mind, I call this Timestamp model lol

Spoiler: code

function GlobalRespawnLoop takes nothing returns nothing
    local integer id = 1
    local real currentTime = TimerGetElapsed(udg_GameTimer)
    local integer p
    local group g
    local unit u
    local boolean nearby


    loop
        exitwhen id > udg_CreepIndex


        // check if this creep is scheduled to respawn
        if udg_CreepRespawnTime[id] > 0.0 and currentTime >= udg_CreepRespawnTime[id] then


            set nearby = false
            set p = 0


            // loop through all players
            loop
                exitwhen p > bj_MAX_PLAYERS or nearby


                if GetPlayerSlotState(Player(p)) == PLAYER_SLOT_STATE_PLAYING then
                    set g = CreateGroup()
                    call GroupEnumUnitsOfPlayer(g, Player(p), null)


                    loop
                        set u = FirstOfGroup(g)
                        exitwhen u == null
                        call GroupRemoveUnit(g, u)


                        if IsUnitAliveBJ(u) and IsUnitInRangeXY(u, udg_CreepX[id], udg_CreepY[id], 1400.0) then
                            set nearby = true
                            exitwhen true
                        endif
                    endloop


                    call DestroyGroup(g)
                    set g = null
                    set u = null
                endif


                set p = p + 1
            endloop


            // if player nearby → delay respawn slightly
            if nearby then
                set udg_CreepRespawnTime[id] = currentTime + 5.0
            else
                // respawn creep
                call SetUnitUserData(CreateUnit(
                    Player(PLAYER_NEUTRAL_AGGRESSIVE),
                    udg_CreepType[id],
                    udg_CreepX[id],
                    udg_CreepY[id],
                    GetRandomReal(0, 360)
                ), id)


                // reset timer
                set udg_CreepRespawnTime[id] = 0.0
            endif


        endif


        set id = id + 1
    endloop
endfunction

---

So basically:

• Check all players( if you want to be extra precise: if IsUnitAliveBJ(u) and GetOwningPlayer(u) != Player(PLAYER_NEUTRAL_AGGRESSIVE) then ; Or even restrict to playing players if needed).
• If someone is nearby push respawn forward by 5s
• Else respawn immediately.

And as for timer, I've seen it as common practice:
call TimerStart(udg_GameTimer, 1000000.0, true, null)

---

Just more thoughts: set g = null and u = null after the loop if you want to follow strict nulling

It works! By the way, is it safe to start a 100k periodic timer? What happens if the timer expires or becomes less than respawn timer? I'm asking because the campaign I'm working on rely on map transition so I'm worried that the timer runs out or break before the campaign is finished. (Although it is unlikely the timer will expire unless the player spend more than 10 hours on the map lol)

What if I start a timer that increases "currentTime" by 1 every second instead? It should work the same as with TimerGetElapsed, no?

One more thing, I'm bit confused about FirstOfGroup(g), do I need to null local unit variables before the endfunction or right after destroying the group?

 

[the.Axolotl]

By the way, is it safe to start a 100k periodic timer? What happens if the timer expires or

Yeah, it’s totally safe. Even a 100k periodic timer won’t really “break” anything when it expires it just loops again since it’s periodic, and TimerGetElapsed resets. In practice, you’ll never hit that limit unless someone is playing for a veeery long time. Also, your respawn times are always relative to the current elapsed time, so nothing desyncs when it loops.

What if I start a timer that increases "currentTime" by 1 every second instead? It should work the same as with TimerGetElapsed, no?

If you want something simpler/clearer, incrementing your own currentTime every second works perfectly fine too. Functionally it’s the same idea, just a bit more manual.

I'm bit confused about FirstOfGroup(g), do I need to null local unit variables before the endfunction or right after destroying the group?

About FirstOfGroup:

You don’t need to null u immediately after destroying the group
Just do set u = null at the end of the function (or after the loop) and you’re gtg!

Nulling is more of a good practice than a strict requirement here, especially since the group is already destroyed.

---

Anyways, I'm glad it works!