[JASS] how to improve?

[mateuspv]

function Trig_Cross_Fire_Conditions takes nothing returns boolean
    if ( not ( GetSpellAbilityId() == 'A01I' ) ) then
        return false
    endif
    return true
endfunction

function Trig_Cross_Fire_Actions takes nothing returns nothing
    set udg_TempLoc51 = GetUnitLoc(GetTriggerUnit())
    set udg_TempLoc54 = GetSpellTargetLoc()
    set udg_TempLoc52 = PolarProjectionBJ(udg_TempLoc51, 300.00, ( GetUnitFacing(GetTriggerUnit()) + 90.00 ))
    set udg_TempLoc53 = PolarProjectionBJ(udg_TempLoc51, 300.00, ( GetUnitFacing(GetTriggerUnit()) - 90.00 ))
    call CreateNUnitsAtLoc( 1, 'h00D', GetOwningPlayer(GetTriggerUnit()), udg_TempLoc52, GetUnitFacing(GetTriggerUnit()) )
    set udg_UnitVarLastUnit[1] = bj_lastCreatedUnit
    call UnitApplyTimedLife(udg_UnitVarLastUnit[1],'BTLF', 1.50)
    call UnitAddAbility(udg_UnitVarLastUnit[1], 'A01L')
    call SetUnitAbilityLevel(udg_UnitVarLastUnit[1], 'A01L', GetUnitAbilityLevel(GetTriggerUnit(), 'A01I') )
    call CreateNUnitsAtLoc( 1, 'h00D', GetOwningPlayer(GetTriggerUnit()), udg_TempLoc53, GetUnitFacing(GetTriggerUnit()) )
    set udg_UnitVarLastUnit[2] = bj_lastCreatedUnit
    call UnitApplyTimedLife(udg_UnitVarLastUnit[2], 'BTLF', 1.50)
    call UnitAddAbility( udg_UnitVarLastUnit[2], 'A01L' )
    call SetUnitAbilityLevel(udg_UnitVarLastUnit[2], 'A01L', GetUnitAbilityLevel(GetTriggerUnit(), 'A01I') )
    call IssuePointOrderLoc( udg_UnitVarLastUnit[1], "carrionswarm", udg_TempLoc54 )
    call IssuePointOrderLoc( udg_UnitVarLastUnit[2], "carrionswarm", udg_TempLoc54 )
    set bj_lastCreatedEffect = AddSpecialEffectLoc("war3mapImported\\BlinkCaster.mdx", udg_TempLoc52)
    call DestroyEffect(bj_lastCreatedEffect)
    set bj_lastCreatedEffect = AddSpecialEffectLoc("war3mapImported\\BlinkCaster.mdx", udg_TempLoc53)
    call DestroyEffect(bj_lastCreatedEffect)
    call RemoveLocation (udg_TempLoc51)
    call RemoveLocation (udg_TempLoc52)
    call RemoveLocation (udg_TempLoc53)
    call RemoveLocation (udg_TempLoc54)
    set bj_lastPlayedSound = gg_snd_BlinkArrival1
    if (gg_snd_BlinkArrival1 != null) then
        call StartSound(gg_snd_BlinkArrival1)
    endif
    set bj_lastPlayedSound = gg_snd_HealingSprayDeath1
    if (gg_snd_HealingSprayDeath1 != null) then
        call StartSound(gg_snd_HealingSprayDeath1)
    endif
    set bj_lastPlayedSound = gg_snd_TheBlackArrow
    if (gg_snd_TheBlackArrow != null) then
        call StartSound(gg_snd_TheBlackArrow)
    endif
    endfunction

//===========================================================================
function InitTrig_Wave takes nothing returns nothing
    set gg_trg_Wave = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( gg_trg_Wave, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddCondition( gg_trg_Wave, Condition( function Trig_Cross_Fire_Conditions ) )
    call TriggerAddAction( gg_trg_Wave, function Trig_Cross_Fire_Actions )
endfunction

Hi, that is my first "convert to custom text"
I read a little and I felt confident to transform some functions in native.

I know that has some functions that I didn't get to modify like "PolarProjectionBJ" and "CreateNUnitsAtLoc" would like to know how to do.

I know that that is not exactly jass
Thank you for her attention, if you has some reading clue I thank
how to improve?

 

[Bannar]

hope others will look on this closer since i'm falling asleep. Get rid of locations and use coordinates instead.

For target point of ability being cast:

local real x = GetSpellTargetX()
local real y = GetSpellTargetY()

For unit's coordinates:

local real x = GetUnitX()
local real y = GetUnitY()

This comparison is converted from GUi isn't it?

function Trig_Cross_Fire_Conditions takes nothing returns boolean
    if ( not ( GetSpellAbilityId() == 'A01I' ) ) then
        return false
    endif
    return true
endfunction

Use this instead:

function Trig_Cross_Fire_Conditions takes nothing returns boolean
    return GetSpellAbilityId() == 'A01I'
endfunction

If you are using jass make use of local variables instead of globals from variable editor.

 

[mateuspv]

yes is converted from GUI
I'm trying to intend

local real x = GetSpellTargetX  ()
local real y = GetSpellTargetY ()

and

local real x = GetUnitX ()
local real y = GetUnitY ()

serves the variable?
*if won't ask a lot, can anybody tell me how it is?
well, I'm trying, thanks

 

[watermelon_1234]

 local real x = GetUnitX ()
 local real y = GetUnitY ()

Don't forget to pass GetTriggerUnit (or a local unit variable set to GetTriggerUnit) to those or else you'll get a syntax error.

Anyway, if you haven't already, get JNGP. It will make using JASS much easier.

Once you have it, you can look up BJ functions so that you can inline them.
For example, PolarProjectionBJ looks like this:

function PolarProjectionBJ takes location source, real dist, real angle returns location
    local real x = GetLocationX(source) + dist * Cos(angle * bj_DEGTORAD)
    local real y = GetLocationY(source) + dist * Sin(angle * bj_DEGTORAD)
    return Location(x, y)
endfunction

Since we don't want to use locations, we would just look at the x and y calculations.
So

set udg_TempLoc52 = PolarProjectionBJ(udg_TempLoc51, 300.00, ( GetUnitFacing(GetTriggerUnit()) + 90.00 ))

would become something like this

// Assume ux and uy are GetUnitX(GetTriggerUnit()) and GetUnitY(GetTriggerUnit())
// face = GetUnitFacing(GetTriggerUnit())
local real offset_x1 = ux + 300 * Cos((face+90)*bj_DEGTORAD) // I converted 90 degress to radians since Cos and Sin only take radians.
local real offset_y1 = uy + 300 * Sin((face+90)*bj_DEGTORAD)

Study CreateNUnitsAtLoc in a similar fashion. It will inline to CreateUnitAtLoc, but you should use CreateUnit to use coordinates instead.

 

[Tirlititi]

Local variables can be used only in 1 function. They are perfect for replacing udg_UnitVarLastUnit or those like it. But you have to declare locals before anything else in the function.
As said before, use coordinates :

set udg_TempLoc51 = GetUnitLoc(GetTriggerUnit())

becomes

local real unitX = GetUnitX(GetTriggerUnit())
local real unitY = GetUnitY(GetTriggerUnit())

Personally, I use CosBJ and not Cos because it does that :

function CosBJ takes real degrees returns real
    return Cos(degrees * bj_DEGTORAD)
endfunction

Using directly "Cos" and a conversion is slightly faster because you don't have a function call. Notice that jasshelper should inline it but doesn't (because "CosBJ" is declared in Blizzard.j). For example, if you create a function like this in your map :

function CosCustom takes real degrees returns real
    return Cos(degrees * bj_DEGTORAD)
endfunction

When saving with the JNGP it automatically transforms CosCustom( angle ) into Cos(angle * bj_DEGTORAD).
Anyway, the whole thing is not important.

CreateNUnitsAtLoc should be replaced by CreateUnit carefully : the arguments are not the sames and, more importantly, CreateUnit doesn't store the created unit in bj_lastCreatedUnit. So :

    call CreateNUnitsAtLoc( 1, 'h00D', GetOwningPlayer(GetTriggerUnit()), udg_TempLoc52, GetUnitFacing(GetTriggerUnit()) )
    set udg_UnitVarLastUnit[1] = bj_lastCreatedUnit

turns into

    set udg_UnitVarLastUnit[1] = CreateUnit( GetOwningPlayer(GetTriggerUnit()), 'h00D', targetPointX, targetPointY, GetUnitFacing(GetTriggerUnit()) )

Finally, you can use locals for storing datas so you don't have to "calculate" them every time you need them. For instance, you use GetUnitFacing(GetTriggerUnit()) several time. You can declare local real unitFacing = GetUnitFacing(GetTriggerUnit()) or even

local unit caster = GetTriggerUnit()
local real unitFacing = GetUnitFacing(caster)

and it will be faster and more readable.

The whole function may be replaced by this (you can probably remove the if (gg_snd_sounds != null) then, though) :

function

function Trig_Cross_Fire_Actions takes nothing returns nothing
    local unit caster = GetTriggerUnit()
    local player owner = GetOwningPlayer(caster)
    local real ux = GetUnitX(caster)
    local real uy = GetUnitY(caster)
    local real facing = GetUnitFacing(caster)
    local integer abilLevel = GetUnitAbilityLevel(caster, 'A01I')
    local real targetX = GetSpellTargetX()
    local real targetY = GetSpellTargetY()
    local real dummy1X = ux + 300*CosBJ(facing+90)
    local real dummy1Y = ux + 300*SinBJ(facing+90)
    local unit dummy1 = CreateUnit( owner, 'h00D', dummy1X, dummy1Y, facing )
    local real dummy2X = ux + 300*CosBJ(facing-90)
    local real dummy2Y = ux + 300*SinBJ(facing-90)
    local unit dummy2 = CreateUnit( owner, 'h00D', dummy2X, dummy2Y, facing )
    call UnitApplyTimedLife(dummy1,'BTLF', 1.50)
    call UnitApplyTimedLife(dummy2,'BTLF', 1.50)
    call UnitAddAbility(dummy1, 'A01L')
    call UnitAddAbility(dummy2, 'A01L')
    call SetUnitAbilityLevel(dummy1, 'A01L', abilLevel )
    call SetUnitAbilityLevel(dummy2, 'A01L', abilLevel )
    call IssuePointOrder(dummy1, "carrionswarm", targetX, targetY )
    call IssuePointOrder(dummy2, "carrionswarm", targetX, targetY )
    call DestroyEffect(AddSpecialEffect("war3mapImported\\BlinkCaster.mdx", dummy1X, dummy1Y ))
    call DestroyEffect(AddSpecialEffect("war3mapImported\\BlinkCaster.mdx", dummy2X, dummy2Y ))
    if (gg_snd_BlinkArrival1 != null) then
        call StartSound(gg_snd_BlinkArrival1)
    endif
    if (gg_snd_HealingSprayDeath1 != null) then
        call StartSound(gg_snd_HealingSprayDeath1)
    endif
    if (gg_snd_TheBlackArrow != null) then
        call StartSound(gg_snd_TheBlackArrow)
    endif
    set caster=null
    set dummy1=null
    set dummy2=null
endfunction

 

[Bannar]

@Tirlititi Code itslef is good, but you could get rid of unit and few real variables, and I still don't understand why CosBJ :S. Anyways with cutting variables it should be somewhat like:

    local real dummyX = ux + 300 * CosBJ(facing+90)
    local real dummyY = ux + 300 * SinBJ(facing+90)

    set bj_lastCreatedUnit = CreateUnit( owner, 'h00D', dummyX, dummyY, facing )
    call UnitApplyTimedLife(bj_lastCreatedUnit,'BTLF', 1.50)
    call UnitAddAbility(bj_lastCreatedUnit, 'A01L')
    call SetUnitAbilityLevel(bj_lastCreatedUnit, 'A01L', abilLevel )
    call IssuePointOrder(bj_lastCreatedUnit, "carrionswarm", targetX, targetY )
    call DestroyEffect(AddSpecialEffect("war3mapImported\\BlinkCaster.mdx", dummyX, dummyY ))

    set dummyX = ux + 300 * CosBJ(facing-90)
    set dummyY = ux + 300 * SinBJ(facing-90)

    set bj_lastCreatedUnit = CreateUnit( owner, 'h00D', dummyX, dummyY, facing )
    call UnitApplyTimedLife(bj_lastCreatedUnit,'BTLF', 1.50)
    call UnitAddAbility(bj_lastCreatedUnit, 'A01L')
    call SetUnitAbilityLevel(bj_lastCreatedUnit, 'A01L', abilLevel )
    call IssuePointOrder(bj_lastCreatedUnit, "carrionswarm", targetX, targetY )
    call DestroyEffect(AddSpecialEffect("war3mapImported\\BlinkCaster.mdx", dummyX, dummyY ))

 

[mateuspv]

Now yes, it is much easier of understanding what this being made.
I looked and I tested Code in jass, what should happen and that when the skill is activated the effect is not created in the point in that was activated and yes on whom
activated (caster) and then move to the point where it was activated
forced to all D: