JASS code not working

[Ivach]

Ok, first of all I wold like to say that I have no experience in JASS whatsoever, and I started learning it few days ago. I read through Daelin's GUI to JASS guide. At chapter 12, about picking units, I tried to understand the code that was given, and I wrote a new one according to that one. The difference is that i used my own variable names and the spell that would be casted, but it doesn't seem to work.

function Distortion_Condition takes nothing returns boolean
return GetSpellAbilityId()=='A000'
endfunction

function Distortion_Action takes nothing returns nothing
local unit u       
local group g
local unit c
local unit d
local location p

set c = GetTriggerUnit()
set p = GetSpellTargetLoc()
set g = GetUnitsInRangeOfLocAll(600.00, p)
loop
      set u = FirstOfGroup(g)
      exitwhen u==null
      if  IsUnitEnemy(u, GetOwningPlayer(c))==true then
      call GroupRemoveUnit(g,u)
      set d = CreateUnitAtLoc(GetOwningPlayer(c), 'h000', GetUnitLoc(u), 0.00)
      call IssueTargetOrderBJ(d, "purge", u)
      call UnitApplyTimedLifeBJ(1.00, 'BTLF', d)
      set d = null
      endif
endloop

set g = null
set u = null
set c = null
set p = null
endfunction

function InitTrig_distortion takes nothing returns nothing
local trigger t
set t=CreateTrigger()
call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_SPELL_EFFECT)
call TriggerAddCondition(t, Condition(function Distortion_Condition))
call TriggerAddAction(t, function Distortion_Action)
endfunction

Any ideas on what might be wrong?

 

[Chocobo]

CODE instead of QUOTE.

The footman has the ability?

 

[AceHart]

The "GroupRemoveUnit" should be outside the "if then".

 

[Ivach]

I moved it outside, but it still doesn't work. Is that the only problem in the trigger?

 

[Chocobo]

Maybe p instead of GetUnitLoc(u)?

 

[Xorifelse]

Make sure the unit has enough mana to cast the ability.
This is a leakless code. Your code leaked 2 locations and a unit leak.

function Distortion_Condition takes nothing returns boolean
return GetSpellAbilityId()=='A000'
endfunction

function Distortion_Action takes nothing returns nothing
    local unit TriggerUnit = GetTriggerUnit()
    local unit UnitInGroup
    local unit TempUnit       
    local group Group = CreateGroup()
    local real UnitX
    local real UnitY
    local location SpellLoc 

        set Group = GetUnitsInRangeOfLocAll(600.00, SpellLoc )
        loop
            set UnitInGroup = FirstOfGroup( Group )
            exitwhen UnitInGroup == null
                call GroupRemoveUnit( Group, UnitInGroup )
                if  IsUnitEnemy( UnitInGroup, GetOwningPlayer( TriggerUnit ) ) == true then
                    set UnitX = GetUnitX( UnitInGroup )
                    set UnitY = GetUnitY( UnitInGroup )
                    set TempUnit = CreateUnit( GetOwningPlayer( TriggerUnit ), 'h000', UnitX, UnitY, 0.00 )
                    call IssueTargetOrderBJ( TempUnit, "purge", UnitInGroup )
                    call UnitApplyTimedLifeBJ( 1.00, 'BTLF', TempUnit )
                    set TempUnit = null
                endif
            set UnitInGroup = null
        endloop
        
    //Nullifying
    call DestroyGroup( Group )
    set Group = null
    set TriggerUnit = null

endfunction

function InitTrig_distortion takes nothing returns nothing
local trigger t
    set t = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddCondition(t, Condition( function Distortion_Condition ) )
    call TriggerAddAction(t, function Distortion_Action )
endfunction

Also make sure.. That using less locations is faster.
Locations are basicly made by Blizzard to make GUI easyer but they leak like hell.

Basicly this is a function created by Blizzard (BJ)

function SetTerrainTypeBJ takes location where, integer terrainType, integer variation, integer area, integer shape returns nothing
    call SetTerrainType(GetLocationX(where), GetLocationY(where), terrainType, variation, area, shape)
endfunction

Now take a good look at this..

native SetTerrainType takes real x, real y, integer terrainType, integer variation, integer area, integer shape returns nothing

basicly its calling a native but instead of using the X, Y its using the location created by Blizzard. Locations leak to fix those:
call RemoveLocation( <Location> )
set <Location> = null

Now basicly that is silly for 2 reasons.
Its calling a native and it creates a leak.

If you use the direct native it go's faster instead of calling 2 things and you dont create a leak.