How do I do this more efficiently?

[sniper_zero]

I want to set the units' health in an area to the average of their current health in percent. I don't have the trigger yet but this is how I plan on doing it.

trigger

    call GroupEnumUnitsInRange(g, x, y, 600, null)
    loop
        set u = FirstOfGroup(g)
        exitwhen u == null
        set i = i + 1
        set Real = (0 + GetUnitStatePercent(u, UNIT_STATE_LIFE, UNIT_STATE_MAX_LIFE)) / i
        call GroupRemoveUnit(g, u)
    endloop
    call GroupEnumUnitsInRange(g, x, y, 600, null)
    loop
        set u = FirstOfGroup(g)
        exitwhen u == null
        call SetUnitState(u, UNIT_STATE_LIFE, GetUnitState(u, UNIT_STATE_MAX_LIFE) * Real)
        call GroupRemoveUnit(g, u)
    endloop

I'd also like to know if GetUnitStatePercent is a good BJ and what's an alternate way to do CountUnitsInGroup?

 

[Doomlord]

Here is GetUnitStatePercent

function GetUnitStatePercent takes unit whichUnit, unitstate whichState, unitstate whichMaxState returns real
    local real value    = GetUnitState(whichUnit, whichState)
    local real maxValue = GetUnitState(whichUnit, whichMaxState)

    // Return 0 for null units.
    if (whichUnit == null) or (maxValue == 0) then
        return 0.0
    endif

    return value / maxValue * 100.0
endfunction

which could be rewritten for added efficiency into

function GetUnitStatePercentEx takes unit u, unitstate whichState, unitstate whichMaxState returns real
    // Check if the unit is still alive
    if not IsUnitType(u, UNIT_TYPE_DEAD) and /*
    */ GetUnitTypeId(u) != 0 then

        return GetUnitState(u, whichState)/GetUnitState(u, whichMaxState)*100.
    endif
    
    return 0.
endfunction

Here is an alternative function for CountUnitsInGroup which I think of on the spot.

function CountUnitsInGroupEx takes group g returns integer
    local unit array w
    local unit u
    local integer i = 0
    local integer j = 0
    
    // Iterate the group
    loop
        set u = FirstOfGroup(g)
        exitwhen u == null
        set w[i] = u
        set i = i + 1
        call GroupRemoveUnit(g, u)
    endloop
    
    // Assign the count to a different variable for storage
    set j = i
    
    // Readd the units to the group
    loop
        exitwhen i < 0
        call GroupAddUnit(g, w[i])
        set w[i] = null
        set i = i - 1
    endloop
    
    // Return the value
    return j
endfunction

They are all freehand coding. I didn't take time to test so I apologize if something unexpected pop up.

 

[sniper_zero]

They look fine from what I can tell. Thanks. At least now that's done with.
How about the trigger? Do I just make this use the functions you made and it's good to go? I thought about just adding call SetUnitState(u, UNIT_STATE_LIFE, GetUnitState(u, UNIT_STATE_MAX_LIFE) * Real) to the first loop but then that wouldn't be right. The units would each get a different average.

Is it good enough as it is?

trigger

    call GroupEnumUnitsInRange(g, x, y, 600, null)
    loop
        set u = FirstOfGroup(g)
        exitwhen u == null
        set i = i + 1
        set Real = (0 + GetUnitStatePercent(u, UNIT_STATE_LIFE, UNIT_STATE_MAX_LIFE)) / i
        call GroupRemoveUnit(g, u)
    endloop
    call GroupEnumUnitsInRange(g, x, y, 600, null)
    loop
        set u = FirstOfGroup(g)
        exitwhen u == null
        call SetUnitState(u, UNIT_STATE_LIFE, GetUnitState(u, UNIT_STATE_MAX_LIFE) * Real)
        call GroupRemoveUnit(g, u)
    endloop

 

[Doomlord]

Yeh well it doesn't really seem right according to what you demand in the main post. Your assignment to the variable Real seems iffy as it will always return the last iterated unit's health % divided by i.

I would rather do it like this.

    call GroupEnumUnitsInRange(g, x, y, 600, null)
    loop
        set u = FirstOfGroup(g)
        exitwhen u == null
        set i = i + 1
        set Real = Real + GetUnitStatePercentEx(u, UNIT_STATE_LIFE, UNIT_STATE_MAX_LIFE)
        call GroupRemoveUnit(g, u)
    endloop

    set Real = Real/100./i

    call GroupEnumUnitsInRange(g, x, y, 600, null)
    loop
        set u = FirstOfGroup(g)
        exitwhen u == null
        call SetWidgetLife(u, GetWidgetLife(u)*Real)
        call GroupRemoveUnit(g, u)
    endloop

 

[Almia]

GetWidgetLife
and
SetWidgetLife
FTW

 

[Doomlord]

GetWidgetLife
and
SetWidgetLife
FTW

Previous post updated because GetWidgetLife and SetWidgetLife are swaggier

 

[WaterKnight]

Why would you need percent? And you do not even reverse it when setting life then, resulting in the units possessing the hundredfold of what they should have afterwards. Furthermore, you already loop through the group, so of course it's more effective to add a counter within the loop instead of calling your CountUnitsInGroupEx function doing it once again. You even will crash the thread when no units are picked (zero division).

 

[Doomlord]

Why would you need percent? And you do not even reverse it when setting life then. Furthermore, you already loop through the group, so of course it's more effective to add a counter within the loop instead of calling your CountUnitsInGroupEx function doing it once again.

I need percent because the OP request a way to do so, ofc it can be rewritten to increase efficiency.

Yes you are right. I will update it.

edit
About zero division, I can add a protection if necessary.

 

[sniper_zero]

Why would you need percent? And you do not even reverse it when setting life then, resulting in the units possessing the hundredfold of what they should have afterwards. Furthermore, you already loop through the group, so of course it's more effective to add a counter within the loop instead of calling your CountUnitsInGroupEx function doing it once again.

I need percent because I want the ability to get the average percent current health so that some units don't get healed to full HP.

Yeah...the trigger sucks. I know it's not effective which is why I asked how to do it more effectively.

The problem I'm having is putting the counter and setting the units' life in the same trigger. The counter would also serve as the divisor and if it's in a firstofgroup loop the unit would get removed, so its life would be set differently compared to the other units.

 

[WaterKnight]

I need percent because I want the ability to get the average percent current health so that some units don't get healed to full HP.

Current life lies between 0 and max life, so dividing it by max life returns a number between 0 and 1. Multiplying the result with a new max life will again transfer this ratio to between 0 and new max life. I am saying you do not need to scale it by 100, which you would need to count back anyway.

 

[sniper_zero]

Okay, so where in the script did I multiply the result with max life? And you're still saying grouping units twice is bad right? What do you suggest?

 

[Doomlord]

Updated to increase efficiency and fix from current life to max life. Sorry I misread your original code.

    call GroupEnumUnitsInRange(g, x, y, 600, null)
    loop
        set u = FirstOfGroup(g)
        exitwhen u == null
        set i = i + 1
        set Real = Real + GetWidgetLife(u)/GetUnitState(u, UNIT_STATE_MAX_LIFE)
        call GroupRemoveUnit(g, u)
    endloop

    // Fail-safe
    if i != 0 then
        set Real = Real/i
    else
        set Real = 1
    endif

    call GroupEnumUnitsInRange(g, x, y, 600, null)
    loop
        set u = FirstOfGroup(g)
        exitwhen u == null
        call SetWidgetLife(u, GetUnitState(u, UNIT_STATE_MAX_LIFE)*Real)
        call GroupRemoveUnit(g, u)
    endloop

Okay, so where in the script did I multiply the result with max life? And you're still saying grouping units twice is bad right? What do you suggest?

He is talking to me

 

[sniper_zero]

Thanks Doomlord. I've been wanting to give you rep for some time now, but it says I have to give some to other people.

 

[Doomlord]

Thanks Doomlord. I've been wanting to give you rep for some time now, but it says I have to give some to other people.

It is alright

 

[WaterKnight]

If no unit was picked for the first loop, you do not need to try picking again nor start the 2nd loop, nor set the Real variable.

 

[Doomlord]

If no unit was picked for the first loop, you do not need to try picking again nor start the 2nd loop, nor set the Real variable.

Thank you. Updated again.

    call GroupEnumUnitsInRange(g, x, y, 600, null)
    loop
        set u = FirstOfGroup(g)
        exitwhen u == null
        set i = i + 1
        set Real = Real + GetWidgetLife(u)/GetUnitState(u, UNIT_STATE_MAX_LIFE)
        call GroupRemoveUnit(g, u)
    endloop

    // Fail-safe
    if i != 0 then
        set Real = Real/i
        
        call GroupEnumUnitsInRange(g, x, y, 600, null)
        
        loop
            set u = FirstOfGroup(g)
            exitwhen u == null
            call SetWidgetLife(u, GetUnitState(u, UNIT_STATE_MAX_LIFE)*Real)
            call GroupRemoveUnit(g, u)
        endloop
    else
        // call DestroyGroup(g)
        // Execute end code here if you got'em
    endif

 

[sniper_zero]

Thanks Good thing I asked before doing the trigger. I totally would've missed the division by zero.