[vJASS] [Solved] Filter units in range, then damage them doesn't work, whats the optimal way of doing this?

[Regno]

scope RetsuBoom
    private struct RetsuBoom
        static timer t = CreateTimer()
        static unit caster
        static effect prep
        static effect flash
        static effect periodic
        static sound RetsuW1Voiceline
        static sound RetsuW2Voiceline
        static method filterGroupUnit takes nothing returns boolean
            return IsUnitEnemy(GetFilterUnit(), GetOwningPlayer(caster))
        endmethod
        static method damageTargets takes nothing returns nothing
            call DisplayTimedTextToPlayer(Player(0), 0, 0, 1, "Damaging: "+GetUnitName(GetEnumUnit()))
            call UnitDamageTarget(caster, GetEnumUnit(), 200, false, false, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_MAGIC, null)
        endmethod
        static method removeEffect takes nothing returns nothing
            call DestroyEffect(prep)
            call DestroyEffect(flash)
        endmethod
        static method onFinish takes nothing returns nothing
            call PauseUnitEx(caster, false)
            call SetUnitInvulnerable(caster, false)
            call TimerStart(t, 3, false, function thistype.removeEffect)
        endmethod
        static method onCast3 takes nothing returns nothing
            local group targets = null
            call SetUnitAnimationByIndex(caster, 5)
            call QueueUnitAnimation(caster, "stand")
          
            call DisplayTimedTextToPlayer(Player(0), 0, 0, 1, "I ran")
            call DestroyEffect(AddSpecialEffect("Effect Blue Ranbu.mdx", GetUnitX(caster), GetUnitY(caster)))
          
            set RetsuW2Voiceline = CreateSound("Sounds\\RetsuW2Voice.mp3", false, false, false, 12700, 12700, "")
            call StartSoundEx(RetsuW2Voiceline, true)
            call PauseUnitEx(caster, true)
            call SetUnitInvulnerable(caster, true)
            // v This is the part thats not working v
            call GroupEnumUnitsInRange(targets, GetUnitX(caster), GetUnitY(caster), 600, Filter(function thistype.filterGroupUnit))
            call ForGroup(targets, function thistype.damageTargets)
            // ^ This is the part thats not working ^
            call TimerStart(t, 0.5, false, function thistype.onFinish)
        endmethod
        static method onCast2 takes nothing returns nothing
          
            set flash = AddSpecialEffectTarget("blinknew2.mdx", caster, "overhead")
            call BlzSetSpecialEffectZ(flash, GetUnitZ(caster)+300)
            call BlzSetSpecialEffectTimeScale(flash, 1.5)
            call TimerStart(t, 1.5, false, function thistype.onCast3)
        endmethod
        static method onCast1 takes nothing returns nothing
            set caster = GetTriggerUnit()
            set RetsuW1Voiceline = CreateSound("Sounds\\RetsuW1Voice.mp3", false, false, false, 12700, 12700, "")
            call StartSoundEx(RetsuW1Voiceline, true)
            set prep = AddSpecialEffectTarget("RetsuW_Prep.mdx", caster, "origin")
            call BlzSetSpecialEffectColor(prep, 5, 5, 255)
            call BlzSetSpecialEffectTimeScale(prep, 0.5)
            call BlzSetSpecialEffectZ(prep, GetUnitZ(caster)+50)
            call TimerStart(t, 0.5, false, function thistype.onCast2)
        endmethod
        static method onInit takes nothing returns nothing
            call RegisterSpellEffectEvent('A000', function thistype.onCast1)
        endmethod
    endstruct
endscope

The part I'm struggling with is in method "onCast3", specifically GroupEnumUnitsInRange what is the reason its not working? I tried printing the units name, but nothing shows up. I'm not entirely sure how to fix it

 

[Regno]

Maybe there is a ForGroup or Group library-utility that I could import which makes this group handling easier?

 

[Uncle]

A group is an object that needs to be initialized:

local group targets = CreateGroup()

Your targets variable acts as a pointer which can track a single Unit Group object or nothing at all. It is currently pointing to "null" in your original code.

This means that you're passing "nothing" to all of the functions that use targets throughout your code. This will cause obvious problems, you can't add units to nothing so the function is going to fail.

Also, since it's an object, you'll want to Destroy it when you're done with it:

// end of code
call DestroyGroup(targets)
set targets = null

This applies to Forces (player groups), Locations (points), Triggers, and Timers as well. You'll find similar API for them:

local force f = CreateForce()
local location l = Location(0, 0)
local trigger trig = CreateTrigger()
local timer t = CreateTimer()

// destroy the objects themselves
call DestroyForce(f)
call RemoveLocation(l)
call DestroyTrigger(trig)
call DestroyTimer(t)

// garbage collect the local variables that were pointing to these objects
set f = null
set l = null
set trig = null
set t = null

There's probably a couple more I'm forgetting.

 

[Pyrogasm]

Uncle is correct. For your reference, the only other way to loop over the units in a group is this:

set g = CreateGroup() //not needed after the first time if the group is being reused
call AddUnitsToGroupHoweverYouDoIt(g)
loop
    set u = FirstOfGroup(g)
    exitwhen u == null
    call GroupRemoveUnit(g,u)

    //do stuff...
endloop

This ultimately clears the group of units by the end of it, so you can just keep reusing the group instead of destroying it and reassigning the variable to a new empty group every time this occurs. That may be a benefit you care about. The real tangible benefit to the method is that it doesn't do the callback in another function, thus allowing you access to any local variables from the function where the group was formed.

 

[Regno]

A group is an object that needs to be initialized:

local group targets = CreateGroup()

Your targets variable acts as a pointer which can track a single Unit Group object or nothing at all. It is currently pointing to "null" in your original code.

This means that you're passing "nothing" to all of the functions that use targets throughout your code. This will cause obvious problems, you can't add units to nothing so the function is going to fail.

Also, since it's an object, you'll want to Destroy it when you're done with it:

// end of code
call DestroyGroup(targets)
set targets = null

This applies to Forces (player groups), Locations (points), Triggers, and Timers as well. You'll find similar API for them:

local force f = CreateForce()
local location l = Location(0, 0)
local trigger trig = CreateTrigger()
local timer t = CreateTimer()

// destroy the objects themselves
call DestroyForce(f)
call RemoveLocation(l)
call DestroyTrigger(trig)
call DestroyTimer(t)

// garbage collect the local variables that were pointing to these objects
set f = null
set l = null
set trig = null
set t = null

There's probably a couple more I'm forgetting.

Uncle is correct. For your reference, the only other way to loop over the units in a group is this:

set g = CreateGroup() //not needed after the first time if the group is being reused
call AddUnitsToGroupHoweverYouDoIt(g)
loop
    set u = FirstOfGroup(g)
    exitwhen u == null
    call GroupRemoveUnit(g,u)

    //do stuff...
endloop

This ultimately clears the group of units by the end of it, so you can just keep reusing the group instead of destroying it and reassigning the variable to a new empty group every time this occurs. That may be a benefit you care about. The real tangible benefit to the method is that it doesn't do the callback in another function, thus allowing you access to any local variables from the function where the group was formed.

Thank you both <3 I completely forgot the group had to be initialized. As for effects, I have to remove them too right? what if I create an effect, and it plays death animation and no longer shows, its still there and I have to remove it right?

I had an issue where removing an effect played its death animation again, so I resorted to

call DestroyEffect(AddSpecialEffect("Effect Blue Ranbu.mdx", GetUnitX(caster), GetUnitY(caster)))

Anyway, many thanks <3

 

[Pyrogasm]

Destroying an effect always plays its death animation if it has one, and if it doesn't have one it will linger in the animation it's in for some time. There's no way to directly avoid this, but you can get around this potential annoyance by moving the effect before you destroy it. I wrote a simple library to do that:

library RemoveEffect
//Destroying an effect always plays its death animation and any associated sounds.
//This library fixes it in a simple way: move the effect to a location where nobody can see or hear it.

//By Pyrogasm, additional help by Bribe
//v1.1

globals
    private real SAFE_X = -3900.
    private real SAFE_Y =  3900.
    private real SAFE_Z = -1000.

    private real TIMESCALE = 10. //doesn't really matter what this is but we set it to > 0 so the animations actually finish
endglobals

function RemoveEffect takes effect e returns nothing
    call BlzSetSpecialEffectAlpha(e, 255)
    call BlzSetSpecialEffectZ(e, SAFE_Z)
    call BlzSetSpecialEffectPosition(e, SAFE_X, SAFE_Y, SAFE_Z)
    call BlzSetSpecialEffectTimeScale(e, TIMESCALE)
    call DestroyEffect(e)
endfunction
endlibrary

 

[Regno]

Destroying an effect always plays its death animation if it has one, and if it doesn't have one it will linger in the animation it's in for some time. There's no way to directly avoid this, but you can get around this potential annoyance by moving the effect before you destroy it. I wrote a simple library to do that:

library RemoveEffect
//Destroying an effect always plays its death animation and any associated sounds.
//This library fixes it in a simple way: move the effect to a location where nobody can see or hear it.

//By Pyrogasm, additional help by Bribe
//v1.1

globals
    private real SAFE_X = -3900.
    private real SAFE_Y =  3900.
    private real SAFE_Z = -1000.

    private real TIMESCALE = 10. //doesn't really matter what this is but we set it to > 0 so the animations actually finish
endglobals

function RemoveEffect takes effect e returns nothing
    call BlzSetSpecialEffectAlpha(e, 255)
    call BlzSetSpecialEffectZ(e, SAFE_Z)
    call BlzSetSpecialEffectPosition(e, SAFE_X, SAFE_Y, SAFE_Z)
    call BlzSetSpecialEffectTimeScale(e, TIMESCALE)
    call DestroyEffect(e)
endfunction
endlibrary

Thank you so much, this library will make it easier to handle effect removals

@Uncle @Pyrogasm CreateGroup() fixed it, thanks a lot