Triggers - Heal All: A tale of ancient GUI and modern JASS

AceHart

Your Friendly Neighborhood Admin
While roaming through ancient maps in a pointless quest for inspiration,
I came upon this beauty:

Trigger:
  • Heal All
    • Events
      • Unit - A unit Starts the effect of an ability
    • Conditions
      • (Ability being cast) Equal to Heal All
    • Actions
      • Unit Group - Pick every unit in (Units owned by (Owner of (Triggering unit)) matching (((Matching unit) is alive) Equal to True)) and do (Actions)
        • Loop - Actions
          • Unit - Set life of (Picked unit) to (Max life of (Picked unit))


It waits for a unit to cast "Heal All", then takes all units of that player, provided it's alive, and sets its health back to max.

Rather simple, right?
Right.

Then again, it's somewhat too simple:
- it leaks
- it doesn't show anything


Also, why wait for "any unit" to do it?
The spell, most likely, will only ever be used by players, not Creeps for example.

I.e. we can "optimize" this by setting the event only for players this actually applies to:

Trigger:
  • Heal All
    • Events
      • Unit - A unit owned by Player 1 (Red) Starts the effect of an ability
      • Unit - A unit owned by Player 2 (Blue) Starts the effect of an ability
      • Unit - A unit owned by Player 3 (Teal) Starts the effect of an ability
      • Unit - A unit owned by Player 4 (Purple) Starts the effect of an ability
    • Conditions
      • (Ability being cast) Equal to Heal All
    • Actions
      • Unit Group - Pick every unit in (Units owned by (Owner of (Triggering unit)) matching (((Matching unit) is alive) Equal to True)) and do (Actions)
        • Loop - Actions
          • Unit - Set life of (Picked unit) to (Max life of (Picked unit))



More importantly though, "Units owned by" creates a unit group.
A new one on every run.
And, once the loop has been used... well, that group is lost.
Unfortunately, it still exists somewhere in memory.
Only, we have no way to get back to it.
It's lost.
A memory leak.

Depending on how many, or how often, or how big, or all of those, your map runs slower and slower over time.
Even to the point of serious lag.

To prevent that, we store the group in a variable, and destroy it later.

Trigger:
  • Heal All
    • Events
      • Unit - A unit owned by Player 1 (Red) Starts the effect of an ability
      • Unit - A unit owned by Player 2 (Blue) Starts the effect of an ability
      • Unit - A unit owned by Player 3 (Teal) Starts the effect of an ability
      • Unit - A unit owned by Player 4 (Purple) Starts the effect of an ability
    • Conditions
      • (Ability being cast) Equal to Heal All
    • Actions
      • Set UnitGroup = (Units owned by (Owner of (Triggering unit)) matching (((Matching unit) is alive) Equal to True))
      • Unit Group - Pick every unit in UnitGroup and do (Actions)
        • Loop - Actions
          • Unit - Set life of (Picked unit) to (Max life of (Picked unit))
      • Custom script: call DestroyGroup( udg_UnitGroup )


Now the group stays in the variable "UnitGroup".
After the loop, we destroy it with a line of custom script (i.e. a JASS line).

Note the "udg_" in front of the variable name.
That's the way the variable editor creates those, when seen from JASS.

So, the trigger works.

It's also fully multi-unit (MUI), multi-player (MPI), multi-everything (WTH) and then some.

Though it's still a bit simple.
It heals the units all right, but we don't really see anything.

More importantly perhaps, our opponent, in the middle of a fight, is not seeing anything either.

In short, it's boring, so let's add some simple eye-candy: a special effect:
Special Effect - Create a special effect attached to the "overhead" of <unit> using <some effect>

I'm going to use the effect "Chest of Gold / Lumber".
It's some golden flame-like thing.
Shown above the head of the units.

Additionally, that effect also needs to be destroyed.
Same reason as above, it would be a (small) memory leak otherwise.


Trigger:
  • Heal All
    • Events
      • Unit - A unit owned by Player 1 (Red) Starts the effect of an ability
      • Unit - A unit owned by Player 2 (Blue) Starts the effect of an ability
      • Unit - A unit owned by Player 3 (Teal) Starts the effect of an ability
      • Unit - A unit owned by Player 4 (Purple) Starts the effect of an ability
    • Conditions
      • (Ability being cast) Equal to Heal All
    • Actions
      • Set UnitGroup = (Units owned by (Owner of (Triggering unit)) matching (((Matching unit) is alive) Equal to True))
      • Unit Group - Pick every unit in UnitGroup and do (Actions)
        • Loop - Actions
          • Unit - Set life of (Picked unit) to (Max life of (Picked unit))
          • Special Effect - Create a special effect attached to the overhead of (Picked unit) using Abilities\Spells\Items\ResourceItems\ResourceEffectTarget.mdl
          • Special Effect - Destroy (Last created special effect)
      • Custom script: call DestroyGroup( udg_UnitGroup )


There we go, a nice GUI spell, with effect, no leak, MUI and it even works.


What more can you ask for?

Well, that's where JASS comes into play.
We want that thing to be optimized.


Basic rules of optimization:
- Don't do anything that doesn't need to be done.
- Don't do anything twice.


Sounds simple enough.

Let's see what that gives us with menu: Edit / Convert to custom script:


JASS:
function Trig_Heal_All_Conditions takes nothing returns boolean
    if ( not ( GetSpellAbilityId() == 'A042' ) ) then
        return false
    endif
    return true
endfunction

function Trig_Heal_All_Func001002002 takes nothing returns boolean
    return ( IsUnitAliveBJ(GetFilterUnit()) == true )
endfunction

function Trig_Heal_All_Func006A takes nothing returns nothing
    call SetUnitLifeBJ( GetEnumUnit(), GetUnitStateSwap(UNIT_STATE_MAX_LIFE, GetEnumUnit()) )
    call AddSpecialEffectTargetUnitBJ( &quot;overhead&quot;, GetEnumUnit(), &quot;Abilities\\Spells\\Items\\ResourceItems\\ResourceEffectTarget.mdl&quot; )
    call DestroyEffectBJ( GetLastCreatedEffectBJ() )
endfunction

function Trig_Heal_All_Actions takes nothing returns nothing
    set udg_UnitGroup = GetUnitsOfPlayerMatching(GetOwningPlayer(GetTriggerUnit()), Condition(function Trig_Heal_All_Func001002002))
    call ForGroupBJ( udg_UnitGroup, function Trig_Heal_All_Func006A )
    call DestroyGroup( udg_UnitGroup )
endfunction

//===========================================================================
function InitTrig_Heal_All takes nothing returns nothing
    set gg_trg_Heal_All = CreateTrigger(  )
    call TriggerRegisterPlayerUnitEventSimple( gg_trg_Heal_All, Player(0), EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerRegisterPlayerUnitEventSimple( gg_trg_Heal_All, Player(1), EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerRegisterPlayerUnitEventSimple( gg_trg_Heal_All, Player(2), EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerRegisterPlayerUnitEventSimple( gg_trg_Heal_All, Player(3), EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddCondition( gg_trg_Heal_All, Condition( function Trig_Heal_All_Conditions ) )
    call TriggerAddAction( gg_trg_Heal_All, function Trig_Heal_All_Actions )
endfunction



Ugly as all hell :p

"Trig_Heal_All_Func001002002"? Nice name...
Plenty of functions here with "BJ" in the name... we'll come to that soon enough.

Given we're going JASS here, we're also going to use the latest available, i.e. what follows requires NewGen.

For starters, we need better function names.

Like this for example:

JASS:
function Conditions takes nothing returns boolean
    if ( not ( GetSpellAbilityId() == 'A042' ) ) then
        return false
    endif
    return true
endfunction

function IsAlive takes nothing returns boolean
    return ( IsUnitAliveBJ(GetFilterUnit()) == true )
endfunction

function Heal takes nothing returns nothing
    call SetUnitLifeBJ( GetEnumUnit(), GetUnitStateSwap(UNIT_STATE_MAX_LIFE, GetEnumUnit()) )
    call AddSpecialEffectTargetUnitBJ( &quot;overhead&quot;, GetEnumUnit(), &quot;Abilities\\Spells\\Items\\ResourceItems\\ResourceEffectTarget.mdl&quot; )
    call DestroyEffectBJ( GetLastCreatedEffectBJ() )
endfunction

function Actions takes nothing returns nothing
    set udg_UnitGroup = GetUnitsOfPlayerMatching(GetOwningPlayer(GetTriggerUnit()), Condition(function IsAlive))
    call ForGroupBJ( udg_UnitGroup, function Heal )
    call DestroyGroup( udg_UnitGroup )
endfunction

function InitTrig_Heal_All takes nothing returns nothing
    set gg_trg_Heal_All = CreateTrigger(  )
    call TriggerRegisterPlayerUnitEventSimple( gg_trg_Heal_All, Player(0), EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerRegisterPlayerUnitEventSimple( gg_trg_Heal_All, Player(1), EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerRegisterPlayerUnitEventSimple( gg_trg_Heal_All, Player(2), EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerRegisterPlayerUnitEventSimple( gg_trg_Heal_All, Player(3), EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddCondition( gg_trg_Heal_All, Condition( function Conditions ) )
    call TriggerAddAction( gg_trg_Heal_All, function Actions )
endfunction


Makes a lot more sense already.
The "init" function prepares the trigger, adds a "condition", and the "actions".
The Actions function gets a group that matches "IsAlive" which at least tells what it is checking,
and the units are healed in a function called Heal.
Easy to read and find.

Though, well, if you have several spells in your map, or some imported ones,
chances are those names are already used.

Which is why the people over at wc3campaigns introduced "scopes".
Function names inside a scope, assuming the function is private, are, well, private to that scope.
I.e. it's no problem to have another scope with the same names.


Current version:

JASS:
scope HEALALL

function Conditions takes nothing returns boolean
    if ( not ( GetSpellAbilityId() == 'A042' ) ) then
        return false
    endif
    return true
endfunction

function IsAlive takes nothing returns boolean
    return ( IsUnitAliveBJ(GetFilterUnit()) == true )
endfunction

function Heal takes nothing returns nothing
    call SetUnitLifeBJ( GetEnumUnit(), GetUnitStateSwap(UNIT_STATE_MAX_LIFE, GetEnumUnit()) )
    call AddSpecialEffectTargetUnitBJ( &quot;overhead&quot;, GetEnumUnit(), &quot;Abilities\\Spells\\Items\\ResourceItems\\ResourceEffectTarget.mdl&quot; )
    call DestroyEffectBJ( GetLastCreatedEffectBJ() )
endfunction

function Actions takes nothing returns nothing
    set udg_UnitGroup = GetUnitsOfPlayerMatching(GetOwningPlayer(GetTriggerUnit()), Condition(function IsAlive))
    call ForGroupBJ( udg_UnitGroup, function Heal )
    call DestroyGroup( udg_UnitGroup )
endfunction

function InitTrig_Heal_All takes nothing returns nothing
    set gg_trg_Heal_All = CreateTrigger(  )
    call TriggerRegisterPlayerUnitEventSimple( gg_trg_Heal_All, Player(0), EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerRegisterPlayerUnitEventSimple( gg_trg_Heal_All, Player(1), EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerRegisterPlayerUnitEventSimple( gg_trg_Heal_All, Player(2), EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerRegisterPlayerUnitEventSimple( gg_trg_Heal_All, Player(3), EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddCondition( gg_trg_Heal_All, Condition( function Conditions ) )
    call TriggerAddAction( gg_trg_Heal_All, function Actions )
endfunction

endscope



The "init" function also assumes the existence of a global variable, of type trigger, called "gg_trg_Heal_All".
Not very flexible...

We want to be able to simply copy this over to a map, change a couple settings and be done with it.
Anything else if too much work :p

That's where we're going to start to "make it better" (for some definition of "better").

We replace that weird InitTrig_Heal_All with an initialzer called from the scope,
and provide our own trigger:



Slightly better.

But, let's stay here some more and try to find out what "TriggerRegisterPlayerUnitEventSimple" is doing.
It's a BJ, it should be in Blizzard.j...

Turns out it is:
JASS:
function TriggerRegisterPlayerUnitEventSimple takes trigger trig, player whichPlayer, playerunitevent whichEvent returns event
    return TriggerRegisterPlayerUnitEvent(trig, whichPlayer, whichEvent, null)
endfunction


Well, with simply adding the extra "null" parameter, we can save some function calls.

JASS:


This is now doing as little as possible to get it working for those 4 players.

Does t actually need to be nulled?
Well... not really. Reason: that trigger will stay around forever anyway.


Let's continue with more interesting things, the condition for example:
JASS:
function Conditions takes nothing returns boolean
    if ( not ( GetSpellAbilityId() == 'A042' ) ) then
        return false
    endif
    return true
endfunction


The actual test is this: "GetSpellAbilityId() == 'A042'".
It returns true or false, depending on whether the spell cast was or not... 'A042'???
That's the object id of the spell.
Can be seen in the object editor with "ctrl + D".

Only, it's not very readable.

You probably know it now from looking it up, but, in three weeks from now?

We better give it a name: "GetSpellAbilityId() == SPELL_HEAL_ALL".
That's easy to read and write.

All we need for this is a new global variable...
Fortunately, that's also possible.
Even private to the scope it's in.

JASS:
globals
    private constant integer SPELL_HEAL_ALL = 'A042'
endglobals


"private" just in case there's some other spell in the map called "spell_heal_all".
"constant" because it's never going to change anyway.
"integer" is the type.

Back to the real stuff, what is that condition doing?

If the ability test is true (it was our spell), then "if not true then return false, otherwise true".
Not true being false, the "if" will fail and we return true.
I.e. if it is true, it will return true.

If the ability test is false (it wasn't out spell), then "if not false then return false, otherwise true".
Not false being true, the "if" passes and it returns false.
I.e. if it is false, it will return false.

In short, it returns the result of that test...

Which is something we can do directly:

JASS:
function Conditions takes nothing returns boolean
    return GetSpellAbilityId() == SPELL_HEAL_ALL
endfunction



So far, we have this:
JASS:
scope HEALALL initializer Init

globals
    private constant integer SPELL_HEAL_ALL = 'A042'
endglobals

private function Conditions takes nothing returns boolean
    return GetSpellAbilityId() == SPELL_HEAL_ALL
endfunction

private function IsAlive takes nothing returns boolean
    return ( IsUnitAliveBJ(GetFilterUnit()) == true )
endfunction

private function Heal takes nothing returns nothing
    call SetUnitLifeBJ( GetEnumUnit(), GetUnitStateSwap(UNIT_STATE_MAX_LIFE, GetEnumUnit()) )
    call AddSpecialEffectTargetUnitBJ( &quot;overhead&quot;, GetEnumUnit(), &quot;Abilities\\Spells\\Items\\ResourceItems\\ResourceEffectTarget.mdl&quot; )
    call DestroyEffectBJ( GetLastCreatedEffectBJ() )
endfunction

private function Actions takes nothing returns nothing
    set udg_UnitGroup = GetUnitsOfPlayerMatching(GetOwningPlayer(GetTriggerUnit()), Condition(function IsAlive))
    call ForGroupBJ( udg_UnitGroup, function Heal )
    call DestroyGroup( udg_UnitGroup )
endfunction

private function Init takes nothing returns nothing
    local trigger t = CreateTrigger()
    call TriggerRegisterPlayerUnitEvent(t, Player(0), EVENT_PLAYER_UNIT_SPELL_EFFECT, null)
    call TriggerRegisterPlayerUnitEvent(t, Player(1), EVENT_PLAYER_UNIT_SPELL_EFFECT, null)
    call TriggerRegisterPlayerUnitEvent(t, Player(2), EVENT_PLAYER_UNIT_SPELL_EFFECT, null)
    call TriggerRegisterPlayerUnitEvent(t, Player(3), EVENT_PLAYER_UNIT_SPELL_EFFECT, null)
    call TriggerAddCondition(t, Condition(function Conditions))
    call TriggerAddAction(t, function Actions)
    set t = null
endfunction

endscope



Things to note:
- "scope" around
- all functions are private
- there's an easy to find "configuration option" to set the Object ID of the spell
- "Init" is as basic as it gets
- the condition is down to one line...


The next part would be the "IsAlive" function.
JASS:
return ( IsUnitAliveBJ(GetFilterUnit()) == true )


The outer () aren't needed:
JASS:
return IsUnitAliveBJ(GetFilterUnit()) == true


"GetFilterUnit()" is called "Matching unit" in GUI.

"IsUnitAliveBJ" can, as before, be found in Blizzard.j:
JASS:
function IsUnitAliveBJ takes unit whichUnit returns boolean
    return not IsUnitDeadBJ(whichUnit)
endfunction


I.e. it calls a function that checks if the unit is not dead...

JASS:
function IsUnitDeadBJ takes unit whichUnit returns boolean
    return GetUnitState(whichUnit, UNIT_STATE_LIFE) &lt;= 0
endfunction


We're getting closer.
Dead means you have no life anymore... (who'd have guessed that :p)

Hence the following:
JASS:
private function IsAlive takes nothing returns boolean
    return GetUnitState(whichUnit, UNIT_STATE_LIFE) &gt; 0.5
endfunction


One function call. Not a call to a call that returns a result that is negated...

Why 0.5?
Because it works better than 0.
Yes, even though GUI is using it.

Well, some recommend 0.405 (or was that 0.407?).
If you feel adventurous, try to locate the thread over at campaigns... Good luck though, it's ages old.

0.5 seems to work.
It's also rather safe to assume a unit with less is "dead enough" for what we need it here.

That one too would then be down to one line, that gets the result immediately.


Latest version:
JASS:
scope HEALALL initializer Init

globals
    private constant integer SPELL_HEAL_ALL = 'A042'
endglobals

private function Conditions takes nothing returns boolean
    return GetSpellAbilityId() == SPELL_HEAL_ALL
endfunction

private function IsAlive takes nothing returns boolean
    return GetUnitState(whichUnit, UNIT_STATE_LIFE) &gt; 0.5
endfunction

private function Heal takes nothing returns nothing
    call SetUnitLifeBJ( GetEnumUnit(), GetUnitStateSwap(UNIT_STATE_MAX_LIFE, GetEnumUnit()) )
    call AddSpecialEffectTargetUnitBJ( &quot;overhead&quot;, GetEnumUnit(), &quot;Abilities\\Spells\\Items\\ResourceItems\\ResourceEffectTarget.mdl&quot; )
    call DestroyEffectBJ( GetLastCreatedEffectBJ() )
endfunction

private function Actions takes nothing returns nothing
    set udg_UnitGroup = GetUnitsOfPlayerMatching(GetOwningPlayer(GetTriggerUnit()), Condition(function IsAlive))
    call ForGroupBJ( udg_UnitGroup, function Heal )
    call DestroyGroup( udg_UnitGroup )
endfunction

private function Init takes nothing returns nothing
    local trigger t = CreateTrigger()
    call TriggerRegisterPlayerUnitEvent(t, Player(0), EVENT_PLAYER_UNIT_SPELL_EFFECT, null)
    call TriggerRegisterPlayerUnitEvent(t, Player(1), EVENT_PLAYER_UNIT_SPELL_EFFECT, null)
    call TriggerRegisterPlayerUnitEvent(t, Player(2), EVENT_PLAYER_UNIT_SPELL_EFFECT, null)
    call TriggerRegisterPlayerUnitEvent(t, Player(3), EVENT_PLAYER_UNIT_SPELL_EFFECT, null)
    call TriggerAddCondition(t, Condition(function Conditions))
    call TriggerAddAction(t, function Actions)
    set t = null
endfunction

endscope



Next would be the function "Heal".


Looks simple, set the "picked unit"'s life to its possible maximum.

Still, SetUnitLifeBJ is this:
JASS:
function SetUnitLifeBJ takes unit whichUnit, real newValue returns nothing
    call SetUnitState(whichUnit, UNIT_STATE_LIFE, RMaxBJ(0,newValue))
endfunction


Yet another function that calls a function to do the work.
Additionally, it also makes sure we never set the health to a negative value.
We are rather sure though the max possible health of a unit is not negative.

That's also one of the reasons those BJs exist: call the native, unless you need to... test something for example.
Anyway, the "newValue" we pass here is the unit's max life from: "GetUnitStateSwap(UNIT_STATE_MAX_LIFE, GetEnumUnit())".
You just never know what people might throw at it.
Here, we do it ourselves, we know what's going on (famous last words).

Another BJ:
JASS:
function GetUnitStateSwap takes unitstate whichState, unit whichUnit returns real
    return GetUnitState(whichUnit, whichState)
endfunction


So, we plug that one back into the original:


One more replace with a native:


Another 4 (3? 5? :p) function calls reduced to 1.


On to the special effect:
JASS:
call AddSpecialEffectTargetUnitBJ( &quot;overhead&quot;, GetEnumUnit(), &quot;Abilities\\Spells\\Items\\ResourceItems\\ResourceEffectTarget.mdl&quot; )
call DestroyEffectBJ( GetLastCreatedEffectBJ() )


For starters, these long effect strings are not readable in code.
And, if you need to change it, you must actually go and look for them...

A better way to organize that is... yet another global constant:
JASS:
private constant string HEAL_EFFECT = &quot;Abilities\\Spells\\Items\\ResourceItems\\ResourceEffectTarget.mdl&quot;


Which goes to the top, is easy to find later, and gives it a name that is much easier to use and read.


"AddSpecialEffectTargetUnitBJ", as you may have guessed, is yet another function that calls a function:
JASS:
function AddSpecialEffectTargetUnitBJ takes string attachPointName, widget targetWidget, string modelName returns effect
    set bj_lastCreatedEffect = AddSpecialEffectTarget(modelName, targetWidget, attachPointName)
    return bj_lastCreatedEffect
endfunction


It creates the effect, stores it in a global variable, and returns that value.

Quick look at "DestroyEffectBJ":
JASS:
function DestroyEffectBJ takes effect whichEffect returns nothing
    call DestroyEffect(whichEffect)
endfunction


And, finally, "GetLastCreatedEffectBJ()":
JASS:
function GetLastCreatedEffectBJ takes nothing returns effect
    return bj_lastCreatedEffect
endfunction


Ok...

So, the effect is created, stored in the global variable bj_lastCreatedEffect.
That variable is asked for, and used, when destroying it.

Which means we can save a couple steps by combining this all together:

JASS:


Somewhat better.
But, the only point of that variable is to have something to pass to the destroy function.
Which we could also do all at once:

JASS:
call DestroyEffect(AddSpecialEffectTarget(HEAL_EFFECT, GetEnumUnit(), &quot;overhead&quot;))



All those parts give us our new "Heal" function:
JASS:
private function Heal takes nothing returns nothing
    call SetUnitState(GetEnumUnit(), UNIT_STATE_LIFE, GetUnitState(GetEnumUnit(), UNIT_STATE_MAX_LIFE))
    call DestroyEffect(AddSpecialEffectTarget(HEAL_EFFECT, GetEnumUnit(), &quot;overhead&quot;))
endfunction



Most recent incarnation:
JASS:
scope HEALALL initializer Init

globals
    private constant integer SPELL_HEAL_ALL = 'A042'
    private constant string HEAL_EFFECT = &quot;Abilities\\Spells\\Items\\ResourceItems\\ResourceEffectTarget.mdl&quot;
endglobals

private function Conditions takes nothing returns boolean
    return GetSpellAbilityId() == SPELL_HEAL_ALL
endfunction

private function IsAlive takes nothing returns boolean
    return GetUnitState(whichUnit, UNIT_STATE_LIFE) &gt; 0.5
endfunction

private function Heal takes nothing returns nothing
    call DestroyEffect(AddSpecialEffectTarget(HEAL_EFFECT, GetEnumUnit(), &quot;overhead&quot;))
    call SetUnitState(GetEnumUnit(), UNIT_STATE_LIFE, GetUnitState(GetEnumUnit(), UNIT_STATE_MAX_LIFE))
endfunction

private function Actions takes nothing returns nothing
    set udg_UnitGroup = GetUnitsOfPlayerMatching(GetOwningPlayer(GetTriggerUnit()), Condition(function IsAlive))
    call ForGroupBJ( udg_UnitGroup, function Heal )
    call DestroyGroup( udg_UnitGroup )
endfunction

private function Init takes nothing returns nothing
    local trigger t = CreateTrigger()
    call TriggerRegisterPlayerUnitEvent(t, Player(0), EVENT_PLAYER_UNIT_SPELL_EFFECT, null)
    call TriggerRegisterPlayerUnitEvent(t, Player(1), EVENT_PLAYER_UNIT_SPELL_EFFECT, null)
    call TriggerRegisterPlayerUnitEvent(t, Player(2), EVENT_PLAYER_UNIT_SPELL_EFFECT, null)
    call TriggerRegisterPlayerUnitEvent(t, Player(3), EVENT_PLAYER_UNIT_SPELL_EFFECT, null)
    call TriggerAddCondition(t, Condition(function Conditions))
    call TriggerAddAction(t, function Actions)
    set t = null
endfunction

endscope



We're coming to the "Actions":
JASS:
// get all units that match the condition
set udg_UnitGroup = GetUnitsOfPlayerMatching(GetOwningPlayer(GetTriggerUnit()), Condition(function IsAlive))

// do something with them
call ForGroupBJ( udg_UnitGroup, function Heal )

// destroy the group
call DestroyGroup( udg_UnitGroup )



The usual look in Blizzard.j:
JASS:
function GetUnitsOfPlayerMatching takes player whichPlayer, boolexpr filter returns group
    local group g = CreateGroup()
    call GroupEnumUnitsOfPlayer(g, whichPlayer, filter)
    call DestroyBoolExpr(filter)
    return g
endfunction


Create a new group,
put all units inside,
destroy the filter ???
return the group.

Well, we can take that part out, which eliminates one function call already.
And the need for a global variable that needs to be created from the variable editor...


Then it tries to use the group:
JASS:
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


This is also where the famous "bj_wantDestroyGroup" is coming from.
I strongly recommend to avoid it.

The interesting part is inside: call ForGroup(whichGroup, callback)


Putting it all back in:
JASS:
// set udg_UnitGroup = GetUnitsOfPlayerMatching(GetOwningPlayer(GetTriggerUnit()), Condition(function IsAlive))
local group g = CreateGroup()
call GroupEnumUnitsOfPlayer(g, GetOwningPlayer(GetTriggerUnit()), Condition(function IsAlive))

// call ForGroupBJ( udg_UnitGroup, function Heal )
call ForGroup(g, function Heal)

// call DestroyGroup( udg_UnitGroup )
call DestroyGroup(g)



What happened to destroying the boolexpr (the condition function)?
That's not needed.
It seems the game keeps them around, and returns the very same one next time you use it.
And a couple other details...
In short, no need to destroy it.


Our function could look like this:
JASS:
private function Actions takes nothing returns nothing
    local group g = CreateGroup()
    call GroupEnumUnitsOfPlayer(g, GetOwningPlayer(GetTriggerUnit()), Condition(function IsAlive))
    call ForGroup(g, function Heal)
    call DestroyGroup(g)
endfunction


There's really no reason to create and destroy a new group on every cast.
If this were a global that we simply keep around...

A new "scope global":
group g = CreateGroup()

And an extra call before using it to be sure any "old" mess is gone:
call GroupClear(g)


All in all, the current spell looks like this:
JASS:
scope HEALALL initializer Init

globals
    private constant integer SPELL_HEAL_ALL = 'A042'
    private constant string HEAL_EFFECT = &quot;Abilities\\Spells\\Items\\ResourceItems\\ResourceEffectTarget.mdl&quot;
    private group g = CreateGroup()
endglobals

private function Conditions takes nothing returns boolean
    return GetSpellAbilityId() == SPELL_HEAL_ALL
endfunction

private function IsAlive takes nothing returns boolean
    return GetUnitState(GetFilterUnit(), UNIT_STATE_LIFE) &gt; 0.5
endfunction

private function Heal takes nothing returns nothing
    call DestroyEffect(AddSpecialEffectTarget(HEAL_EFFECT, GetEnumUnit(), &quot;overhead&quot;))
    call SetUnitState(GetEnumUnit(), UNIT_STATE_LIFE, GetUnitState(GetEnumUnit(), UNIT_STATE_MAX_LIFE))
endfunction

private function Actions takes nothing returns nothing
    call GroupClear(g)
    call GroupEnumUnitsOfPlayer(g, GetOwningPlayer(GetTriggerUnit()), Condition(function IsAlive))
    call ForGroup(g, function Heal)
endfunction

private function Init takes nothing returns nothing
    local trigger t = CreateTrigger()
    call TriggerRegisterPlayerUnitEvent(t, Player(0), EVENT_PLAYER_UNIT_SPELL_EFFECT, null)
    call TriggerRegisterPlayerUnitEvent(t, Player(1), EVENT_PLAYER_UNIT_SPELL_EFFECT, null)
    call TriggerRegisterPlayerUnitEvent(t, Player(2), EVENT_PLAYER_UNIT_SPELL_EFFECT, null)
    call TriggerRegisterPlayerUnitEvent(t, Player(3), EVENT_PLAYER_UNIT_SPELL_EFFECT, null)
    call TriggerAddCondition(t, Condition(function Conditions))
    call TriggerAddAction(t, function Actions)
    set t = null
endfunction

endscope



This already looks like production code.
It's clean, easy and simple.

And yet, there's one more step we can do to simplify this:
We're running a condition over a bunch of units, remember all of them, then do something to them.

However, by the time we tell the "enum" function to remember this unit,
we already know we're going to do something to that unit.
So, why not do it immediately?
Then we wouldn't even need to remember it.
We also wouldn't need to loop over the units a second time.

Just do it all at once in the condition.

I.e. instead of "GetUnitState(whichUnit, UNIT_STATE_LIFE) > 0.5",
we put an "if" around, do the healing actions as needed,
and return false at the end.

Why false?
Because we don't need the game to collect any units in the group.
We're already done with them...

Combining "IsAlive" and "Heal" leads to something like this:
JASS:
private function Heal takes nothing returns boolean
    if GetUnitState(GetFilterUnit(), UNIT_STATE_LIFE) &gt; 0.5 then
        call DestroyEffect(AddSpecialEffectTarget(HEAL_EFFECT, GetFilterUnit(), &quot;overhead&quot;))
        call SetUnitState(GetFilterUnit(), UNIT_STATE_LIFE, GetUnitState(GetFilterUnit(), UNIT_STATE_MAX_LIFE))
    endif
    return false
endfunction


Note the change from "Picked unit" to "Matching unit".
And the change from "returns nothing" to "returns boolean" as it is now a condition function.


And a somewhat simpler "Actions" function:
JASS:
private function Actions takes nothing returns nothing
    call GroupClear(g)
    call GroupEnumUnitsOfPlayer(g, GetOwningPlayer(GetTriggerUnit()), Condition(function Heal))
endfunction



The vastly improved script:

JASS:
scope HEALALL initializer Init

globals
    private constant integer SPELL_HEAL_ALL = 'A042'
    private constant string HEAL_EFFECT = &quot;Abilities\\Spells\\Items\\ResourceItems\\ResourceEffectTarget.mdl&quot;
    private group g = CreateGroup()
endglobals

private function Conditions takes nothing returns boolean
    return GetSpellAbilityId() == SPELL_HEAL_ALL
endfunction

private function Heal takes nothing returns boolean
    if GetUnitState(GetFilterUnit(), UNIT_STATE_LIFE) &gt; 0.5 then
        call DestroyEffect(AddSpecialEffectTarget(HEAL_EFFECT, GetFilterUnit(), &quot;overhead&quot;))
        call SetUnitState(GetFilterUnit(), UNIT_STATE_LIFE, GetUnitState(GetFilterUnit(), UNIT_STATE_MAX_LIFE))
    endif
    return false
endfunction

private function Actions takes nothing returns nothing
    call GroupClear(g)
    call GroupEnumUnitsOfPlayer(g, GetOwningPlayer(GetTriggerUnit()), Condition(function Heal))
endfunction

private function Init takes nothing returns nothing
    local trigger t = CreateTrigger()
    call TriggerRegisterPlayerUnitEvent(t, Player(0), EVENT_PLAYER_UNIT_SPELL_EFFECT, null)
    call TriggerRegisterPlayerUnitEvent(t, Player(1), EVENT_PLAYER_UNIT_SPELL_EFFECT, null)
    call TriggerRegisterPlayerUnitEvent(t, Player(2), EVENT_PLAYER_UNIT_SPELL_EFFECT, null)
    call TriggerRegisterPlayerUnitEvent(t, Player(3), EVENT_PLAYER_UNIT_SPELL_EFFECT, null)
    call TriggerAddCondition(t, Condition(function Conditions))
    call TriggerAddAction(t, function Actions)
    set t = null
endfunction

endscope



Are we there yet?

Nearly... :p

"Don't do stuff more than once".
Our heal function uses GetFilterUnit() rather often.
Assuming a player has more living units than dead units, the actions inside the if will run more often than not.

As such, that could use a local variable, to reduce the calls to one.


Based on the same, there's also "Condition(function Heal)".
Every cast, turn that into a boolexpr, find out you already have it, return the same one.
We can save some micro-work here too by storing it once, and using that variable instead.



Final script:

JASS:
scope HEALALL initializer Init

globals
    private constant integer SPELL_HEAL_ALL = 'A042'
    private constant string HEAL_EFFECT = &quot;Abilities\\Spells\\Items\\ResourceItems\\ResourceEffectTarget.mdl&quot;
    private group g = CreateGroup()
    private boolexpr b
endglobals

private function Conditions takes nothing returns boolean
    return GetSpellAbilityId() == SPELL_HEAL_ALL
endfunction

private function Heal takes nothing returns boolean
    local unit u = GetFilterUnit()
    if GetUnitState(u, UNIT_STATE_LIFE) &gt; 0.5 then
        call DestroyEffect(AddSpecialEffectTarget(HEAL_EFFECT, u, &quot;overhead&quot;))
        call SetUnitState(u, UNIT_STATE_LIFE, GetUnitState(u, UNIT_STATE_MAX_LIFE))
    endif
    set u = null
    return false
endfunction

private function Actions takes nothing returns nothing
    call GroupClear(g)
    call GroupEnumUnitsOfPlayer(g, GetOwningPlayer(GetTriggerUnit()), b)
endfunction

private function Init takes nothing returns nothing
    local trigger t = CreateTrigger()
    call TriggerRegisterPlayerUnitEvent(t, Player(0), EVENT_PLAYER_UNIT_SPELL_EFFECT, null)
    call TriggerRegisterPlayerUnitEvent(t, Player(1), EVENT_PLAYER_UNIT_SPELL_EFFECT, null)
    call TriggerRegisterPlayerUnitEvent(t, Player(2), EVENT_PLAYER_UNIT_SPELL_EFFECT, null)
    call TriggerRegisterPlayerUnitEvent(t, Player(3), EVENT_PLAYER_UNIT_SPELL_EFFECT, null)
    call TriggerAddCondition(t, Condition(function Conditions))
    call TriggerAddAction(t, function Actions)
    set t = null
    set b = Condition(function Heal)
endfunction

endscope




And, finally, how to put this into your own map as a spell?
Well, copy&paste the script, either to some existing JASS trigger, or convert one, or the map header.
Edit the spell ID as needed to match your ability.
Done.


Remember what it is supposed to do?
Take all units of the caster and fully heal them.


The core activity is this:


Which does:
Take all units of the caster, and fully heal them.
With special effect.


In three lines.
Natives only.
JASS.
Pure.


Yours,
AceHart
 

Darthfett

Aerospace/Cybersecurity Software Engineer
Still, it seems like an awfully long length to go to just to fix a small spell :p
Yes it is, but he explained every single step on the way. That way anyone can do it, even if they have never even touched JASS before.

He's not only teaching good coding practice, but he's also giving people a start on JASS, which is a step in the right direction. :)

Usually I never even touch the GUI, and I automatically lookup non-BJ functions, so I never even have to do anything. If you start coding it right, you'll never have to do any optimization at the end.
 

Ghostwind

o________o
I love you. <3<3<3

Not really, but this is really helpful. I'm not sure though, is this a spell or a tutorial? :confused:
 

XeNiM666

I lurk for pizza
ahhhh yes.

my plans of learning JASS has now been continued.

+rep

EDIT: Sorry AceHart
I must spread it out first before giving it to AceHart again
 

cleeezzz

The Undead Ranger.
Why false?
Because we don't need the game to collect any units in the group.
We're already done with them...
i still dont get that. how does it differ from the function returning nothing?
 

Forty

New Member
he does the actions in the filter function and returns false in the filter to not add units to the group. whats so difficult? its faster than picking them and then looping through the units again.
 

AceHart

Your Friendly Neighborhood Admin
The "awfully long length"(sic) is not from the difficulty in doing this.
It's the number of steps taken to write it all down.

Just doing it takes under 10 minutes, if you know what you are doing.
Compared to the time it takes to make an actual map, that's nothing.


Actually, the final GUI version also works perfectly fine.
It's just not very good.
Basically, it's the difference between "it works" and "it runs".
The JASS version runs.


As for the simplicity of the spell, yes, it is.
However, if you changed, for example, to enum to get units in range, with an extra condition to check for "is an enemy",
you have a spell that can do something to nearby enemy units.

Without the need to do much.
And it would be optimized already, right from the start.


For a GUI "coder", this might be a reason to get started on better things.
For a JASS coder, this might show a thing or two... if nothing else, consider it a framework to get started.


Either way, I consider it interesting enough to 1. do it, 2. post it.

:p
 

Romek

Super Moderator
Staff member
I'd make the trigger work for all players too. (Except neutral)
Either use a loop, or use the BJ. That could also explain that not all BJ's suck ass :p

This also shows how a 2-line GUI spell can become about 30 lines of JASS code. Awesome :p

Overall though, this is awesome. It's very detailed for beginners learning JASS.
Also, I think this should be a tutorial, not a spell
 

The Undaddy

Creating with the power of rage
>>Also, I think this should be a tutorial, not a spell
Maybe a spell tutorial (tutorial spell?) :p
Anyways,looks like "posting it" took longer than "doing it".:)
Nice one
 

Cheesy

some fucker
Nice post AceHart!

I am fairly new to Jass, though I can code fairly well in it, and this helped me a lot. Showed me how to optimize a code. :thup:

PS: I think you should chance [Spell] to [Tutorial / Spell]
 

AceHart

Your Friendly Neighborhood Admin
> will it make any spell lagless

While this spellorial is somewhere between good and great or better, depending on who's reading it, it isn't magical.

The impossible is rather easy, even though it takes a bit.
But for miracles, it takes ages...
 

Matrix

New Member
Still, it seems like an awfully long length to go to just to fix a small spell :p
:D

Why it is tagged as Spell but not as a tutorial? It is the most understandable tut I've ever seen.... Only dumbas can't understand and use jass after that.
All Gui-Users must be transformed into jassers...all humans into undeads
 
General chit-chat
Help Users
  • No one is chatting at the moment.
  • tom_mai78101 tom_mai78101:
    I have a GIF though
  • tom_mai78101 tom_mai78101:
    In the GIF, I've configured it so the lower the "Main Value" is, the better. I had to compress it pretty far down though.
  • tom_mai78101 tom_mai78101:
    And this is the thread with the video that shows the wall clip. https://tasvideos.org/Forum/Topics/23453
    +2
  • tom_mai78101 tom_mai78101:
    Hmm, about the Headline News, I noticed threads are being moved into the subforum (Health News, Environmental News, etc.). When that happens, the TH Forum Home page loses the articles, and instead would show old articles posted 1 or 2 weeks ago.
  • tom_mai78101 tom_mai78101:
    What do we do with the Home page?
  • Ghan Ghan:
    I added those forums to the filter for that widget.
    +1
  • tom_mai78101 tom_mai78101:
    Oh nice. They're back. Thanks.
  • tom_mai78101 tom_mai78101:
    Now I think it makes more sense for me to put news in their own subforums, without worry.
  • The Helper The Helper:
    Awesome Ghan thanks! I was purposely not moving the first 15 news articles in Headline news to the different subforums but I guess I don't have to do that now?
  • tom_mai78101 tom_mai78101:
    Question: Is there a way to remove thread redirects? It creates a copy of the moved thread and takes up space, and I am leaning towards wanting to remove them in the Headline News. But if they have an expiration date, I guess I'm fine with it.
  • The Helper The Helper:
    If you move a thread please leave a permanent redirect. You can delete any redirects after 6 months. The redirects are left to help Search Engines find the moved content.
  • tom_mai78101 tom_mai78101:
    What if you move the permanent redirect, not the thread?
  • The Helper The Helper:
    I think that works but I have not messed with it. You can delete redirects though if you have to that will not delete the original thread
  • The Helper The Helper:
    if a redirect ends up in the same forum as the post it goes to though I think the redirect drops or fails or something but they are not bugged out and when you are working on an indirect the original post is safe.
  • The Helper The Helper:
    Happy Early Friday :)
    +1
  • V-SNES V-SNES:
    Happy Friday :)
  • tom_mai78101 tom_mai78101:
    Fun Friday for me
  • tom_mai78101 tom_mai78101:
    Happy Fun Friday to all.
    +2
  • The Helper The Helper:
    Happy Sunday everyone!!!
  • V-SNES V-SNES:
    Happy Sunday!!!
    +1
  • jonas jonas:
    Happy monday :p
  • jonas jonas:
    Everyone hates mondays?
    +1
  • The Helper The Helper:
    Happy Tuesday!
  • jonas jonas:
    Happy belated tuesday

    Members online

    No members online now.

    Affiliates

    Hive Workshop NUON Dome World Editor Tutorials

    Network Sponsors

    Apex Steel Pipe - Buys and sells Steel Pipe.
    Top