[JASS] Efficency problem

[dardas]

hey.. i made a loop to find a right value out of an array..
so i did this:

    private function GetUnitDataCount takes unit u returns integer
    loop
    if AS[2] == AS[1] then
        set AS[2] = 1
    else
        set AS[2] = AS[2] + 1
    endif
    
    exitwhen AS[2] == AS[1] or GetUnitTypeId(u) == UnitType[AS[2]]
    if AS[2] == AS[1] and GetUnitTypeId(u) != UnitType[AS[2]] then
        call BJDebugMsg("You have not initialized the unit type trying to attack.")
        call BJDebugMsg(" Unit name is: " + GetUnitName(u))
    endif
    endloop
    
    return AS[2]
    endfunction

im pretty sure that this will lag heavly if ill call it many times, (many unit's using the function)
cuz of my old projectiles system.
so i want to know if theres a better way to make it?

oh yeah, AS[2] = Table

thanks in advance.

 

[watermelon_1234]

What exactly are you trying to do?
What is UnitType? What is AS[1]?

Is it important that the key needs to be an integer value? If it isn't, you could use HandleTable which would probably be the best way to do this.

 

[dardas]

well.. its used for a custom attack system,
im gonna replace the whole regular warcraft III attacking system with a new 1...
for me and my friend's game..
basicly AS[1] is a table, it counts max amount of units added to the list.
heres most of the code.. (exept custom attack, and the globals)

TheCode

    function RegisterUnitType takes integer TypeRaw returns integer
        set AS[1] = AS[1] + 1
        set UnitType[AS[1]] = TypeRaw
        set UnitTypeAttackType[AS[1]] = DefaultAttackType
        set UnitTypeDamageType[AS[1]] = DefaultDamageType
        set UnitTypeWeaponType[AS[1]] = DefaultWeaponType
        set UnitTypeAbilityRaw[AS[1]] = DefaultAbilityRaw
        return AS[1]
    endfunction
    
    function RegisterUnitDamage takes integer TypeCount, real Damage returns nothing
        set UnitTypeDamage[TypeCount] = Damage
    endfunction
    
    function RegisterUnitBoolean takes integer TypeCount, boolean IgnoreArmor returns nothing
        set UnitTypeArmorBool[TypeCount] = IgnoreArmor
    endfunction
    
    function RegisterUnitSword takes integer TypeCount, real SwordDistance returns nothing
        set UnitTypeSwordLenght[TypeCount] = SwordDistance
    endfunction
    
    function RegisterUnitAttackType takes integer TypeCount, attacktype ATTACK_TYPE returns nothing
        set UnitTypeAttackType[TypeCount] = ATTACK_TYPE
    endfunction
    
    function RegisterUnitDamageType takes integer TypeCount, damagetype DAMAGE_TYPE returns nothing
        set UnitTypeDamageType[TypeCount] = DAMAGE_TYPE
    endfunction
    
    function RegisterUnitWeaponType takes integer TypeCount, weapontype WEAPON_TYPE returns nothing
        set UnitTypeWeaponType[TypeCount] = WEAPON_TYPE
    endfunction
    
    function RegisterUnitRange takes integer TypeCount, boolean IsUnitRanged returns nothing
        set UnitTypeUnitRanged[TypeCount] = IsUnitRanged
    endfunction
    
    function RegisterUnitAnimationName takes integer TypeCount, string AnimationName returns nothing
        set UnitTypeAnimationName[TypeCount] = AnimationName
    endfunction
    
    function RegisterUnitAbilityRaw takes integer TypeCount, integer AbilityRaw returns nothing
        set UnitTypeAbilityRaw[TypeCount] = AbilityRaw
    endfunction
    
    function RegisterUnitMissleSpeed takes integer TypeCount, real MissleSpeed returns nothing
        set UnitTypeSpeed[TypeCount] = MissleSpeed
    endfunction
    
    function RegisterUnitWeaponDelay takes integer TypeCount, real WeaponDelay returns nothing
        set UnitTypeAttackCooldown[TypeCount] = WeaponDelay
    endfunction
    
    function RegisterUnitWeaponModel takes integer TypeCount, string EffectName returns nothing
        set UnitTypeEffectName[TypeCount] = EffectName
    endfunction
    
    private function GetUnitDataCount takes unit u returns integer
    loop
    if AS[2] == AS[1] then
        set AS[2] = 1
    else
        set AS[2] = AS[2] + 1
    endif
    
    exitwhen AS[2] == AS[1] or GetUnitTypeId(u) == UnitType[AS[2]]
    if AS[2] == AS[1] and GetUnitTypeId(u) != UnitType[AS[2]] then
        call BJDebugMsg("You have not initialized the unit type trying to attack.")
        call BJDebugMsg(" Unit name is: " + GetUnitName(u))
    endif
    endloop
    
    return AS[2]
    endfunction

edit: heres a example on how i call them..

Example

        set NewUnitType = RegisterUnitType('hspt') // Spell Breaker
        call RegisterUnitDamage(NewUnitType, 235)
        call RegisterUnitBoolean(NewUnitType, true)
        call RegisterUnitSword(NewUnitType, 450)
        call RegisterUnitAttackType(NewUnitType, ATTACK_TYPE_NORMAL)
        call RegisterUnitDamageType(NewUnitType, DAMAGE_TYPE_NORMAL)
        call RegisterUnitWeaponType(NewUnitType, WEAPON_TYPE_WHOKNOWS)
        call RegisterUnitRange(NewUnitType, true)
        call RegisterUnitWeaponDelay(NewUnitType, 0.75)
        call RegisterUnitMissleSpeed(NewUnitType, 35)
        call RegisterUnitAnimationName(NewUnitType, "attack")
        call RegisterUnitWeaponModel(NewUnitType, "Abilities\\Weapons\\BloodElfSpellThiefMISSILE\\BloodElfSpellThiefMISSILE.mdl")

 

[Berb]

By the looks of it you could simplify things quite a bit by using JassHelper.

 

[dardas]

nah, i use library, globals, etc.. (it requires quite a handy amout of stuff too.. just didnt show in the script the full code)

 

[Berb]

Could you try to explain exactly what you are trying to do again? I understand that you need to find a more efficient method of doing something but what you are doing I am not sure of.

 

[dardas]

im trying to replace wc III attack engine,
i've built a shooting system allready, im right now on the "attack",
so i registered all units i needed, and replaced their attacks.
so right now it works.. but now i want to find a better way to do that loop...

 

[Berb]

Well I understand that you're trying to replace unit attacks, but what I need to know is how you are "registering" your units; from the looks of it you register specific attack data to each type of unit, or a unit ID. In this case why don't you just use a hashtable?

private function GetUnitDataCount takes unit u returns integer
loop
if AS[2] == AS[1] then
    set AS[2] = 1
else
    set AS[2] = AS[2] + 1
endif

exitwhen AS[2] == AS[1] or GetUnitTypeId(u) == UnitType[AS[2]]
if AS[2] == AS[1] and GetUnitTypeId(u) != UnitType[AS[2]] then
    call BJDebugMsg("You have not initialized the unit type trying to attack.")
    call BJDebugMsg(" Unit name is: " + GetUnitName(u))
endif
endloop

return AS[2]
endfunction

You still haven't really explained any of this function. What exactly do you mean by "Data Count"? Why are you setting the value of this table to 1? I really don't have any clue what you're trying to accomplish with that loop, as you haven't really used those references AS[1] and AS[2] anywhere else in the code you've made available.

Also if you use vJass then why do you have separate arrays for attack-type and such, it could all be done with a single data structure to associate the data together, which it should be.

W.I.P.

struct attackdata
    real damage
    real range
    
    boolean ignoreArmor
    
    attacktype atktype
    damagetype dmgtype
    
    private static hashtable attackDataTable = InitHashtable()
    
    method addUnitType takes integer id returns boolean
        if not HaveSavedInteger(thistype.attackDataTable, id, 0) then
            call SaveInteger(thistype.attackDataTable, id, 0, attackdata.create())
            return true
        endif
        debug call BJDebugMsg("A unit is being added that has already been registered.")
        return false
    endmethod
    method addUnitTypeDamage takes integer id, real dmg returns boolean
        if not HaveSavedInteger(thistype.attackDataTable, id, 0) then
            debug call BJDebugMsg("Attempt to set the damage property of a unit-type that hasn't been registered.")
            return false
        endif
        set attackdata(LoadInteger(thistype.attackDataTable, id, 0)).damage = dmg
        return true
    endmethod
    
    method getUnitTypeDamage takes integer id returns real
        if not HaveSavedInteger(thistype.attackDataTable, id, 0) then
            debug call BJDebugMsg("Attempt to reference the damage property of a unit-type that hasn't been registered.")
            return 0
        endif
        return attackdata(LoadInteger(thistype.attackDataTable, id, 0)).damage
    endmethod
    method getUnitTypeRange takes integer id returns real
        if not HaveSavedInteger(thistype.attackDataTable, id, 0) then
            debug call BJDebugMsg("Attempt to reference the range property of a unit-type that hasn't been registered.")
            return 0
        endif
        return attackdata(LoadInteger(thistype.attackDataTable, id, 0)).range
    endmethod
endstruct

 

[dardas]

well.. i've removed it, gonna remake it with structs.. i guess it is best solution..
thought i will have to learn struct mui basics.. dam ;p
well,. thanks for the help, and nice job on the projectile system btw ^^ +rep

 

[Berb]

Lol thanks.

By the way I'll give you another method that you can add that will give your struct the capability (given you've somewhat followed what I've showed). To add to that there is also a mistake in the code I gave above, the method addUnitType should most likely be a static method.

    static method operator [] takes integer id returns thistype
        return LoadInteger(thistype.attackDataTable, id, 0)
    endmethod

This will allow you to use the syntax short-cut to reference the attack-data associated with a specific ID simply as attackdata['h000' - this example is associated with the Footman unit ID, it is just an example. This way you can do stuff like:

set attackdata['h000'].range = 128
set attackdata['h000'].damage = 12
set attackdata['h000'].atktype = ATTACK_TYPE_MELEE

And so on... there are lots of areas in vJass that you can take advantage of to optimize your syntax, most of which I didn't even take full advantage of when making my projectiles system - so don't feel bad if you can't nail it as long as you're trying

-- Okay, because I found many problems with my previous implementation I'm going to post you a new one to follow. This one is a little better.

New Format

module AttackDataDamage
    real            damage
endmodule
module AttackDataRange
    real            range
endmodule

struct AttackData

    private         integer     DATA__ID        
    private static  hashtable   DATA__table     = InitHashtable()
    
    implement   AttackDataDamage
    implement   AttackDataRange
    
    static method operator [] takes integer id returns thistype
        return LoadInteger(DATA__table, id, 0)
    endmethod
    
    method onDestroy takes nothing returns nothing
        call RemoveSavedInteger(thistype.DATA__table, DATA__id, 0)
    endmethod
    static method create takes integer id returns thistype
        local thistype d = 0
            if not HaveSavedInteger(DATA__table, id, 0) then
                set d = allocate()
                set d.DATA__id = id
                call SaveInteger(DATA__table, id, 0, d)
            endif
            return d
    endmethod

endstruct

Example Usage

        local AttackData yourData = AttackData.create('hfoo')
        set yourData.range = 128
        set yourData.damage = 12
        
        ..or.. (both of these execute identically)
        
        call AttackData.create('hfoo')
        set AttackData['hfoo'].range = 128
        set AttackData['hfoo'].damage = 12

The reason I modified the script is because there were completely unnecessary methods. There is no need to have "get" and "set" methods when the members are likely going to be public. In addition I've completely replaced addUnitType with a static "create" method which makes more sense, also included a method that will remove the attack-data (in the unlikely event that this is necessary).

The static method operator denoted by [] (array syntax) will reference an attack-data indexed by ID (which is given as an array parameter) and from there you can use the members from implemented modules (such as range and damage given in the example).

 

[dardas]

Modules are not the best part of me..
i've tried implenting a module using jasshelper manual like.. 50 times, and when i finnaly made it,
i realised i cant do it again, and it was pure luck.. so im kinda stuck,
and i really find destroy/create alot smarter then what i've done..
if i wont fail again on module part, this will come alot more in handy ^^

thanks =]

Edit DATA__ID is not a member of StructName..

edit: you encouraged me to remake all my projectile system in vjass xD (ranged attacks..) you have no clue how annoying it is..
its like.. the seventh time i remake it o.o each time it can handle more and more.. (it was 800 missles with 3 - 8 fps drop, then after reset it got better again)
so i wanna check out how good it will be like this..

and this is a correct way for a loop?

        private static method MissleSplit takes nothing returns nothing
            local thistype LocalThis = 0
            if this != 0 then
                loop
                
                    if LocalThis == this then
                        set LocalThis = 1
                    else
                        set LocalThis = LocalThis + 1
                    endif
                    
                        if LocalThis.Usage == true then
                            call MissleOne(LocalThis)
                        else
                            call MissleTwo(LocalThis)
                        endif
                
                exitwhen LocalThis == this
                endloop
            endif
        endmethod

or how should i do it to make it better?

btw, this is a static that calls the StructName.allocate() on each creations..