Very Simple Group Filter

[XeRo13g]

The Simple spell causes units around the target unit to take damage based on some variables, simple as that.
-I don't want to keep the group, thus using a empty global group (gr) declared at initialization.

function Simple_Filter takes nothing returns boolean
local unit uf=GetFilterUnit()
//The next line shows what needs to be done with the filtered units
//call UnitDamageTarget(u,uf,dmg,true,false,ATTACK_TYPE_NORMAL,DAMAGE_TYPE_FIRE,WEAPON_TYPE_WHOKNOWS)
set uf=null
return false
endfunction

function Trig_Simple_Actions takes nothing returns nothing
local unit u=GetTriggerUnit()
local unit ust=GetSpellTargetUnit()
local real x=GetUnitX(ust)
local real y=GetUnitY(ust)
local real dmg=40*GetUnitAbilityLevel(u,GetSpellAbilityId())+GetHeroStr(u,true)
local real aoe=300.

call GroupEnumUnitsInRange(gr,x,y,aoe,Filter(function Simple_Filter))
set u=null
set ust=null
endfunction

-As far as I understand, there is no way to pass parameters through a filter, so this is how I've tried to go about it:

 scope HB
    globals
        private unit uu
        private unit uust
        private real dmg
    endglobals
function Simple_Filter takes nothing returns boolean
local unit uf=GetFilterUnit()
call UnitDamageTarget(uu,uf,dmg,true,false,ATTACK_TYPE_NORMAL,DAMAGE_TYPE_FIRE,WEAPON_TYPE_WHOKNOWS)
set uf=null
return false
endfunction

function Trig_Simple_Actions takes nothing returns nothing
local real x
local real y
local real aoe=300.
set uu=GetTriggerUnit()
set uust=GetSpellTargetUnit()
set x=GetUnitX(uust)
set y=GetUnitY(uust)
set dmg=40*GetUnitAbilityLevel(uu,GetSpellAbilityId())+GetHeroStr(uu,true)

call GroupEnumUnitsInRange(gr,x,y,aoe,Filter(function Hellfire_Bolt_Filter))

endfunction
endscope

Is this acceptable? Nulling of private globals is required?

What is the best way to deal with this?
Thanks in advance

EDIT: Corrected an error - added what I've done so far

 

[luorax]

Use globals to pass the needed variables:

globals
    private unit Caster
    private unit Target
    private real Damage
    private constant group ENUM_GROUP=CreateGroup() //this is not needed if you use GroupUtils
endglobals

function Simple_Filter takes nothing returns boolean
    call UnitDamageTarget(Caster,Target,Damage,true,false,ATTACK_TYPE_NORMAL,DAMAGE_TYPE_FIRE,WEAPON_TYPE_WHOKNOWS)
    return false
endfunction

function Trig_Simple_Actions takes nothing returns nothing
    set Caster=GetTriggerUnit()
    set Target=GetSpellTargetUnit()
    set Damage=40*GetUnitAbilityLevel(Caster,GetSpellAbilityId())+GetHeroStr(Caster,true)

    call GroupEnumUnitsInRange(ENUM_GROUP,GetUnitX(Target),GetUnitY(Target),300.,Filter(function Simple_Filter))
endfunction

 

[XeRo13g]

Use globals to pass the needed variables:

globals
    private unit Caster
    private unit Target
    private real Damage
    private constant group ENUM_GROUP=CreateGroup() //this is not needed if you use GroupUtils
endglobals

function Simple_Filter takes nothing returns boolean
    call UnitDamageTarget(Caster,Target,Damage,true,false,ATTACK_TYPE_NORMAL,DAMAGE_TYPE_FIRE,WEAPON_TYPE_WHOKNOWS)
    return false
endfunction

function Trig_Simple_Actions takes nothing returns nothing
    set Caster=GetTriggerUnit()
    set Target=GetSpellTargetUnit()
    set Damage=40*GetUnitAbilityLevel(Caster,GetSpellAbilityId())+GetHeroStr(Caster,true)

    call GroupEnumUnitsInRange(ENUM_GROUP,GetUnitX(Target),GetUnitY(Target),300.,Filter(function Simple_Filter))
endfunction

-I don't currently use any external system. Is there a problem using a universal empty group (gr) for all similar occasions?

So, if I use those private globals above the trigger, I don't need to use scope?
My main concern was that those globals should be able to be re-declared to other spells(using the same names) and some of them will require to keep the values stored in them without conflict with this or any other trigger.
Basically, globals acting like locals.

EDIT:
-It still requires me to enter scope and include the whole spell in it.
-It seems that the private globals do not hold their values the second time the trigger is used(much like locals). If I don't null them, isn't it a leak?

 

[luorax]

They don't hold their values, since they're redeclared each time the trigger fires. You have to place it in a scope or library. They don't have to be private if you want to modify them later on, it's up to you. And you should be more specific about holding their values; this snippet was meant to pass the needed variables to the filter. Storing them for later use takes some extra globals.

-I don't currently use any external system. Is there a problem using a universal empty group (gr) for all similar occasions?

It's fine, if that's the only one static group. But please follow the general naming conventions and don't name your variables like that.

 

[XeRo13g]

It seems that I don't understand the mechanics behind the -private- prefix.

They don't hold their values, since they're redeclared each time the trigger fires.

I thought those were normal globals, just available in this specific trigger. It seems like the global block runs each time the trigger runs and erases any value it might have had - Thus my concern for leaks.

They don't have to be private if you want to modify them later on, it's up to you.

If I don't use the -private- prefix, wouldn't that cause conflict with other triggers using the same-named variables, or it's fine due to declaring them inside a scope? Please excuse my ignorance.

And you should be more specific about holding their values; this snippet was meant to pass the needed variables to the filter. Storing them for later use takes some extra globals.

I thought those globals would keep their values when it comes to this specific trigger. I don't actually need those to keep any value in this example, I just want to make sure they won't leak that value and to understand how this works.

It's fine, if that's the only one static group. But please follow the general naming conventions and don't name your variables like that.

The group was declared like this:

globals
group gr=CreateGroup()
endglobals

Unfortunately, I'm already using it in several occasions and the bad name will not be easy to fix but it will be done in time, thanks for indicating that.
-Do I need to use any prefix to that group to make it static? Will it make a difference?
-It is the only group I use for that purpose, would it be a problem if there was another one?

 

[luorax]

private globals are just like normal globals, you got it all wrong. (that might be my fault - when I said redeclared, I meant its value) The only difference is that they get a prefix depending on the namespace name, and JH pops up an error when you try to use a private global outside its namespace. Example:

library Test
    globals
        private group EnumGroup
    endglobals
endlibrary

Becomes after compilation:

globals
    group Test_EnumGroup
endglobals

If I don't use the -private- prefix, wouldn't that cause conflict with other triggers using the same-named variables, or it's fine due to declaring them inside a scope? Please excuse my ignorance.

It would.

The group was declared like this:

I mean in the whole map. If you have more of those static groups in your map, it's worthy to implement GroupUtils, or create one and use it everywhere.

I thought those globals would keep their values when it comes to this specific trigger. I don't actually need those to keep any value in this example, I just want to make sure they won't leak that value and to understand how this works.

Globals do not leak at all. Globals are really different from locals. They have initial values (unlike locals, that can freeze threads), they don't have to be nulled (nor have function arguments) and they keep their values.

EDIT: and well, my favourite "tutorial": http://www.wc3c.net/vexorian/jasshelpermanual.html
I can't stop linking this, because it was so damn helpful when I was beginner.

EDIT2: well, they way I'm doing it on my map looks like this:

scope foo

struct Data extends array
    implement Alloc
    
    readonly unit caster
    readonly unit target
    readonly real damage
    static method new takes unit caster,unit target,real damage returns thistype
        local thistype this=thistype.allocate()
        set this.caster=caster
        set this.target=target
        set this.damage=damage
        return this
    endmethod
endstruct

struct FooSpell extends SpellStruct
    implement SpellStruct
    
    group tempGroup

    private method getDamage takes nothing returns real
        return 40*this.level+GetHeroStr(this.caster,true)
    endmethod
    private static method callback takes nothing returns nothing
        local Data data=GetGroupData(GetEnumGroup())
        call UnitDamageTarget(data.caster,GetEnumUnit(),data.damage,true,false,ATTACK_TYPE_NORMAL,DAMAGE_TYPE_FIRE,WEAPON_TYPE_WHOKNOWS)
    endmethod
    implement EnemyModule
    private method onEffect takes nothing returns nothing
        local Data data=Data.create(this.caster,this.targetUnit,this.getDamage())
        set this.tempGroup=NewGroupEx(data)
        call this.enumUnitsTargetUnit(this.tempGroup,this.aoe,function thistype.callback)
        call data.deallocate()
        call ReleaseGroup(this.tempGoup)
    endmethod
    private static method onInit takes nothing returns nothing
        set thistype.abil='A000'
        set thistype.defaultAoE=500.
    endmethod
endstruct
endscope

However this one uses dozens of external scripts and structs. (and it might contain syntax errors, hell, it's handwritten)

 

[XeRo13g]

If you have more of those static groups in your map, it's worthy to implement GroupUtils, or create one and use it everywhere.

Actually, I'm using that bad named (gr) group for everything in the map. Do I need to add any prefix to that? Constant or Static? Will it help?

Globals do not leak at all. Globals are really different from locals. They have initial values (unlike locals, that can freeze threads), they don't have to be nulled (nor have function arguments) and they keep their values.

If that is true, then I get some weird behavior. If I try to get the value of a global before it's set, the trigger doesn't run at all. If I set an initial value for the global at it's declaration, it's fine.

Thanks for helping luorax, I really appreciate it.

PS. I've read this tutorial far too many times and I'm reading it again as we speak. The problem comes when I try to use those in action

PS2. I've just noticed your edit before I post my reply. That code seems a bit scary, I'll try to study it asap.

 

[luorax]

Constant and static are the same, but constant can't be used in structs, whilst static can only be used in structs.

If that is true, then I get some weird behavior. If I try to get the value of a global before it's set, the trigger doesn't run at all. If I set an initial value for the global at it's declaration, it's fine.

Hmm, then maybe that's only true for global arrays? I was not completely sure about that.

PS. I've read this tutorial far too many times and I'm reading it again as we speak. The problem comes when I try to use those in action

Practice is the key here bro

PS2. I've just noticed your edit before I post my reply. That code seems a bit scary, I'll try to study it asap.

It's a really, really short and easy one Do it and you'll see.

 

[Ayanami]

It's actually better to use [ljass]FirstOfGroup()[/ljass] than group filters now, because null boolexpr do not leak anymore. This way, you can use local values. Below is a simple trigger that deals 100 damage to enemies within 500 area of the caster.

globals
    private group G = bj_lastCreatedGroup // you can use this rather than creating an empty group simply for enumeration
endglobals

private function OnAction takes nothing returns nothing
    local unit caster = GetTriggerUnit()
    local unit u
    
    call GroupEnumUnitsInRange(G, GetUnitX(u), GetUnitY(u), 500, null)
    loop
        set u = FirstOfGroup(G)
        exitwhen u == null
        call GroupRemoveUnit(G, u)

        if not IsUnitType(u, UNIT_TYPE_DEAD) then // include all conditions here
            call UnitDamageTarget(caster, u, 100, true, false, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_MAGIC, null)
        endif
    endloop

    set caster = null
endfunction

With this, you won't even need to pass variables via globals. You can directly use locals.

 

[Bribe]

I second that, Ayanami, I was going to post that he doesn't need enum filter. Really the benefit of using a filter is only if you plan to keep the group. If you're doing a once-off enumeration (like he is) then he's way better with the FirstOfGroup loop.

 

[Sgqvur]

>Really the benefit of using a filter is only if you plan to keep the group.

It's kinda annoying (4 additional lines) but the group can be preserved after the first of group enumeration:

globals
    private group G = CreateGroup() // someone might use gui =) bj_lastCreatedGroup // you can use this rather than creating an empty group simply for enumeration
    private group tmpg = CreateGroup()
    private group swapg
endglobals

private function OnAction takes nothing returns nothing
    local unit caster = GetTriggerUnit()
    local unit u
    
    call GroupEnumUnitsInRange(G, GetUnitX(u), GetUnitY(u), 500, null)
    loop
        set u = FirstOfGroup(G)
        exitwhen u == null
        call GroupRemoveUnit(G, u)

        call GroupAddUnit(tmpg, u) // (1)

        if not IsUnitType(u, UNIT_TYPE_DEAD) then // include all conditions here
            call UnitDamageTarget(caster, u, 100, true, false, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_MAGIC, null)
        endif
    endloop // after this line what was in G is now in tmpg, G is empty
    set swapg = G // (2)
    set G = tmpg // (3)
    set tmpg = swapg // (4)
    // so we simply swap them, tmpg is now empty and G has it's original units before the enumeration

    // do something else with G
    call BJDebugMsg(GetUnitName(FirstOfGroup(G)))
  
    set caster = null
endfunction

 

[Dirac]

if G is empty at start there's no need for a swap, just
set G = tmpg
would do the trick, after all there's no group to back up there.
Remember to destroy the other group though (the one G was before)

 

[Bribe]

bj_lastCreatedGroup doesn't cause problems with GUI in 99.9999% of cases and 0% of sane cases anyway. If you are doing recursive things that can interfere with GUI's attempt to get the exact value of "(Last created unit group)" you're doing something wrong, because you could only really get that from inside a "unit is indexed" event, and no one really uses that command anyway (CreateNUnitsAtLoc usually gets passed the value of "1") and that's not considering such a rare thing would also be used in conjunction with a JASS script.