[JASS] Can't get this spell to run

[ZIR]

Hi can somebody look over this spell it is my first attempt to make a spell in jass so please don't moan about the style of writing. Okay the problem is that the spell only functions well when used first time and it only damages 3 instead of 4 units.

function Trig_Nether_Swap_Conditions takes nothing returns boolean
    return GetSpellAbilityId() == 'AOw2'
endfunction

function Trig_Nether_Swap_Actions takes nothing returns nothing
    local unit c = GetTriggerUnit()
    local real xc = GetUnitX(c)
    local real yc = GetUnitY(c)
    local real array life 
    local integer level = GetHeroLevel(c)
    local real damage = level*500
    local rect spellrect = Rect(xc-1000,yc-1000,xc+1000,yc+1000)
    local unit array dummys 
    local group damagedunits = GetUnitsInRangeOfLocAll(512,GetUnitLoc(c))
    local integer maxdamagedunits = level *4
    local integer loophelp = 0
    set bj_forLoopAIndex = 1
    set bj_forLoopAIndexEnd = 36
    loop
        exitwhen loophelp>=maxdamagedunits
        set damagedunits = GetUnitsInRangeOfLocAll(512,GetUnitLoc(c))
        set dummys[loophelp] = GroupPickRandomUnit(damagedunits)
        set life[loophelp] = GetUnitState(dummys[loophelp],UNIT_STATE_LIFE)
        call SetUnitState (dummys[loophelp],UNIT_STATE_LIFE,life[loophelp]-damage)
        call GroupRemoveUnit (damagedunits,dummys[loophelp])
        set dummys[loophelp] = null
        set loophelp = loophelp+1
    endloop
    set loophelp = 0
    loop
        exitwhen bj_forLoopAIndex > bj_forLoopAIndexEnd
        call CreateNUnitsAtLoc( 1, 'hfoo', Player(0), PolarProjectionBJ(GetRectCenter(spellrect), 50.00, I2R(( GetForLoopIndexA() * ( 360 / 36 ) ))), bj_UNIT_FACING )
        set dummys[bj_forLoopAIndex] = GetLastCreatedUnit()
        set bj_forLoopAIndex = bj_forLoopAIndex + 1
    endloop
    loop
        exitwhen bj_forLoopAIndexEnd < 0
        call KillUnit(dummys[bj_forLoopAIndexEnd])
        set dummys[bj_forLoopAIndexEnd] = null
        set bj_forLoopAIndexEnd = bj_forLoopAIndexEnd - 1
    endloop
    set c = null
endfunction

//===========================================================================
function InitTrig_Nether_Swap takes nothing returns nothing
    set gg_trg_Nether_Swap = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ(gg_trg_Nether_Swap, EVENT_PLAYER_UNIT_SPELL_EFFECT)
    call TriggerAddCondition(gg_trg_Nether_Swap, Condition(function Trig_Nether_Swap_Conditions))
    call TriggerAddAction(gg_trg_Nether_Swap, function Trig_Nether_Swap_Actions)
endfunction

 

[Berb]

What is this supposed to do?

 

[DevilDuke1995]

Firstly WTF IS IS IT SUPPOSED TO DO?

And thats no way to damage sth.

 call SetUnitState (dummys[loophelp],UNIT_STATE_LIFE,life[loophelp]-damage)

Use this:

• call UnitDamageTarget

And u should upload a demo map

 

[alext]

bug hunting

In the first loop of the action function, you call

set damagedunits = GetUnitsInRangeOfLocAll(512,GetUnitLoc(c))

inside the loop. I think this should be located before the beginning of the loop since it initializes the group. I think this should solve one of the bugs, that the spell affects too few units. Since the choice of the unit is random inside the loop, it would pick 4 times a random unit from the same group (because the group is reset inside the loop). So it would only be a matter of luck if the chosen units would be distinct.

The problem of the spell only working once then no more might come from the following anomaly Ive noticed:

In the third loop you call

call KillUnit(dummys[bj_forLoopAIndexEnd])

The values of bj_forLoopAIndexEnd run from 36 to 0, with the last call being

call KillUnit(dummys[ 0 ])

however, when you populate the dummys array in the second loop, you start out at index 1. So essentially, at index 0 there is heaven knows what because it's never set. So when you call KillUnit( ??? ) the result can be considered unpredictable. I've had cases when calling functions on un-initialized array elements resulted in the trigger becoming disabled at run-time.

LEAKS
I know you didn't ask, but the code leaks badly. I've spotted the following leaks:

local rect spellrect = Rect(xc-1000,yc-1000,xc+1000,yc+1000)
this rect is allocated and never removed ( I think the function is DestroyRect )

local group damagedunits = GetUnitsInRangeOfLocAll(512,GetUnitLoc(c))
GetUnitLoc(c) creates a location and sends it to GetUnitsInRangeOfLocAll but nobody deletes the location after it has been used

set damagedunits = GetUnitsInRangeOfLocAll(512,GetUnitLoc(c))
Inside the loop, the same leak will occur as above

damagedunits is never deleted with a RemoveGroup(). I can't be 100% sure on this without extensive testing, but I think groups don't get un-allocated from memory when all units inside them have been removed.

call CreateNUnitsAtLoc( 1, 'hfoo', Player(0), PolarProjectionBJ(GetRectCenter(spellrect), 50.00, I2R(( GetForLoopIndexA() * ( 360 / 36 ) ))), bj_UNIT_FACING )
GetRectCenter() creates a location and sends it to CreateNUnitsAtLoc but the location is never deleted with RemoveLocation()

Hope this helps

 

[ZIR]

It should damage level of hero * 4 random enemies in the are around the caster.
It functions the first time well but the second and every time after the second not. Here is a demo map and I didn't done the leaks I want at first getting it running. The paladan has the abilitie which will starts this trigger it's thunderclap.