[JASS] Quick check about leaks

[superkar]

I made this simple function to get units that are distant less then R from (X,Y) but more then r. Does group g leak? Do I have to make it global to destroy it?

function GetUnitsInCrown takes real x, real y, real rmin, real rmax returns group
        local unit u
        local location loc = Location(x,y)
        local group temp = CreateGroup()
        local group g = CreateGroup()
        call GroupEnumUnitsInRange(temp, x, y, rmax, null)
        if (rmin>0) then
        loop
            set u = FirstOfGroup(temp)
            exitwhen u == null
            if not (IsUnitInRangeLoc(u, loc, rmin)) then
                call GroupAddUnit(g, u)
            endif
            call GroupRemoveUnit(temp, u)
        endloop
        endif
        set u = null
        set loc = null
        set temp = null
        call DestroyGroup(temp)
        call RemoveLocation(loc)
        return g
    endfunction

Oh I just realized I'm also nulling the locals before destroying them silliness

 

[PurgeandFire]

Yes, as you've mentioned, you have to destroy the objects before you null the pointer to them. And yes, you should assign g to a global and return the global, otherwise you can't null the local before the end of the function.

If I were to rewrite it, I'd write it as:

    globals
        group returnGroup = CreateGroup()
    endglobals  

    function GetUnitsInCrown takes real x, real y, real rmin, real rmax returns group
        local unit u
        local group temp = CreateGroup()
        call GroupEnumUnitsInRange(temp, x, y, rmax, null)
        if (rmin>0) then
            loop
                set u = FirstOfGroup(temp)
                exitwhen u == null
                if not (IsUnitInRangeXY(u, x, y, rmin)) then
                    call GroupAddUnit(returnGroup, u)
                endif
                call GroupRemoveUnit(temp, u)
            endloop
        endif
        call DestroyGroup(temp)
        set temp = null
        return returnGroup
    endfunction

You don't need to null u in this case, since it is already pointing to null by the end of the loop (since the exitwhen is when u == null). You can destroy the group later on wherever you use it. For example, if you use GetUnitsInCrown in some function, you can simply destroy it when you're done using it:

function Example takes nothing returns nothing
    local group g = GetUnitsInCrown(...)
    // do stuff with the group
    call DestroyGroup(g)
    set g = null
endfunction

The only reason I switched to a global was because the function in the first post wasn't able to null the local group "g" since it had to be returned from the function.

 

[superkar]

Ok! Thank you very much!