Conditions vs. Actions

[Bribe]

This isn't about speed and TriggerSleepAction, it's about thread integrity. I've read a post from Cohadar on wc3c.net (from 2008, mind you), and this is what he says:

Both Conditions and Actions should be used in a way Blizzard intended them to be used.

Doing stuff like this is bad:

function Condition takes nothing returns boolean
    if (GetSpellAbilityId() == 'A000') then
        //do stuff
    endif
    return false
endfunction

It clogs out the main game thread which can lead to considerable event response lag (time between you click on an ability and time it executes)

Also you cannot use TriggerSleepAction(seconds) inside a Condition and TriggerSleedCondition() function does not exist

That said it is OK to optimize some simple triggers by moving stuff from action to a condition but nothing too heavy.
For example this is OK:

library Example

globals
    private group G = CreateGroup()
endglobals

// here some periodic function does something to units in a group

function Condition takes nothing returns boolean
    if (GetSpellAbilityId() == 'A000') then
        call GroupAddUnit(G, GetSpellTargetUnit())
        set TimeLeft[GetSpellTargetUnit()] = GetUnitAbilityLevel(GetTriggerUnit(), 'A000')*10.
    endif
    return false
endfunction

endlibrary

This is not ok:

function Condition takes nothing returns boolean
    if (GetSpellAbilityId() == 'A000') then
        call Dummy_create(GetTriggerUnit(), AID_DUMMY, GetUnitAbilityLevel(hero, AID_SPELL), 5.)
        call IssueTargetOrderById(bj_lastCreatedUnit, OID_DUMMY, GetSpellTargetUnit())
    return false
endfunction

Spells with heavy stuff like creating dummy casters or sliding units or spells with custom effects should never be done in conditions.

In fact in 99% of cases you should use Conditions and Actions the way they are intended to be used.

That 1% that differs are stuff like DamageDetection systems where you register OnDamage trigger with default Condition and the call TriggerExecute from it for a specific stuff.
Here is such a condition from ORBEngine:

Spoiler

//=========================================================================
//  MAIN ENGINE
//=========================================================================
//  Units that use OrbAttacks need to have AID_ORB_ATTACK
//  Units that use OrbShields need to have AID_ORB_SHIELD
//=========================================================================
private function UnitDamaged takes nothing returns boolean
    local integer i
    local unit attacker
    local unit victim
    
    if GetEventDamage()<=0.0 then
        return false
    endif
    
    // recursion prevention
    if SEM > 1 then
        call BJDebugMsg(SCOPE_PREFIX+"UnitDamaged - SEM>1")
        return false
    endif
    
    set attacker = GetEventDamageSource()
    set victim = GetTriggerUnit()
    
    // Orb Spells on SEM==0 must deal minimum damage (0..1) because they don't triger Orb Shields
    if (GetUnitAbilityLevel(attacker, ORBConfig_SPELL) > 0) and (SEM==0) then
        set SEM = SEM + 1  //----------------//
        set i = 0
        loop
            exitwhen i >= SpellCounter
            if GetUnitAbilityLevel(attacker, SpellId<i>) &gt; 0 then
                //call TextTag_Unit(victim, SpellDescription<i>, &quot;|c0000FF00&quot;)
                if TriggerEvaluate(SpellTrigger<i>) then
                    call TriggerExecute(SpellTrigger<i>)
                endif
                exitwhen true // no dummy casts 2 different orb spells
            endif
            set i = i + 1
        endloop
        set SEM = SEM - 1  //----------------//
    // if unit has custom shield ability
    elseif (GetUnitAbilityLevel(victim, ORBConfig_SHIELD) &gt; 0) then 
        set i = 0
        loop
            exitwhen i &gt;= ShieldCounter
            if GetUnitAbilityLevel(victim, ShieldId<i>) &gt; 0 then
                //call TextTag_Unit(victim, ShieldDescription<i>, &quot;|c000000FF&quot;)
                if TriggerEvaluate(ShieldTrigger<i>) then
                    call TriggerExecute(ShieldTrigger<i>)
                endif
            endif
            set i = i + 1
        endloop
    endif

    // unit special attack
    if (GetUnitAbilityLevel(attacker, ORBConfig_ATTACK) &gt; 0) and (SEM==0) and (attacker!=victim) then
        set SEM = SEM + 1  //----------------//
        set i = 0
        loop
            exitwhen i &gt;= AttackCounter
            if GetUnitAbilityLevel(attacker, AttackId<i>) &gt; 0 then
                //call TextTag_Unit(attacker, AttackDescription<i>, &quot;|c00FF0000&quot;)
                if TriggerEvaluate(AttackTrigger<i>) then
                    call TriggerExecute(AttackTrigger<i>)
                endif
                exitwhen true // prevent using 2 orb attacks at same time
            endif
            set i = i + 1
        endloop
        set SEM = SEM - 1  //----------------//
    endif

    // debugging
    // 0 - standard damage
    // 1 - custom damage
    //if SEM == 1 then
    //    call TextTag_Unit(GetTriggerUnit(), I2S(R2I(GetEventDamage())), &quot;|c00FFFFFF&quot;)
    //endif
    
    set attacker = null
    set victim = null
    
    return false
endfunction
</i></i></i></i></i></i></i></i></i></i></i></i>

========================================================
But I doubt you will be making this kind of triggers any time soon,
so the answer to your question is:
Use Conditions and Actions like Blizzard does.

This challenges many common practices. Should I change my approach, use short-and-wasteful conditions and start using actions again?

 

[Azlier]

I've made a map or two in which the triggers use conditions exclusively (except for the scenarios where I need to start a new thread to do something) and there have been no noticeable effects whatsoever.

I would continue use of conditions.

 

[Jesus4Lyf]

I've made a map or two in which the triggers use conditions exclusively (except for the scenarios where I need to start a new thread to do something) and there have been no noticeable effects whatsoever.

I would continue use of conditions.

Likewise. Cohadar comes up with all kinds of things, some of which I listen to, some of which I ignore. This would fall under the ignore category - I honestly don't think anything in JASS is really that taxing. Really. AIDS structs are all instantiated at the same time as part of a filter (filters occur before any conditions are evaluated) and I've never seen that induce problems. I think the whole "JASS is inefficient" thing is a myth, personally. Just code appropriately (keep complexity down) - you'll be fine.

Always good reading, though. Keeps perspective broad.

Edit:
Oh, and lol @ the recursion prevention. My solution is always to support recursion!