[vJASS] Spell Efficiency

[pks9067]

Hello! This is first vJass spell I'm attempting.
It shoots 7 flame balls which move in curve.

Is this an efficient way of making a spell?

scope FlameBarrage

    private keyword Data

    globals
        private Data tmps
    endglobals

    private struct MData extends array
    
        effect eff
        real p0x
        real p0y
        real p1x
        real p1y
        real p2x
        real p2y
        real count

    endstruct

    private struct Data
    
        group missiles
        group damaged
        integer count
        
        method onDestroy takes nothing returns nothing
            call ReleaseGroup(.missiles)
            call ReleaseGroup(.damaged)
            call .stopPeriodic()
        endmethod
        
        private static method Damage takes nothing returns boolean
            local unit u = GetFilterUnit()
            local FBBuff fb
            if EnemyCheck(u) and (not IsUnitInGroup(u,tmps.damaged)) then
                call GroupAddUnit(tmps.damaged,u)
                set fb = FBBuff.create(u)
                set fb.caster = GroupSource
                call fb.destroyTimed(6)
                call Damage_Spell(GroupSource,u,85)
            endif
            set u = null
            return false
        endmethod
        
        private static method Fly takes nothing returns nothing
            local unit u = GetEnumUnit()
            local MData dat = MData[GetUnitIndex(u)]
            local real t = 1-dat.count
            local real tt = t*t
            local real td = 2*t*dat.count
            local real dd = dat.count*dat.count
            local real X = tt*dat.p0x + td*dat.p1x + dd*dat.p2x
            local real Y = tt*dat.p0y + td*dat.p1y + dd*dat.p2y
            call SetUnitX(u,X)
            call SetUnitY(u,Y)
            if dat.count >= 1 then
                set tmps.count = tmps.count-1
                call GroupRemoveUnit(tmps.missiles,u)
                call DestroyEffect(dat.eff)
                call DestroyEffect(AddSpecialEffect("Abilities\\Spells\\Other\\Incinerate\\FireLordDeathExplode.mdl",X,Y))
                set GroupSource = hero[GetPlayerId(GetOwningPlayer(u))]
                call GroupEnumUnitsInArea(ENUM_GROUP,X,Y,90,Filter(function Data.Damage))
                set dat.eff = null
            endif
            set dat.count = dat.count+.05
            set u = null
        endmethod
        
        private method periodic takes nothing returns nothing
            if this.count > 0 then
                set tmps = this
                call ForGroup(this.missiles,function Data.Fly)
            else
                call this.destroy()
            endif
        endmethod
        implement T32x
        
        private static method DummyCreate takes player p, real x, real y, real x1, real y1, real angle returns nothing
            local unit dummy = CreateUnit(p,'hpea',x,y,0)
            local MData dat = MData[GetUnitIndex(dummy)]
            set dat.eff = AddSpecialEffectTarget("Abilities\\Weapons\\RedDragonBreath\\RedDragonMissile.mdl",dummy,"origin")
            set dat.p0x = x
            set dat.p0y = y
            set dat.p1x = x+300*Cos(angle)
            set dat.p1y = y+300*Sin(angle)
            set dat.p2x = x1+GetRandomReal(-112.5,112.5)
            set dat.p2y = y1+GetRandomReal(-112.5,112.5)
            set dat.count = 0
            call UnitApplyTimedLife(dummy,'BTLF',1.2)
            call GroupAddUnit(tmps.missiles,dummy)
            set tmps.count = tmps.count+1
            set dummy = null
        endmethod

        static method Actions takes nothing returns nothing
            local thistype dat = thistype.allocate()
            local unit caster = GetTriggerUnit()
            local real x = GetUnitX(caster)
            local real y = GetUnitY(caster)
            local real x1 = GetSpellTargetX()
            local real y1 = GetSpellTargetY()
            local real angle = Atan2(y1-y,x1-x)
            local real count = 7
            set dat.missiles = NewGroup()
            set dat.damaged = NewGroup()
            
            set tmps = dat
            loop
                exitwhen count == 0
                call thistype.DummyCreate(GetOwningPlayer(caster),x,y,x1,y1,angle+(180-45*count)*bj_DEGTORAD)
                set count = count-1
            endloop
            
            call dat.startPeriodic()
        endmethod
          
        static method onInit takes nothing returns nothing
             call TriggerAddAction(GT_RegisterStartsEffectEvent(CreateTrigger(),'A041'),function Data.Actions)
        endmethod

    endstruct

endscope

 

[Dr Super Good]

It depends.

If you have thousands of spells that means tens of thousands of arrays which is rather inefficient memory wise but most importantly produce a huge trigger script to compile. Hashtables might be slower but can allow you to allocate variable space only as you need it saving map load time and memory. Benchmarks are needed to tell the exact benifits (if any) but for small projects and for execution speed using arrays is by far the best way.

You also create objects even if the spell will never be cast in a game sessions (such as the using hero is never choosen). Equally well you also register it as if it can always be cast even if no such units ever exist. Although one can not argue with general practices in spell making, I do imagine it being more efficient to have a initialization system to control when the spells get set up and also a small scope register (like a single unit) to reduce the scope of the code running (stupid to make it register on all casts in a map where only 1 hero can cast it). Obviously you would need a deregister system in the case the hero itself gets removed or ability unlearned. This however is a bit off topic of the spell.

 

[Bribe]

Your alignment got messed up here:

    endmethod
            
    static method onInit takes nothing returns nothing
        call TriggerAddAction(GT_RegisterStartsEffectEvent(CreateTrigger( ), 'A031' ), function thistype.Actions)
    endmethod

    endstruct

You also should use "extends array" on structs that don't generate instances.

 

[pks9067]

thank you very much
what does extends array do?

 

[Nestharus]

what does extends array do?

Removes all vjass generated code for that struct, thus making it totally empty except for what you write.


Furthermore, I don't really suggest using groups for this. It makes a lot more sense to use a stack actually.

Also try to save on your calculations

Math is heavy in JASS

            local real X = (t*t*MData[id].p0x)+(2*t*MData[id].count*MData[id].p1x)+(MData[id].count*MData[id].count*MData[id].p2x)
            local real Y = (t*t*MData[id].p0y)+(2*t*MData[id].count*MData[id].p1y)+(MData[id].count*MData[id].count*MData[id].p2y)

Loop backwards so that you can compare to 0.

            loop
                exitwhen count > 4
                set dummy = CreateUnit(p,'hpea',x,y,0)
                set id = GetUnitIndex(dummy)
                set MData[id].eff = AddSpecialEffectTarget("Abilities\\Weapons\\LavaSpawnMissile\\LavaSpawnMissile.mdl",dummy,"origin")
                set MData[id].p0x = x
                set MData[id].p0y = y
                set MData[id].p1x = x+300*Cos(angle+(120-60*count)*bj_DEGTORAD)
                set MData[id].p1y = y+300*Sin(angle+(120-60*count)*bj_DEGTORAD)
                set MData[id].p2x = x1+GetRandomReal(-100,100)
                set MData[id].p2y = y1+GetRandomReal(-100,100)
                set MData[id].count = 0
                call GroupAddUnit(Missiles,dummy)
                set count = count+1
            endloop

You can store these calculations into two variables to save on more math since they don't change from instance to instance

                set MData[id].p1x = x+300*Cos(angle+(120-60*count)*bj_DEGTORAD)
                set MData[id].p1y = y+300*Sin(angle+(120-60*count)*bj_DEGTORAD)

 

[pks9067]

thanks for great tips
is stack one of your snippets?