[JASS] My first vJass spell, is it efficient?

[Zypher]

My first vJass spell, is it efficient?

Units caught in the AoE would be affected by a random effect of the following.
1 ~ Stunned for Heroes Level
2 ~ Damage Units by Heroes Level x 50
3 ~ Hero Loses 50% of Max Health (Can kill)
4 ~ Debuffs unit's armor by Heroes Level
5 ~ Debuffs unit's attack speed, damage and movement speed by Heroes Int

vJass

//===========================================================================\\
//===============================SETUP=======================================\\
//===========================================================================\\ 
// 1. Copy into your map                                                     \\
// 2. Copy ability Armor Debuff (-4) into your map                           \\
// 3. Set rawcodes for setup data and change to your liking                  \\
// 4. Enjoy                                                                  \\
//                                                                           \\
// Author: Zypher@USeast                                                     \\
//===========================================================================\\
scope Random5 initializer Init
    globals
        private constant integer SPELL_ID = 'ANsi'  //the rawcode of the spell
        private constant integer DUMMY_ID = 'hf00'  //raw code of the dummy unit
        private constant integer ARMOR_DEBUFF_ID = 'A000' //rawcode of custom item ability Armor Debuff (-4)
        private constant integer ATTACK_SPEED_DEBUFF_ID = 'A001' //rawcode of custom item ability Attack Speed Debuff (-0.3)
        private constant integer DAMAGE_DEBUFF_ID = 'A002' //rawcode of custom item ability Damage Debuff (-15)
        private constant damagetype D_TYPE = DAMAGE_TYPE_NORMAL //the attack type of the spell
        private constant attacktype A_TYPE = ATTACK_TYPE_MAGIC  //the damage type of the spell
        private constant weapontype W_TYPE = WEAPON_TYPE_WHOKNOWS //default weapontype change if you want to
        private constant string AOE_EFFECT = "Units\\NightElf\\Wisp\\WispExplode.mdl"  //effect created on cast of Aoe location
        private constant string DAMAGE_EFFECT = "Abilities\\Weapons\\Bolt\\BoltImpact.mdl"  //effect that will be created when we damage units
        private constant string STUN_EFFECT = "Abilities\\Spells\\Human\\Thunderclap\\ThunderclapTarget.mdl" //stun effect path
        private constant boolean IS_RANGED = true //spell damage is ranged attack
        private constant boolean IS_MELEE = false //spell damage is melee attack.
        private constant boolean INT_INCLUDE_BONUSES = true //decides if bonuses should effect hero stats for the spell
    endglobals
    
    private function Aoe takes integer level returns real
    //returns the Aoe the spell will affect
        return I2R(level * 175)
    endfunction
    
    private function Stuntime takes integer level returns real
    //returns the stun time to enemies
        return I2R(level * 1)
    endfunction
    
    private function Percenthealthloss takes nothing returns integer
    //returns the % of healthloss the caster takes
        return 50
    endfunction
    
    private function armorloss takes integer level returns integer
    //returns the amount of armor lost by target units
        return level
    endfunction
    
    private function armorlosstime takes integer level returns real
    //returns the time armor loss is active on target units
        return I2R(20 + (level * 10))
    endfunction
    
    private function Damage takes integer level returns real
    //returns the damage to enemies
        return I2R(level * 50)
    endfunction
    
    private function Targets takes nothing returns boolean
    //the units the spell will affect
    local unit t = GetEnumUnit()
        return (GetWidgetLife(t) > 0.405) and (IsUnitType(t, UNIT_TYPE_STRUCTURE) == false) and (IsUnitType(t, UNIT_TYPE_MAGIC_IMMUNE) == false) and (IsUnitType(t, UNIT_TYPE_FLYING) == false)
    endfunction
    
    private function debufftime takes integer level returns real
        return I2R(20 + (level * 10))
    endfunction
    
    private function movespeeddown takes integer Int, real movespeed returns real
     //returns the movespeed of the unit minus the intelligence of the hero
    return movespeed - I2R(Int)
    endfunction
    
    private function movespeedup takes integer Int, real movespeed returns real
     //returns the movespeed of the unit plus the intelligence of the hero
    return movespeed + I2R(Int)
    endfunction
    
//===========================================================================\\
//=============================SETUP END=====================================\\
//===========================================================================\\
    globals
        private unit udg_temp_unit
        private integer udg_level
        private unit udg_caster
        private integer INT
    endglobals
    
    private function Conditions takes nothing returns boolean
        return GetSpellAbilityId() == SPELL_ID
    endfunction
    
    private function stun takes nothing returns nothing
        local unit u = udg_temp_unit
        local effect e = AddSpecialEffectTarget(STUN_EFFECT, u, "overhead")
        call PauseUnit(u, true)
        call TriggerSleepAction(Stuntime(udg_level))
        call PauseUnit(u, false)
        call DestroyEffect(e)
        set u = null
        set e = null
    endfunction
    
    private function unitarmorloss takes nothing returns nothing
        local unit u = udg_temp_unit
        call UnitAddAbility( u, ARMOR_DEBUFF_ID )
        call TriggerSleepAction(armorlosstime(udg_level))
        call UnitRemoveAbility( u, ARMOR_DEBUFF_ID)
        set u = null
    endfunction
    
    private function executeunitarmorloss takes nothing returns nothing
        local unit u = GetEnumUnit()
        set udg_temp_unit = u
        call ExecuteFunc("unitarmorloss")
        set u = null
    endfunction
    
    private function executestun takes nothing returns nothing
        local unit u = GetEnumUnit()
        set udg_temp_unit = u
        call ExecuteFunc("stun")
        set u = null
    endfunction
    
    private function damage takes nothing returns nothing
        local unit u = GetEnumUnit()
        call UnitDamageTarget(udg_caster, u, Damage(udg_level), IS_MELEE, IS_RANGED, A_TYPE, D_TYPE, W_TYPE)
        set u = null
    endfunction

    private function killunit takes unit u returns nothing
        call KillUnit(u)
        call TriggerSleepAction(7.00)
        call RemoveUnit(u)
        set u = null
    endfunction
    
    private function debuff takes nothing returns nothing
        local unit u = udg_temp_unit
        local real movespeed = GetUnitMoveSpeed(u)
            call SetUnitMoveSpeed( u, ( movespeeddown( INT , movespeed )))
                call UnitAddAbility( u, ATTACK_SPEED_DEBUFF_ID )
                    call UnitAddAbility( u, DAMAGE_DEBUFF_ID )
        call TriggerSleepAction(debufftime(udg_level))
            call SetUnitMoveSpeed( u, ( movespeedup( INT , movespeed )))
                call UnitRemoveAbility( u, ATTACK_SPEED_DEBUFF_ID)
                    call UnitRemoveAbility( u, DAMAGE_DEBUFF_ID ) 
        set u = null
    endfunction
    
    private function executedebuff takes nothing returns nothing
        local unit u = GetEnumUnit()
        set udg_temp_unit = u
        call ExecuteFunc("debuff")
        set u = null
    endfunction
    
    private function Actions takes nothing returns nothing
        local location spellLoc = GetSpellTargetLoc()
        local real spellX = GetLocationX(spellLoc)
        local real spellY = GetLocationY(spellLoc)
        local unit caster = GetTriggerUnit()
        local integer level = GetUnitAbilityLevel(caster, SPELL_ID)
        local group g = CreateGroup()
        local real realAoe = Aoe(level)
        local integer random = GetRandomInt( 1, 5)
        call GroupEnumUnitsInRangeOfLoc( g, Location(spellX, spellY), realAoe, Filter(function Targets))
        set udg_level = level
        set udg_caster = caster
        set INT = GetHeroInt(udg_caster, INT_INCLUDE_BONUSES)
        if random == 1 then
            call ForGroup(g, function executestun)
        endif   
        if random == 2 then
            call ForGroup(g, function damage)
        endif
        if random == 3 then
            if GetUnitStatePercent(udg_caster, UNIT_STATE_LIFE, UNIT_STATE_MAX_LIFE) <= 50 then
            call killunit(udg_caster)
            call SetUnitState(udg_caster, UNIT_STATE_LIFE, GetUnitState(udg_caster, UNIT_STATE_MAX_LIFE) * RMaxBJ(0,( GetUnitStatePercent(udg_caster, UNIT_STATE_LIFE, UNIT_STATE_MAX_LIFE) - Percenthealthloss() )) * 0.01)
            endif
        else
        endif
        if random == 4 then
        call ForGroup(g, function executeunitarmorloss)
        else
        endif
        if random == 5 then
        call ForGroup(g, function executedebuff)
        else
        endif
        call RemoveLocation(spellLoc)
        set spellLoc = null
        set caster = null
        call DestroyGroup(g)
        set g = null
    endfunction
        
    private function Init takes nothing returns nothing
        local trigger Random5 = CreateTrigger()
        call TriggerRegisterAnyUnitEventBJ( Random5, EVENT_PLAYER_UNIT_SPELL_EFFECT )
        call TriggerAddCondition( Random5, Condition( function Conditions ) )
        call TriggerAddAction( Random5, function Actions )
        set Random5 = null
        //preloading effects
        call Preload(AOE_EFFECT)
        call Preload(DAMAGE_EFFECT)
        call Preload(STUN_EFFECT)
        
        //preloading the ability
        set bj_lastCreatedUnit = CreateUnit(Player(0), DUMMY_ID, 0, 0, 0)
        call UnitAddAbility(bj_lastCreatedUnit, SPELL_ID)
        call KillUnit(bj_lastCreatedUnit)
        endfunction
endscope

This post will be updated as I learn how awful my coding is.

 

[xD.Schurke]

the only advantages of vJass you are using is the global declaration, yo dont use structs at all.
Also for a normal Jass code it's not efficient, you use triggersleep function and the ForGroup function
i can say it's not efficient at all, first you should learn how to use jass in an efficient way before trying vjass

 

[Eleandor]

Trigger sleep action is efficient... It's just not accurate when it comes down to small numbers.
The ForGroup function isn't inefficient either as far as I know. I don't think there's a big difference between that and a FirstOfGroup loop.

So those things have nothing to do with efficiency.

However, when you have a function like

private function damage takes nothing returns nothing
        local unit u = GetEnumUnit()
        call UnitDamageTarget(udg_caster, u, Damage(udg_level), IS_MELEE, IS_RANGED, A_TYPE, D_TYPE, W_TYPE)
        set u = null
    endfunction

you should better simply use

private function damage takes nothing returns nothing
        call UnitDamageTarget(udg_caster, GetEnumUnit(), Damage(udg_level), IS_MELEE, IS_RANGED, A_TYPE, D_TYPE, W_TYPE)
    endfunction

 

[xD.Schurke]

Trigger sleep action is efficient... It's just not accurate when it comes down to small numbers.
The ForGroup function isn't inefficient either as far as I know. I don't think there's a big difference between that and a FirstOfGroup loop.

So those things have nothing to do with efficiency.

However, when you have a function like

private function damage takes nothing returns nothing
        local unit u = GetEnumUnit()
        call UnitDamageTarget(udg_caster, u, Damage(udg_level), IS_MELEE, IS_RANGED, A_TYPE, D_TYPE, W_TYPE)
        set u = null
    endfunction

you should better simply use

private function damage takes nothing returns nothing
        call UnitDamageTarget(udg_caster, GetEnumUnit(), Damage(udg_level), IS_MELEE, IS_RANGED, A_TYPE, D_TYPE, W_TYPE)
    endfunction

first of all, it depends on what you understand under efficience. the forgroup function is inefficient, because you call another function and that makes it slower so forgroup = inefficient
also to use for each thing another function is not that efficient

 

[Justify]

Trigger sleep action is efficient... It's just not accurate when it comes down to small numbers.

No, it's always inaccurate like hell... start a timer, wait 2 seconds and stop it, read the elapsed time. It will be at around 2.2-2.3 seconds :/

 

[Eleandor]

first of all, it depends on what you understand under efficience. the forgroup function is inefficient, because you call another function and that makes it slower so forgroup = inefficient

Orly?

local group f = ...
local group g = a copy of f, as you'll need f for other purposes and don't want to clear it
local unit u
loop
    set u = FirstOfGroup(g)
    exitwhen u == null
    call GroupRemoveUnit
endloop

I count 2 function calls per unit in the group. The ForGroup function probably uses a simple iterator through all units in the group and thus doesn't need to create a dummy group, copy all units from the original group to the dummy group, then make a loop by retrieving the first unit of the dummy group, remove it and repeat. Thus, ForGroup is most probably implemented in a more efficient way than you could possibly do in jass.

Besides, efficiency isn't measured in how many function calls are made. Efficiency is measured in time, i.e. how long it takes for a function to execute. And in my experience, both ForGroup and the loop alternative are equally fast. The only disadvantage ForGroup has is that it can't pass parameters to the callback code, and thus it's "easier" to use the alternative loop.

No, it's always inaccurate like hell... start a timer, wait 2 seconds and stop it, read the elapsed time. It will be at around 2.2-2.3 seconds :/

And it's always the same offset of around 0.25 seconds. So if you need to wait 2 seconds, you wait 1.75 seconds instead, so it'll be "near" 2 seconds. Close enough at least, with a decent accuracy (when it comes to waiting a "long" time such as 2 seconds, it's not that bad that you're 0,1 seconds off).

While it's "always" inaccurate, you can't really say it's inaccurate "like hell". An inaccuracy of 0,2 seconds on a 30 seconds wait is not a big deal, hence why I said the inaccuracy of TriggerSleepAction only causes problems on small time intervals, i.e. waiting less than 0,25 seconds.

That doesn't mean I'm trying to support the use of TriggerSleepAction over timers. Timers are obviously more accurate, but it doesn't mean that a script using TriggerSleepAction is a bad script.

 

[xD.Schurke]

Orly?

local group f = ...
local group g = a copy of f, as you'll need f for other purposes and don't want to clear it
local unit u
loop
    set u = FirstOfGroup(g)
    exitwhen u == null
    call GroupRemoveUnit
endloop

I count 2 function calls per unit in the group. The ForGroup function probably uses a simple iterator through all units in the group and thus doesn't need to create a dummy group, copy all units from the original group to the dummy group, then make a loop by retrieving the first unit of the dummy group, remove it and repeat. Thus, ForGroup is most probably implemented in a more efficient way than you could possibly do in jass.

thats not what i ment, in the function refering to the forgroup function you normaly have some more calls, that means u first call another function to call some other functions, but with the group loop, you dont call another function to call other functions (hope you understand what i mean)

 

[Eleandor]

No, I don't understand what you mean.

 

[xD.Schurke]

i try to explain it in another way

group loop:
calls natives min. 2 per unit

ForGroup:
calls custom function which calls nativs

 

[Eleandor]

right, but the natives in the custom function have to be called inside the group loop too:


group loop =
FirstOfGroup call
GroupRemoveUnit call
all native calls to be done (n calls)

ForGroup =
callback function call containing
all native calls to be done (n calls)

so the group loop still has 1 more function call per unit.

 

[xD.Schurke]

you are right, but may the group loop is faster, dunno

 

[Eleandor]

It's about the same, but group loop is easier to use

 

[LeP]

But, if you use ForGroup(), the ForGroup-Action has its own Microop-count so you can use some more complex actions in it.
If you would use those 'complex-actions' in you exitwhen u==null loop you would probaly hit the op-limit.
(if i remeber correctly :>)

 

[Zypher]

the only advantages of vJass you are using is the global declaration, yo dont use structs at all.
Also for a normal Jass code it's not efficient, you use triggersleep function and the ForGroup function
i can say it's not efficient at all, first you should learn how to use jass in an efficient way before trying vjass

I think I will implement timers and structs into the script, I just need to learn them a bit more first. Could you point out what's wrong so i can improve? (besides forgroup and timers)

 

[Eleandor]

But, if you use ForGroup(), the ForGroup-Action has its own Microop-count so you can use some more complex actions in it.
If you would use those 'complex-actions' in you exitwhen u==null loop you would probaly hit the op-limit.
(if i remeber correctly :>)

They aren't complex actions at all, and I've never had any problems with it. In fact, I've never really had any big problems with the op-limit in the first place, except those rare occasions when I made a stupid mistake causing my script to be stuck in an infinite loop or something like that...

 

[Justify]

But, if you use ForGroup(), the ForGroup-Action has its own Microop-count so you can use some more complex actions in it.
If you would use those 'complex-actions' in you exitwhen u==null loop you would probaly hit the op-limit.
(if i remeber correctly :>)

In the moment you hit the op-limit because of such a loop, your map has to lag like hell. Why? Cause to get 300.000 Microops, you need either a freaking long loop with hundreds of calls or thousands of units in your map

 

[LeP]

Ofc in most times hitting the op limit in an group-loop jsut happen due bad coding BUT if you really need a own op-counter for each unit,you'll be glad to know that ForGroup() has its own op-counter.

For example a particle-system should use ForGroup() for the collision, imo.

 

[Deaod]

1.) dont use TSA.
2.) Use proper indentation
3.) make use of elseif statements.
4.) dont crash WC3 by using ExecuteFun() on invalid functions (add SCOPE_PRIVATE)
5.) dont use locals for every bit of code. I can understand that they are useful, but please remember that they need to be allocated, thus using locals is slower, when you only access the value of the local once.
6.) if GetUnitStatePercent(udg_caster, UNIT_STATE_LIFE, UNIT_STATE_MAX_LIFE) <= 50 then i see a hardcoded balance related value that is already in your calibration section.
7.) if you already kill the caster, then why do you bother changing its health?
8.) every units handleid will never get recycled when you have had it inside the AoE of this spell.


unrelated:
ForGroup() makes a group leak when destroyed. Thats why we recycle groups.
Oh, and you probably wont ever create something thats faster than ForGroup().

another thing: you probably dont want to know how fast you can reach the op-limit. And this is in no way related to framelags.

 

[Zypher]

Deaod thanks, that was what I was looking for.