wc3campaigns
WC3C Homepage - www.wc3c.netUser Control Panel (Requires Log-In)Engage in discussions with other users and join contests in the WC3C forums!Read one of our many tutorials, ranging in difficulty from beginner to advanced!Show off your artistic talents in the WC3C Gallery!Download quality models, textures, spells (vJASS/JASS), systems, and scripts!Download maps that have passed through our rigorous approval process!

Go Back   Wc3C.net > Warcraft III Modding > Developer's Corner > Triggers & Scripts
User Name
Password
Register Rules Get Hosted! Chat Pastebin FAQ and Rules Members List Calendar



Reply
 
Thread Tools Search this Thread
Old 03-05-2011, 05:01 PM   #1
Tom_Kazansky
User
 
Tom_Kazansky's Avatar
 
Join Date: Apr 2009
Posts: 74

Tom_Kazansky is on a distinguished road (13)

Send a message via Yahoo to Tom_Kazansky
Default I have two ways to do thing, which is more efficient?

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
Tom_Kazansky is offline   Reply With Quote
Sponsored Links - Login to hide this ad!
Old 03-06-2011, 03:56 AM   #2
PurgeandFire111
User
 
PurgeandFire111's Avatar
 
Join Date: Dec 2006
Posts: 253

PurgeandFire111 will become famous soon enough (58)PurgeandFire111 will become famous soon enough (58)

Default

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.
PurgeandFire111 is offline   Reply With Quote
Old 03-06-2011, 05:32 AM   #3
Tom_Kazansky
User
 
Tom_Kazansky's Avatar
 
Join Date: Apr 2009
Posts: 74

Tom_Kazansky is on a distinguished road (13)

Send a message via Yahoo to Tom_Kazansky
Default

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 :)

Last edited by Tom_Kazansky : 03-06-2011 at 05:32 AM.
Tom_Kazansky is offline   Reply With Quote
Old 03-06-2011, 07:42 PM   #4
Anitarf
Procrastination Incarnate


Development Director
 
Join Date: Feb 2004
Posts: 8,190

Submissions (19)

Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)

2008 Spell olympics - Fire - SilverApproved Map: Old School Alliance TacticsHero Contest #2 - 3rd PlaceSpell making session 2 winner

Default

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.
__________________
Anitarf is offline   Reply With Quote
Old 03-08-2011, 01:24 AM   #5
Tom_Kazansky
User
 
Tom_Kazansky's Avatar
 
Join Date: Apr 2009
Posts: 74

Tom_Kazansky is on a distinguished road (13)

Send a message via Yahoo to Tom_Kazansky
Default

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!
Tom_Kazansky is offline   Reply With Quote
Old 03-13-2011, 01:01 AM   #6
Anitarf
Procrastination Incarnate


Development Director
 
Join Date: Feb 2004
Posts: 8,190

Submissions (19)

Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)

2008 Spell olympics - Fire - SilverApproved Map: Old School Alliance TacticsHero Contest #2 - 3rd PlaceSpell making session 2 winner

Default

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.
__________________
Anitarf is offline   Reply With Quote
Old 03-13-2011, 03:09 AM   #7
Tom_Kazansky
User
 
Tom_Kazansky's Avatar
 
Join Date: Apr 2009
Posts: 74

Tom_Kazansky is on a distinguished road (13)

Send a message via Yahoo to Tom_Kazansky
Default

thanks, I will now reorganize my code
I have already used AutoIndex but haven't used AutoData, I will look into it and ABuff
Tom_Kazansky is offline   Reply With Quote
Reply


Thread Tools Search this Thread
Search this Thread:

Advanced Search

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

vB code is On
Smilies are On
[IMG] code is On
HTML code is Off


All times are GMT. The time now is 12:16 PM.


Affiliates
The Hubb The JASS Vault Clan WEnW Campaign Creations Clan CBS GamesModding Flixreel Videos

Powered by vBulletin (Copyright ©2000 - 2019, Jelsoft Enterprises Ltd).
Hosted by www.OICcam.com
IT Support and Services provided by Executive IT Services