[Solved] GetUnitsOfPlayerAndTypeId have leak?

[tlstkwjr]

Then how can I fix it?

 

[pOke]

function GetUnitsOfPlayerAndTypeId takes player whichPlayer, integer unitid returns group
    local group g = CreateGroup()
    set bj_groupEnumTypeId = unitid
    call GroupEnumUnitsOfPlayer(g, whichPlayer, filterGetUnitsOfPlayerAndTypeId)
    return g
endfunction

It doesn't leak if you save the returned group to a variable and then destroy it.

 

[tlstkwjr]

function GetUnitsOfPlayerAndTypeId takes player whichPlayer, integer unitid returns group
    local group g = CreateGroup()
    set bj_groupEnumTypeId = unitid
    call GroupEnumUnitsOfPlayer(g, whichPlayer, filterGetUnitsOfPlayerAndTypeId)
    return g
endfunction

It doesn't leak if you save the returned group to a variable and then destroy it.

    set udg_CHECKincome = GetUnitsOfPlayerAndTypeId(ConvertedPlayer(GetForLoopIndexA()), 'ogre')
    set bj_wantDestroyGroup = true
    set udg_PLAYER_POINT[GetForLoopIndexA()] = CountUnitsInGroup(udg_CHECKincome)
    call DestroyGroup(udg_CHECKincome)
    set udg_CHECKincome = null

I'm using this. So It is okay right?

 

[Chaosy]

set bj_wantDestroyGroup = true
is probably not needed.

 

[KILLCIDE]

Why not just do this?

// use a global group created on map init that you will just recycle (I'll call it udg_UnitGroup)
function SomeFunction takes nothing returns nothing
    local player p = Player(0)
    local integer count = 0
    local unit u
  
    call GroupEnumUnitsInRange(udg_UnitGroup, someX, someY, someRadius, null)
    loop
        set u = FirstOfGroup(udg_UnitGroup)
        exitwhen (u == null)
      
        if (GetUnitTypeId(u) == 'ogre') and (GetOwningPlayer(u) == p) then
            set count = count + 1
        endif
      
        call GroupRemoveUnit(g, u)
    endloop
endfunction

 

[Chaosy]

Because good BJ functions are awesome.

DNRY.

 

[KILLCIDE]

CountUnitsInGroup is a good BJ function. However, GetUnitsOfPlayerAndTypeId is not xD you can easily count the units and filter them out with a simple FirstOfGroup loop.

Also, from the looks if it, he does all of that simply to find the number of units of a unit type in the map. If he made it to a function that returns an integer, then:

// use a global group created on map init that you will just recycle (I'll call it udg_UnitGroup)
function CountUnitType takes integer unitType, player p returns integer
    local integer count = 0
    local unit u
 
    call GroupEnumUnitsInRange(udg_UnitGroup, someX, someY, someRadius, null)
    loop
        set u = FirstOfGroup(udg_UnitGroup)
        exitwhen (u == null)
  
        if (GetUnitTypeId(u) == unitType) and (GetOwningPlayer(u) == p) then
            set count = count + 1
        endif
  
        call GroupRemoveUnit(g, u)
    endloop
 
    return count
endfunction

function SomeFunction takes nothing returns nothing
    local integer i = 0

    loop
        exitwhen (i > blahblah)
        set udg_PLAYER_POINT[i] = CountUnitType(Player(i), 'ogre')
        set i = i + 1
    endloop
endfunction

 

[tlstkwjr]

Wow. okay. I will use it. thanks!

 

[Chaosy]

IMO any function that contains multiple lines of code are good functions. Unless they leak - but even then you can fix the leak so that's no issue.

As far as I am concerned, one function call is easier to use than a loop. So.. win!

 

[pOke]

@tlstkwjr

Well it could be optimized but it should be fine for leaks.

@Chaosy

He needs the set bj_wantDestroy = true so the Counted local group is cleared.

@KILLCIDE

I'm not sure how Count is any better or worse than GetUnitsOfPlayerAndType

Both could be done with a FirstOfGroup loop so both are equally "bad".

 

[KILLCIDE]

Unless they leak - but even then you can fix the leak so that's no issue.

You fix the leak by making your own function

As far as I am concerned, one function call is easier to use than a loop. So.. win!

I am using one function? I combined both CountUnitsInGroup and GetUnitsOfPlayerAndTypeId into one function

I'm not sure how Count is any better or worse than GetUnitsOfPlayerAndType

Instead of using an integer counter that increments everytime you add an unit (assuming that someone adds / removes units dynamically), you can just add and remove units freely and use CountUnitsInGroup when you have to.

 

[ZiBitheWand3r3r]

function CountUnitType takes integer unitType, player p returns integer
    local integer count = 0
    local unit u
 
    call GroupEnumUnitsInRange(udg_UnitGroup, someX, someY, someRadius, null)
    loop
        set u = FirstOfGroup(udg_UnitGroup)
        exitwhen (u == null)
 
        if (GetUnitTypeId(u) == unitType) and (GetOwningPlayer(u) == p) then
            set count = count + 1
        endif
 
        call GroupRemoveUnit(g, u)
    endloop
 
    return count
endfunction

-->

native GetPlayerUnitTypeCount takes player p, integer unitid returns integer

//counts only alive units/heroes/structures
//counts also units during training; also counts buildings under construction
//counts building during upgrade (like upgrade town hall-->keep in progress, counts as keep)
//counts also heroes during revieving

 

[KILLCIDE]

Given by the snippet he gave us, it looks like an income system. Units and Heroes that are still "training" or "reviving" shouldn't be included in the income amount.

 

[Dr Super Good]

They leak because of the local declared local handle variable reference counter leak.

It doesn't leak if you save the returned group to a variable and then destroy it.

Yes it does leak... "local group g" is not null at the time of function return. Hence it leaks a handle reference.

 

[Wietlol]

You either have to pass that group as parameter, or use a global group... or dont use it at all