HomeUser Control Panel (unavailable in archive)ForumsTutorialsArt GalleryResourcesMaps
Wc3C.net » Warcraft III Modding » Developer's Corner » Triggers & Scripts » I have two ways to do thing, which is more efficient?

I have two ways to do thing, which is more efficient?

03-05-2011, 04:01 PM#1
Tom_Kazansky
I have a global trigger that would call a function and in this function, there are code that check and do things. Example like below:

Collapse JASS:
function TrigAct takes nothing returns nothing
    local unit c// = ...
    local unit a// = ...
    local integer lvl// = ...
    
    set lvl = GetUnitAbilityLevel(c,'A000')
    if lvl > 0 and GetRandomInt(1,100) <= 10+5*lvl then
        // other things
    endif
    
    set lvl = GetUnitAbilityLevel(c,'A001')
    if lvl > 0 and not IsUnitType(a,UNIT_TYPE_MAGIC_IMMUNE) then
        // other things
    endif
    
    //... and there are about 20 of other similar "if" (this will increase in the future)
    
endfunction

the trigger is global means that function will run with every unit so, obviously, those if will be ran and if there are too many ifs, the performance will be reduced, right?

now I think of an alternate way: use function interface

all the ifs are in seperate functions and when the ability is learned, add the consistant functions to the "functions to be called" (I cant think of the right word at the moment, sorry) like so:

Collapse JASS:
function interface TrigActEvent takes unit c, unit a returns nothing
globals
    TrigActEvent array eventCheck [100] // these are "functions to be called"
    integer eventCheckCount = 0
endglobals

function TrigAct takes nothing returns nothing
    local unit c// = ...
    local unit a// = ...
    local integer i = 0
    loop
        exitwhen i == eventCheckCount
        call eventCheck[i].evaluate(c,a)
        set i = i + 1
    endloop
endfunction

function EventCheckA000 takes unit c, unit a returns nothing
    local integer lvl = GetUnitAbilityLevel(c,'A000')
    if lvl > 0 and GetRandomInt(1,100) <= 10+5*lvl then
        // other things
    endif
endfunction

function EventCheckA001 takes unit c, unit a returns nothing
    local integer lvl = GetUnitAbilityLevel(c,'A001')
    if lvl > 0 and not IsUnitType(a,UNIT_TYPE_MAGIC_IMMUNE) then
        // other things
    endif
endfunction

// other functions for other "if" (and yea, about 20 of them, the number will increase in the future)

// this is the action of "hero learn skill" trigger
function HeroLearnSkillAct takes nothing returns nothing
    local unit c// = ...
    local integer abi// = ...
    local integer lvl// = ...
    if abi=='A000' and lvl==1 then
        set eventCheck[eventCheckCount] = EventCheckA000
        set eventCheckCount = eventCheckCount + 1
    elseif abi=='A000' and lvl==1 then
        set eventCheck[eventCheckCount] = EventCheckA001
        set eventCheckCount = eventCheckCount + 1
    // and more
    endif
endfunction

those "EventCheck" functions are normal functions and static method (of struct)

so, with the above way, the "ifs" are only ran when the ability is learned so I think it's more efficient
but am I abusing the function interface?
is this way as efficient as I think?

thanks for reading
p.s: sorry that I dont have a better title for the thread
03-06-2011, 02:56 AM#2
PurgeandFire111
I would personally use the first method. The second method is a bit more odd and it also internally creates a trigger and evaluates it.

It won't be that much of a difference, especially since you are not doing anything resource heavy. What you are doing should be perfectly fine, it should work very well whether it be 20, 50, 100, or maybe even 1000 if-blocks.
03-06-2011, 04:32 AM#3
Tom_Kazansky
I forgot to mention that the if-block could be... complex, I'm talking about it checks many conditions but they're only around "IsUnitType".

also, I don't know if the number of if-block could get to 100 or not, probably it could but since it's not that heavy so I guess it's fine.

I will wait for other opinion :)
03-06-2011, 06:42 PM#4
Anitarf
The ifs are likely a lot faster than doing a trigger evaluate, so the first approach would likely be faster, unless the ratio between the total number of checks and the number of checks you would actually activate with the second approach is something absurd like 1:100.

Neither of these two approaches seem particularly good, though. There is likely a better way to do this, but I'd need to know what exactly you are trying to do before I can suggest anything more specific. The first thing I thought of when I saw the code was SpellEvent, but it seems you are doing something slightly different.
03-08-2011, 12:24 AM#5
Tom_Kazansky
oh... ok, I will be more specific now:

this is the shortened code:

Expand JASS:

please see the function onAttackCheck, this function is ran whenever a unit attacks and finishes its "damage point" animation. There highlighted code is the "EventCheck" thing in the first post

below in onSwingCheck, there are the similar code highlighted as well.

the code works as the following:
a unit is attacked -> onSwingCheck -> Swing --- (finish damage point) ---> onAttackCheck ---> ... (others)
onSwingCheck is called from action of a trigger
onAttackCheck is called from "callback function" of a one-short timer

the above code is the first method mentioned in first post, I'm planing to rewrite it follow the second method but I'm not sure if it's more efficient.

tell me if you need any other info

also, is this function too heavy:
Collapse JASS:
function UnitMatch takes unit c, unit f, string fil returns boolean
    local integer i = 0
    local integer array ch
    local boolean ok = true
    local integer inx
    local player p
    if fil=="" then
        return true
    endif
    if GetUnitAbilityLevel(f,tj_IgnoredUnit)>0 then
        return false
    endif
    set inx = GetUnitId(f)
    set p = GetOwningPlayer(c)
    loop
        exitwhen i == 20
        set ch[i] = S2I(SubString(fil,i,i+1))
        set i = i + 1
    endloop
    if ch[0]!=0 then
        set ok = ok and IsUnitType(f,UNIT_TYPE_GROUND)==(ch[0]==1)
    endif
    if ch[1]!=0 then
        set ok = ok and UnitAlive(f)==(ch[1]==1)
    endif
    if ch[2]!=0 then
        if ch[2] < 3 then
            set ok = ok and IsUnitEnemy(f,p)==(ch[2]==1)
        else
            set ok = ok and GetOwningPlayer(f)==p
        endif
    endif
    if ch[3]!=0 then
        set ok = ok and IsUnitType(f,UNIT_TYPE_MELEE_ATTACKER)==(ch[3]==1)
    endif
    if ch[4]!=0 then
        if ch[4]==3 then
            set ok = ok and IsUnitType(f,UNIT_TYPE_HERO)==true
        else
            set ok = ok and UnitData[inx].isHero==(ch[4]==1)
        endif
    endif
    if ch[5]!=0 then
        set ok = ok and IsUnitTypeChampion(f)==(ch[5]==1)
    endif
    if ch[6]!=0 then
        set ok = ok and IsUnitTypeElite(f)==(ch[6]==1)
    endif
    if ch[7]!=0 then
        set ok = ok and IsUnitType(f,UNIT_TYPE_MECHANICAL)==(ch[7]==1)
    endif
    if ch[8]!=0 then
        set ok = ok and IsUnitType(f,UNIT_TYPE_STRUCTURE)==(ch[8]==1)
    endif
    if ch[9]!=0 then
        set ok = ok and IsUnitType(f,UNIT_TYPE_MAGIC_IMMUNE)==(ch[9]==1)
    endif
    if ch[10]!=0 then
        set ok = ok and IsUnitType(f,UNIT_TYPE_ETHEREAL)==(ch[10]==1)
    endif
    if ch[11]!=0 then
        set ok = ok and IsUnitType(f,UNIT_TYPE_POLYMORPHED)==(ch[11]==1)
    endif
    if ch[12]!=0 then
        set ok = ok and IsUnitType(f,UNIT_TYPE_SNARED)==(ch[12]==1)
    endif
    if ch[13]!=0 then
        set ok = ok and IsUnitType(f,UNIT_TYPE_SLEEPING)==(ch[13]==1)
    endif
    if ch[14]!=0 then
        set ok = ok and (GetUnitAbilityLevel(f,tj_IsWardCheck)>0)==(ch[14]==1)
    endif
    if ch[15]!=0 then
        set ok = ok and IsUnitIllusion(f)==(ch[15]==1)
    endif
    if ch[16]!=0 then
        set ok = ok and IsUnitInvulnerable(f)==(ch[16]==1)
    endif
    if ch[17]!=0 then
        set ok = ok and IsUnitVisibleEx(f,p)==(ch[17]==1)
    endif
    if ch[18]!=0 then
        set ok = ok and (IsUnitFogged(f,p) or IsUnitMasked(f,p))!=(ch[18]==1)
    endif
    if ch[19]!=0 then
        set ok = ok and (GetUnitState(f,UNIT_STATE_MAX_MANA)>0)==(ch[19]==1)
    endif
    return ok
endfunction

thanks for reading!
03-13-2011, 12:01 AM#6
Anitarf
Hardcoding all the effects from various abilities into one trigger like that is not a very good way of organizing your code. Each ability should have all the ability-specific code in its own library or scope.

Furthermore, checking for all abilities one each event is inefficient; checking for all abilities that were picked isn't much better, you'll have fewer checks but each one will need a trigger evaluate. You will need to use evaluates anyway if you want to move the spell specific code to separate libraries like mentioned above, but there's a better way to do this: simply keep a list of picked abilities on a per-unit basis. This way, you will only do a check if the unit has the ability, so you don't even need to check, just do an .evaluate on the ability's event function.

Libraries that allow you to efficiently maintain a list of structs on a per-unit basis already exist, I recommend AutoIndex with its AutoData feature. There's also ABuff which is a bit more high-level and already contains code for handling attack and damage events, which seem like they should correspond with your onSwingCheck and onAttackCheck events.
03-13-2011, 01:09 AM#7
Tom_Kazansky
thanks, I will now reorganize my code
I have already used AutoIndex but haven't used AutoData, I will look into it and ABuff
Wc3C.net archive — static reconstruction. No accounts, no posting.
Hosted by www.hiveworkshop.com