• Listen to a special audio message from Bill Roper to the Hive Workshop community (Bill is a former Vice President of Blizzard Entertainment, Producer, Designer, Musician, Voice Actor) 🔗Click here to hear his message!
  • Read Evilhog's interview with Gregory Alper, the original composer of the music for WarCraft: Orcs & Humans 🔗Click here to read the full interview.

[vJASS] optimizing

Status
Not open for further replies.
hello i was just looking to see if this is as optimal as possible. if it isnt plz give me some better ways to do things thanks.
JASS:
scope HeroAbilitySelection initializer Ability_Selection

globals
    trigger trg_AbilitySelection
    private integer array lumberCost
    private integer array itemIDS
    private integer array abilityIDS
    private unit array abilitySellers
    private integer array itemIntDataArray
    private integer array itemGroups
endglobals

private function addItemsToStockLoop takes integer p, integer i, integer start, integer end returns nothing
    loop
        exitwhen start > end
        call AddItemToStock( abilitySellers[((p*4)+i)], itemIDS[start], 1, 1)
        set start = start + 1
    endloop
endfunction

private function addItemsToStock takes integer p, integer Loop returns nothing
    local integer i
    local integer start
    local integer end
    set i = Loop/11
    set start = i*11
    if start > 32 then 
        set end = 39
    else
        set end = start + 10
    endif
    call addItemsToStockLoop( p, i, start, end)
endfunction

private function getAbilityGroup takes integer Loop returns integer
    local integer L = 1
    local integer e
    loop
        exitwhen L > 13
        if Loop < itemGroups[L] then
            return L
        endif
        set L = L + 1
    endloop
    return L
endfunction

private function addNewAbility takes integer p, integer Loop returns nothing
    local integer i
    local integer L
    local integer e
    set L = getAbilityGroup( Loop)
    set e = (itemGroups[L]-1)
    set i = itemGroups[(L-1)]
    loop
        exitwhen i > e
        set itemIntDataArray[i] = 0
        set i = i + 1
    endloop
    set itemIntDataArray[Loop] = 1
    call addItemsToStock( p, Loop)
endfunction

private function Actions takes nothing returns nothing
    local integer p = GetPlayerId( GetTriggerPlayer())
    local integer start
    local integer end
    local item titem = GetManipulatedItem()
    local integer Loop = 0
    local integer abilvl
    local integer id = GetItemTypeId( titem)
    call text( "ability selection actions works", 0)
    call RemoveItem( titem)
    loop
        exitwhen Loop > 39
        if id == itemIDS[Loop] then
            set abilvl = GetUnitAbilityLevel( Selected_Hero_Player, abilityIDS[Loop])
            if itemIntDataArray[Loop] == 1 then
                if abilvl == 0 then
                    call UnitAddAbility( Selected_Hero_Player, abilityIDS[Loop])
                    call addNewAbility( p, Loop)
                elseif abilvl == 100 then
                    call text( "Congradulations u Reached the Max Lvl for this Ability", p)
                    set itemIntDataArray[Loop] = 0
                    call SetPlayerState( Player(p), PLAYER_STATE_RESOURCE_LUMBER, (GetPlayerState( Player(p), PLAYER_STATE_RESOURCE_LUMBER)+lumberCost[Loop]))
                    call addItemsToStock( p, Loop)
                else
                    call IncUnitAbilityLevel( Selected_Hero_Player, abilityIDS[Loop])
                    call addItemsToStock( p, Loop)
                endif
            elseif itemIntDataArray[Loop] == 0 then
                call SetPlayerState( Player(p), PLAYER_STATE_RESOURCE_LUMBER, (GetPlayerState( Player(p), PLAYER_STATE_RESOURCE_LUMBER)+lumberCost[Loop]))
                call text( "Sry u can't buy this item", p)
                call addItemsToStock( p, Loop)
            endif
        endif
        set Loop = Loop + 1
    endloop
    set titem = null
endfunction

private function destroyItems takes nothing returns nothing
    call RemoveItem( GetEnumItem())
endfunction

private function settingLumberCosts takes nothing returns nothing
    local integer Loop = 0
    local unit u = gg_unit_n01M_0127
    local integer maxLumber = 1000000 // 1 mil
    local integer itemLumber
    local rect r = gg_rct_Delete_Items
    loop
        exitwhen Loop > 39
        call SetPlayerState( Player(11), PLAYER_STATE_RESOURCE_LUMBER, maxLumber)
        call AddItemToStock( u, itemIDS[Loop], 1, 1)
        call IssueNeutralImmediateOrderById( Player(11), u, itemIDS[Loop])
        call RemoveItemFromStock( u, itemIDS[Loop])
        set itemLumber = GetPlayerState( Player(11), PLAYER_STATE_RESOURCE_LUMBER)
        set lumberCost[Loop] = maxLumber - itemLumber
        set Loop = Loop + 1
    endloop
    call EnumItemsInRect( r, null, function destroyItems)
    call ShowUnit( u, false )
    call RemoveUnit( u)
    set u = null
    set r = null
endfunction

private function itemToStockSetup takes nothing returns nothing
    local integer Loop = 0
    local integer i = 0
    local integer ids = 0
    local integer loopE = ((40*2)-1) // number of abilities * number of players in map
    loop   
        exitwhen Loop > loopE
        call AddItemToStock( abilitySellers[i], itemIDS[ids], 1, 1)
        set itemIntDataArray[Loop] = 1
        if ids == 10 or ids == 21 or ids == 32 then
            set i = i + 1
        elseif ids == 39 then
            set i = i + 1
            set ids = -1
        endif
        set ids = ids + 1
        set Loop = Loop + 1
    endloop
endfunction

private function preloadAbilities takes nothing returns nothing
    local unit u
    local integer Loop = 0
    set u = CreateUnit( Player(11), 'hfoo', 0, 0, 0)
    call ShowUnit( u, false)
    loop
        exitwhen Loop > 39
        call UnitAddAbility( u, abilityIDS[Loop])
        call UnitRemoveAbility( u, abilityIDS[Loop])
        set Loop = Loop + 1
    endloop
    call RemoveUnit( u)
    set u = null
endfunction

private function hash takes nothing returns nothing
    local integer Loop = 0
    loop
        exitwhen Loop > 39
        call SaveBoolean( conditionHash, itemIDS[Loop], 0, true)
        set Loop = Loop + 1
    endloop
endfunction

private function startup takes nothing returns nothing //this whole thing just sets the data in arrays
    set itemIDS[0] = 'I004' // Divine Shield ITEM ID
    set itemIDS[1] = 'I005' // Mana Shield ITEM ID
    set itemIDS[2] = 'I006' // Mirror Image ITEM ID
    set itemIDS[3] = 'I007' // Wind Walk ITEM ID
    set itemIDS[4] = 'I00E' // Volcano ITEM ID
    set itemIDS[5] = 'I00F' // Earthquake ITEM ID
    set itemIDS[6] = 'I00G' // Stampede ITEM ID
    set itemIDS[7] = 'I00H' // Flame Strike ITEM ID
    set itemIDS[8] = 'I00O' // Shadow Strike ITEM ID
    set itemIDS[9] = 'I00P' // Storm Bolt ITEM ID
    set itemIDS[10] = 'I00Q' // Soul Burn ITEM ID
    set itemIDS[11] = 'I00R' // Bash ITEM ID
    set itemIDS[12] = 'I00S' // Critical Strike ITEM ID
    set itemIDS[13] = 'I00T' // Spiked Carapace ITEM ID
    set itemIDS[14] = 'I00U' // Cleaving Attack ITEM ID
    set itemIDS[15] = 'I00X' // Death Coil ITEM ID
    set itemIDS[16] = 'I00Y' // Holy Light ITEM ID
    set itemIDS[17] = 'I00Z' // Death Pact ITEM ID
    set itemIDS[18] = 'I010' // Life Drain ITEM ID
    set itemIDS[19] = 'I008' // Avatar ITEM ID
    set itemIDS[20] = 'I009' // Metamorphosis ITEM ID
    set itemIDS[21] = 'I00A' // Vengeance ITEM ID
    set itemIDS[22] = 'I00I' // Tornado ITEM ID
    set itemIDS[23] = 'I00J' // Bladestorm ITEM ID
    set itemIDS[24] = 'I00K' // Incinerate ITEM ID
    set itemIDS[25] = 'I00L' // Immolation ITEM ID
    set itemIDS[26] = 'I00M' // Impale ITEM ID
    set itemIDS[27] = 'I00N' // Carrion Swarm ITEM ID
    set itemIDS[28] = 'I00V' // Sleep ITEM ID
    set itemIDS[29] = 'I00W' // Charm ITEM ID
    set itemIDS[30] = 'I011' // Fan of Knives ITEM ID
    set itemIDS[31] = 'I012' // Thunder Clap ITEM ID
    set itemIDS[32] = 'I013' // Howl of Terror ITEM ID
    set itemIDS[33] = 'I001' // Chain Lightning ITEM ID
    set itemIDS[34] = 'I000' // Forked Lightning ITEM ID
    set itemIDS[35] = 'I002' // Blink ITEM ID
    set itemIDS[36] = 'I003' // Mass Teleport ITEM ID
    set itemIDS[37] = 'I00B' // Blizzard ITEM ID
    set itemIDS[38] = 'I00C' // Rain of Fire ITEM ID
    set itemIDS[39] = 'I00D' // Starfall ITEM ID
    set abilityIDS[0] = 'A017' // Divine Shield ITEM ID
    set abilityIDS[1] = 'A02D' // Mana Shield ITEM ID
    set abilityIDS[2] = 'A006' // Mirror Image ITEM ID
    set abilityIDS[3] = 'A00I' // Wind Walk ITEM ID
    set abilityIDS[4] = 'A02F' // Volcano ITEM ID
    set abilityIDS[5] = 'A003' // Earthquake ITEM ID
    set abilityIDS[6] = 'A02I' // Stampede ITEM ID
    set abilityIDS[7] = 'A018' // Flame Strike ITEM ID
    set abilityIDS[8] = 'A02L' // Shadow Strike ITEM ID
    set abilityIDS[9] = 'A01X' // Storm Bolt ITEM ID
    set abilityIDS[10] = 'A02M' // Soul Burn ITEM ID
    set abilityIDS[11] = 'A012' // Bash ITEM ID
    set abilityIDS[12] = 'A002' // Critical Strike ITEM ID
    set abilityIDS[13] = 'A023' // Spiked Carapace ITEM ID
    set abilityIDS[14] = 'A02N' // Cleaving Attack ITEM ID
    set abilityIDS[15] = 'A00Q' // Death Coil ITEM ID
    set abilityIDS[16] = 'A01B' // Holy Light ITEM ID
    set abilityIDS[17] = 'A00R' // Death Pact ITEM ID
    set abilityIDS[18] = 'A02P' // Life Drain ITEM ID
    set abilityIDS[19] = 'A010' // Avatar ITEM ID
    set abilityIDS[20] = 'A029' // Metamorphosis ITEM ID
    set abilityIDS[21] = 'A02B' // Vengeance ITEM ID
    set abilityIDS[22] = 'A02J' // Tornado ITEM ID
    set abilityIDS[23] = 'A020' // Bladestorm ITEM ID
    set abilityIDS[24] = 'A02K' // Incinerate ITEM ID
    set abilityIDS[25] = 'A028' // Immolation ITEM ID
    set abilityIDS[26] = 'A021' // Impale ITEM ID
    set abilityIDS[27] = 'A00P' // Carrion Swarm ITEM ID
    set abilityIDS[28] = 'A022' // Sleep ITEM ID
    set abilityIDS[29] = 'A02Q' // Charm ITEM ID
    set abilityIDS[30] = 'A027' // Fan of Knives ITEM ID
    set abilityIDS[31] = 'A01Z' // Thunder Clap ITEM ID
    set abilityIDS[32] = 'A02Q' // Howl of Terror ITEM ID
    set abilityIDS[33] = 'A000' // Chain Lightning ITEM ID
    set abilityIDS[34] = 'A02C' // Forked Lightning ITEM ID
    set abilityIDS[35] = 'A024' // Blink ITEM ID
    set abilityIDS[36] = 'A01E' // Mass Teleport ITEM ID
    set abilityIDS[37] = 'A014' // Blizzard ITEM ID
    set abilityIDS[38] = 'A02E' // Rain of Fire ITEM ID
    set abilityIDS[39] = 'A02A' // Starfall ITEM ID
    set itemGroups[0] = 0  // 1st item in next group
    set itemGroups[1] = 4  // 1st item in next group
    set itemGroups[2] = 8  // 1st item in next group
    set itemGroups[3] = 11  // 1st item in next group
    set itemGroups[4] = 15  // 1st item in next group
    set itemGroups[5] = 19  // 1st item in next group
    set itemGroups[6] = 22  // 1st item in next group
    set itemGroups[7] = 24  // 1st item in next group
    set itemGroups[8] = 26  // 1st item in next group
    set itemGroups[9] = 28  // 1st item in next group
    set itemGroups[10] = 30  // 1st item in next group
    set itemGroups[11] = 33  // 1st item in next group
    set itemGroups[12] = 35  // 1st item in next group
    set itemGroups[13] = 37  // 1st item in next group
    set itemGroups[14] = 40  // 1st item in next group
    set abilitySellers[0] = gg_unit_n01L_0119
    set abilitySellers[1] = gg_unit_n01L_0120
    set abilitySellers[2] = gg_unit_n01L_0121
    set abilitySellers[3] = gg_unit_n01L_0122
    set abilitySellers[4] = gg_unit_n01L_0123
    set abilitySellers[5] = gg_unit_n01L_0124
    set abilitySellers[6] = gg_unit_n01L_0125
    set abilitySellers[7] = gg_unit_n01L_0126
    call hash()
    call preloadAbilities()
    call itemToStockSetup()
    call settingLumberCosts()
endfunction

private function conditions takes nothing returns boolean
    return HaveSavedBoolean( conditionHash, GetItemTypeId( GetManipulatedItem()), 0)
endfunction

//===========================================================================
private function Ability_Selection takes nothing returns nothing
    set trg_AbilitySelection = CreateTrigger()
    call TriggerRegisterPlayerUnitEvent( trg_AbilitySelection, Player(0), EVENT_PLAYER_UNIT_PICKUP_ITEM, null )
    call TriggerRegisterPlayerUnitEvent( trg_AbilitySelection, Player(1), EVENT_PLAYER_UNIT_PICKUP_ITEM, null )
    call TriggerRegisterPlayerUnitEvent( trg_AbilitySelection, Player(2), EVENT_PLAYER_UNIT_PICKUP_ITEM, null )
    call TriggerRegisterPlayerUnitEvent( trg_AbilitySelection, Player(3), EVENT_PLAYER_UNIT_PICKUP_ITEM, null )
    call startup()
    call TriggerAddAction( trg_AbilitySelection, function Actions )
    call TriggerAddCondition( trg_AbilitySelection, function conditions )
endfunction

endscope
 
Last edited:
JASS:
id = id/4*4
id2 = id + 4
loop
    set itemIntDataArray[id] = 0
    set id = id + 1
    exitwhen id == id2
endloop

That's the most obvious improvement that I see thus far =P. I don't really want to read all of your code, only read down to your huge elseif chain.

That will give 4 array positions to each id though. I'd have to go through more of the code to allow for any # of positions for any given id.
 

Dr Super Good

Spell Reviewer
Level 64
Joined
Jan 18, 2005
Messages
27,258
JASS:
private function destroyItems takes nothing returns nothing
    local item Item
    set Item = GetEnumItem()
    call RemoveItem( Item)
    set Item = null
endfunction

Could just be...

JASS:
private function destroyItems takes nothing returns nothing
    call RemoveItem(GetEnumItem())
endfunction

And...

JASS:
private function itemToStockSetup takes nothing returns nothing
    local integer Loop = 0
    local integer i = 0
    local integer index = 0
    local integer ids = 0
    local integer players = 4 // number of max players in map // why is a constant declared here?!
    local integer abilities = 40 // number of abilities // why is a constant declared here?!
    local integer loopE = ((abilities*players)-1)
    loop   
        exitwhen Loop > loopE
        call AddItemToStock( abilitySellers[i], itemIDS[ids], 1, 1)
        set itemIntDataArray[Loop] = 1
        if index == 10 or index == 21 or index == 32 then
            set i = i + 1
        elseif index == 39 then
            set i = 0
            set ids = 0
        endif
        set ids = ids + 1
        set index = index + 1 // this has same value as Loop?
        set Loop = Loop + 1 // thiis has same value as index?
    endloop
endfunction

You could probably merge those two locals for speed. The constants should not be declared as locals as that causes their values to be computed each time the function is run (even if the function is only run once it is still bad practice).
 
i put them as local variables because tht function is only run once. the abilities and players i just had in there so i didnt forget at the time lol i changed it tho. the item thing i had a bug and i wanted to see if tht would fix it, just forgot to switch it back lol thx. and the index is same as loop because i was tired lol i just didnt notice thx tho i will change tht

edit: changed to this and i also changed the item deletion back
JASS:
private function itemToStockSetup takes nothing returns nothing
    local integer Loop = 0
    local integer i = 0
    local integer ids = 0
    local integer loopE = ((40*4)-1) // number of abilities * number of players in map
    loop   
        exitwhen Loop > loopE
        call AddItemToStock( abilitySellers[i], itemIDS[ids], 1, 1)
        set itemIntDataArray[Loop] = 1
        if Loop == 10 or Loop == 21 or Loop == 32 then
            set i = i + 1
        elseif Loop == 39 then
            set i = 0
            set ids = 0
        endif
        set ids = ids + 1
        set Loop = Loop + 1
    endloop
endfunction

i think im gonna make a itemgroup array variable next to get rid of tht huge ite i have in the addabilities function

edit: i always reworked it a little made the big ite block a little more readable. updated first post w new trigger. also if u look in the settinglumberCosts function i havei call player state resource lumber 80 times would it be better to put tht into a variable or would it not really matter ? ive seen them set to a variable in some triggers but not others.

shortened the huge ite block to from this
JASS:
private function addNewAbility takes integer p, integer Loop returns nothing
    if Loop == 0 or Loop == 1 or Loop == 2 or Loop == 3 then
        set itemIntDataArray[0] = 0
        set itemIntDataArray[1] = 0
        set itemIntDataArray[2] = 0
        set itemIntDataArray[3] = 0
        set itemIntDataArray[Loop] = 1
    elseif Loop == 4 or Loop == 5 or Loop == 6 or Loop == 7 then
        set itemIntDataArray[4] = 0
        set itemIntDataArray[5] = 0
        set itemIntDataArray[6] = 0
        set itemIntDataArray[7] = 0
        set itemIntDataArray[Loop] = 1
    elseif Loop == 8 or Loop == 9 or Loop == 10 then
        set itemIntDataArray[8] = 0
        set itemIntDataArray[9] = 0
        set itemIntDataArray[10] = 0
        set itemIntDataArray[Loop] = 1
    elseif Loop == 11 or Loop == 12 or Loop == 13 or Loop == 14 then
        set itemIntDataArray[11] = 0
        set itemIntDataArray[12] = 0
        set itemIntDataArray[13] = 0
        set itemIntDataArray[14] = 0
        set itemIntDataArray[Loop] = 1
    elseif Loop == 15 or Loop == 16 or Loop == 17 or Loop == 18 then
        set itemIntDataArray[15] = 0
        set itemIntDataArray[16] = 0
        set itemIntDataArray[17] = 0
        set itemIntDataArray[18] = 0
        set itemIntDataArray[Loop] = 1
    elseif Loop == 19 or Loop == 20 or Loop == 21 then
        set itemIntDataArray[19] = 0
        set itemIntDataArray[20] = 0
        set itemIntDataArray[21] = 0
        set itemIntDataArray[Loop] = 1
    elseif Loop == 22 or Loop == 23 then
        set itemIntDataArray[22] = 0
        set itemIntDataArray[23] = 0
        set itemIntDataArray[Loop] = 1
    elseif Loop == 24 or Loop == 25 then
        set itemIntDataArray[24] = 0
        set itemIntDataArray[25] = 0
        set itemIntDataArray[Loop] = 1
    elseif Loop == 26 or Loop == 27 then
        set itemIntDataArray[26] = 0
        set itemIntDataArray[27] = 0
        set itemIntDataArray[Loop] = 1
    elseif Loop == 28 or Loop == 29 then
        set itemIntDataArray[28] = 0
        set itemIntDataArray[29] = 0
        set itemIntDataArray[Loop] = 1
    elseif Loop == 30 or Loop == 31 or Loop == 32 then
        set itemIntDataArray[30] = 0
        set itemIntDataArray[31] = 0
        set itemIntDataArray[32] = 0
        set itemIntDataArray[Loop] = 1
    elseif Loop == 33 or Loop == 34 then
        set itemIntDataArray[33] = 0
        set itemIntDataArray[34] = 0
        set itemIntDataArray[Loop] = 1
    elseif Loop == 35 or Loop == 36 then
        set itemIntDataArray[35] = 0
        set itemIntDataArray[36] = 0
        set itemIntDataArray[Loop] = 1
    elseif Loop == 37 or Loop == 38 or Loop == 39 then
        set itemIntDataArray[37] = 0
        set itemIntDataArray[38] = 0
        set itemIntDataArray[39] = 0
        set itemIntDataArray[Loop] = 1
    endif
    call addItemsToStock( p, Loop)
endfunction
to this
JASS:
private function getAbilityGroup takes integer Loop returns integer
    local integer L = 1
    local integer e
    loop
        exitwhen L > 13
        if Loop < itemGroups[L] then
            return L
        endif
        set L = L + 1
    endloop
    return L
endfunction

private function addNewAbility takes integer p, integer Loop returns nothing
    local integer i
    local integer L
    local integer e
    set L = getAbilityGroup( Loop)
    set e = (itemGroups[L]-1)
    set i = itemGroups[(L-1)]
    loop
        exitwhen i > e
        set itemIntDataArray[i] = 0
        set i = i + 1
    endloop
    set itemIntDataArray[Loop] = 1
    call addItemsToStock( p, Loop)
endfunction
 
Last edited:
Level 37
Joined
Mar 6, 2006
Messages
9,243
JASS:
private function conditions takes nothing returns boolean
    local integer id = GetItemTypeId( GetManipulatedItem())
    return id==itemIDS[0] or id==itemIDS[1] or id==itemIDS[2] or id==itemIDS[3] or id==itemIDS[4] or id==itemIDS[5] or id==itemIDS[6] or id==itemIDS[7] or id==itemIDS[8] or id==itemIDS[9] or id==itemIDS[10] or id==itemIDS[11] or id==itemIDS[12] or id==itemIDS[13] or id==itemIDS[14] or id==itemIDS[15] or id==itemIDS[16] or id==itemIDS[17] or id==itemIDS[18] or id==itemIDS[19] or id==itemIDS[20] or id==itemIDS[21] or id==itemIDS[22] or id==itemIDS[23] or id==itemIDS[24] or id==itemIDS[25] or id==itemIDS[26] or id==itemIDS[27] or id==itemIDS[28] or id==itemIDS[29] or id==itemIDS[30] or id==itemIDS[31] or id==itemIDS[32] or id==itemIDS[33] or id==itemIDS[34] or id==itemIDS[35] or id==itemIDS[36] or id==itemIDS[37] or id==itemIDS[38] or id==itemIDS[39]
endfunction
->
JASS:
return HaveSavedBoolean(hashtable, GetItemTypeId( GetManipulatedItem()), 0)

Save a boolean for each item type into a hashtable: call SaveBoolean(hashtable, 'I000', 0, true)
Replace I000 with the raw code of each item.
 

Dr Super Good

Spell Reviewer
Level 64
Joined
Jan 18, 2005
Messages
27,258
In case you were wondering, hashtables have a lookup order of approximately O(1) under low utilization conditions. By this I mean they behave similar to an array lookup as opposed to having to check every element in turn (a linear search) which can vary in cost with the number of elements involved.
 
Level 37
Joined
Mar 6, 2006
Messages
9,243
The O notation is used to tell how much the processing time (or storage space or something else) increases in relation to the number of inputs.

O(1) means that you only need to do one lookup no matter how many elements you stored. Look at the hashtable optimization above. You can store 1000 booleans and still load the one you need with one function call. O(1) is very good.

Arrays would be O( (n+1)/2 ). If you have array size of 4, you do either 1, 2, 3 or 4 checks assuming each index is equally propable to be the right one. On average that is (1+2+3+4)/4 = 2,5 lookups, which is (4+1)/2, (n+1)/2.
 
Status
Not open for further replies.
Top