[VJASS] Script optimization

[lobster]

How can i optimize this script for cleanliness and effciency?

library CombatEngine 
globals
    private   real              array   update 
    private   multiboard        array   mbList 
    private   Multibar          array   battleGage
    private   integer           array   value
    private   real                      normalUpdate = .05
    private   integer updateAmount = 1
    private   AbilityData array abilities
    private   integer totalAbilities = 0
endglobals

struct AbilityData
    integer Spell = 0
    integer Cost = 0
endstruct
//========================================================

function RegisterAbility takes integer S, integer C returns nothing

    set abilities[totalAbilities] = AbilityData.create()
    set abilities[totalAbilities].Spell = S
    set abilities[totalAbilities].Cost = C
    set totalAbilities = totalAbilities + 1
 
endfunction

private function UpdateMultiboard takes nothing returns nothing
    local integer i = 0

    loop
    exitwhen i == 12
        set value[i] = value[i] + updateAmount
        call battleGage[i].UpdateValue(value[i],true)
        set i = i+1

    endloop
endfunction
//============================================
private function DrainBar takes nothing returns boolean
local integer i = 0
loop
    exitwhen i == totalAbilities
    if GetSpellAbilityId() == abilities[i].Spell then
        if value[GetPlayerId(GetOwningPlayer(GetTriggerUnit()))] >= abilities[i].Cost then
            set value[GetPlayerId(GetOwningPlayer(GetTriggerUnit()))] = value[GetPlayerId(GetOwningPlayer(GetTriggerUnit()))] - abilities[i].Cost
            call BJDebugMsg("Sucess")
        else
            call IssueImmediateOrder(GetTriggerUnit(), "stop")
            call BJDebugMsg("Failure")
        endif
    endif
set i = i +1

endloop
return false
endfunction



//========================================================
 
function MultiboardCreation takes player p,string title, integer col, integer row, integer barInitValue returns nothing
    
    local trigger T2 = null
    local timer Per = null

    set update[GetPlayerId(p)] = normalUpdate
    set Per = CreateTimer()
    call TimerStart(Per, update[GetPlayerId(p)] ,true, function UpdateMultiboard)

    set mbList[GetPlayerId(p)] = CreateMultiboard()
   
    call MultiboardSetColumnCount(mbList[GetPlayerId(p)], col)
    call MultiboardSetRowCount(mbList[GetPlayerId(p)], row)
    call MultiboardSetTitleText(mbList[GetPlayerId(p)], title)
    set battleGage[GetPlayerId(p)] = Multibar.create(mbList[GetPlayerId(p)],0,0,16,100,10,2)
   
    call MultiboardDisplay(mbList[GetPlayerId(p)], true)
   
    set value[GetPlayerId(p)] = barInitValue
    
    set T2 = CreateTrigger()
 
    
    call TriggerAddCondition(T2, function DrainBar)
    call TriggerRegisterAnyUnitEventBJ(T2,EVENT_PLAYER_UNIT_SPELL_CAST )
    
endfunction


endlibrary

 

[watermelon_1234]

Use GetTriggerUnit() instead of GetSpellAbilityUnit()

What's the point of T1?

You could initialize T2 and Per.

You'll probably want to make those globals, and function DrainBar private. (Maybe the struct too?)

Out of curiosity, what exactly is this supposed to do?

 

[lobster]

Use GetTriggerUnit() instead of GetSpellAbilityUnit()

What's the point of T1?

You could initialize T2 and Per.

You'll probably want to make those globals, and function DrainBar private. (Maybe the struct too?)

Out of curiosity, what exactly is this supposed to do?

All fixed (will be updated soon)

Its purpose is like a custom mana bar, leaving your mana open for other things, like maybe if you wanted to use it as energy that slowly drains until it reaches 0, upon which you die.

Anything else?

 

[Adiktuz]

seems okay... there's just one thing I want to ask, wheres the struct called Multibar? ^^

 

[lobster]

seems okay... there's just one thing I want to ask, wheres the struct called Multibar? ^^

its in a library in a seperate trigger. Im using someone elses library.
http://www.wc3c.net/showthread.php?t=98138

 

[Adiktuz]

oh... okay. ^^...

 

[watermelon_1234]

    local trigger T2 = null
    local timer Per = null

Oh no, I meant

    local trigger T2 = CreateTrigger()
    local timer Per = CreateTimer()

Also, you could put constant for some of the globals. (Like totalAbilities.) It makes it slightly faster.

In function UpdateMultiboard, shouldn't it only update for the players that have the multiboard created for them?

 

[lobster]

Oh no, I meant

    local trigger T2 = CreateTrigger()
    local timer Per = CreateTimer()

Also, you could put constant for some of the globals. (Like totalAbilities.) It makes it slightly faster.

In function UpdateMultiboard, shouldn't it only update for the players that have the multiboard created for them?

Hmm, iye, i could do that

As for update multiboard, wouldnt that require adding more code, making it more unreadable? along with a varaible to check whether the player has a multiboard

 

[watermelon_1234]

As for update multiboard, wouldnt that require adding more code, making it more unreadable? along with a varaible to check whether the player has a multiboard

True, but it would become more efficient. Actually I think it's more efficient to have only one global timer do the multiboard updating and have a global variable which determines if the multiboard should be updated for a player.

 

[lobster]

True, but it would become more efficient. Actually I think it's more efficient to have only one global timer do the multiboard updating and have a global variable which determines if the multiboard should be updated for a player.

This is an issue for functionality over readability.

As for the timer, then you couldn't have speed adjusting abilities