[Solved] [Lua] Struggling with unit filters and getting thoroughly understanding GroupEnumUnitsInRange.

[Cobalt]

Hi!

I've decided to get into Lua scripting, and I want to make a (very) simple spell just to become more familiar with it. However, I'm struggling quite a bit with getting unit groups to work. I have this code:

EXPLOSION_ID = 1093677104
EXPLOSION_DAMAGE_EFFECT = 'Abilities\\Spells\\Human\\Thunderclap\\ThunderClapCaster.mdl'

EXPLOSION_DAMAGE = 100.0
EXPLOSION_DAMAGE_MOD = 0.75

EXPLOSION_RADIUS = 450.0

function InitTrig_explosion()
    local gg_trg_explosion  = CreateTrigger()
    TriggerRegisterAnyUnitEventBJ(gg_trg_explosion, EVENT_PLAYER_UNIT_SPELL_EFFECT)
    TriggerAddAction(gg_trg_explosion, Trig_explosion_Actions)
end

function Trig_explosion_Actions()
    if GetSpellAbilityId() == EXPLOSION_ID then
        local caster = GetTriggerUnit()
        local p = GetUnitLoc(caster)
        
        --Filters out invalid targets, i.e. dead and friendly units.
        GroupEnumUnitsInRangeOfLoc(tempGroup, p, EXPLOSION_RADIUS, Filter(IsValidEnemyTarget()))

        --Iterates through the filtered group by picking the first unit, executing actions, and then removing it from the group.
        for i = 1, CountUnitsInGroup(tempGroup) do
            u = FirstOfGroup(tempGroup)
            GroupRemoveUnit(tempGroup, u)
            UnitDamageTarget(caster, u, EXPLOSION_DAMAGE * EXPLOSION_DAMAGE_MOD, true, false, ATTACK_TYPE_MAGIC, DAMAGE_TYPE_MAGIC, null)
        end

        DestroyEffect(AddSpecialEffectLoc(EXPLOSION_DAMAGE_EFFECT, p))

        RemoveLocation(p)
        p = null
        u = null     
    end
end

IsValidEnemyTarget and tempGroup are kept in another custom script in the World Editor, and look like this:

tempGroup = CreateGroup()

function IsValidEnemyTarget()
    local u = GetFilterUnit()
    return IsUnitEnemy(u, GetOwningPlayer(GetTriggerUnit())) and GetWidgetLife(u) > .405 
end

I've tried troubleshooting for a while now, but I can't figure out why I can't get GroupEnumUnitsInRange to work properly.

If I remove the filter, it seems to work, but of course without actually filtering the units.

Thank you for any help!

 

[ScrewTheTrees]

Filter(IsValidEnemyTarget())

Calls the function, instead of passing it directly as an argument., You probably want to remove the () after it.
It should throw a error for invalid argument, but in order to catch lua errors you need to use pcall/xpcall.

 

[Uncle]

Some random pointers:

You don't have to null the location/unit variables and in Lua you write "nil" instead of "null".

A single trigger with the event EVENT_PLAYER_UNIT_SPELL_EFFECT can be used for all of your abilities. Simply store the functions in a table using their ability id as the index:

SpellEffects = {} -- create the table that will contain ALL of your spell functions, these run when a unit starts the effect of an ability
EXPLOSION_ID = 1093677104
SpellEffects[EXPLOSION_ID] = Trig_explosion_Actions -- store the spell function in the table using it's id as the index

-- Now you can have a single trigger handle every single spell effect in the game
function CreateSpellEffectTrigger()
    local trig = CreateTrigger()
    TriggerRegisterAnyUnitEventBJ(trig, EVENT_PLAYER_UNIT_SPELL_EFFECT)
    TriggerAddAction(trig, function ()
        local id = GetSpellAbilityId()
        SpellEffects[id]() -- this runs the function associated with this spell id (so in this case Trig_explosion_Actions)
    end)
end

You can also see that the function that runs in TriggerAddAction can be created right there on the spot. It doesn't need to be a separate function. This means that it will recognize any local variables that you may have set beforehand. This is extremely useful and makes your life a lot easier.

Here's some code I threw together that should work similar to your own code:

function DamageExplosion()
    local caster = GetTriggerUnit()
    local target
    local player = GetTriggerPlayer() -- casting player
    local x = GetSpellTargetX()
    local y = GetSpellTargetY()
    local g = CreateGroup()
    -- settings
    local aoe = 450
    local dmgMod = 0.75
    local dmg = 100 * dmgMod -- do the math now instead of once for each picked unit
    local sfx = "Abilities\\Spells\\Human\\Thunderclap\\ThunderClapCaster.mdl"
    -- your condition that will filter out unwanted units
    local cond = Condition(function() return
        IsUnitEnemy(GetFilterUnit(),player)
        and not IsUnitType(GetFilterUnit(), UNIT_TYPE_DEAD)
        and not IsUnitType(GetFilterUnit(), UNIT_TYPE_STRUCTURE)
        and not BlzIsUnitInvulnerable(GetFilterUnit())
    end)
    -- group enemies and damage them
    GroupEnumUnitsInRange(g, x, y, aoe, cond)
    ForGroup(g, function()
        target = GetEnumUnit()
        UnitDamageTarget(caster, target, dmg, nil, nil, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_MAGIC, nil) -- you can tweak the settings here
    end)
    -- special effect
    DestroyEffect(AddSpecialEffect(sfx, x, y))
    -- clean up
    DestroyGroup(g)
    DestroyCondition(cond)
end

Maybe you can make it more efficient using FirstOfGroup and in other ways, but I figured I'd post a working example. Also, try to use coordinates whenever possible, Location is less efficient and often unnecessary. Additionally, if you're using a BJ function, check if there's a non-BJ version and use that instead. Another one that comes to mind, you have to use the "FourCC()" function when referencing rawcodes. This converts the string into an integer. This is also useful for storing spell ids.
So instead of doing EXPLOSION_ID = 1093677104, you would do:

EXPLOSION_ID = FourCC("A000")

With "A000" being the rawcode of the ability.

 

[Cobalt]

Some random pointers:

You don't have to null the location/unit variables. Also, in Lua you write "nil" instead of "null".

A single trigger with the event EVENT_PLAYER_UNIT_SPELL_EFFECT can be used for all of your abilities. Simply store the functions in a table using their ability id as the index:

SpellEffects = {} -- create the table that will contain ALL of your spell functions that run when a unit starts the effect of an ability
EXPLOSION_ID = 1093677104
SpellEffects[EXPLOSION_ID] = Trig_explosion_Actions -- store the spell function in the table using it's id as the index

-- Now you can have a single trigger handle every single spell effect in the game
function CreateSpellEffectTrigger()
    local trig = CreateTrigger()
    TriggerRegisterAnyUnitEventBJ(trig, EVENT_PLAYER_UNIT_SPELL_EFFECT)
    TriggerAddAction(trig, function ()
        local id = GetSpellAbilityId()
        SpellEffects[id]() -- this runs the function associated with this spell id (so in this case Trig_explosion_Actions)
    end)
end

You can also see that the function that runs in TriggerAddAction can be created right there on the spot. It doesn't need to be a separate function. This means that it will recognize any local variables that you may have set beforehand. This is extremely useful and makes your life a lot easier.

Here's some code I threw together that should work:

function DamageExplosion()
    local caster = GetTriggerUnit()
    local target
    local player = GetTriggerPlayer() -- casting player
    local x = GetSpellTargetX()
    local y = GetSpellTargetY()
    local g = CreateGroup()
    -- settings
    local aoe = 450
    local dmgMod = 0.75
    local dmg = 100 * dmgMod -- do the math now instead of once for each picked unit
    local sfx = "Abilities\\Spells\\Human\\Thunderclap\\ThunderClapCaster.mdl"
    local cond = Condition(function() return
        IsUnitEnemy(GetFilterUnit(),player)
        and not IsUnitType(GetFilterUnit(), UNIT_TYPE_DEAD)
        and not IsUnitType(GetFilterUnit(), UNIT_TYPE_STRUCTURE)
        and not BlzIsUnitInvulnerable(GetFilterUnit())
    end)
    -- group enemies and damage them
    GroupEnumUnitsInRange(g, x, y, aoe, cond)
    ForGroup(g, function()
        target = GetEnumUnit()
        UnitDamageTarget(caster, target, dmg, nil, nil, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_MAGIC, nil) -- you can tweak the settings here
    end)
    -- special effect
    DestroyEffect(AddSpecialEffect(sfx, x, y))
    -- clean up
    DestroyGroup(g)
    DestroyCondition(cond)
end

Maybe you can make it more efficient using FirstOfGroup and in other ways, but I figured I'd post a working example. Also, try to use coordinates whenever possible, Location is less efficient and often unnecessary. Additionally, if you're using a BJ function, see if there's a non-BJ version and use that instead.

Thank you for all the help, it works like a charm now.

I'm especially grateful for the CreateSpellEffectTrigger function, it really simplifies things.