Spell: Sanctify

[Zakamutt]

I don't have a map, but I have a triggered spell scriptz!

This script allows you to deal damage and healing around a point x and y, and differentiates between allies and enemies. I suppose it's not very complex, but care.

Put it in your map header and call it with custom script or just plain JASS, be sure to satisfy those parameters and you should be ready to go, leakfree!

trig is triggering unit.

 
function SancFilt takes nothing returns boolean
return GetWidgetLife(GetFilterUnit()) > 0.405
endfunction
 
function Sanctify takes real x, real y, unit trig, real area, real heal, real dmg returns nothing
  local group g = CreateGroup()
  local unit u
  local real hp
  call GroupEnumUnitsInRange(g, x,y,area,Filter(function SancFilt))
   loop
     set u = FirstOfGroup(g)
     exitwhen u == null
     set hp = GetUnitState(u, UNIT_STATE_LIFE)
     if ( IsUnitAlly(u, GetOwningPlayer(trig)) ) then
     call SetWidgetLife(u, hp + heal)
     else
     call SetWidgetLife(u, hp - dmg)
     endif
     call GroupRemoveUnit(g, u)
     endloop
  call DestroyGroup(g)
  set u = null
  set g = null
  endfunction

[DiamondBlade]

This isn't leak free.
You're not nulling 'u' in function 'SancFilt'.
Though that could be easily avoided:

function SancFilt takes nothing returns boolean
local unit u = GetFilterUnit()
if (GetUnitState(u, UNIT_STATE_LIFE) > 0) then //IsUnitAlive
return true
endif
return false
endfunction

Could be:

function SancFilt takes nothing returns boolean
return (GetUnitState(GetFilterUnit(), UNIT_STATE_LIFE) > 0) //IsUnitAlive
endfunction

Or better yet:

function SancFilt takes nothing returns boolean
return GetWidgetLife(GetFilterUnit()) > 0.405 // GetWidgetLife is faster, and shorter. .405 is the actual life at which units are considered dead.
endfunction

You may also want to consider 'IsUnitType(u, UNIT_TYPE_DEAD) == true', as a dead units health can be (accidentally) set above 1, and the above conditions would fail.

Destroying filters isn't a good idea due to how Warcraft handles them (similarly to strings, that is), and hence there's rarely a reason to ever need to assign them to locals.
There's also no point in using locals if they're only being used once, as there are no advantages whatsoever, and it's slower, longer to type, and they have to be nulled.
Function arguments don't need to be nulled - they're properly recycled automatically.
Players don't need to be nulled as they're never destroyed.

'GetUnitState(u, UNIT_STATE_LIFE)' could be 'GetWidgetLife(u)', as it's faster and shorter.
'SetUnitState(u, UNIT_STATE_LIFE, <value>)' could be 'SetWidgetLife(u, <value>)' for the same reasons.

[Zakamutt]

Code changed in line with your suggestions. I didn't really focus on the filter, I suppose that's why I did not catch the leak.

Function arguments automatically recycled... interesting. Though I guess not surprising.

IsUnitType is not really a problem. If the map has no issues due to it increasing the life of dead units, it has no issues with this also increasing or decreasing their life. Which check is faster?

[DiamondBlade]

> Function arguments automatically recycled... interesting. Though I guess not surprising.
It's nice to know Blizzard only fucked up partially.

> Which check is faster?
'GetWidgetLife' is much faster. There's a downside to 'IsUnitType', though I can't remember it off the top of my head. Speed isn't really important anyway; nobody can tell the difference. Use whichever you prefer.