[Spell] Does this ability leak?

[Doomlord]

I am still fairly new to JASS and I don't know much about checking all the leaks of an ability so I need help. Here is my code for a spell

function Trig_Frost_Shard_Conditions takes nothing returns boolean
    return ( GetSpellAbilityId() == 'A005' ) 
endfunction

function Frost_Shard_Effect_Periodic takes nothing returns nothing
local timer t = GetExpiredTimer()
local unit target = LoadUnitHandleBJ(0, GetHandleIdBJ(t), udg_Hashtable)
local unit dummy = LoadUnitHandleBJ(1, GetHandleIdBJ(t), udg_Hashtable)
local unit caster = LoadUnitHandleBJ(2, GetHandleIdBJ(t), udg_Hashtable)
local location l1 = GetUnitLoc (target)
local location l = GetUnitLoc (dummy)
local location l2
local real angle

if (DistanceBetweenPoints(l, l1) <= 30) then
if IsUnitAliveBJ(target) == true then
call KillUnit (dummy)
call UnitDamageTarget(caster, target, 500, true, false, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_FIRE, null)
call RemoveLocation (l1)
call RemoveLocation (l)
set l = null
set l1= null
call PauseTimer (t)
call DestroyTimer (t)
set t = null
set caster = null
set target = null
set dummy = null
else
call KillUnit (dummy)
call RemoveLocation (l1)
call RemoveLocation (l)
set l = null
set l1= null
call PauseTimer (t)
call DestroyTimer (t)
set t = null
set caster = null
set target = null
set dummy = null
endif
else
set angle = AngleBetweenPoints(l, l1)
set l2 = PolarProjectionBJ (l, 20, angle)
call SetUnitPositionLocFacingBJ(dummy, l2, angle)
call RemoveLocation (l2)
set l2 = null
endif
set target = null
set caster = null
set dummy = null
endfunction

function Frost_Shard_Effect takes unit caster, unit u, location l returns nothing
local unit dummy
local timer t = CreateTimer ()
local real angle

set angle = AngleBetweenPoints (GetUnitLoc (caster), GetUnitLoc(u))
set dummy = CreateUnitAtLoc(GetOwningPlayer(caster), 'h003', l, angle)
call SaveUnitHandleBJ(u, 0, GetHandleIdBJ(t), udg_Hashtable)
call SaveUnitHandleBJ(dummy, 1, GetHandleIdBJ(t), udg_Hashtable)
call SaveUnitHandleBJ(caster, 2, GetHandleIdBJ(t), udg_Hashtable)
call TimerStart (t, 0.02, true, function Frost_Shard_Effect_Periodic)
set l = null
set dummy = null
set caster = null
set u = null
set t = null
endfunction

function Frost_Shard_Explode takes unit caster, location l returns nothing
local timer t = GetExpiredTimer()
local group g
local unit u

set g = GetUnitsInRangeOfLocAll(1000.00, l)
loop
    set u = FirstOfGroup (g)
    exitwhen u == null
    if IsUnitEnemy(u, GetOwningPlayer(caster)) == true and IsUnitAliveBJ(u) == true then   
         call Frost_Shard_Effect (caster, u, l)
    endif
    call GroupRemoveUnitSimple (u, g)   
endloop

call DestroyGroup(g)
set g = null
set caster = null
endfunction

function Frost_Shard_Periodic takes nothing returns nothing
local trigger trig = GetTriggeringTrigger ()
local unit caster = LoadUnitHandleBJ(0, GetHandleIdBJ(trig), udg_Hashtable)
local unit target = LoadUnitHandleBJ(1, GetHandleId(trig), udg_Hashtable)
local unit dummy = LoadUnitHandleBJ(2, GetHandleId(trig), udg_Hashtable)
local location l = GetUnitLoc (dummy)
local location l1 = GetUnitLoc (target)
local location l2
local real angle
local triggeraction tga = LoadTriggerActionHandleBJ (3, GetHandleIdBJ (trig), udg_Hashtable)

if(GetTriggerEventId()==EVENT_UNIT_DEATH)then
if(GetDyingUnit() == target)then
call KillUnit (dummy)
call RemoveLocation (l)
call RemoveLocation (l1)
set l = null
set l1 = null
set caster = null
set target = null
set dummy = null
call DisableTrigger (trig)
call TriggerRemoveAction (trig, tga)
call DestroyTrigger (trig)
set tga = null
set trig = null
endif
else
if (DistanceBetweenPoints(l, l1) <= 30) then
call KillUnit (dummy)
call UnitDamageTarget(caster, target, 1000, true, false, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_FIRE, null)
if IsUnitAliveBJ (target) == false then
call Frost_Shard_Explode(caster, l1)
else
endif
call RemoveLocation (l)
call RemoveLocation (l1)
set caster = null
set target = null
set dummy = null
set l = null
set l1 = null
call DisableTrigger (trig)
call TriggerRemoveAction (trig, tga)
call DestroyTrigger (trig)
set tga = null
set trig = null
else
set angle = AngleBetweenPoints(l, l1)
set l2 = PolarProjectionBJ (l, 24, angle)
call SetUnitPositionLocFacingBJ(dummy, l2, angle)
call RemoveLocation (l2)
set l2 = null
endif
endif
endfunction

function Trig_Frost_Shard_Actions takes nothing returns nothing
local unit caster = GetTriggerUnit()
local unit target = GetSpellTargetUnit()
local unit dummy
local trigger trig = CreateTrigger ()
local location l = GetUnitLoc(caster)
local real angle

set angle = AngleBetweenPoints (GetUnitLoc (caster), GetUnitLoc(target))
set dummy = CreateUnitAtLoc(GetOwningPlayer(caster), 'h002', l, angle)
call TriggerRegisterTimerEventPeriodic(trig, 0.02)
call TriggerRegisterUnitEvent(trig, target, EVENT_UNIT_DEATH)
call SaveTriggerActionHandleBJ( TriggerAddAction( trig, function Frost_Shard_Periodic ), 3, GetHandleIdBJ (trig), udg_Hashtable)
call SaveUnitHandleBJ( caster, 0, GetHandleIdBJ(trig), udg_Hashtable )
call SaveUnitHandleBJ( target, 1, GetHandleIdBJ(trig), udg_Hashtable )
call SaveUnitHandleBJ( dummy, 2, GetHandleIdBJ(trig), udg_Hashtable )

call RemoveLocation (l)
set l = null
set caster = null
set target = null
set dummy = null
set trig = null
endfunction

//===========================================================================
function InitTrig_Frost_Shard takes nothing returns nothing
    set gg_trg_Frost_Shard = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( gg_trg_Frost_Shard, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddCondition( gg_trg_Frost_Shard, Condition( function Trig_Frost_Shard_Conditions ) )
    call TriggerAddAction( gg_trg_Frost_Shard, function Trig_Frost_Shard_Actions )
endfunction

So I have two questions:

• Does this skill leak? And if it leaks, where?
• Do I need to clear the hashtable or will the values (except for locations and unit groups) be recycled?

Test map attached. +rep for helpers. Thanks in advance.

P/S: It also would be great if you could point out anything that is wrong with my other spells but that is optional.

EDIT: Fixed some of the minor coding bugs in the posted code. Test map is also updated.

 

[Doomlord]

Bump.

Sorry guys but I need a response as soon as possible.

 

[Dr Super Good]

set angle = AngleBetweenPoints (GetUnitLoc (caster), GetUnitLoc(u))

Leaks 2 location objects. They are generated by GetUnitLoc but never Removed.

I advise you inline some of those BJ functions as an optimization.

 

[Doomlord]

Leaks 2 location objects. They are generated by GetUnitLoc but never Removed.

I advise you inline some of those BJ functions as an optimization.

Okay got it. But I am doing these spells for The Chosen Ones campaign. I am fine with using the natives. Won't be needing the optimizations.

Other than the mentioned leak, anything else?

 

[Dr. Boom]

Moin moin =)

I saw this topic yesterday, I thought no problems to check if a spell leaks but then I saw your code and I was lost at all.

First of all pay attention, I recode the whole spell, because I can't deal with normal jass like you did, now its vJass.

library FrostShard initializer init

    globals
        private constant integer SPELL_ID = 'A000'
        private constant integer DUMMY_ID = 'dum1'
        
        private constant real SHARD_SPEED = 20.
        private constant real SHARD_EXPLODE_DISTANCE = 50.
        private constant real SHARD_SPREAD_AREA = 400.
        private constant real SHARD_DAMAGE = 500.
        
        // don't change this \\
        
        private constant hashtable HASH = InitHashtable()
        private constant real LOOP_TIME = 0.03125
        private integer TEMP
    endglobals
    
    struct FrostShard
        unit    caster  =   null
        unit    target  =   null
        unit    dummy   =   null
        player  owner   =   null
        
        real    face    =   0.

        static method spread takes nothing returns nothing
            local timer t2 = GetExpiredTimer()
            local thistype this = LoadInteger(HASH,GetHandleId(t2),4)
            local unit dummy  = LoadUnitHandle(HASH,GetHandleId(t2),2)
            local unit target = LoadUnitHandle(HASH,GetHandleId(t2),1)
            local real facing = LoadReal(HASH,GetHandleId(t2),3)
            local real x
            local real y
            
            set x = GetUnitX(target) - GetUnitX(dummy)
            set y = GetUnitY(target) - GetUnitY(dummy)
            
            if x * x + y * y >= SHARD_EXPLODE_DISTANCE * SHARD_EXPLODE_DISTANCE then
                set x = GetUnitX(dummy) + SHARD_SPEED * Cos(facing)
                set y = GetUnitY(dummy) + SHARD_SPEED * Sin(facing)
                call SetUnitX(dummy,x)
                call SetUnitY(dummy,y)
            else
                call RemoveUnit(dummy)
                call UnitDamageTarget(.caster,target,SHARD_DAMAGE,true,false,ATTACK_TYPE_NORMAL,DAMAGE_TYPE_NORMAL,WEAPON_TYPE_WHOKNOWS)
                call PauseTimer(t2)
                call DestroyTimer(t2)
                call .destroy()
            endif
            
            set t2 = null
        endmethod

        static method init_spread takes nothing returns boolean
            local thistype this = TEMP
            local unit u1 = GetFilterUnit()
            local unit u2
            local timer t2 = CreateTimer()
            
            if GetWidgetLife(u1) > 0.405 and IsUnitEnemy(u1,owner) and IsUnitType(u1,UNIT_TYPE_STRUCTURE) == false and IsUnitType(u1,UNIT_TYPE_MAGIC_IMMUNE) == false then
                set .face = Atan2(GetUnitY(u1) - GetUnitY(.target),GetUnitX(u1) - GetUnitX(.target))
                set u2 = CreateUnit(.owner,DUMMY_ID,GetUnitX(.target),GetUnitY(.target),.face * 57.2957795)
                
                call SaveUnitHandle(HASH,GetHandleId(t2),1,u1)
                call SaveUnitHandle(HASH,GetHandleId(t2),2,u2)
                call SaveReal(HASH,GetHandleId(t2),3,.face)
                call SaveInteger(HASH,GetHandleId(t2),4,this)
                call TimerStart(t2,LOOP_TIME,true,function thistype.spread)
                set u2 = null
            endif
            set u1 = null
            set t2 = null
            return false
        endmethod

        static method periodic takes nothing returns nothing
            local timer t1 = GetExpiredTimer()
            local thistype this = LoadInteger(HASH,GetHandleId(t1),0)
            local group g = CreateGroup()
            local real x
            local real y
            
            set x = GetUnitX(.target) - GetUnitX(.dummy)
            set y = GetUnitY(.target) - GetUnitY(.dummy)
            
            if x * x + y * y >= SHARD_EXPLODE_DISTANCE * SHARD_EXPLODE_DISTANCE then
                set x = GetUnitX(.dummy) + SHARD_SPEED * Cos(.face)
                set y = GetUnitY(.dummy) + SHARD_SPEED * Sin(.face)
                call SetUnitX(.dummy,x)
                call SetUnitY(.dummy,y)
            else
                call RemoveUnit(.dummy)
                call UnitDamageTarget(.caster,.target,SHARD_DAMAGE,true,false,ATTACK_TYPE_NORMAL,DAMAGE_TYPE_NORMAL,WEAPON_TYPE_WHOKNOWS)
                call GroupEnumUnitsInRange(g,GetUnitX(.target),GetUnitY(.target),SHARD_SPREAD_AREA,Condition(function thistype.init_spread))
                call PauseTimer(t1)
                call DestroyTimer(t1)
            endif
            
            call DestroyGroup(g)
            set t1 = null
            set g = null
        endmethod
        
        static method create takes unit caster, unit target returns thistype
            local thistype this = .allocate()
            local timer t1 = CreateTimer()
            
            set .caster = caster
            set .target = target
            set .owner = GetOwningPlayer(.caster)
            set .face = Atan2(GetUnitY(.target) - GetUnitY(.caster), GetUnitX(.target) - GetUnitY(.caster))
            set .dummy = CreateUnit(.owner,DUMMY_ID,GetUnitX(.caster),GetUnitY(.caster),.face * 57.2957795)
            set TEMP = this
            
            call SaveInteger(HASH,GetHandleId(t1),0,this)
            call TimerStart(t1,LOOP_TIME,true,function thistype.periodic)
            set t1 = null
            return this
        endmethod
    endstruct
    
    private function condition takes nothing returns boolean
        if GetSpellAbilityId() == SPELL_ID then
            call FrostShard.create(GetTriggerUnit(),GetSpellTargetUnit())
        endif
        return false
    endfunction
    
    private function init takes nothing returns nothing
        local trigger t = CreateTrigger()
        local integer i = 0
        
        loop
            exitwhen i == 15
            call TriggerRegisterPlayerUnitEvent(t,Player(i),EVENT_PLAYER_UNIT_SPELL_EFFECT,null)
            set i = i + 1
        endloop
        call TriggerAddCondition(t,function condition)
        
        set t = null
    endfunction
endlibrary

If you need any help with vJass just ask and @Dr Super Good: Well there are a lot more improvements for this spell as some red bj's at all ...

Greetings and Peace
Dr. Boom

 

[Doomlord]

Like I already said I can't code in vJASS. I know it is very good and all but I am doing it for a campaign and campaigns don't support vJASS. I hate the normal JASS too but it doesn't seem like I have any other choice.

 

[Dr. Boom]

Campaigns don't support vJass? Sorry I didn't know that

Then some other needs to help you, because I don't get this normal Jass way at all.
(Next time if you code, please use a better format as I posted the code, then its a bit easier to read it)

 

[Doomlord]

Campaigns don't support vJass? Sorry I didn't know that

Then some other needs to help you, because I don't get this normal Jass way at all.
(Next time if you code, please use a better format as I posted the code, then its a bit easier to read it)

Yeah fine I know JASS is always the target of all hate around because it can't stand a chance against vJASS. But two questions still remain:

1/Are the values (except unit groups and locations) in hashtables recycled or I need to clear them manually?

2/When a function uses, for example a location, should the variable be removed in the function that uses it or in the previous function? And do I need to null the variables that the function takes?

 

[Dr Super Good]

You are not using the natives, that is the problem... You are using BJ adapters to the natives which result in a custom function call which is very expensive in JASS.

Campaigns do support vJASS, just you need to save the map externally and re-import it each time.

You really do not need to use vJASS for everything.

 

[Doomlord]

You are not using the natives, that is the problem... You are using BJ adapters to the natives which result in a custom function call which is very expensive in JASS.

Wow never know that. How should I improve it then? Can you give me some examples?

 

[Dr. Boom]

Wow never know that. How should I improve it then? Can you give me some examples?

You just should use the Jass NewGen Pack and update it with the newest "JassHelper". With help of this JassHelper you learn a good jass very easy.
Also we don't use locations here, instead we use x and y coordinates.

 

[Doomlord]

Why shouldn't you use locations? I see lots of JASS spells that use coordinates but I never know why.

And also I am kinda in a crisis here. I need to submit the spells at the end of this week. Some native functions to replace the BJ's would be of great help. I will check out the JASS NewGen and JassHelper later.

 

[Dr. Boom]

Well mate I don't know the reason anymore, but when I started with jass everyone said to me I have to use coordinates instead of locations...
I only know one reason so far for using an location

GetLocationZ

for a better height calculation.

Well it's a lot of work there but lets get a bit into it:

LoadUnitHandleBJ(0, GetHandleIdBJ(t), udg_Hashtable)
// should be
LoadUnitHandle(udg_Hashtable,GetHandleId(t),0)
// now you can fix the other two

DistanceBetweenPoints(l, l1) <= 30
// here we need coordination
local real dx = GetUnitX(target) - GetUnitX(dummy)
local real dy = GetUnitY(target) - GetUnitY(dummy)

if dx * dx + dy * dy <= 30 * 30 then
//all actions here

if IsUnitAliveBJ(target) == true then
// we use:
if GetWidgetLife(target) > 0.405 then

PolarProjectionBJ
// fix:
local real x = X of source + dist * Cos(angle * bj_DEGTORAD)
local real y = Y of source + dist * Sin(angle * bj_DEGTORAD)

AngleBetweenPoints
// the fix:
bj_RADTODEG * Atan2(GetLocationY(locB) - GetLocationY(locA), GetLocationX(locB) - GetLocationX(locA))
// you don't need to use GetLocationX or GetLocationY, you can use GetUnitX and GetUnitY for coordinates

SaveUnitHandleBJ(u, 0, GetHandleIdBJ(t), udg_Hashtable)
// the fix:
SaveUnitHandle(udg_Hashtable,GetHandleId(t),0,u)

GetUnitsInRangeOfLocAll(1000.00, l)
// the fix
call GroupEnumUnitsInRange(group,x,y,range,filter/null)

call GroupRemoveUnitSimple (u, g)   
// the fix
call GroupRemoveUnit(g,u)

About "LoadTriggerActionHandleBJ" and SaveTriggerActionHandleBJ I don't know anything, since I don't need these actions.

TriggerRegisterTimerEventPeriodic(trig, 0.02)
// the fix:
TriggerRegisterTimerEvent(trig,0.03125,true)
// 0.03125 is the LOWEST value to use. True means its periodic

call TriggerRegisterAnyUnitEventBJ( gg_trg_Frost_Shard, EVENT_PLAYER_UNIT_SPELL_EFFECT )
// some people will say "you don't need to fix this blabla" but its red so we fix it!
local integer i = 0

loop
   exitwhen i == 15
   call TriggerRegisterPlayerUnitEvent(trigger,Player(i),EVENT_PLAYER_UNIT_SPELL_EFFECT,null)
   set i = i + 1
endloop

Okay I just wrote everything by hand, I hope I didn't make something wrong, but you said you are new to jass and I really recommend JassHelper (which only works with jass NewGen pack). It helped me a lot when I started with jass =)

Edit: Bah just burn all bj's: bj_DEGTORAD = 57,2957795

 

[Doomlord]

Well now I realized how lousy a programmer I am but no prob I learned lots of new things after all. Thanks very much. +rep to both of you.

EDIT: About this

GetUnitsInRangeOfLocAll(1000.00, l)
// the fix
call GroupEnumUnitsInRange(group,x,y,range,filter/null)

Can you show me how to do the filter thing? I realized now that I never know how to do such a basic thing. And one more thing, after I use this do I need another function for the group?

 

[Dr. Boom]

Okay I just try to give an example how I use it okay?

function name2 takes nothing returns boolean
    local unit u = GetFilterUnit()
    
    if GetWidgetLife(u) > 0.405 and IsUnitEnemy(u,Player(0)) and IsUnitType(u,UNIT_TYPE_MAGIC_IMMUNE) == false then //just a filter..
        call UnitDamageTarget()
    endif
    
    set u = null
    return false
endfunction

function name1 takes nothing returns nothing
    local group g = CreateGroup()

    call GroupEnumUnitsInRange(g,x,y,range,Condition(function name2))
   
    call DestroyGroup(g)
    set g = null
endfunction

 

[Doomlord]

Okay thanks Dr.Boom.

 

[edo494]

Well mate I don't know the reason anymore, but when I started with jass everyone said to me I have to use coordinates instead of locations...
I only know one reason so far for using an location

GetLocationZ

for a better height calculation.

from common.j

// This function is asynchronous. The values it returns are not guaranteed synchronous between each player.
//  If you attempt to use it in a synchronous manner, it may cause a desync.
native GetLocationZ             takes location whichLocation returns real

his code should be(I only tab-bed it out not rewrite):

function Trig_Frost_Shard_Conditions takes nothing returns boolean
    return ( GetSpellAbilityId() == 'A005' ) 
endfunction

function Frost_Shard_Effect_Periodic takes nothing returns nothing
    local timer t = GetExpiredTimer()
    local unit target = LoadUnitHandleBJ(0, GetHandleIdBJ(t), udg_Hashtable)
    local unit dummy = LoadUnitHandleBJ(1, GetHandleIdBJ(t), udg_Hashtable)
    local unit caster = LoadUnitHandleBJ(2, GetHandleIdBJ(t), udg_Hashtable)
    local location l1 = GetUnitLoc (target)
    local location l = GetUnitLoc (dummy)
    local location l2
    local real angle
    if (DistanceBetweenPoints(l, l1) <= 30) then
        if IsUnitAliveBJ(target) == true then
            call KillUnit (dummy)
            call UnitDamageTarget(caster, target, 500, true, false, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_FIRE, null)
            call RemoveLocation (l1)
            call RemoveLocation (l)
            set l = null
            set l1= null
            call PauseTimer (t)
            call DestroyTimer (t)
            set t = null
            set caster = null
            set target = null
            set dummy = null
        else
            call KillUnit (dummy)
            call RemoveLocation (l1)
            call RemoveLocation (l)
            set l = null
            set l1= null
            call PauseTimer (t)
            call DestroyTimer (t)
            set t = null
            set caster = null
            set target = null
            set dummy = null
        endif
    else
        set angle = AngleBetweenPoints(l, l1)
        set l2 = PolarProjectionBJ (l, 20, angle)
        call SetUnitPositionLocFacingBJ(dummy, l2, angle)
        call RemoveLocation (l2)
        set l2 = null
    endif
    set target = null
    set caster = null
    set dummy = null
endfunction

function Frost_Shard_Effect takes unit caster, unit u, location l returns nothing
    local unit dummy
    local timer t = CreateTimer ()
    local real angle
    set angle = AngleBetweenPoints (GetUnitLoc (caster), GetUnitLoc(u))
    set dummy = CreateUnitAtLoc(GetOwningPlayer(caster), 'h003', l, angle)
    call SaveUnitHandleBJ(u, 0, GetHandleIdBJ(t), udg_Hashtable)
    call SaveUnitHandleBJ(dummy, 1, GetHandleIdBJ(t), udg_Hashtable)
    call SaveUnitHandleBJ(caster, 2, GetHandleIdBJ(t), udg_Hashtable)
    call TimerStart(t, 0.02, true, function Frost_Shard_Effect_Periodic)
    set l = null
    set dummy = null
    set caster = null
    set u = null
    set t = null
endfunction

function Frost_Shard_Explode takes unit caster, location l returns nothing
    local timer t = GetExpiredTimer()
    local group g
    local unit u
    set g = GetUnitsInRangeOfLocAll(1000.00, l)
    loop
        set u = FirstOfGroup (g)
        exitwhen u == null
        if IsUnitEnemy(u, GetOwningPlayer(caster)) == true and IsUnitAliveBJ(u) == true then   
            call Frost_Shard_Effect (caster, u, l)
        endif
        call GroupRemoveUnitSimple (u, g)   
    endloop
    call DestroyGroup(g)
    set g = null
    set caster = null
endfunction

function Frost_Shard_Periodic takes nothing returns nothing
    local trigger trig = GetTriggeringTrigger ()
    local unit caster = LoadUnitHandleBJ(0, GetHandleIdBJ(trig), udg_Hashtable)
    local unit target = LoadUnitHandleBJ(1, GetHandleId(trig), udg_Hashtable)
    local unit dummy = LoadUnitHandleBJ(2, GetHandleId(trig), udg_Hashtable)
    local location l = GetUnitLoc (dummy)
    local location l1 = GetUnitLoc (target)
    local location l2
    local real angle
    local triggeraction tga = LoadTriggerActionHandleBJ (3, GetHandleIdBJ (trig), udg_Hashtable)
    if(GetTriggerEventId()==EVENT_UNIT_DEATH)then
        if(GetDyingUnit() == target)then
            call KillUnit (dummy)
            call RemoveLocation (l)
            call RemoveLocation (l1)
            set l = null
            set l1 = null
            set caster = null
            set target = null
            set dummy = null
            call DisableTrigger (trig)
            call TriggerRemoveAction (trig, tga)
            call DestroyTrigger (trig)
            set tga = null
            set trig = null
        endif
    else
        if (DistanceBetweenPoints(l, l1) <= 30) then
            call KillUnit (dummy)
            call UnitDamageTarget(caster, target, 1000, true, false, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_FIRE, null)
            if IsUnitAliveBJ (target) == false then
                call Frost_Shard_Explode(caster, l1)
            else
            endif
            call RemoveLocation (l)
            call RemoveLocation (l1)
            set caster = null
            set target = null
            set dummy = null
            set l = null
            set l1 = null
            call DisableTrigger (trig)
            call TriggerRemoveAction (trig, tga)
            call DestroyTrigger (trig)
            set tga = null
            set trig = null
        else
            set angle = AngleBetweenPoints(l, l1)
            set l2 = PolarProjectionBJ (l, 24, angle)
            call SetUnitPositionLocFacingBJ(dummy, l2, angle)
            call RemoveLocation (l2)
            set l2 = null
        endif
    endif
endfunction

function Trig_Frost_Shard_Actions takes nothing returns nothing
    local unit caster = GetTriggerUnit()
    local unit target = GetSpellTargetUnit()
    local unit dummy
    local trigger trig = CreateTrigger ()
    local location l = GetUnitLoc(caster)
    local real angle
    set angle = AngleBetweenPoints (GetUnitLoc (caster), GetUnitLoc(target))
    set dummy = CreateUnitAtLoc(GetOwningPlayer(caster), 'h002', l, angle)
    call TriggerRegisterTimerEventPeriodic(trig, 0.02)
    call TriggerRegisterUnitEvent(trig, target, EVENT_UNIT_DEATH)
    call SaveTriggerActionHandleBJ( TriggerAddAction( trig, function Frost_Shard_Periodic ), 3, GetHandleIdBJ (trig), udg_Hashtable)
    call SaveUnitHandleBJ( caster, 0, GetHandleIdBJ(trig), udg_Hashtable )
    call SaveUnitHandleBJ( target, 1, GetHandleIdBJ(trig), udg_Hashtable )
    call SaveUnitHandleBJ( dummy, 2, GetHandleIdBJ(trig), udg_Hashtable )
    call RemoveLocation (l)
    set l = null
    set caster = null
    set target = null
    set dummy = null
    set trig = null
endfunction

//===========================================================================
function InitTrig_Frost_Shard takes nothing returns nothing
    set gg_trg_Frost_Shard = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( gg_trg_Frost_Shard, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddCondition( gg_trg_Frost_Shard, Condition( function Trig_Frost_Shard_Conditions ) )
    call TriggerAddAction( gg_trg_Frost_Shard, function Trig_Frost_Shard_Actions )
endfunction

doomlord did you spot the difference in readability in mine code and yours?

 

[Dr. Boom]

.. doomlord did you spot the difference in readability in mine code and yours?

... you forgot the space between the locals and the action script :x
And we know what the GetLocationZ does, we just thought about good using for Locations instead of coordinates =)

 

[edo494]

why should i put space there, its quite obvious the local and call i think(not if you use stone-age we(the original one))

 

[Doomlord]

How do you do the tabbed thing? I tried using the tab key myself but it doesn't seem to help.

 

[Dr. Boom]

That is why I said you should use Jass NewGen Pack with JassHelper =) When you use it you can press tab and you got it.
[1x tab is (here in hive) 4x space - but I don't know if the normal editor work with it]

 

[Doomlord]

Anyway here is my rewritten code. The spell works and there is no red text Thanks to all of you especially Dr. Boom.

function Trig_Frost_Shard_Conditions takes nothing returns boolean
    return ( GetSpellAbilityId() == 'A005' ) 
endfunction

function Frost_Shard_Effect_Periodic takes nothing returns nothing
    local timer t = GetExpiredTimer()
    local unit target = LoadUnitHandle(udg_Hashtable, GetHandleId(t), 0)
    local unit dummy = LoadUnitHandle(udg_Hashtable, GetHandleId(t), 1)
    local unit caster = LoadUnitHandle(udg_Hashtable, GetHandleId(t), 2)
    local real x1 = GetUnitX(dummy)
    local real y1 = GetUnitY(dummy)
    local real x2 = GetUnitX(target)
    local real y2 = GetUnitY(target)
    local real x3
    local real y3
    local real angle
    
    if SquareRoot( (x2-x1) * (x2-x1) + (y2-y1) * (y2-y1) ) < 30 then 
        if GetWidgetLife(target) > 0.405 then
            call UnitDamageTarget(caster, target, 500, true, false, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_FIRE, null)
        endif
        call KillUnit (dummy)
        call PauseTimer (t)
        call DestroyTimer (t)
    else
        set angle = Atan2(y2-y1, x2-x1)
        set x3 = x1 + 36 * Cos(angle)
        set y3 = y1 + 36 * Sin(angle)
        call SetUnitPosition(dummy, x3, y3)
        call SetUnitFacing(dummy, angle*bj_RADTODEG)
    endif
    set target = null
    set caster = null
    set dummy = null
    set t = null
endfunction

function Frost_Shard_Effect takes unit caster, unit u, real x2, real y2 returns nothing
local unit dummy
local timer t = CreateTimer ()
local real angle
local real x1 = GetUnitX(u)
local real y1 = GetUnitY(u)

set angle = Atan2(y1-y2, x1-x2)
set dummy = CreateUnit( GetOwningPlayer(caster), 'h003', x2, y2, angle )
call SaveUnitHandle( udg_Hashtable, GetHandleId(t), 0, u)
call SaveUnitHandle( udg_Hashtable, GetHandleId(t), 1, dummy)
call SaveUnitHandle( udg_Hashtable, GetHandleId(t), 2, caster)
call TimerStart (t, 0.03125, true, function Frost_Shard_Effect_Periodic)
set dummy = null
set caster = null
set u = null
set t = null
endfunction


function Frost_Shard_Periodic takes nothing returns nothing
local trigger trig = GetTriggeringTrigger ()
local unit caster = LoadUnitHandle(udg_Hashtable, GetHandleId(trig), 0)
local unit target = LoadUnitHandle(udg_Hashtable, GetHandleId(trig), 1)
local unit dummy = LoadUnitHandle(udg_Hashtable, GetHandleId(trig), 2)
local real x1 = GetUnitX(dummy)
local real y1 = GetUnitY(dummy)
local real x2 = GetUnitX(target)
local real y2 = GetUnitY(target)
local real x3
local real y3
local real angle
local unit u
local group g = CreateGroup()
local triggeraction tga = LoadTriggerActionHandle (udg_Hashtable, GetHandleId (trig), 3)

if(GetTriggerEventId()==EVENT_UNIT_DEATH)then
if(GetDyingUnit() == target)then
call KillUnit (dummy)
call DestroyGroup(g)
set g = null
set caster = null
set target = null
set dummy = null
call DisableTrigger (trig)
call TriggerRemoveAction (trig, tga)
call DestroyTrigger (trig)
set tga = null
set trig = null
endif
else
if SquareRoot( (x2-x1) * (x2-x1) + (y2-y1) * (y2-y1) ) < 30 then
call KillUnit (dummy)
call UnitDamageTarget(caster, target, 1000, true, false, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_FIRE, null)
if GetWidgetLife(target) < 0.405 then
call GroupEnumUnitsInRange(g, x2, y2, 1000, null)
    loop
        set u = FirstOfGroup(g)
        exitwhen u == null
    if IsUnitEnemy(u, GetOwningPlayer(caster)) == true and GetWidgetLife(u) > 0.405 then        
        call Frost_Shard_Effect (caster, u, x2, y2)
    endif
        call GroupRemoveUnit(g, u)
    endloop
call DestroyGroup(g)
set g = null
endif
set caster = null
set target = null
set dummy = null
call DisableTrigger (trig)
call TriggerRemoveAction (trig, tga)
call DestroyTrigger (trig)
set tga = null
set trig = null
else
set angle = Atan2(y2-y1, x2-x1)
set x3 = x1 + 36 * Cos(angle)
set y3 = y1 + 36 * Sin(angle)
call SetUnitPosition(dummy, x3, y3)
call SetUnitFacing(dummy, angle*bj_RADTODEG)
endif
endif
endfunction

function Trig_Frost_Shard_Actions takes nothing returns nothing
local unit caster = GetTriggerUnit()
local unit target = GetSpellTargetUnit()
local unit dummy
local trigger trig = CreateTrigger ()
local real x = GetUnitX (caster)
local real y = GetUnitY (caster)
local real x1 = GetUnitX (target)
local real y1 = GetUnitY (target)
local real angle

set angle = Atan2(y1-y, x1-x)
set dummy = CreateUnit( GetOwningPlayer(caster), 'h002', x, y, angle*bj_RADTODEG )
call TriggerRegisterTimerEvent(trig, 0.03125, true)
call TriggerRegisterUnitEvent(trig, target, EVENT_UNIT_DEATH)
call SaveTriggerActionHandle(udg_Hashtable , GetHandleId (trig), 3, TriggerAddAction( trig, function Frost_Shard_Periodic ))
call SaveUnitHandle( udg_Hashtable, GetHandleId(trig), 0, caster)
call SaveUnitHandle( udg_Hashtable, GetHandleId(trig), 1, target)
call SaveUnitHandle( udg_Hashtable, GetHandleId(trig), 2, dummy)

set caster = null
set target = null
set dummy = null
set trig = null
endfunction

//===========================================================================
function InitTrig_Frost_Shard takes nothing returns nothing
    local integer i = 0    
    set gg_trg_Frost_Shard = CreateTrigger(  )
    loop
    exitwhen i == 15
    call TriggerRegisterPlayerUnitEvent(gg_trg_Frost_Shard,Player(i),EVENT_PLAYER_UNIT_SPELL_EFFECT,null)
    set i = i + 1
    endloop
    call TriggerAddCondition( gg_trg_Frost_Shard, Condition( function Trig_Frost_Shard_Conditions ) )
    call TriggerAddAction( gg_trg_Frost_Shard, function Trig_Frost_Shard_Actions )
endfunction

And sorry it was not tabbed.

EDIT: I used this native in another function (I don't know if it is BJ or not) and when I used Hive to check the rewritten codes it is marked red.

ModuloInteger

I read somewhere else that not all BJ's are bad but is this ok for me to use this one? Or do I need to find an alternative?

 

[Dr. Boom]

Moin moin =)

Haha, actually I'm the wrong person to discuss, which BJ's are "good" and which are "bad", because my opinion is to kill them all! Some people say like the BJ "TriggerRegisterEvent" you can use the Modulo BJ, but I wouldn't use it so:

if ModuloInteger(12, 5) == 2 then
 
// 12 = dividend, 5 = divisor
// formula: dividend - (dividend / divisor) * divisor
local integer m = 12 - (12 / 5) * 5

// to avoid a nagativ m value; formula: integer-varaible = integer-varable + divisor
if m < 0 then
    set m = m + 5
endif

// to use the varable now:
if m == 2 then
    //actions
endif

As a note: If these BJ's would be bad, then Hive should customize there "jass in-lining script" and change the color to not red. Until they don't do this, I'll fix every BJ, in special red-marked BJ's, I find =)

Edit: As I said bj_DEGTORAD = 57,2957795 - so if you would mind please

Greetings and Peace
Dr. Boom