[vJASS] Small error in a trigger

[Erkki2]

Hello. I've been trying to make a spell, which throws projectiles (dummy units) into air around the caster, and damages enemies when they land. So far, the spell works fine, there's just one thing that has to be fixed: on some rare cases, a dummy unit is created and it isn't moved and therefore it isn't removed either. This code isn't even nearly optimal, but I can't find any clear errors. Here's the trigger:

trigger

scope SeedFlare initializer init

private struct Data
    unit u
    real damage
    integer executions
endstruct

private struct Dummy
    unit u
    real dist
    real d
    real dx
    real dy
    unit c
    real damage
endstruct

globals
    private Data array DataList
    private Dummy array DummyList
    private integer Count = 0
    private integer DummyCount = 0
    private timer T = CreateTimer()
    private real Area = 1000
    private real Height = 1500
    private real Speed = 1.5
endglobals

private function True takes nothing returns boolean
    return true
endfunction

private function Loop takes nothing returns nothing
    local Data dat
    local Dummy dum
    local integer i = 0
    local location p
    local group g
    local unit u
    local real angle
    local real dist
    local real x
    local real y
    
    loop
        exitwhen i >= Count
        set dat = DataList[i]
        set angle = GetRandomReal(0, 2*bj_PI)
        set dum = Dummy.create()
        set dum.dist = GetRandomReal(1, Area)
        set dum.u = CreateUnit(GetOwningPlayer(dat.u), 'n00Z', GetUnitX(dat.u), GetUnitY(dat.u), angle)
        set dum.d = 0
        set dum.damage = dat.damage
        set dum.c = dat.u
        set dum.dx = Cos(angle) * dum.dist * Speed * 0.01
        set dum.dy = Sin(angle) * dum.dist * Speed * 0.01
        set DummyList[DummyCount] = dum
        set DummyCount = DummyCount + 1
        
        set dat.executions = dat.executions - 1
        if dat.executions == 0 then
            call dat.destroy()
            set Count = Count - 1
            set DataList[i] = DataList[Count]
        endif
        set i = i + 1
    endloop
    
    set i = 0
    
    loop
        exitwhen i >= DummyCount
        set dum = DummyList[i]
        call SetUnitX(dum.u, GetUnitX(dum.u) + dum.dx)
        call SetUnitY(dum.u, GetUnitY(dum.u) + dum.dy)
        set dum.d = dum.d + SquareRoot(dum.dx*dum.dx + dum.dy*dum.dy)
        call SetUnitFlyHeight(dum.u, Parabola(dum.dist, Height, dum.d), 0)
        
        set x = GetUnitX(dum.u)
        set y = GetUnitY(dum.u)
        
        if dum.d > dum.dist then
        
            set p = Location(x, y)
            set g = GetUnitsInRangeOfLocMatching(100, p, Filter(function True))
            call DestroyEffect(AddSpecialEffect("Abilities\\Spells\\NightElf\\ManaBurn\\ManaBurnTarget.mdl", x, y))
            loop
                set u = FirstOfGroup(g)
                exitwhen u == null
                if IsPlayerEnemy(GetOwningPlayer(u), GetOwningPlayer(dum.c)) and not IsUnitType(u, UNIT_TYPE_STRUCTURE) then
                    call UnitDamageTarget(dum.c, u, dum.damage,true, false, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_MAGIC, WEAPON_TYPE_WHOKNOWS)
                endif  
                call GroupRemoveUnit(g, u)
            endloop
            call RemoveLocation(p)
            call DestroyGroup(g)
            
            call RemoveUnit(dum.u)
            call dum.destroy()
            
            set DummyCount = DummyCount - 1
            set DummyList[i] = DummyList[DummyCount]
        endif
        set i = i + 1
    endloop
    
    if DummyCount == 0 then
        call PauseTimer (T)
    endif
        
endfunction

private function Actions takes nothing returns nothing
    local Data dat = Data.create()
    set DataList[Count] = dat
    set dat.u = GetTriggerUnit() 
    set dat.executions = 50
    set dat.damage = (30 + GetHeroInt(dat.u,true)) * (1 + 0.2 * GetUnitAbilityLevel(dat.u, 'A027'))
    set Count = Count + 1
    if Count == 1 then
        call TimerStart(T, 0.04, true, function Loop)
    endif
endfunction

private function Conditions takes nothing returns boolean
    return GetSpellAbilityId() == 'A027'
endfunction

private function init takes nothing returns nothing
    local trigger t = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_SPELL_EFFECT)
    call TriggerAddCondition(t, Condition(function Conditions))
    call TriggerAddAction(t, function Actions)
endfunction

endscope

I believe that the mistake is somewhere in the first loop of the Loop function. I'd be grateful if anyone could spot the mistake. Any other improvement ideas are also welcome.

 

[Maker]

All the instances take the same time to ececute, right? So the order of expiration is the same as order of creation.

I think the error occurs when you deindex.

Example, you have indexes
[0], [1], [2] where [0] is created first and will expire first.
You subtract 1 from count so the system thinks there are only indexes [0] and [1] to loop through.
Then you set = [count], which means what was in [1] is set into [0]. [2] will remain in [2]. The next time the loop runs, it loops through [0] and [1], which are the same instance and [2] won't be looped through since count is 1.

The solution is to first set = [count] and only then set count = count - 1

Use GroupEnumUnitsInRange instead of the bj, use coordinates instead of locations.

 

[Doomlord]

In addition to Maker's useful advices, I would like to elaborate a bit more on the GroupEnumUnitsInRange he mentioned.

Instead of:

private function True takes nothing returns boolean
    return true
endfunction

            set g = GetUnitsInRangeOfLocMatching(100, p, Filter(function True))
            call DestroyEffect(AddSpecialEffect("Abilities\\Spells\\NightElf\\ManaBurn\\ManaBurnTarget.mdl", x, y))
            loop
                set u = FirstOfGroup(g)
                exitwhen u == null
                if IsPlayerEnemy(GetOwningPlayer(u), GetOwningPlayer(dum.c)) and not IsUnitType(u, UNIT_TYPE_STRUCTURE) then
                    call UnitDamageTarget(dum.c, u, dum.damage,true, false, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_MAGIC, WEAPON_TYPE_WHOKNOWS)
                endif  
                call GroupRemoveUnit(g, u)
            endloop
            call RemoveLocation(p)
            call DestroyGroup(g)

It could be:

            call DestroyEffect(AddSpecialEffect("Abilities\\Spells\\NightElf\\ManaBurn\\ManaBurnTarget.mdl", x, y))
            set g = CreateGroup()
            call GroupEnumUnitsInRange(g, x, y, 100, null)
            loop
                set u = FirstOfGroup(g)
                exitwhen u == null
                if IsUnitEnemy(u, GetOwningPlayer(dum.c)) and not IsUnitType(u, UNIT_TYPE_STRUCTURE) then
                    call UnitDamageTarget(dum.c, u, dum.damage, false, false, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_MAGIC, WEAPON_TYPE_WHOKNOWS)
                endif  
                call GroupRemoveUnit(g, u)
            endloop
            call DestroyGroup(g)

• Using IsUnitEnemy can help you cut down on an unnecessary call for GetOwningPlayer(u).
• The two booleans for the UnitDamageTarget should be false.

 

[Erkki2]

Thanks to both, I didn't know about those functions. They look quite useful.

About the indexing, I tried that and it didn't work. I'm not 100% sure if I got right what you meant.

For example, if there are indexes [0], [1], and [2], then count is 3. If [0] is deleted, count will be set to 2, and the struct at [2] wil be moved to index [0]. That should be ok.

I noticed one other thing that was wrong. I should only increase i in else statement, because i shouldn't be increased if a struct is destroyed and replaced with another. I changed that but still the spell doesn't work correctly.