More Efficent Code?

03-10-2009, 02:58 PM

JASS:

scope FlyStrike initializer Init

globals
    private constant integer FLYID = 'Amrf'
    private constant integer SPELL = 'A000'
    private constant integer STUN = 'A001'
    private constant real TIME = 0.03
    private constant real SPEED = 700
    private boolexpr Checker
    private constant string EFFECT = "Objects\\Spawnmodels\\Undead\\ImpaleTargetDust\\ImpaleTargetDust.mdl"
    private data Var
endglobals

private struct data
    unit caster
    real cx
    real cy
    real tx
    real ty
    timer fly
    timer down
    timer kill
    timer knock
    location target
    group g
    real DISTANCE


static method create takes nothing returns data
    local data d = data.allocate()
    local unit dummy 
    set d.caster = GetTriggerUnit()
    set d.cx = GetUnitX(d.caster)
    set d.cy = GetUnitY(d.caster)
    set d.g = CreateGroup()
    set dummy = CreateUnit(GetOwningPlayer(d.caster),'hfoo',d.cx,d.cy,GetUnitFacing(d.caster))
    set d.target = GetSpellTargetLoc()
    set d.tx = GetLocationX(d.target)
    set d.ty = GetLocationY(d.target)
    set d.fly = NewTimer()
    set d.down = NewTimer()
    set d.kill = NewTimer()
    set d.knock = NewTimer()
    set d.DISTANCE = 450
    call UnitAddAbility(dummy,'A001')
    call IssueTargetOrder(dummy,"thunderbolt",d.caster)
    call UnitApplyTimedLife(dummy,'BTLF',3)
    call SetUnitPathing(d.caster,false)
    call UnitAddAbility(d.caster,FLYID)
    call UnitRemoveAbility(d.caster,FLYID)
    return d
endmethod

method onDestroy takes nothing returns nothing
call ReleaseTimer(.knock)
endmethod
    endstruct

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

private function Check takes nothing returns boolean
    local data d = Var
    local unit u = GetEnumUnit()
    return IsUnitEnemy(d.caster, GetOwningPlayer(u)) and GetFilterUnit() != d.caster
endfunction

private function Push takes nothing returns nothing
    local data d = data(GetTimerData(GetExpiredTimer()))
    local unit t = GetEnumUnit()
    local real ux = GetUnitX(t)
    local real uy = GetUnitY(t)
    local real angle = Atan2(uy-d.cy,ux-d.cx)
    local real cos = Cos(angle)
    local real sin = Sin(angle)
    set ux = ux + cos * TIME * SPEED
    set uy = uy + sin * TIME * SPEED
    call SetUnitPosition(t,ux,uy)
    call DestroyEffect(AddSpecialEffect(EFFECT,ux,uy))
endfunction

private function Knock takes nothing returns nothing
    local data d = data(GetTimerData(GetExpiredTimer()))
    set d.DISTANCE = d.DISTANCE - TIME * SPEED
    if d.DISTANCE <= 0 then
    call d.destroy()
    endif
    call ForGroup(d.g,function Push)
endfunction

private function Down takes nothing returns nothing
    local data d = data(GetTimerData(GetExpiredTimer()))
    call SetUnitAnimation(d.caster,"attack slam")
    call SetUnitPathing(d.caster,true)
endfunction

private function Kill takes nothing returns nothing
    local data d = data(GetTimerData(GetExpiredTimer()))
    set Var = d
    call GroupEnumUnitsInRange(d.g,d.tx,d.ty,300,Checker)
    call SetTimerData(d.knock,integer (d))
    call TimerStart(d.knock,TIME,true,function Knock)
endfunction


private function Fly takes nothing returns nothing
    local data d = data(GetTimerData(GetExpiredTimer()))
    call SetUnitX(d.caster,d.tx)
    call SetUnitY(d.caster,d.ty)
    set d.cx = GetUnitX(d.caster)
    set d.cy = GetUnitY(d.caster)
    call SetUnitFlyHeight(d.caster,0,1500)
endfunction

private function Actions takes nothing returns nothing
    local data d = data.create()
    call SetTimerData(d.fly,integer (d))
    call SetTimerData(d.down,integer (d))
    call SetTimerData(d.kill,integer (d))
    call SetUnitFlyHeight(d.caster,999999,1500)
    call TimerStart(d.down,2.5,false,function Down)
    call TimerStart(d.fly,1.5,false,function Fly)
    call TimerStart(d.kill,3.2,false,function Kill)
endfunction

//===========================================================================
private function Init takes nothing returns nothing
    local trigger t = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ( t, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddAction( t, function Actions )
    call TriggerAddCondition(t,Condition(function Conditions))
    set Checker = Condition(function Check)
endfunction

endscope

How can I make this thing more efficent? It uses alot and alot of timer for a single spell to get animations and stuff right.

The red parts I have a question to ask , can it be replaced with

JASS:

private keyword data

or not?

I want to make this thing more efficent , tell me how to

.

It's late , I will check back later , post your helpful posts and I will ask questions when I am back, I am expecting xombie hehe..

03-10-2009, 03:01 PM	#2
xombie	No you cannot replace private data Var with private keyword data.

03-10-2009, 03:01 PM	#3
Vexorian	private keyword data Just add it before the globals declaration. You may also declare the variable bellow the struct's declaration.

03-10-2009, 03:05 PM

xombie

Quote:

Originally Posted by wraithseeker

I am expecting xombie hehe..

Lmao. Oh - after further investigation (I knew something didn't look right) I find its a different spell altogether.

Okay, so wraithseeker, explain to me what this ability does for reference.

03-10-2009, 05:31 PM

peq

A single timer should be enough since you can always start the same timer aggain.

start with

JASS:

call TimerStart(d.tim,1.5,false,function Fly)

then at the end of function fly start it aggain

JASS:

call TimerStart(d.tim,1.0,false,function Down)

and at the end of function Down:

JASS:

    call TimerStart(d.tim,0.7,false,function Kill)

and in function Kill:

JASS:

call TimerStart(d.tim,TIME,true,function Knock)

But I think the thing which really takes most of the performance is calling this every 0.03s:

JASS:

call DestroyEffect(AddSpecialEffect(EFFECT,ux,uy))

Try calling it with a 10% chance or something like that.

Maybe you can also get a little bit more speed by calculation Cos and Sin only once in the Kill-function but this might change the behavior of the spell.

My last suggestion: Use Lists or Arrays to store the affected units instead of a group. I think it is faster as you can use a simple loop instead of ForGroup.

03-10-2009, 05:49 PM

xombie

You should really try to organize your code a little better though, because part of making it more efficient is making it easier to read/change. I noticed you're getting a little better at indenting your code but there are still things (like your struct variable declarations) that could use work. Here's an example:

JASS:

private struct data
    unit caster = null
    group enum = null
    location loc = null

    timer fly = null
    timer down = null
    timer kill = null
    timer knock = null

    real cx
    real cy
    real tx
    real ty
    real distance

I remember from somewhere at the back of my head that handles should be initialized to (null) in the struct declaration. Also, you probably shouldn't use all capitals for a struct variable, like you did for real DISTANCE. Its false emphasis, and makes the code look bad.

Quote:

Originally Posted by wraithseeker

JASS:

static method create takes nothing returns data
    local data d = data.allocate()
    local unit dummy 
    set d.caster = GetTriggerUnit()
    set d.cx = GetUnitX(d.caster)
    set d.cy = GetUnitY(d.caster)
    set d.g = CreateGroup()
    set dummy = CreateUnit(GetOwningPlayer(d.caster),'hfoo',d.cx,d.cy,GetUnitFacing(d.caster))
    set d.target = GetSpellTargetLoc()
    set d.tx = GetLocationX(d.target)
    set d.ty = GetLocationY(d.target)
    set d.fly = NewTimer()
    set d.down = NewTimer()
    set d.kill = NewTimer()
    set d.knock = NewTimer()
    set d.DISTANCE = 450
    call UnitAddAbility(dummy,'A001')
    call IssueTargetOrder(dummy,"thunderbolt",d.caster)
    call UnitApplyTimedLife(dummy,'BTLF',3)
    call SetUnitPathing(d.caster,false)
    call UnitAddAbility(d.caster,FLYID)
    call UnitRemoveAbility(d.caster,FLYID)
    return d
endmethod

The Titanic sank as far as I can remember.

03-10-2009, 09:23 PM	#7
darkwulfv	One way to organize your code better would be to make a struct create function, rather than individually setting them. In that create function, you could just take the location and set the x/y's to GetLocationX/Y respectively. It's quite helpful. EDIT: Disregard that, you have one. But I don't think you can use "GetTriggerUnit()" in it like that.... Can you?

03-10-2009, 10:41 PM	#8
xombie	It still runs in the same thread, so yea, you can :P

03-11-2009, 08:31 AM

wraithseeker

Quote:

Originally Posted by xombie

The Titanic sank as far as I can remember.

Meaning?

03-11-2009, 09:09 AM	#10
DioD	private data Var === integer FlyStrike__Var check result code sometime.

03-11-2009, 09:22 AM	#11
wraithseeker	Huh?

03-11-2009, 11:07 AM	#12
xombie	He's telling you that private data Var compiles into integer FlyStrike__Var.

03-11-2009, 12:52 PM

#13

DioD

Struct reference is integer, not some other data type.

JASS:

private function Actions takes nothing returns nothing
    local data d = data.create()
    call SetTimerData(d.fly,integer (d))
    call SetTimerData(d.down,integer (d))
    call SetTimerData(d.kill,integer (d))
    call SetUnitFlyHeight(d.caster,999999,1500)
    call TimerStart(d.down,2.5,false,function Down)
    call TimerStart(d.fly,1.5,false,function Fly)
    call TimerStart(d.kill,3.2,false,function Kill)
endfunction

integer statement is useless, d is already integer.

03-11-2009, 12:59 PM

#14

xombie

DioD, even though what you say is true, there really is no down-side to using it, and in certain cases it makes the code easier to follow. Its more of a practice.

Quote:

Originally Posted by xombie

The Titanic sank as far as I can remember.

Quote:

Originally Posted by wraithseeker

Meaning?

It means even though your code weighs thousands of times less than water, it would still sink, because of the metaphorical enormity of its block-ness. Its like a block-ness monster.

03-11-2009, 01:38 PM	#15
Viikuna-	I think you need to use integer() when doing some comparisons like: JASS: if integer(d) != 0 then or JASS: if integer(d) < 120 then But Im not sure. Too lazy to test.