[JASS] Is it fully optimize?

[Septimus]

Even though I am sure the spells was 100% efficient, but I am not taking any chances.

Thus, I am posting this list of spells and would like an opinion of where it could be optimize. Of course, it got to be jass not vjass since the spells was for campaign.

Multiple Shot Spells

function Trig_Multiple_Shot_Conditions takes nothing returns boolean
    return GetSpellAbilityId() == 'A62P'
endfunction

function Trig_Multiple_Shot_Actions takes nothing returns nothing
 local unit caster=GetTriggerUnit()
 local unit lastcreated
 local location loc=GetSpellTargetLoc()
 local real x1=GetUnitX(caster)
 local real y1=GetUnitY(caster)
 local real x2=GetLocationX(loc)
 local real y2=GetLocationY(loc)
 local real missilefacing=bj_RADTODEG*Atan2(y2-y1,x2-x1)
 local real angle=(GetUnitFacing(caster)-(GetUnitAbilityLevel(caster,'A62P') * 10.00 ))
 local real offsetx
 local real offsety
 local integer index=1
    loop
        exitwhen index>(GetUnitAbilityLevel(caster,'A62P')*5)
        set lastcreated=CreateUnit(GetOwningPlayer(caster),'e605',x1,y1,missilefacing)
        call UnitApplyTimedLife(lastcreated,'BTLF',2)
        set offsetx=x2+150*Cos(angle*bj_DEGTORAD)
        set offsety=y2+150*Sin(angle*bj_DEGTORAD)
        call IssuePointOrder(lastcreated,"breathoffire",offsetx,offsety)
        set lastcreated=null
        set angle=angle+4.00
        set index=index+1
    endloop
    set caster=null
    call RemoveLocation(loc)
endfunction

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

constant function MSDF takes nothing returns boolean
    return true
endfunction

function InitTrig_Multiple_Shot takes nothing returns nothing
    local trigger T = CreateTrigger()
    local integer TI = 0
    local filterfunc FF = Filter(function MSDF)
    loop
    exitwhen (TI >= bj_MAX_PLAYER_SLOTS)
    call TriggerRegisterPlayerUnitEvent(T, Player(TI), EVENT_PLAYER_UNIT_SPELL_EFFECT, FF)
    set TI = TI + 1
    endloop
    call DestroyBoolExpr(FF)
    call DestroyFilter(FF)
    call TriggerAddAction(T,function Trig_Multiple_Shot_Actions)
    call TriggerAddCondition(T,Condition(function Trig_Multiple_Shot_Conditions))
    set FF = null
    set T = null
endfunction

Napalm Shock Spells

function Trig_Napalm_Shock_Conditions takes nothing returns boolean
    return GetSpellAbilityId() == 'A62A'
endfunction

function Trig_Napalm_Shock_Actions takes nothing returns nothing
 local unit caster=GetTriggerUnit()
 local unit target=GetSpellTargetUnit()
 local unit lastcreated
 local real x1=GetUnitX(caster)
 local real y1=GetUnitY(caster)
 local real x2=GetUnitX(target)
 local real y2=GetUnitY(target)
 local real x3
 local real y3
 local real damage=40+(GetUnitAbilityLevel(caster,'A62A')*15)
 local integer index=1
 local real dx=x2-x1
 local real dy=y2-y1
 local real distance=SquareRoot(dx*dx+dy*dy)/100
 local real angle=bj_RADTODEG*Atan2(y2-y1,x2-x1)
    call UnitDamageTarget(caster,target,damage,true,false,ATTACK_TYPE_NORMAL,DAMAGE_TYPE_NORMAL,WEAPON_TYPE_WHOKNOWS)
    call DestroyEffect(AddSpecialEffect("Abilities\\Spells\\Other\\Volcano\\VolcanoDeath.mdl",x1,y1))
    loop
        exitwhen index>R2I(distance)
        set x3=x1+(udg_NS_Distance+100)*Cos(angle*bj_DEGTORAD)
        set y3=y1+(udg_NS_Distance+100)*Sin(angle*bj_DEGTORAD)
        set lastcreated=CreateUnit(GetOwningPlayer(caster),'n61G',x3,y3,angle)
        set udg_NS_Distance=udg_NS_Distance+100
        call UnitApplyTimedLife(lastcreated,'BTLF',0.2)
        set lastcreated=null
        set index=index+1
    endloop
    set lastcreated=CreateUnit(GetOwningPlayer(caster),'n61E',x2,y2,angle)
    call UnitApplyTimedLife(lastcreated,'BTLF',3)
    call UnitAddAbility(lastcreated,'A629')
    call SetUnitAbilityLevel(lastcreated,'A629',GetUnitAbilityLevel(caster,'A62A'))
    call IssueTargetOrder(lastcreated,"shadowstrike",target)
    set udg_NS_Distance=0.00
endfunction

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

constant function NSDF takes nothing returns boolean
    return true
endfunction

function InitTrig_Napalm_Shock takes nothing returns nothing
    local trigger T = CreateTrigger()
    local integer TI = 0
    local filterfunc FF = Filter(function NSDF)
    loop
    exitwhen (TI >= bj_MAX_PLAYER_SLOTS)
    call TriggerRegisterPlayerUnitEvent(T, Player(TI), EVENT_PLAYER_UNIT_SPELL_EFFECT, FF)
    set TI = TI + 1
    endloop
    call DestroyBoolExpr(FF)
    call DestroyFilter(FF)
    call TriggerAddAction(T,function Trig_Napalm_Shock_Actions)
    call TriggerAddCondition(T,Condition(function Trig_Napalm_Shock_Conditions))
    set FF = null
    set T = null
endfunction

Phoenix Swap Spells

function Trig_Phoenix_Swap_Conditions takes nothing returns boolean
    return GetSpellAbilityId() == 'A634'
endfunction

function Trig_Phoenix_Swap_Actions takes nothing returns nothing
 local unit caster=GetTriggerUnit()
 local unit target=GetSpellTargetUnit()
    local real x1=GetUnitX(caster)
    local real x2=GetUnitX(target)
    local real y1=GetUnitY(caster)
    local real y2=GetUnitY(target)
    call SetUnitX(caster,x2)
    call SetUnitX(target,x1)
    call SetUnitY(caster,y2)
    call SetUnitY(target,y1)
    call DestroyEffect(AddSpecialEffect("Abilities\\Spells\\Other\\Doom\\DoomDeath.mdl",x1,y1))
    call DestroyEffect(AddSpecialEffect("Abilities\\Spells\\Other\\Doom\\DoomDeath.mdl",x2,y2))
    if IsUnitEnemy(target,GetOwningPlayer(caster))==true then
        call UnitDamageTarget(caster,target,50.00,true,false,ATTACK_TYPE_NORMAL,DAMAGE_TYPE_NORMAL,WEAPON_TYPE_WHOKNOWS)
    endif
    set caster=null
    set target=null
endfunction

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

constant function PSDF takes nothing returns boolean
    return true
endfunction

function InitTrig_Phoenix_Swap takes nothing returns nothing
    local trigger T = CreateTrigger()
    local integer TI = 0
    local filterfunc FF = Filter(function PSDF)
    loop
    exitwhen (TI >= bj_MAX_PLAYER_SLOTS)
    call TriggerRegisterPlayerUnitEvent(T, Player(TI), EVENT_PLAYER_UNIT_SPELL_EFFECT, FF)
    set TI = TI + 1
    endloop
    call DestroyBoolExpr(FF)
    call DestroyFilter(FF)
    call TriggerAddAction(T,function Trig_Phoenix_Swap_Actions)
    call TriggerAddCondition(T,Condition(function Trig_Phoenix_Swap_Conditions))
    set FF = null
    set T = null
endfunction

Massive Conversion Spells

function Trig_Massive_Conversion_Conditions takes nothing returns boolean
    return GetSpellAbilityId() == 'A62J'
endfunction

function Boolean takes nothing returns boolean
    return (IsUnitType(GetFilterUnit(),UNIT_TYPE_STRUCTURE)==false) and (IsUnitType(GetFilterUnit(),UNIT_TYPE_MAGIC_IMMUNE)==false) and (GetWidgetLife(GetFilterUnit())>.415) and (IsUnitType(GetFilterUnit(), UNIT_TYPE_HERO)==false) and (IsUnitEnemy(GetFilterUnit(),GetOwningPlayer(GetTriggerUnit()))==true)
endfunction

function Trig_Massive_Conversion_Actions takes nothing returns nothing
 local unit caster=GetTriggerUnit()
 local unit target
 local effect sfx
 local real x1=GetUnitX(caster)
 local real y1=GetUnitY(caster)
 local real x2
 local real y2
    call GroupEnumUnitsInRange(udg_MC_Group,x1,y1,500,Condition(function Boolean))
    set target=FirstOfGroup(udg_MC_Group)
    loop
        exitwhen target==null
        call SetUnitOwner(target,GetOwningPlayer(caster),true)
        set x2=GetUnitX(target)
        set y2=GetUnitY(target)
        set sfx=AddSpecialEffect("Abilities\\Spells\\Other\\Charm\\CharmTarget.mdl",x2,y2)
        call GroupRemoveUnit(udg_MC_Group,target)
        set target=FirstOfGroup(udg_MC_Group)
        call DestroyEffect(sfx)
        set sfx=null
    endloop
    set caster=null
endfunction

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

constant function MCDF takes nothing returns boolean
    return true
endfunction

function InitTrig_Massive_Conversion takes nothing returns nothing
    local trigger T = CreateTrigger()
    local integer TI = 0
    local filterfunc FF = Filter(function MCDF)
    loop
    exitwhen (TI >= bj_MAX_PLAYER_SLOTS)
    call TriggerRegisterPlayerUnitEvent(T, Player(TI), EVENT_PLAYER_UNIT_SPELL_EFFECT, FF)
    set TI = TI + 1
    endloop
    call DestroyBoolExpr(FF)
    call DestroyFilter(FF)
    call TriggerAddAction(T,function Trig_Massive_Conversion_Actions)
    call TriggerAddCondition(T,Condition(function Trig_Massive_Conversion_Conditions))
    set FF = null
    set T = null
endfunction

Demonic Rampage Spells

function Trig_Demonic_Rampage_Conditions takes nothing returns boolean
    return GetSpellAbilityId() == 'A62U'
endfunction

function Trig_Demonic_Rampage_Actions takes nothing returns nothing
 local unit caster=GetTriggerUnit()
 local unit lastcreated
 local location loc=GetSpellTargetLoc()
 local effect sfx
 local real x1=GetLocationX(loc)
 local real y1=GetLocationY(loc)
 local real x2
 local real y2
 local real facingangle
 local integer index=1
    set lastcreated=CreateUnit(GetOwningPlayer(caster),'n61P',x1,y1,270)
    call UnitApplyTimedLife(lastcreated,'BTLF',3)
    call IssuePointOrder(lastcreated,"stasistrap",x1,y1)
    set lastcreated=null
    set sfx=AddSpecialEffect("Abilities\\Spells\\Undead\\DarkRitual\\DarkRitualTarget.mdl",x1,y1)
    call DestroyEffect(sfx)
    loop
        exitwhen index>(GetUnitAbilityLevel(caster,'A62U')*3)
        set x2=x1+700*Cos((udg_angle+(120.00/I2R(GetUnitAbilityLevel(caster,'A62U'))))*bj_DEGTORAD)
        set y2=y1+700*Sin((udg_angle+(120.00/I2R(GetUnitAbilityLevel(caster,'A62U'))))*bj_DEGTORAD)
        set facingangle=bj_RADTODEG*Atan2(y1-y2,x1-x2)
        set lastcreated=CreateUnit(GetOwningPlayer(caster),'n61O',x2,y2,facingangle)
        call SetUnitTimeScale(lastcreated,1.3)
        call UnitApplyTimedLife(lastcreated,'BTLF',6)
        set lastcreated=null
        set udg_angle=(udg_angle+(120.00/I2R(GetUnitAbilityLevel(caster,'A62U'))))
        set index=index+1
    endloop
    set udg_angle=0.00
    set sfx=null
    set caster=null
    call RemoveLocation(loc)
endfunction

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

constant function DRDF takes nothing returns boolean
    return true
endfunction

function InitTrig_Demonic_Rampage takes nothing returns nothing
    local trigger T = CreateTrigger()
    local integer TI = 0
    local filterfunc FF = Filter(function DRDF)
    loop
    exitwhen (TI >= bj_MAX_PLAYER_SLOTS)
    call TriggerRegisterPlayerUnitEvent(T, Player(TI), EVENT_PLAYER_UNIT_SPELL_EFFECT, FF)
    set TI = TI + 1
    endloop
    call DestroyBoolExpr(FF)
    call DestroyFilter(FF)
    call TriggerAddAction(T,function Trig_Demonic_Rampage_Actions)
    call TriggerAddCondition(T,Condition(function Trig_Demonic_Rampage_Conditions))
    set FF = null
    set T = null
endfunction

Energy Greed Spells

function Trig_Energy_Greed_Conditions takes nothing returns boolean
    return GetUnitAbilityLevel(GetTriggerUnit(),'A62F')>0
endfunction

function Trig_Energy_Greed_Actions takes nothing returns nothing
 local unit caster=GetTriggerUnit()
 local integer level=GetUnitAbilityLevel(caster,'A62F')
 local real manabefore=GetUnitState(caster,UNIT_STATE_MANA)
 local real manaafter
 local real manaadd
 local real x
 local real y
 local effect sfx=null
    call TriggerSleepAction(0.05)
    set manaafter=GetUnitState(caster,UNIT_STATE_MANA)
    call TriggerSleepAction(0.02)
    set manaadd=((manabefore-manaafter)/2)
    if GetRandomInt(1,10)<=level then
        set x=GetUnitX(caster)
        set y=GetUnitY(caster)
        call SetUnitState(caster,UNIT_STATE_MANA,(GetUnitState(caster,UNIT_STATE_MANA)+manaadd))
        set sfx=AddSpecialEffect("Abilities\\Spells\\Human\\Resurrect\\ResurrectTarget.mdl",x,y)
        call TriggerSleepAction(0.01)
        call DestroyEffect(sfx)
        set sfx=null
    endif
   set caster=null 
endfunction

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

constant function EGDF takes nothing returns boolean
    return true
endfunction

function InitTrig_Energy_Greed takes nothing returns nothing
    local trigger T = CreateTrigger()
    local integer TI = 0
    local filterfunc FF = Filter(function EGDF)
    loop
    exitwhen (TI >= bj_MAX_PLAYER_SLOTS)
    call TriggerRegisterPlayerUnitEvent(T, Player(TI), EVENT_PLAYER_UNIT_SPELL_EFFECT, FF)
    set TI = TI + 1
    endloop
    call DestroyBoolExpr(FF)
    call DestroyFilter(FF)
    call TriggerAddAction(T,function Trig_Energy_Greed_Conditions)
    call TriggerAddCondition(T,Condition(function Trig_Energy_Greed_Actions))
    set FF = null
    set T = null
endfunction

Spells

 

[Element of Water]

For the multiple shot things, you should set GetUnitAbilityLevel(caster,'A62P')*5 to a local variable and use that in the exitwhen rather than repeating using the function.

 

[DiscipleOfLife]

In the first one you could iterate in reverse order to save GetUnitAbilityLevel calls:

 local integer index=GetUnitAbilityLevel(caster,'A62P')*5
    loop
        exitwhen index==0
        set lastcreated=CreateUnit(GetOwningPlayer(caster),'e605',x1,y1,missilefacing)
        call UnitApplyTimedLife(lastcreated,'BTLF',2)
        set offsetx=x2+150*Cos(angle*bj_DEGTORAD)
        set offsety=y2+150*Sin(angle*bj_DEGTORAD)
        call IssuePointOrder(lastcreated,"breathoffire",offsetx,offsety)
        set lastcreated=null
        set angle=angle+4.00
        set index=index-1
    endloop
    set caster=null
    call RemoveLocation(loc)

Also IssuePointOrderById is a bit faster than the string version.
[EDIT]Get ConvOrder here.

loop
    exitwhen (TI >= bj_MAX_PLAYER_SLOTS)
    call TriggerRegisterPlayerUnitEvent(T, Player(TI), EVENT_PLAYER_UNIT_SPELL_EFFECT, FF)
    set TI = TI + 1
    endloop

Use the BJ instead of that. If you're doing something only once during the whole game, it doesn't matter if it takes 0.0001 or 0.0002 seconds... If you're concerned about the filter leak, just make a leakless function to replace it.



You're not nulling locations anywhere, though you are removing them .



In the second one you can avoid some R2I calls if you loop backwards like in the first one.




if IsUnitEnemy(target,GetOwningPlayer(caster))==true then
-->
if IsUnitEnemy(target,GetOwningPlayer(caster)) then




In the second last spell you could also loop backwards and save some GetUnitAbilityLevel calls.


Other than for the above stuff I don't think you can optimize this much more, nor would it really matter if you could.

 

[Septimus]

Ribenamania helping me in converting all those spells from gui to jass, i did't notice it since there is over 20 spells that I need to browse through.

Would fix it accordingly.

 

[Dr Super Good]

Try using 1 constant dummy filter which returns the same value between all spells. That way you do not really have to destroy it as it is only 1 handle that can be used between X number of spells.

 

[-JonNny]

...........

MSS

- Change those names Trig_Multiple_Shot_Conditions and remove the InitTrig_ prefix, initialize it via scope then

- call RemoveLocation(loc)[icode=jass] you do remove the location but you should also null it - ive never seen nulling a Trigger, im pretty sure its useless - Do not create so many multiple function of the same thing, use one single one [code=jass]constant function MSDF takes nothing returns boolean return true endfunction[/code] ... [code=jass]constant function NSDF takes nothing returns boolean return true endfunction[/code] then you could also remove that [code=jass] call DestroyBoolExpr(FF) call DestroyFilter(FF)[/code] also i doubt thats this is really necessary - As DSG already mentioned do not create thousands of dummies use one global one or atleast one per spell Most of those things are in all other Scripts too, i wont mention each time again to remove prefixes ect![/hidden] [hidden=NSS] -Your setting the local variable distance but you use the udg_NS_Distance one, this shouldd not work properly... -you do nothing with the dummy? then you can inlie that creation and remove the useless variable [icode=jass]call UnitApplyTimedLife(CreateUnit(GetOwningPlayer(caster),'n61G',x3,y3,angle),'BTLF',0.2)

-You do not have to null globals, also you doont have to null reals and integers...

- You do not null the caster , target and the last lastcreated unit variables

PSS

nothing which wasnt mentioned above

MCS

- (GetWidgetLife(GetFilterUnit())>.415) afaik its 0.405 and not 0.415

- this loop is completely useless

    call GroupEnumUnitsInRange(udg_MC_Group,x1,y1,500,Condition(function Boolean))
    set target=FirstOfGroup(udg_MC_Group)
    loop
        exitwhen target==null
        call SetUnitOwner(target,GetOwningPlayer(caster),true)
        set x2=GetUnitX(target)
        set y2=GetUnitY(target)
        set sfx=AddSpecialEffect("Abilities\\Spells\\Other\\Charm\\CharmTarget.mdl",x2,y2)
        call GroupRemoveUnit(udg_MC_Group,target)
        set target=FirstOfGroup(udg_MC_Group)
        call DestroyEffect(sfx)
        set sfx=null
    endloop

i dont know what it should have been.
if you want ONE "random" target you can replace the loop with a condition if target == null , but then you have to null the variable later if its want empty, also you have to GroupClear() the grup
if you want to loop through each unit that set target=FirstOfGroup(udg_MC_Group) must be INSIDE inside the loop...

- inlinde that effect and remove the sfx variable, else you have to null he effect variablee

DRS

- Why do you use a global angle for that? (seen that before but forgot the mention i guess)
btw, you can remove that useless conversion from DEG to RAD and RAD to DEG again

-null the location

EGS

-ive already mention those TSA before when looking above it
also TSA smaller than 0.27 is useless cuz 0.27 is the smallest it can do

 

[Septimus]

i want jass, not vjass.

 

[-JonNny]

ok, missed that
but that only affects the scope which would replace those prefixes
every other sugggstion can still be done

 

[Viikuna]

You should not create dummies on spell cast like that, CreateUnit is super slow and everything. Just create one dummy in map init and use it to cast all those spells. ( If you need details, ask JonNny )

Also, doing all your stuff in triggercondition and not using
triggeraction at all gives you some speed, since conditions are faster than actions.