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

AceHart

Your Friendly Neighborhood Admin
Reaction score
1,497
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() == &#039;A042&#039; ) ) 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() == &#039;A042&#039; ) ) 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() == &#039;A042&#039; ) ) 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() == &#039;A042&#039; ) ) 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 = &#039;A042&#039;
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 = &#039;A042&#039;
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 = &#039;A042&#039;
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 = &#039;A042&#039;
    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 = &#039;A042&#039;
    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 = &#039;A042&#039;
    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 = &#039;A042&#039;
    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
 
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.
 
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:
 
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
 
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?
 
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.
 
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
 
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
 
>>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
 
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]
 
i have a small question about this spell/tutorials/whatever :).
will it make any spell lagless????
 
> 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...
 
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.
  • Varine Varine:
    +1
  • Varine Varine:
    I've been really into this
  • Varine Varine:
    It's a musical retelling of The Odyssey
  • Varine Varine:
    Also if you want mixed drinks without alcohol, there's a handful of non-alcoholic 'spirits'. Ritual Zero and Free Spirit are the two brands I've tried and they are alright. They don't have a TON of options, but they have some gin and whiskey alternatives that are fun to play with.
  • Varine Varine:
    I got a couple bottles to make some mixy drinks around holidays when I mostly want to drink, they aren't exact replacements but they are surprisingly close.
  • Varine Varine:
    I ended up with some hop water things that I really like instead for when I want to feel like I'm participating
  • Varine Varine:
    I just got moved to unsupervised probation, so I can get away with drinking a bit now. I technically am not supposed to, but I don't think I get checked anymore. I really want to smoke weed but it scares me
  • The Helper The Helper:
    Happy Wednesday! I am not feeling it today, my teeth are bothering me, or the fragments that were left behind are coming up and it is extra painful today. Don't ever let them tell you that getting full dental implants is easy. They have to pull all your teeth to do those and apparently they are not very good at getting all the teeth out all the time.
    +1
  • The Helper The Helper:
    That is true with Dentures too. I am taking the day off and working from home. The big difference is though, I am not drinking anymore so I have a nice tea and coffee regimen set up :)
  • Varine Varine:
    I wish i had a 3D scanner
  • Varine Varine:
    That would make what I'm doing SO much easier
  • Varine Varine:
    I guess I could just take a picture of it and then use that as a reference. I have a circuit board with some weird holes I can't get measured right
  • Varine Varine:
    Oh I got a Klipper mod for my big printer! It comes with a bunch of parts and I don't totally understand what it does but apparently it's way better than Marlin, which is I think what it currently runs
    +1
  • The Helper The Helper:
    Nice!
  • Varine Varine:
    You're people in Florida safe? The only couple I know are safe but I think they evacuated.
  • The Helper The Helper:
    They are without power but safe. Flood damage and stuff alot of tornados hit east Florida. No fatalities or injuries to any of my people that I know of
  • The Helper The Helper:
    They have been through all the Florida hurricanes they are like me I have been through every texas hurricane for the last 50 years.
  • The Helper The Helper:
    Never evacuated.
  • The Helper The Helper:
    Check out this bad ass Chicken Noodle soup recipe - https://www.thehelper.net/threads/crack-chicken-noodle-soup.196687/
  • The Helper The Helper:
    Happy Saturday!
    +1

      The Helper Discord

      Members online

      Affiliates

      Hive Workshop NUON Dome World Editor Tutorials

      Network Sponsors

      Apex Steel Pipe - Buys and sells Steel Pipe.
      Top