[JASS] Help with getting BJ function native Please

[AlphaChkn]

I need to pick all units within range of a spell cast that are enemies and do something to them. I was optimizing my code and came across this BJ. I need a way to make my own version but i dont quite understand the code.

function ForGroupBJ takes group whichGroup, code callback returns nothing
    // If the user wants the group destroyed, remember that fact and clear
    // the flag, in case it is used again in the callback.
    local boolean wantDestroy = bj_wantDestroyGroup
    set bj_wantDestroyGroup = false

    call ForGroup(whichGroup, callback)

    // If the user wants the group destroyed, do so now.
    if (wantDestroy) then
        call DestroyGroup(whichGroup)
    endif
endfunction

What exactly is callback? And how would I make my own version of this? Thanks.

 

[Earth-Fury]

just use ForGroup(). All the BJ does there is destroy the group if bj_wantDestroyGroup is set to true. It is most defintly preferable to destroy the group "manually" with a call to DestroyGroup()

And as for what callback is, its a pointer to a function defined with the function keyword in a constant maner, such as: function MyFunc. Note that code-type function pointers can not be passed paramiters (they must take nothing) and their return value will not be used. Boolexprs are like the native code type, except the functions must return boolean. What ForGroup does is execute that function for each unit in a group. You can use GetEnumUnit() inside the callback to reffer to the current unit in the group. Eg:

function MyCallback takes nothing returns nothing
    // do whatever for each unit in the group
endfunction

function abc takes nothing returns nothing
    call ForGroup(SomeGlobalGroup, function MyCallback)
endfunction

 

[AlphaChkn]

So for every unit picked with ForGroup, the callback function will be executed on them? If im interpreting what you said correctly...

Also. Which group would I later destroy using DestroyGroup()
after using ForGroup?

thanks for help so far. +rep.

 

[Earth-Fury]

When you create something, you must destroy it otherwise it tends to stick around. Units are automatically destroyed after they fully decay and such. The BJ ForGroupBJ() is used in the GUI, where destroying groups isn't readily possible. So blizzard decided to check if a variable is set, and if so, destroy the group that is passed to ForGroup. Of course, it don't think they ever actually made it possible to set the flag variable without JASS.

So, if you create a group, use it for ForGroup, then never use it again, you must destroy it. Otherwise, it is likely to leak (have no valid refrences / pointers pointing to it) and simply take up memory untill the game ends. Really, any time you create anything that is of a desended type of handle, you must worry about cleaning it up. (Except for objects which will exist for a whole game, like the player type, or the few things that clean themselves up, like units. Also, do not destroy boolexprs.) You may wanan check the tutorials section to learn more about memory leaks..

And yes, ForGroup enumerates throgh a group one unit at a time, calling the callback function on each enumeration. You use GetEnumUnit() to get the currently enumerating unit. (unless im far too forgetful, and thats not the correct function... heh) Its woth mentioning that ForGroup is supposedly faster in executing a function then a normal function call.

 

[AlphaChkn]

Awesome thanks so much for the great exp. I get the whole deal about memory leaks. Just wasnt sure what name the group that is created was assigned.

Is it just that first parameter that lets you choose the group name?

call ForGroup(SomeGlobalGroup, function MyCallback)

 

[Earth-Fury]

The first paramiter takes anything of the group type. So any variable or function or such that returns a group can be passed to the first paramiter. But again, remember: if you create it, you must destroy it, so something like this is stupid:

call ForGroup(CreateGroup(), function MyCallback)

As you of course can not destroy a group you can't reference.

 

[XieLong]

I'm quite sure that the also the native ForGroup is suspected to leak itself, no matter if the group is destroyed, but I don't know excatly why ^^ [jass=Because of this, there's a little workaround to avoid the ForGroup]local group g = <yourGroup>
local group h = CreateGroup()
local unit u
loop
set u = FirstOfGroup(g)
exitwhen u == null
call GroupRemoveUnit(g, u)
call GroupAddUnit(h, u)
// all the actions you want to do with the picked unit (u = picked unit)
endloop
// with this second loop, all the units are putted back to <yourGroup>
// if you dont need <yourGroup> afterwards, you don't need this second loop
loop
set u = FirstOfGroup(h)
exitwhen u == null
call GroupRemoveUnit(h, u)
call GroupAddUnit(g, u)
endloop
set g = null
call DestroyGroup(h)
set h = null
set u = null[/code]This 'loop-method' has another clear advantage: There's no callback function, and this leads to the point that you dont have to pass any variables through globals or gamecache.

 

[AlphaChkn]

is there any reason that in the first loop, you are adding units to the group h? couldnt you just do the stuff to the units and then null group g at the end?

**EDIT**
nvm. I think I see. You need to remove units from the original group g in order to change what the first unit is for the FirstOfGroup() function. The group h is just temp storage so that g can be restored later?

So then I have a new question then. If you dont need to restore g, do you need h?

 

[Marcelo Hossomi]

If you don't need to do anything else, I'm quite sure h is not needed. You have already done what you wanted so the groups are empty and useless now, I guess

Yes, the use of h is to restore g after (I don't see any other use for it xD)

 

[AlphaChkn]

Thanks you much for the help.

 

[aznricepuff]

You have some really redundant code:

Assuming his group was originally a global and he wants that group preserved, you don't need to loop through group h and add tht back to group g - just save group h to the global pointer:

local group g = <yourGroup>
local group h = CreateGroup()
local unit u
loop
    set u = FirstOfGroup(g)
    exitwhen u == null
    call GroupRemoveUnit(g, u)
    call GroupAddUnit(h, u)
    // all the actions you want to do with the picked unit (u = picked unit)
endloop
//save global (or w/e) group as the newly created group h with all of the original units, this way you avoid running through essentially the same group twice
set <yourGroup> = h
set g = null
call DestroyGroup(g)
set h = null
set u = null

 

[AlphaChkn]

Ok now Im having a problem. For some reason when I try to test the map Im getting an access violation error message. here is the message:

When my trigger is turned off I dont get the message. So im guessing its some kind of error in the code that makes it past syntax checker.

Here is the code. thanks.

function Trig_Warriors_Calling_Conditions takes nothing returns boolean    
    return ( GetSpellAbilityId() == 'A00F' )
endfunction

function Is_Enemy_Unit takes nothing returns boolean
    return ( IsUnitEnemy( GetEnumUnit(), GetOwningPlayer(GetSpellAbilityUnit()) ) )
endfunction    

function Trig_Warriors_Calling_Actions takes nothing returns nothing
    local unit Main_Caster = GetSpellAbilityUnit()
    local unit Dummy_Caster
    local unit Picked_Unit
    local group Units_Called = CreateGroup()
    local location Main_Cast_Loc = GetUnitLoc(Main_Caster)
    
    call GroupEnumUnitsInRangeOfLoc(Units_Called, Main_Cast_Loc, 400, Condition(function Is_Enemy_Unit) )
    
    loop
        set Picked_Unit = FirstOfGroup(Units_Called)
        exitwhen ( Picked_Unit == null )
        call GroupRemoveUnit(Units_Called, Picked_Unit)
        call SetUnitPositionLoc(Picked_Unit, Main_Cast_Loc)
    endloop
    
    call DestroyGroup(Units_Called)
    set Units_Called = null
    set Main_Caster = null
    set Dummy_Caster = null
    set Picked_Unit = null
    call RemoveLocation(Main_Cast_Loc)
    set Main_Cast_Loc = null   
    
endfunction

//==== Init Trigger Warriors_Calling ====
function InitTrig_Warriors_Calling takes nothing returns nothing
    local integer event_player_index    
    set gg_trg_Warriors_Calling = CreateTrigger()    
    set event_player_index = 0
    
    loop
        exitwhen  (event_player_index == 16)
        call TriggerRegisterPlayerUnitEvent( gg_trg_Warriors_Calling, Player(event_player_index), EVENT_PLAYER_UNIT_SPELL_FINISH, null)
        set event_player_index = ( event_player_index + 1 )
    endloop    
    
    call TriggerAddCondition(gg_trg_Warriors_Calling, Condition(function Trig_Warriors_Calling_Conditions))
    call TriggerAddAction(gg_trg_Warriors_Calling, function Trig_Warriors_Calling_Actions)
endfunction

*****EDIT*****
Nvm. I found it. Player IDs start with 0. I had my Event Register loop up to Player(16) which im giessing caused the prog to crash. Oops.

loop
        exitwhen  (event_player_index == 17)
        call TriggerRegisterPlayerUnitEvent( gg_trg_Warriors_Calling, Player(event_player_index), EVENT_PLAYER_UNIT_SPELL_FINISH, null)
        set event_player_index = ( event_player_index + 1 )
    endloop

*****EDIT 2*****
Im having a problem with the enum function condition in the code above. for some reason the spell is picking enemy and non-enemy units to the casting units location. It is only supposed to move enemies. Anyone able to tell my why?

 

[Earth-Fury]

You have some really redundant code:

Assuming his group was originally a global and he wants that group preserved, you don't need to loop through group h and add tht back to group g - just save group h to the global pointer:

local group g = <yourGroup>
local group h = CreateGroup()
local unit u
loop
    set u = FirstOfGroup(g)
    exitwhen u == null
    call GroupRemoveUnit(g, u)
    call GroupAddUnit(h, u)
    // all the actions you want to do with the picked unit (u = picked unit)
endloop
//save global (or w/e) group as the newly created group h with all of the original units, this way you avoid running through essentially the same group twice
set <yourGroup> = h
set g = null
call DestroyGroup(g)
set h = null
set u = null

errr

local group g
local unit enum

call GroupAddGroup(<someGroup>, g)

loop
    set enum = FirstOfGroup(g)
    exitwhen enum == null
    call GroupRemoveUnit(g, enum)

    // loop code
endloop

call DestroyGroup(g)
set g = null
set enum = null

You may wish to inline the GroupAddGroup, but its not critical or needed unless your code is iterating at a VERY low interval. Some BJs do have their uses...

Also, you null g before you destroy it, thus you destroy a null handle... which does nothing good.

 

[XieLong]

But if you look the function 'GroupAddGroup' you see that is based on the ForGroup function, which we tried to avoid...

 

[Earth-Fury]

But if you look the function 'GroupAddGroup' you see that is based on the ForGroup function, which we tried to avoid...

The only reason to avoid ForGroup is because of the callback structure. Most often, its just a lot more convinient to just use a FirstOfGroup() loop. Also, you remove the issue of having to send data in to the ForGroup() by using a FirstOfGroup() loop. However, ForGroup is actually rather optimized compated to a FirstOfGroup() loop. Even if the iterative logic ends up costing the same, you have fewer function calls inside. (GetEnumUnit() VS FirstOfGroup() GroupRemoveUnit())

As for some kind of leak in ForGroup, you'd be conting bits in the land of gigabytes.

Saving a lot of extra logic within a script, abstracting it to an API like blizzard.j, is a good thing. Makes code more readable and compact, and thus more maintainable and easyer to modify.

 

[HINDYhat]

FirstOfGroup is actually rather optimized compated to a FirstOfGroup() loop.

Wake up

And yes, ForGroup is actually pretty fast. Faster than a FirstOfGroup() loop (and yes I've timed both), so when you don't need the advantages of a FirstOfGroup() loop (like no local passing), you're better off using ForGroup.

 

[AlphaChkn]

*****EDIT 2*****
Im having a problem with the enum function condition in the code above. for some reason the spell is picking enemy and non-enemy units to the casting units location. It is only supposed to move enemies. Anyone able to tell my why?

I didnt even need the part that saved the group. Im only doing one thing to it then it dont need it anymore.

Im just concerned with why its pulling both enemies and friendlies to the casters pos.

thanks.

 

[Marcelo Hossomi]

The function Is_Unit_Enemy, taken by the GroupEnumUnitsInRangeOfLoc function is a filter, so you shouldn't use GetEnumUnit in Is_Unit_Enemy, but GetFilterUnit. This will refer to the unit being filtered, and not picked yet.

In GUI, it would be Matching Unit.

 

[Jacek]

So, how many % of speedup did you get from replacing that ForGroupBJ with native?

 

[AlphaChkn]

IDK lol....

Its not like I can measure. But its noticable when its a map with alot of triggers running. Also I would assume the performance decrease from leaks grows cumulatively: The more you use the trigger with the leak the worse the performance of the whole game gets.

And thanks to everyone that helped. +Reps to those that havnt gotten yet.

**EDIT**
I tried the GetFilterUnit() Function for the filter conditional and it worked amazingly. Thanks once again for everyone that helped.

**EDIT**
Got the part working where the units are summoned to the caster's position. For some reason though the dummy unit wont cast its Slam spell after the units are summoned. Can anyone see why that would be?

function Trig_Warriors_Calling_Conditions takes nothing returns boolean    
    return ( GetSpellAbilityId() == 'A00F' )
endfunction

function Is_Enemy_Unit takes nothing returns boolean    
    return ( IsUnitEnemy( GetFilterUnit(), GetOwningPlayer(GetSpellAbilityUnit()) ) )
endfunction    

function Trig_Warriors_Calling_Actions takes nothing returns nothing
    local unit Main_Caster = GetSpellAbilityUnit()
    local unit Dummy_Caster
    local unit Picked_Unit
    local group Units_Called = CreateGroup()
    local location Main_Cast_Loc = GetUnitLoc(GetSpellAbilityUnit())
    local integer Level_Ability = GetUnitAbilityLevel(Main_Caster, 'A00F')
    
    call GroupEnumUnitsInRangeOfLoc(Units_Called, Main_Cast_Loc, 400, Condition(function Is_Enemy_Unit) )
    
    loop
        set Picked_Unit = FirstOfGroup(Units_Called)
        exitwhen ( Picked_Unit == null )
        call GroupRemoveUnit(Units_Called, Picked_Unit)
        call SetUnitPositionLoc(Picked_Unit, Main_Cast_Loc)
    endloop
    
    call CreateNUnitsAtLoc(1, 'n008', GetOwningPlayer(GetSpellAbilityUnit()), Main_Cast_Loc, 270)
    set Dummy_Caster = GetLastCreatedUnit()
    call UnitApplyTimedLife(Dummy_Caster, 'BTLF', 5)
    call UnitAddAbility(Dummy_Caster, 'a00G')
    call SetUnitAbilityLevel(Dummy_Caster, 'a00G', Level_Ability)
    call IssueImmediateOrder(Dummy_Caster, "creepthunderclap")   
    
    call DestroyGroup(Units_Called)
    set Units_Called = null
    set Main_Caster = null
    set Dummy_Caster = null
    set Picked_Unit = null
    call RemoveLocation(Main_Cast_Loc)
    set Main_Cast_Loc = null   
    
endfunction

//==== Init Trigger Warriors_Calling ====
function InitTrig_Warriors_Calling takes nothing returns nothing
    local integer event_player_index    
    set gg_trg_Warriors_Calling = CreateTrigger()    
    set event_player_index = 0
    
    loop
        exitwhen  (event_player_index == 16)
        call TriggerRegisterPlayerUnitEvent( gg_trg_Warriors_Calling, Player(event_player_index), EVENT_PLAYER_UNIT_SPELL_FINISH, null)
        set event_player_index = ( event_player_index + 1 )
    endloop    
    
    call TriggerAddCondition(gg_trg_Warriors_Calling, Condition(function Trig_Warriors_Calling_Conditions))
    call TriggerAddAction(gg_trg_Warriors_Calling, function Trig_Warriors_Calling_Actions)
endfunction

 

[Marcelo Hossomi]

Try changing

call CreateNUnitsAtLoc(1, 'n008', GetOwningPlayer(GetSpellAbilityUnit()), Main_Cast_Loc, 270)
set Dummy_Caster = GetLastCreatedUnit()

to

set Dummy_Caster = CreateUnitAtLoc( GetOwningPlayer(GetSpellAbilityUnit()), 'n008', Main_Cast_Loc, 270 )

(The best would use coordinates and the function CreateUnit(player, unitid, x, y, face), but let's simplify xD)
(and remember you can't use GetLastCreatedUnit() to refer to the unit created by CreateUnitAtLoc or CreateUnit)

I think the correct raw code for the ability would be 'A00G' ( case sensitive ).
And make sure the dummy has enough mana to cast the ability, or the ability costs no mana \o/

 

[AlphaChkn]

Ahh damn I feel teh stupehdz for not realizing about the limits of GetLastCreatedUnit() function. Thanks so much once again.