[JASS] Managing groups in damage over time spell

[Woodenplank]

So for my latest dip into the WC3 Editor I finally decided to learn JASS. It's been going well, except that I struggle with carrying groups through time(rs).
I think I can best explain my issue by explaining the spell I'm making.

Hellfury Strike - Slams a target foe in melee range, dealing damage. If 3 or more enemies are within range of the main target this ability deals double damage, but is distributed evenly among all targets.

Now this part of the trigger ability works! However, the ability may be further modified by the so-called "Arms of Astaroth" (or AoA for short) upgrade. This should make the ability do an additional 25% lingering damage over 5 seconds.
This is the part I'm struggling with.

function Trig_HellfuryStrike_Conditions takes nothing returns boolean
    return (GetSpellAbilityId() == 'A00I')
endfunction


function TargetCondition takes nothing returns boolean
    if IsUnitType(GetFilterUnit(), UNIT_TYPE_STRUCTURE) then
        return false
    elseif IsUnitType(GetFilterUnit(), UNIT_TYPE_ETHEREAL) then
        return false
    elseif IsUnitType(GetFilterUnit(), UNIT_TYPE_FLYING) then
        return false
    elseif IsUnitDeadBJ(GetFilterUnit()) then
        return false
    elseif not IsUnitEnemy(GetFilterUnit(), GetOwningPlayer(GetTriggerUnit())) then
        return false
    else
        return true
    endif
endfunction

function AoA_dot takes nothing returns nothing
    local timer t = GetExpiredTimer()
    local integer id = GetHandleId(t)

    local unit pu
    local unit cast = LoadUnitHandle(udg_Spell_Table, id, 1)
    local real damage = LoadReal(udg_Spell_Table, id, 2)
    local group ug = LoadGroupHandle(udg_Spell_Table, id, 3)
    local integer count = LoadInteger(udg_Spell_Table, id, 4) + 1
    call SaveInteger(udg_Spell_Table, id, 4, count)

    if (count < 5) then
        loop
            set pu = FirstOfGroup(ug)
            exitwhen (pu == null)
            call UnitDamageTarget(cast, pu, damage, true, false, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_NORMAL, null)
            call DestroyEffect(AddSpecialEffect("Abilities\\Spells\\Human\\Feedback\\SpellBreakerAttack.mdl", GetUnitX(pu), GetUnitY(pu)))
            call GroupRemoveUnit(ug, pu)
        endloop
        call TimerStart(t, 1.0, false, function AoA_dot)
    else
        // clean memory
        call FlushChildHashtable(udg_Spell_Table, id)
        call PauseTimer(t)
        call DestroyTimer(t)
    endif

    call DestroyGroup(ug)
    set ug = null
    set pu = null
    set cast = null
    set t = null
endfunction


function Trig_HellfuryStrike_Actions takes nothing returns nothing
    local unit cast = GetTriggerUnit()
    local unit targ = GetSpellTargetUnit()
   
    local real x = GetSpellTargetX()
    local real y = GetSpellTargetY()

    local real damage = 100
    local real aoe = 200
    local group ug = CreateGroup()
    local unit pu
    local integer targetcount = 1
    local filterfunc targetfilter = Filter(function TargetCondition)
   
    /*     If ARMS OF ASTAROTH is not active, these are not needed
        but must be declared here for locality.
    */
        local timer t = CreateTimer()
        local integer id = GetHandleId(t)
        local boolean AoAactive = (udg_HF_activeHellion[GetUnitUserData(cast)] == 1)  // this looks wierd, I reckon, but it DOES work like I want it to

    call GroupEnumUnitsInRange(ug, x, y, aoe, targetfilter)
    if AoAactive then
        call SaveGroupHandle(udg_Spell_Table, id, 3, ug)
    endif       

    set targetcount = CountUnitsInGroup(ug)
    if (targetcount >= 4) then
        // AoE spread damage
        set damage = (damage*2)/targetcount
        loop
            set pu = FirstOfGroup(ug)
            exitwhen (pu == null)
            call UnitDamageTarget(cast, pu, damage, true, false, ATTACK_TYPE_HERO, DAMAGE_TYPE_ENHANCED, null)    // Hero damage, ignore armor
            call DestroyEffect(AddSpecialEffect("Abilities\\Weapons\\DemolisherFireMissile\\DemolisherFireMissile.mdl", GetUnitX(pu) , GetUnitY(pu)))
            call GroupRemoveUnit(ug, pu)
        endloop
    else
        // Single target damage
        call UnitDamageTarget(cast, targ, damage, true, false, ATTACK_TYPE_HERO, DAMAGE_TYPE_ENHANCED, null)
        call DestroyEffect(AddSpecialEffect("Abilities\\Weapons\\DemolisherFireMissile\\DemolisherFireMissile.mdl", GetUnitX(targ) , GetUnitY(targ)))
    endif

    /*
        Save additional info for ARMS OF ASTAROTH dot damage
    */
    if AoAactive then
        call SaveUnitHandle(udg_Spell_Table, id, 1, cast)
        call SaveReal(udg_Spell_Table, id, 2, damage/20)
        call SaveInteger(udg_Spell_Table, id, 4, 0)
        call TimerStart(t, 1.0, false, function AoA_dot)
    endif

    call DestroyFilter(targetfilter)
    call DestroyGroup(ug)
    set ug = null
    set t = null
    set cast = null
    set targ = null
    set pu = null
   
endfunction

The trouble is that, however I try, the group doesn't seem to get properly passed to the AoA_dot function (after the timer expiration).
I've also tried using a global variable, some udg_HeFu_group; adding units to that and saving its handle, but it amounts to the same thing - which is to say; nothing.

BJDebugMsgs confirm that that the AoAactive boolean IS TRUE! And the counter properly ticks up each of the 5 expected times.
However, the number of units getting passed to the AoA_dot function is always zero, regardless of me saving/loading the group handle of a local or global variable.


Again, I'm new to JASS. If there's a simpler and/or better way of passing groups around to delayed actions - or if there's anything else inherently offputting about my trigger - then I'm happy to take advice!

 

[Nichilus]

Although in "Trig_HellfuryStrike_Actions" you do

if AoAactive then
    call SaveGroupHandle(udg_Spell_Table, id, 3, ug)
endif

at the end of that function you also do

call DestroyGroup(ug)

The solution is to destroy group only if AoAactive is not true.

 

[Woodenplank]

Although in "Trig_HellfuryStrike_Actions" you do

if AoAactive then
    call SaveGroupHandle(udg_Spell_Table, id, 3, ug)
endif

at the end of that function you also do

call DestroyGroup(ug)

The solution is to destroy group only if AoAactive is not true.

Thanks for the reply, but I thought about that, but even when I did

EnumUnitsInRange(blablabla, udg_UniqueGlobalGroup)

and

SaveGroupHandle(blablabla, udg_UniqueGlobalGroup)

It still didn't work; although the global group was never destroyed.

 

[Nichilus]

unit groups are stored by reference.

local group ug = CreateGroup()

creates a unit group and assigns its reference to variable 'ug'.

call SaveGroupHandle(udg_Spell_Table, id, 3, ug)

This takes the reference from 'ug' and stores it in Spell_Table hashtable.
It does not create a copy of the group, instead both Spell_Table and ug variable reference same object (same unit group).

call DestroyGroup(ug)

This does not clear 'ug' variable. It destroys the object that the 'ug' variable is referencing.
That affects Spell_Table as well, as it referenced the same object.

Simply put, it is something like this:

// after: local group ug = CreateGroup()
ug --> *unit_group object*

// after: call SaveGroupHandle(udg_Spell_Table, id, 3, ug)
ug --> *unit_group object* <-- udg_Spell_Table

// after: call DestroyGroup(ug)
ug --> *null* <-- udg_Spell_Table

It still didn't work; although the global group was never destroyed.

Can't really say what was going on as you did not post that script.

 

[Woodenplank]

Can't really say what was going on as you did not post that script.

Ah sorry, I thought I had covered it in the first post, but I can see how I was confusing the point.

The attempt with a global variable, udg_HeFu_group, did not work either.
Curiously, it appears to do ONE instance of damage on ONE unit the first time it's used, then stops functioning. As though the global variable ceases to function/exist after the first step of looping through it.
I'm certain I'm not destroying this one, and I even tried commenting out the "ug=null" statement, but no change.

// Other bits of code as before...

function AoA_dot takes nothing returns nothing
    local timer t = GetExpiredTimer()
    local integer id = GetHandleId(t)

    local unit pu
    local unit cast = LoadUnitHandle(udg_Spell_Table, id, 1)
    local real damage = LoadReal(udg_Spell_Table, id, 2)
    local group ug = LoadGroupHandle(udg_Spell_Table, id, 3)
    local integer count = LoadInteger(udg_Spell_Table, id, 4) + 1
    call SaveInteger(udg_Spell_Table, id, 4, count)

    if (count < 5) then
        loop
            set pu = FirstOfGroup(ug)
            exitwhen (pu == null)
            call UnitDamageTarget(cast, pu, damage, true, false, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_NORMAL, null)
            call DestroyEffect(AddSpecialEffect("Abilities\\Spells\\Human\\Feedback\\SpellBreakerAttack.mdl", GetUnitX(pu), GetUnitY(pu)))
            call GroupRemoveUnit(ug, pu)
        endloop
        call TimerStart(t, 1.0, false, function AoA_dot)
    else
        // clean memory
        call FlushChildHashtable(udg_Spell_Table, id)
        call PauseTimer(t)
        call DestroyTimer(t)
    endif

    set ug = null
    set pu = null
    set cast = null
    set t = null
endfunction


function Trig_HellfuryStrike_Actions takes nothing returns nothing
    local unit cast = GetTriggerUnit()
    local unit targ = GetSpellTargetUnit()

    local real damage = 100
    local real x = GetSpellTargetX()
    local real y = GetSpellTargetY()

    local real aoe = 200
    local group ug = CreateGroup()
    local unit pu
    local integer targetcount = 1
    local filterfunc targetfilter = Filter(function TargetCondition)
    
    /*     If ARMS OF ASTAROTH is not active, these are not needed 
        but must be declared here for locality.
    */
        local timer t = CreateTimer()
        local integer id = GetHandleId(t)
        local boolean AoAactive = (udg_HF_activeHellion[GetUnitUserData(cast)] == 1)

    call GroupEnumUnitsInRange(ug, x, y, aoe, targetfilter)
    if AoAactive then
        call GroupEnumUnitsInRange(udg_HeFu_group, x, y, aoe, targetfilter)
        call SaveGroupHandle(udg_Spell_Table, id, 3, udg_HeFu_group)
    endif        

    set targetcount = CountUnitsInGroup(ug)
    if (targetcount >= 4) then
        // AoE spread damage
        set damage = (damage*2)/targetcount
        loop
            set pu = FirstOfGroup(ug)
            exitwhen (pu == null)
            call UnitDamageTarget(cast, pu, damage, true, false, ATTACK_TYPE_HERO, DAMAGE_TYPE_ENHANCED, null)    // Hero damage, ignore armor
            call DestroyEffect(AddSpecialEffect("Abilities\\Weapons\\DemolisherFireMissile\\DemolisherFireMissile.mdl", GetUnitX(pu) , GetUnitY(pu)))
            call GroupRemoveUnit(ug, pu)
        endloop
    else
        // Single target damage
        call UnitDamageTarget(cast, targ, damage, true, false, ATTACK_TYPE_HERO, DAMAGE_TYPE_ENHANCED, null)
        call DestroyEffect(AddSpecialEffect("Abilities\\Weapons\\DemolisherFireMissile\\DemolisherFireMissile.mdl", GetUnitX(targ) , GetUnitY(targ)))
    endif

    if AoAactive then
        call SaveUnitHandle(udg_Spell_Table, id, 1, cast)
        call SaveReal(udg_Spell_Table, id, 2, damage/20)
        call SaveInteger(udg_Spell_Table, id, 4, 0)
        call TimerStart(t, 1.0, false, function AoA_dot)
    endif

    call DestroyGroup(ug)
    call DestroyFilter(targetfilter)
    set ug = null
    set t = null
    set cast = null
    set targ = null
    set pu = null
endfunction

I see now I should've posted this initially. Regardless; I tried it before and it didn't work either.

 

[Uncle]

I realize this isn't helpful to solving your current issue, but have you thought about learning Lua instead? Unless you're stuck on an older version, Jass is an objectively worse language that can't be used outside of Warcraft 3. You could literally waste 100's of hours of your time if you invest heavily into developing maps using it.

I'd say if you're beginning your programming journey, choose the modern option that was added for good reason: Because Jass sucks.

Of course you lose access to some existing Jass resources, but Lua alternatives exist - and they can be made faster and better.

I personally develop my maps in C# which then compiles to Lua: Home

C# is used in Unity and Godot, two of the most popular game engines. So you could take your Wc3 map code, and the knowledge you've accumulated throughout development, and easily transfer those over to a standalone game.

 

[Nichilus]

Ah sorry, I thought I had covered it in the first post, but I can see how I was confusing the point.

There does not seem to be anything wrong with the script you posted, but the following comment is interesting:

Curiously, it appears to do ONE instance of damage on ONE unit the first time it's used, then stops functioning. As though the global variable ceases to function/exist after the first step of looping through it.

This points to you destroying that unit group somewhere. Global unit group variables are initialized with an instance of unit group. Hence why it works first time. But once something else destroys the group, then it stops working and nothing else seems to create and assign unit group to udg_HeFu_group

 

[Woodenplank]

I realize this isn't helpful to solving your current issue, but have you thought about learning Lua instead? Unless you're stuck on an older version, Jass is an objectively worse language that can't be used outside of Warcraft 3. You could literally waste 100's of hours of your time if you invest heavily into developing maps using it.

I'd say if you're beginning your programming journey, choose the modern option that was added for good reason: Because Jass sucks.

Of course you lose access to some existing Jass resources, but Lua alternatives exist - and they can be made faster and better.

I personally develop my maps in C# which then compiles to Lua: Home

C# is used in Unity and Godot, two of the most popular game engines. So you could take your Wc3 map code, and the knowledge you've accumulated throughout development, and easily transfer those over to a standalone game.

I heard that Lua now integrates to WC3 code, but frankly I haven't been keeping up. I only semi-recently got back into WC3 from years of hiatus. I've noticed the new natives in GUI, but that's about it.
Can you points me towards a good place to start learning Lua - WC3 oriented that is? Perhaps I should just search here on Hive?

p.s. Thanks for the headsup! There's a real risk of suck-cost fallacy here, if I'd spent too much time learning and getting used to JASS

p.s.s. I'm more of a C++ guy than C# (Read: I've never tried C#)

---

There does not seem to be anything wrong with the script you posted, but the following comment is interesting:

This points to you destroying that unit group somewhere. Global unit group variables are initialized with an instance of unit group. Hence why it works first time. But once something else destroys the group, then it stops working and nothing else seems to create and assign unit group to udg_HeFu_group

So after some fiddling around, I've found the issue.

I copied the "damaging loop" style from another trigger - because that's the only way I've "learnt" how to do it.

        loop
            set pu = FirstOfGroup(ug)
            exitwhen (pu == null)
            call UnitDamageTarget(cast, pu, damage, true, false, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_NORMAL, null)
            call DestroyEffect(AddSpecialEffect("Abilities\\Spells\\Human\\Feedback\\SpellBreakerAttack.mdl", GetUnitX(pu), GetUnitY(pu)))
            call GroupRemoveUnit(ug, pu)
        endloop

And indeed, it works perfectly fine for the instant damage, or repeating target-checking.
However, in the case of the DoT, the target group has to stay intact through each iteration. Removing units prevents further interaction (which is why I only saw ONE dot tick).

I changed it to the following (which I haphazardly put together by looking at how GUI converts to jass text):

function GroupDamage takes nothing returns nothing
    call UnitDamageTarget(GetTriggerUnit(), GetEnumUnit(), udg_HeFu_dmg, true, false, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_NORMAL, null)
    call DestroyEffect(AddSpecialEffect("Abilities\\Spells\\Human\\Feedback\\SpellBreakerAttack.mdl", GetUnitX(GetEnumUnit()), GetUnitY(GetEnumUnit())))
endfunction

function AoA_dot takes nothing returns nothing
    local timer t = GetExpiredTimer()
    local integer id = GetHandleId(t)

    local unit pu
    local unit cast = LoadUnitHandle(udg_Spell_Table, id, 1)
    local real damage = LoadReal(udg_Spell_Table, id, 2)
    local group ug = LoadGroupHandle(udg_Spell_Table, id, 3)
    local integer count = LoadInteger(udg_Spell_Table, id, 4) + 1
    call SaveInteger(udg_Spell_Table, id, 4, count)

    if (count < 5) then
        call ForGroupBJ( ug, function GroupDamage)
        call TimerStart(t, 1.0, false, function AoA_dot)
    else
        // clean memory
        call DestroyGroup(ug)
        call FlushChildHashtable(udg_Spell_Table, id)
        call PauseTimer(t)
        call DestroyTimer(t)
    endif

    set ug = null
    set pu = null
    set cast = null
    set t = null
endfunction

And it works.
But with issues, of course. Like this isn't MUI, I don't think? The group handle would only ever refer to that one global variable, HeFu_ug, so if multiple casters were using the ability, all the DoT interactions would get mixed up.
Secondly; since it seems you can't pass arguments to the function call of ForGroupBJ(), I have to use GetTriggerUnit(). It's fine here, but wouldn't work in other cases.

... Hence me asking for general advice!
My way of "damaging unit group" clearly ruined the functionality of this trigger.

 

[Nichilus]

Is GetTriggerUnit() really working? Try printing that unit's name. My guess would be that it does not return anything, since AoA_dot is started by a timer, not by a unit event.

Anyway, what you could do is:

• don't use unit group from a global variable. Just use the local "ug" you create in Trig_HellfuryStrike_Actions and do not destroy it in that function, only save it into hashtable.
• In AoA_dot you already destroy the loaded unit group... which would be in this case the one from global variable? If so, that could be an issue, but if you use the locally created group, it should be OK and solve your MUI issues (this way, each caster has his own locally created unit group).
• In udg_Spell_Table you could store the caster under ID of the target (in other words, reverse what you have). Since in GroupDamage you have access to the target, you can get its id and use it to load the caster. This can avoid the issue with GetTriggerUnit