# How do I do this more efficiently?

#### sniper_zero

Level 3
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.

JASS:
``````    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

Level 16
Here is GetUnitStatePercent

JASS:
``````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

JASS:
``````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.

JASS:
``````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

Level 3
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?

JASS:
``````    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

Level 16
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.

JASS:
``````    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``````

Last edited:

#### Doomlord

Level 16
`GetWidgetLife`
and
`SetWidgetLife`
FTW

Previous post updated because `GetWidgetLife` and `SetWidgetLife` are swaggier

#### WaterKnight

Level 25
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

Level 16
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

Level 3
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

Level 25
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

Level 3
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

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

JASS:
``````    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

Level 3
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

Level 16
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

Level 25
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

Level 16
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.

JASS:
``````    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

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

Replies
6
Views
664
Replies
11
Views
695
Replies
14
Views
786
Replies
2
Views
495
Replies
2
Views
382