[JASS] Projectile spell problem

[Garfield1337]

I made a simple projectile spell that works like: Unit throws a dagger,if the dagger encounters a unit of ancient classification,it will stun it.
Here's the trigger:

Trigger

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

function Throw_Dagger_Actions takes nothing returns nothing
    local unit u = CreateUnit(GetOwningPlayer(GetTriggerUnit()),'h00A',GetUnitX(GetTriggerUnit()),GetUnitY(GetTriggerUnit()),bj_RADTODEG * Atan2(GetSpellTargetY() - GetUnitY(GetTriggerUnit()),GetSpellTargetX() - GetUnitX(GetTriggerUnit())))
    call GroupAddUnit(udg_Daggers,u)
    call SaveReal(udg_Hashtable,GetHandleId(u),0,0.00)
    set u = null
endfunction

function Dagger_Filter takes nothing returns boolean
    return IsUnitType(GetFilterUnit(),UNIT_TYPE_ANCIENT) and GetOwningPlayer(GetFilterUnit()) != GetOwningPlayer(GetEnumUnit())
endfunction

function Group_Count takes nothing returns nothing
    set udg_Integer = udg_Integer + 1
endfunction

function Dagger_Loop takes nothing returns nothing
    local real x = GetUnitX(GetEnumUnit()) + 30 * Cos(GetUnitFacing(GetEnumUnit()) * bj_DEGTORAD)
    local real y = GetUnitY(GetEnumUnit()) + 30 * Sin(GetUnitFacing(GetEnumUnit()) * bj_DEGTORAD)
    local group g
    local unit u
    call SetUnitX(GetEnumUnit(),x)
    call SetUnitY(GetEnumUnit(),y)
    call SaveReal(udg_Hashtable,GetHandleId(GetEnumUnit()),0,LoadReal(udg_Hashtable,GetHandleId(GetEnumUnit()),0) + 30.00)
    call GroupEnumUnitsInRange(g,x,y,40.00,Filter(function Dagger_Filter))
    set udg_Integer = 0
    call ForGroup(g,function Group_Count)
    if udg_Integer > 0 then
        set u = CreateUnit(GetOwningPlayer(GetEnumUnit()),'h005',x,y,0.00)
        call UnitApplyTimedLife(u,'BTLF',1.00)
        call IssueTargetOrder(u,"thunderbolt",GetClosestUnit(g,x,y))
        call GroupRemoveUnit(udg_Daggers,GetEnumUnit())
        call KillUnit(GetEnumUnit())
    elseif LoadReal(udg_Hashtable,GetHandleId(GetEnumUnit()),0) > 900.00 then
        call GroupRemoveUnit(udg_Daggers,GetEnumUnit())
        call KillUnit(GetEnumUnit())
    endif
    call DestroyGroup(g)
    set g = null
    set u = null
endfunction

function Dagger_Loop_Init takes nothing returns nothing
    call ForGroup(udg_Daggers,function Dagger_Loop)
endfunction

//===========================================================================
function InitTrig_Throw_Dagger takes nothing returns nothing
    local trigger t = CreateTrigger()
    set gg_trg_Throw_Dagger = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( gg_trg_Throw_Dagger, EVENT_PLAYER_UNIT_SPELL_EFFECT)
    call TriggerAddCondition( gg_trg_Throw_Dagger, Condition(function Throw_Dagger_Condition))
    call TriggerAddAction( gg_trg_Throw_Dagger, function Throw_Dagger_Actions)
    call TriggerRegisterTimerEvent(t,0.03,true)
    call TriggerAddAction(t, function Dagger_Loop_Init)
    set t = null
endfunction

But it doesn't seem to work. The projectile moves,but doesn't hit any unit,also it doesn't get removed when it passes more than 900.0 range which is supposed to. Actually,when i tried putting some debug messages,it seems they don't work if they're placed bellow GroupEnumUnitsInRange function in Dagger_Loop function!


Also i use function GetClosestUnit,which i made:

Trigger

function GetClosestUnit takes group g,real x,real y returns unit
    local real r1 = 0.00
    local real r2 = 4000.00
    local unit u1
    local unit u2
    local group a = g
    loop
        set u1 = FirstOfGroup(a)
        exitwhen u1 == null
        set r1 = SquareRoot((GetUnitX(u1) - x) * (GetUnitX(u1) - x) + (GetUnitY(u1) - y) * (GetUnitY(u1) - y))
        if r1 < r2 then
            set r2 = r1
            set u2 = u1
        endif
        call GroupRemoveUnit(a,u1)
    endloop
    call DestroyGroup(a)
    set a = null
    set u1 = null
    set u2 = null
    return u2
endfunction

It might affect problem,but it shouldn't.

+rep for anyone who helps me solve this.

 

[watermelon_1234]

There is something wrong with GetClosestUnit.

    set u2 = null
    return u2

I haven't really looked at why the distance check is failing. You did make sure you created the hashtable, right?

 

[Garfield1337]

Thanks for pointing out the flaw in that function.
Also i would be very grateful if you can improve code and point any flaws in it.
Oh and i have a question: Do i need to null local unit variables? If so,how would i do it in GetClosestUnit function?

You did make sure you created the hashtable, right?

Yes. I was using debug messages to check if distance passed works and it did...atleast,when it saved value into hashtable.
I have this at Init trigger:

set udg_Hashtable = InitHashtable()

 

[watermelon_1234]

Oh and i have a question: Do i need to null local unit variables? If so,how would i do it in GetClosestUnit function?

Yes, you do. You can avoid this problem if you use a global variable.

Anyway, here's the code I attempted to improve. I'm not sure if I fixed the distance check but I did find another problem: you never set the local group g to CreateGroup in Dagger_Loop.
To use the code, you'll need to make a global unit variable, TempUnit.

function GetClosestUnit takes group g,real x,real y returns unit
    local real r1 = 0.00
    local real r2 = 4000.00
    local unit u1
    local group a = g    
    loop
        set u1 = FirstOfGroup(a)
        exitwhen u1 == null
        set r1 = SquareRoot((GetUnitX(u1) - x) * (GetUnitX(u1) - x) + (GetUnitY(u1) - y) * (GetUnitY(u1) - y))
        if r1 < r2 then
            set r2 = r1
            set udg_TempUnit = u1 // We'll just use a global variable to avoid having to null it.
        endif
        call GroupRemoveUnit(a,u1)
    endloop
    call DestroyGroup(a)
    set a = null 
    // set u1 = null // Not necessary because it would already be nulled.
    return udg_TempUnit
endfunction


function Throw_Dagger_Condition takes nothing returns boolean
    local unit u
    local real x
    local real y
    if GetSpellAbilityId() == 'A000' then
        set x = GetUnitX(GetTriggerUnit())
        set y = GetUnitY(GetTriggerUnit())
        set u = CreateUnit(GetTriggerPlayer(),'h00A',x,y,bj_RADTODEG * Atan2(GetSpellTargetY() - y,GetSpellTargetX() - x))
        call GroupAddUnit(udg_Daggers,u)
        call SaveReal(udg_Hashtable,GetHandleId(u),0,0.00)
        set u = null
    endif
    return false
endfunction

// Combined Group_Count into this.
function Dagger_Filter takes nothing returns boolean
    if IsUnitType(GetFilterUnit(),UNIT_TYPE_ANCIENT) and GetOwningPlayer(GetFilterUnit()) != GetOwningPlayer(udg_TempUnit) then
        set udg_Integer = udg_Integer + 1
        return true
    endif
    return false
endfunction

function Dagger_Loop takes nothing returns nothing   
    local real x 
    local real y 
    local group g = CreateGroup() // You forgot this.
    local real dist  // Just to make things look neater.
    local unit u
    set udg_TempUnit = GetEnumUnit() // You  need to use a global in order to correctly pass this to Dagger_Filter.
    set x = GetUnitX(udg_TempUnit) + 30 * Cos(GetUnitFacing(udg_TempUnit) * bj_DEGTORAD)
    set y = GetUnitY(udg_TempUnit) + 30 * Sin(GetUnitFacing(udg_TempUnit) * bj_DEGTORAD)
    set dist = LoadReal(udg_Hashtable,GetHandleId(udg_TempUnit),0) + 30.00 // Note I added 30 to change the way things worked.
    call SetUnitX(udg_TempUnit,x)
    call SetUnitY(udg_TempUnit,y)    
    set udg_Integer = 0
    call GroupEnumUnitsInRange(g,x,y,40.00,Filter(function Dagger_Filter)) 
    if udg_Integer > 0 then
        set u = CreateUnit(GetOwningPlayer(udg_TempUnit),'h005',x,y,0.00)
        call UnitApplyTimedLife(u,'BTLF',1.00)
        call IssueTargetOrder(u,"thunderbolt",GetClosestUnit(g,x,y))
        call GroupRemoveUnit(udg_Daggers,udg_TempUnit)
        call KillUnit(udg_TempUnit)
    elseif dist > 900.00 then
        call GroupRemoveUnit(udg_Daggers,udg_TempUnit)
        call KillUnit(udg_TempUnit)
    else // The real will only be saved into the hashtable if the dist is lower than 900.
        call SaveReal(udg_Hashtable,GetHandleId(udg_TempUnit),0,dist)
    endif
    call DestroyGroup(g)
    set g = null
    set u = null
endfunction

function Dagger_Loop_Init takes nothing returns boolean
    call ForGroup(udg_Daggers,function Dagger_Loop)
    return false
endfunction

//===========================================================================
function InitTrig_Throw_Dagger takes nothing returns nothing
    local trigger t = CreateTrigger() // You could just stick to using this for both triggers.
    call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_SPELL_EFFECT)
    call TriggerAddCondition(t, Condition(function Throw_Dagger_Condition)) // Trigger conditions are better.
    
    set t = CreateTrigger()    
    call TriggerRegisterTimerEvent(t,0.03,true)
    call TriggerAddCondition(t, Condition(function Dagger_Loop_Init)) // Trigger conditions are better. Oh, and you actually don't need Condition for this thanks to jasshelper.
     
    set t = null
endfunction

 

[Frotty]

I think you should Create the group "g" before attempting to add Units to it.

And anyway local groups arent the finest way, rather use one global group and just clear it up, because even if you destroy the group it always leaves a small leak behind.
You could also just use the lib TimerUtils (the utils Libs are very useful anyway) which recycels the groups for you.

When you kill the Unit you should propably also remove the stored real, tho it doesnt really matter.

 

[Garfield1337]

@Watermelon
The missing CreateGroup() was the problem. Also thanks a lot for taking your time to improve my code.
@Frotty
Thank you too for taking your time.

+rep for both.