[vJASS] Help optimize this script

[jim7777]

currently making modifying some spell (seismic slam) since i kinda like the fly fly fly code of it (don't bother asking why )
but i don't know why this spell keeps on giving an fps drop of around 2-6 on each cast

scope GrindingPunisher initializer Init

    globals
    // Configurables:

    // Skills:
        private constant integer Abil_id = 'A022'
        // Raw code of the main hero ability.
        private constant integer Flytrick = 'Amrf'
        // Raw code for "Crow Form". Does not need to be changed in most cases.

    // Others:
        private constant real Interval = 0.03   // Interval for periodic timer.
        private constant real Diff = 0.00       // Distance from cast point that the unit will land.
        private constant real Timescale = 0  // How fast the unit spins.
        private constant real SLASH_DISTANCE = 225.
        private constant real TREE_RADIUS = 225
        private constant real AIR_TIME = 0.8
        private constant real SECTORSIZE=0.0 //Set the size of the sector you want to apply the effect to, between -1 and 1
        private constant real RADIUS = 250
        private constant real MAX_HEIGHT = 350
        private constant integer MAX_SLASH = 2
        private constant string End_anim = "stand"
        // End animation of the unit.
        private constant string Dirt_sfx = "war3mapImported\\NewDirtEXNofire.mdx"
        // Effect made at the end of the spell on land.
        private constant string WEAPON = "Abilities\\Weapons\\PhoenixMissile\\Phoenix_Missile.mdl"
        // Same as above but for landing on water.
        private constant boolean Destroytrees = true
        // Whether or not to destroy trees.
    endglobals

    //=======================================================================

    //=======================================================================//
    //  DO NOT TOUCH PAST THIS POINT UNLESS YOU KNOW WHAT YOUR ARE DOING!!   //
    //=======================================================================//

    private keyword Data

    globals
        private rect Rect1 = null
        private integer index
        private group dmgGroup = null
        private real maxLoop = (AIR_TIME / Interval)
        private real heightCalc = SquareRoot(MAX_HEIGHT)
    endglobals

    //=======================================================================
    private function KillEnumDestructables takes nothing returns nothing
        if IsDestructableTree(GetEnumDestructable()) then
            call KillDestructable(GetEnumDestructable())
        endif
    endfunction

    //=======================================================================
    private struct Data
        unit cast
        real cos
        real sin
        real movedist
        real dmg
        real height
        real facing
        effect eff
        integer laps
        integer slash
        timer time 

        static method CheckAirGroup takes nothing returns boolean
            local thistype dat = index
            local unit f = GetFilterUnit()
            local real angBetween
            //local real theta
            local real cx = GetUnitX(dat.cast)
            local real cy = GetUnitY(dat.cast)
            if not IsUnitType(f,UNIT_TYPE_DEAD) and IsUnitEnemy(f,GetOwningPlayer(dat.cast)) and not IsUnitType(f,UNIT_TYPE_STRUCTURE) then 
                set angBetween = Atan2(GetUnitY(f)-cy,GetUnitX(f)-cx)
                /*set theta = Cos(dat.facing-angBetween)
                if theta>SECTORSIZE then*/
                    if Damage_Spell(dat.cast,f,dat.dmg) then
                        call ShowFloatText_Damage(f,dat.dmg)
                        call KBS_BeginEx(f,SLASH_DISTANCE,AIR_TIME,angBetween,null,TREE_RADIUS,false,true)
                    endif
                //endif
            endif
            set f = null
            return false
        endmethod
        
        static method CheckGroup takes nothing returns boolean
            local thistype dat = index
            return IsUnitEnemy(GetFilterUnit(),GetOwningPlayer(dat.cast)) and not IsUnitType(GetFilterUnit(),UNIT_TYPE_DEAD) and not IsUnitType(GetFilterUnit(),UNIT_TYPE_STRUCTURE) 
        endmethod
        
        static method DamageGroup takes nothing returns nothing
            local thistype dat = index
            local unit f = GetEnumUnit()
            local real cx = GetUnitX(dat.cast)
            local real cy = GetUnitY(dat.cast)
            local real angBetween = Atan2(GetUnitY(f)-cy,GetUnitX(f)-cx)
            local real theta = Cos(dat.facing-angBetween)
            if theta>SECTORSIZE then
                if Damage_Spell(dat.cast,f,dat.dmg) then
                    call ShowFloatText_Damage(f,dat.dmg)
                    call KBS_BeginEx(f,SLASH_DISTANCE,1.5,angBetween,null,TREE_RADIUS,false,true)
                endif
            endif
            set f = null
        endmethod
        
        static method Update takes nothing returns nothing
            local thistype this = GetTimerData(GetExpiredTimer())
            local real count = 0
            local real height = 0
            local real newx = 0
            local real newy = 0
            if this.slash < MAX_SLASH then
                set this.laps = this.laps + 1
                if this.laps >= maxLoop then
                    set this.slash = this.slash + 1
                    set this.laps = 1
                    set index = this
                    call GroupEnumUnitsInRange(dmgGroup,newx,newy,RADIUS,Condition(function thistype.CheckGroup))
                    //call ForGroup(dmgGroup,function thistype.DamageGroup)
                else
                    set count = (50/maxLoop)*this.laps
                    call BJDebugMsg(R2S(count) + "/" + R2S(TimerGetElapsed(this.time)))
                    set height = (count-heightCalc) * (count-heightCalc)
                    set newx = GetUnitX(this.cast) + this.movedist * this.cos
                    set newy = GetUnitY(this.cast) + this.movedist * this.sin
                    if IsTerrainPathable(newx,newy,PATHING_TYPE_WALKABILITY) == false then
                        call SetUnitX(this.cast,newx)
                        call SetUnitY(this.cast,newy)
                    endif
                    call SetUnitFlyHeight(this.cast,MAX_HEIGHT-height,0.00)
                endif
            else
                set newx = GetUnitX(this.cast)
                set newy = GetUnitY(this.cast)
                set index = this
                call GroupEnumUnitsInRange(dmgGroup,newx,newy,RADIUS,Condition(function thistype.CheckGroup))
                //call ForGroup(dmgGroup,function thistype.CheckGroup)
                call SetUnitFlyHeight(this.cast,GetUnitDefaultFlyHeight(this.cast),0.00)
                call SetUnitAnimation(this.cast,"Stand")
                call SetUnitTimeScale(this.cast,1)
                call PauseUnit(this.cast,false)
                call DestroyEffect(this.eff)
                if Destroytrees then
                    call SetRect(Rect1,newx-TREE_RADIUS,newy-TREE_RADIUS,newx+TREE_RADIUS,newy+TREE_RADIUS)
                    call EnumDestructablesInRect(Rect1,null,function KillEnumDestructables)
                endif
                //call DestroyEffect(AddSpecialEffect(Dirt_sfx,newx,newy))
                call SetUnitX(this.cast,newx)
                call SetUnitY(this.cast,newy)
                call ReleaseTimer(this.time)
                call this.destroy()
            endif
        endmethod             
        
        static method start takes nothing returns nothing
            local thistype this = thistype.allocate()
            local real targx = GetSpellTargetX()
            local real targy = GetSpellTargetY()
            local integer lvl = GetUnitAbilityLevel(GetTriggerUnit(),Abil_id)
            set this.cast = GetTriggerUnit()
            set this.facing = GetUnitFacing(this.cast)// Atan2(targy-GetUnitY(this.cast),targx-GetUnitX(this.cast))
            set this.cos = Cos(this.facing* bj_DEGTORAD)
            set this.sin = Sin(this.facing* bj_DEGTORAD)
            set this.movedist = SLASH_DISTANCE/maxLoop
            set this.slash = 0
            set this.height = 0
            set this.laps = 0
            set this.dmg = GetHeroStr(this.cast,true)*(0.5+(0.25*lvl))
            set this.eff = AddSpecialEffectTarget(WEAPON,this.cast,"weapon")
            call UnitAddAbility(this.cast,Flytrick)
            call UnitRemoveAbility(this.cast,Flytrick)
            call SetUnitAnimation(this.cast,"Stand Ready")
            call SetUnitTimeScale(this.cast,Timescale)    
            call PauseUnit(this.cast,true)
            set this.time = NewTimer()
            call SetTimerData(this.time,this)
            call TimerStart(this.time,Interval,true,function thistype.Update)
            set index = this
            call GroupEnumUnitsInRange(dmgGroup,GetUnitX(this.cast),GetUnitY(this.cast),RADIUS,Condition(function thistype.CheckAirGroup))
            //call ForGroup(dmgGroup,function thistype.CheckAirGroup)
        endmethod
        
        static method conds takes nothing returns boolean
            if GetSpellAbilityId() == Abil_id then
                call thistype.start.execute()
            endif
            return false
        endmethod
        
        static method onInit takes nothing returns nothing
            /*local trigger t = CreateTrigger()
            //call GT_RegisterStartsEffectEvent(t,Abil_id)
            call TriggerAddAction(t,function thistype.start)*/
            call CommonEvent.registerPlayerUnitEvent(EVENT_PLAYER_UNIT_SPELL_EFFECT,Condition(function thistype.conds)) 
            //set t = null
        endmethod
    endstruct

    //=======================================================================
    /*private function Conditions takes nothing returns boolean
        return GetSpellAbilityId() == Abil_id
    endfunction*/
    //=======================================================================
    private function Init takes nothing returns nothing
        local integer i = 0
        call AbilityPreload(Abil_id)
        call Preload(WEAPON)
        call Preload(Dirt_sfx)
        set Rect1 = Rect(0.00,0.00,1.00,1.00)
        set dmgGroup = CreateGroup()
    endfunction

endscope

 

[baassee]

woah this is a mess, my eyes really hurts. I would say it all reduces fps not the greatest code I've seen if I put it like that

 

[jim7777]

ohh yeah, forgot to mention that the base code (around 70%) of this spell is from an old, ancient spell that you can find by asking Mr. Google

 

[Magtheridon96]

Tip:

Make the struct extend an array, then, add these to the members of the struct:

private static integer ic = 0
private static integer ir = 0
private thistype rn

Intead of declaring a local thistype this and setting it ".allocate()" (which doesn't exist in structs that extend arrays), do this:

local thistype this
if 0==ir then
    set ic=ic+1
    set this=ic
else
    set this=ir
    set ir=.rn
endif

And, instead of calling ".deallocate()", do this:

set .rn=ir
set ir=this

That should increase efficiency.

Now, another thing, don't use GTrigger -_-
Use CommonEvent in the Jass section.
It requires Event by Nestharus.

And instead of TriggerAddAction, just use TriggerAddCondition for the actions and return false at the end.

    private function Conditions takes nothing returns boolean
        return GetSpellAbilityId() == Abil_id
    endfunction

This function is completely useless


edit

method onDestroy takes nothing returns nothing

GAAAAAAAAHH!
Just use a destroy method instead >.<

 

[jim7777]

updated the code
**not using commonevent D: kinda long API implementation D:

anyways, removing the special effect reduces the fps dropping by 1-2 the rest are from the Update method ( i think ) dunno whats wrong with that method

 

[WaterKnight]

For testing, just deactivate parts of the code to get a clue whether the performance drop derives from these places. Also can you show the functions ShowFloatText_Damage and KBS_BeginEx?

 

[Bannar]

    private function Area takes integer lvl returns real
        return 225.00 // Area of effect for destroying trees.
    endfunction

It would be better if you either get rid of passed integer variable with 'takes nothing' or do: 'return (lvl * 0) + 225.'.

 

[Troll-Brain]

Changing the allocate/destroy struct instances methods won't have a noticeable effect.

Focus on the periodic thingies.
Instead of ForForce try to code directly in the boolexpr of GroupEnum... and always return false inside this boolexpr since you don't need to keep units during time.
Avoiding ForFoce will open less new threads which are costly.
Also don't use locations but directly functions which are using X/Y coordinates, this way you won't have to create/destroy handles which are costly.
And if you use GetLocationZ you can use a static point and use MoveLocation instead of create/destroy a new location.

I don't remember how vJass handles timer callbacks which are lower in the script than the call.

method start : (call TimerStart(this.time,Interval,true,function thistype.Update)

It could evaluate the method update, to be sure place the method update more on the top (above start).
TriggerEvaluate is much less efficient than a simple function call.

By default vJass evaluates methods which are below the call of it, but you can disable this automatic behavior.

http://www.hiveworkshop.com/forums/1996864-post13.html

 

[Magtheridon96]

**not using commonevent D: kinda long API implementation D:

I updated it this morning
The API is now only about 4-6 functions I think.

 

[watermelon_1234]

No point in CheckAirGroup, use DmgAirGroup directly in your group enumeration.

Should be using this.level consistently in the start method.

No point at all in using locations in this spell code.

Arrange your methods so that methods call methods above them.

local real angBetween=Atan2(GetUnitY(f)-dat.casty,GetUnitX(f)-dat.castx)
local real theta = Cos(dat.facing-angBetween)

are pretty heavy calculations; you should calculate them only if the unit meets the conditions.

For booleans, == false isn't really necessary, just put not before the boolean, like not IsUnitType(f, UNIT_TYPE_DEAD)

 

[jim7777]

updated with the new suggestions and code cleaning, still having some lots of fps dropping D:

kinda have some debug
and the start method reduces the fps by around 0.5 - 1.xx (but it will return to normal by 0.2-0.7 or never)

then when update method comes, fps drops superbly around 2-12 fps (after the spell, fps will return by 2-4 fps)

so if fps is 60
it will drop to 47-52 fps remaining

 

[Troll-Brain]

For testing, just deactivate parts of the code to get a clue whether the performance drop derives from these places. Also can you show the functions ShowFloatText_Damage and KBS_BeginEx?

It could be because of these functions, care to share them ?

 

[jim7777]

well i tried commenting every function about the group same problem occurs

perhaps someone may make a similar spell eh?

 

[Troll-Brain]

GroupEnumInRect should be more efficient (in theory), i mean instead of checking for all units and calculate the distances it should only care about units which are in the rect.
You don't need create dynamically a rect, you can move a static one.

But while you enum this you will have to check their distance to the center of this rect, so the overhead could be worse in fact.

Also using globals instead of locals in the periodic and group enum filters should be more efficient (probably not in a noticeable way though).

Finally, try to optimize your map script with wc3mapoptimizer.

 

[jim7777]

hmm ok but still, i thought of having too much calculations on my spell...

or maybe really someone may make a similar but efficient/faster script for the Update method ??

or someone help me optimize the start method @@ i'm thinking of some data isn't parsed/used well, i don't know where

edit
after some long series of test, i found out that these lines makes super fps drop:

set angle = Atan2((GetSpellTargetY()-this.casty),(GetSpellTargetX()-this.castx))
set this.cos = Cos(angle)
set this.sin = Sin(angle)

i don't know why.. what's the problem with this line @_@

 

[Troll-Brain]

You mean the one in CheckGroup ?

Because if you're talking about the one in the init method that doesn't make any sense since it's called only one time when the spell event fire.

 

[jim7777]

the one in the start method, i really don't know why and as you said, it only runs once.

but still, fps dropping still occur in the update method.. This code is really awesome D:

 

[Troll-Brain]

Hmm, make sure the start effect event fire only one time by displaying a text message when this event fire, because depending the spell based it could fire X times and maybe also with the pause/unpause unit stuff.

 

[Magtheridon96]

Cos and Sin are very slow functions.

 

[Troll-Brain]

Cos and Sin are very slow functions.

Not in jass if my last old benchmark was good (i compared Cos, Sin vs a global array read + R2I).
The array read was slower, and about the same without R2I (integer as an index but that doesn't make sense )

But i used the japi custom natives and wasn't aware about the matter of string/function names though.

 

[jim7777]

ok updated the script with common event.. dunno but it now gives a fast drop rate @_@

never mind the debug msg, i just use it for some debugging purposes

 

[Troll-Brain]

Now your method CheckGroup is fairly useless, you add or not units but never play with them (each time you use GroupEnum... on a group you clear it)

 

[Magtheridon96]

ok updated the script with common event..

omg i luv uz <3
lol

One question: What are your computer's technical specifications? (RAM, Processor speed, number of processors, number of cores, etc..)

 

[jim7777]

Intel Core i3 2.93GHz (up to 3.10Ghz)
4GB ram
Win7 32 bit
NVIDIA Graphics Card (nvm the model)

Now your method CheckGroup is fairly useless, you add or not units but never play with them (each time you use GroupEnum... on a group you clear it)

i don't get it lol

 

[Magtheridon96]

Intel Core i3 2.93GHz (up to 3.10Ghz)
4GB ram
Win7 32 bit
NVIDIA Graphics Card (nvm the model)

Then I'm sure it's your code and not your computer >.<

Maybe if you comment out a few things and run the script, we could know what's causing it to lag for sure.

 

[Troll-Brain]

        static method CheckGroup takes nothing returns boolean
            local thistype dat = index
            return IsUnitEnemy(GetFilterUnit(),GetOwningPlayer(dat.cast)) and not IsUnitType(GetFilterUnit(),UNIT_TYPE_DEAD) and not IsUnitType(GetFilterUnit(),UNIT_TYPE_STRUCTURE) 
        endmethod

Now check your GroupEnum functions using this filter, you add or not the filter unit to dmdGroup, but dgmGroup is always reset by an other GroupEnum before you do anything with these units in the group.

Actually you could just do that with this filter, the result will be the same, and more efficient (but still useless) :

static method CheckGroup takes nothing returns boolean
   return false
endmethod

That could explain why you don't have fps drop anymore.