[JASS] new with jass need help finding a few errors and help with lightning effects

[OneKlicKill]

Here is what i have compiled so far.
the spell is meant to hit a signal target (finger of death) then for each unit (which is a ward) within range is then supposed to hit the unit for an additional x damage (with lighting effect finger of death) this unit then dies)
help appreciated!

function Trig_Deaths_Light_Actions takes nothing returns nothing
    local unit a = GetEventTargetUnit()
    local group b = GetUnitsInRangeOfLocAll( 1000, GetUnitLoc(GetSpellTargetUnit()))
    local integer c = 0
    local location d = GetUnitLoc(GetSpellTargetUnit())
    local player f
    local unit e
    local lightning array g
    set f = GetOwningPlayer(GetEventTargetUnit())
        loop
            set e = FirstOfGroup(b)
            exitwhen b == null
            if( GetUnitTypeId(e) == 'o001' ) then
                call DamageUnit( f, e, 35, true, true, ATTACK_TYPE_MAGIC, DAMAGE_TYPE_LIGHTNING)
                call KillUnit (e)
                set g[c] AddLightningLoc('AFOD',GetUnitLoc(a),GetUnitLoc(e))
                 set c = c + 1
                 // ignore elseif's for now ill fix them based on the first if              //
          //  elseif( GetUnitTypeId(e) == 'o003' ) then                                     //
             //   call DamageUnit( f, e, 45, ATTACK_TYPE_MAGIC, DAMAGE_TYPE_LIGHTNING)      //
             //   call KillUnit (e)                                                         //
          //  elseif( GetUnitTypeId(e) == 'o002' ) then                                     //
            //    call DamageUnit( f, e, 55, ATTACK_TYPE_MAGIC, DAMAGE_TYPE_LIGHTNING)      //
              //  call KillUnit (e)                                                         //
          //  elseif( GetUnitTypeId(e) == 'o004' ) then                                     //
            //    call DamageUnit( f, e, 65, ATTACK_TYPE_MAGIC, DAMAGE_TYPE_LIGHTNING)      //
             //   call KillUnit (e)                                                         //
          //  elseif( GetUnitTypeId(e) == 'o000' ) then                                     //
            //    call DamageUnit( f, e, 75, ATTACK_TYPE_MAGIC, DAMAGE_TYPE_LIGHTNING)      //
            //    call KillUnit (e)                                                         //
            endif
            call GroupRemoveUnit(b,e)    
        endloop
        // want to add wait time (to let the lighnting effects show)
        loop
            exitwhen c == 0
            call DestroyLightning g[c]  // Will not Destroy lighting for some reson, when i tell it to destroy something inside of the loop and its outside it wont work, but i cant but the timer inside the loop any ideas or solutions//
            set c = c - 1
        endloop
endfunction

//===========================================================================//
function InitTrig_Deaths_Light takes nothing returns nothing
    set gg_trg_Deaths_Light = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ( gg_trg_Deaths_Light, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddAction( gg_trg_Deaths_Light, function Trig_Deaths_Light_Actions )
endfunction

//==========================================================================//
function Trig_Deaths_Light_Func001A takes nothing returns nothing
endfunction

i know this is probable crap but im trying to learn this, sick of clicking in GuI

 

[OneKlicKill]

Never mind i figured it out on my own for the most part, feel free to sugest improvements/ fix leaks(not realy sure how to do that for the most part)

here's my final

function Death_Ward_Kill takes unit h, unit e, lightning g, integer i returns nothing
    local effect S1 = AddSpecialEffect("Abilities\\Spells\\Demon\\DemonBoltImpact\\DemonBoltImpact.mdl" ,GetUnitX(e),GetUnitY(e))
    local effect S2 = AddSpecialEffect("Abilities\\Spells\\Demon\\DemonBoltImpact\\DemonBoltImpact.mdl" ,GetUnitX(h),GetUnitY(h))
    call UnitDamageTarget( e, h, i, true, false, (ATTACK_TYPE_MAGIC), (DAMAGE_TYPE_LIGHTNING), null)
    call TriggerSleepAction (.27)
    call DestroyLightning(g)
    call KillUnit (e)
    call DestroyEffect(S1)
    call DestroyEffect(S2)
endfunction
//===================================================================//
function Trig_Deaths_Light_Actions takes nothing returns nothing
    local unit a = GetEventTargetUnit()
    local group b = GetUnitsInRangeOfLocAll( 1000, GetUnitLoc(GetSpellTargetUnit()))
    local unit e
    local lightning g
    local unit h = GetSpellTargetUnit()
    if ( GetSpellAbilityId()=='A00W') then
        loop
            set e = FirstOfGroup(b)
            exitwhen b == null
            if( GetUnitTypeId(e) == 'o001' ) then
                    set g = AddLightning( "AFOD" , true, GetUnitX(h),GetUnitY(h),GetUnitX(e),GetUnitY(e))
                    call Death_Ward_Kill (h, e, g,35)
                elseif( GetUnitTypeId(e) == 'o003' ) then
                    set g = AddLightning( "AFOD" , true, GetUnitX(h),GetUnitY(h),GetUnitX(e),GetUnitY(e))
                    call Death_Ward_Kill (h, e, g,45)                                                        
                elseif( GetUnitTypeId(e) == 'o002' ) then
                    set g = AddLightning( "AFOD" , true, GetUnitX(h),GetUnitY(h),GetUnitX(e),GetUnitY(e))
                    call Death_Ward_Kill (h, e, g,55)
                elseif( GetUnitTypeId(e) == 'o004' ) then
                    set g = AddLightning( "AFOD" , true, GetUnitX(h),GetUnitY(h),GetUnitX(e),GetUnitY(e))
                    call Death_Ward_Kill (h, e, g,65)
                elseif( GetUnitTypeId(e) == 'o000' ) then
                    set g = AddLightning( "AFOD" , true, GetUnitX(h),GetUnitY(h),GetUnitX(e),GetUnitY(e))
                    call Death_Ward_Kill (h, e, g,76)
            endif
            call GroupRemoveUnit(b,e)
        endloop
    endif
endfunction

//===========================================================================
function InitTrig_Deaths_Light takes nothing returns nothing
    set gg_trg_Deaths_Light = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( gg_trg_Deaths_Light, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddAction( gg_trg_Deaths_Light, function Trig_Deaths_Light_Actions )
endfunction

 

[xAbysSx]

//I am not optimizing this 100%
//But I think this is much better than what you have
//Optimized by xAbysSx
//Comments added - Enjoy!
//P.S. Contact me by PM if you are having troubles, maybe Ill help you optimize it more too.

function Death_Ward_Kill takes unit h, unit e, lightning g, integer i returns nothing
    local effect S1 = AddSpecialEffect("Abilities\\Spells\\Demon\\DemonBoltImpact\\DemonBoltImpact.mdl" ,GetUnitX(e),GetUnitY(e))
    local effect S2 = AddSpecialEffect("Abilities\\Spells\\Demon\\DemonBoltImpact\\DemonBoltImpact.mdl" ,GetUnitX(h),GetUnitY(h))
    call UnitDamageTarget(e, h, i, true, false, ATTACK_TYPE_MAGIC, DAMAGE_TYPE_LIGHTNING, null)
    //You need to use a timer, do NOT use trigger sleep action
    //Polled Wait it a little more accurate than TSA, TSA's can easily cause desyncs
    //Wait - Destroy All
    call PolledWait(.27)
    call DestroyLightning(g)
    call KillUnit(e)
    call DestroyEffect(S1)
    call DestroyEffect(S2)
    
    //Clean Leaks
    set S1 = null
    set S2 = null
endfunction

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

function Actions takes nothing returns nothing
    local unit a  = GetEventTargetUnit()
    local group b = GetUnitsInRangeOfLocAll(1000, GetUnitLoc(GetSpellTargetUnit()))
    local unit e
    local lightning g
    local unit h  = GetSpellTargetUnit()
    
    if ( GetSpellAbilityId()=='A00W') then
        loop
            set e = FirstOfGroup(b)
            exitwhen b == null
            
            set g = AddLightning( "AFOD" , true, GetUnitX(h),GetUnitY(h),GetUnitX(e),GetUnitY(e))
            //The above function was set in each if as the first function
            //So I removed them all to make it save space and speed
            //Since they were all exactly the same
            if( GetUnitTypeId(e) == 'o001' ) then
                    call Death_Ward_Kill (h, e, g,35)
                elseif( GetUnitTypeId(e) == 'o003' ) then
                    call Death_Ward_Kill (h, e, g,45)
                elseif( GetUnitTypeId(e) == 'o002' ) then
                    call Death_Ward_Kill (h, e, g,55)
                elseif( GetUnitTypeId(e) == 'o004' ) then
                    call Death_Ward_Kill (h, e, g,65)
                elseif( GetUnitTypeId(e) == 'o000' ) then
                    call Death_Ward_Kill (h, e, g,76)
            endif
            call GroupRemoveUnit(b,e)
        endloop
    endif
    
    //Clean Leaks
    call DestroyGroup(b)
    set a = null
    set g = null
    set h = null
    set e = null
endfunction

//===========================================================================
function InitTrig_Deaths_Light takes nothing returns nothing
    local trigger t = CreateTrigger( )
    
    call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_SPELL_EFFECT)
    call TriggerAddAction(t, function Actions)
    //Clean Leak
    set t = null
endfunction

 

[OneKlicKill]

yes ur totaly right about the polled wait thing but to make it even easyer i made a dummy ability to make the lightning effects so that i dont even have that problem this is what i finished last night and im very content with it

show

function Trig_Deaths_Light_Actions takes nothing returns nothing
    local unit caster = GetTriggerUnit()
    local group wards
    local unit ward
    local location loc
    local unit target = GetSpellTargetUnit()
    local integer spell_level 
    local filterfunc WardC = Filter(function WardCheck)
    set loc = GetUnitLoc(caster)
    set wards = GetUnitsInRangeOfLocMatching(1000,loc, WardC)
    call RemoveLocation(loc)
    set spell_level = GetUnitAbilityLevel(caster,'A00W')
    if ( GetSpellAbilityId()=='A00W') then
        if( spell_level == 1 ) then
            loop
            set ward = FirstOfGroup(wards)
            exitwhen wards == null
                if(GetOwningPlayer(caster)==GetOwningPlayer(ward)) then
                call UnitDamageTarget( ward, target, 35, true, false, (ATTACK_TYPE_MAGIC), (DAMAGE_TYPE_LIGHTNING), null)
                call IssueTargetOrder(ward, "fingerofdeath",target)
                endif
                call GroupRemoveUnit(wards,ward)
            endloop
        elseif( spell_level == 2 ) then
            loop
            set ward = FirstOfGroup(wards)
            exitwhen wards == null
                if(GetOwningPlayer(caster)==GetOwningPlayer(ward))  then
                call UnitDamageTarget( ward, target, 45, true, false, (ATTACK_TYPE_MAGIC), (DAMAGE_TYPE_LIGHTNING), null)
                call IssueTargetOrder(ward, "fingerofdeath",target)
                endif
                call GroupRemoveUnit(wards,ward)
            endloop
        elseif( spell_level == 3 ) then
            loop
            set ward = FirstOfGroup(wards)
            exitwhen wards == null
                if(GetOwningPlayer(caster)==GetOwningPlayer(ward))then
                call UnitDamageTarget( ward, target, 55, true, false, (ATTACK_TYPE_MAGIC), (DAMAGE_TYPE_LIGHTNING), null)
                call IssueTargetOrder(ward, "fingerofdeath",target)
                endif
                call GroupRemoveUnit(wards,ward)
            endloop
        elseif( spell_level == 4 ) then
            loop
            set ward = FirstOfGroup(wards)
            exitwhen wards == null
                if (GetOwningPlayer(caster)==GetOwningPlayer(ward))  then
                call UnitDamageTarget( ward, target, 65, true, false, (ATTACK_TYPE_MAGIC), (DAMAGE_TYPE_LIGHTNING), null)
                call IssueTargetOrder(ward, "fingerofdeath",target)
                endif
                call GroupRemoveUnit(wards,ward)
            endloop
        elseif( spell_level == 5 ) then
            loop
            set ward = FirstOfGroup(wards)
            exitwhen wards == null
                if(GetOwningPlayer(caster)==GetOwningPlayer(ward))then
                call UnitDamageTarget( ward, target, 75, true, false, (ATTACK_TYPE_MAGIC), (DAMAGE_TYPE_LIGHTNING), null)
                call IssueTargetOrder(ward, "fingerofdeath",target)
                endif
                call GroupRemoveUnit(wards,ward)
            endloop
        endif
        call GroupRemoveUnit(wards,ward)
    endif
    call DestroyGroup(wards)
    call DestroyFilter(WardC)
    set target = null
    set caster = null
    set ward = null
endfunction

feel free to sugest any thing else i can clean up any leaks or any thin like tht

 

[xAbysSx]

Again, you are repeating a lot of things that is very unnecessary, here is your new optimized code with comments to help.

//Optimized by xAbysSx
//Enjoy the new code!
//Some part may malfunction due to me not knowing how the function(s) are supposed to look like in the map.
//But its unlikely.. If this has any problems, PM me the post link and the problem.

function DeathsLightActions takes nothing returns nothing
    local unit caster = GetTriggerUnit()
    local group wards
    local unit ward
    local location loc
    local unit target = GetSpellTargetUnit()
    local integer spell_level
    local filterfunc WardC = Filter(function WardCheck)//I dont know if you just took this out when posted, but doesnt exist here.
    //You also are missing the function that creates the trigger (at the bottom).
    //Add this so that you dont have to repeat Unit Damage Target, repeats are bad.
    local integer dmg

    set loc = GetUnitLoc(caster)
    //BJs are bad, but using one is ok :p
    set wards = GetUnitsInRangeOfLocMatching(1000,loc, WardC)
    call RemoveLocation(loc)
    set spell_level = GetUnitAbilityLevel(caster, 'A00W')
    if (GetSpellAbilityId() == 'A00W') then
        //Remove multiple if these if's that were unnecessary
        if (GetOwningPlayer(caster) == GetOwningPlayer(ward)) then
            //Removed multiple loops that were unnecessary
            loop
            set ward = FirstOfGroup(wards)
            exitwhen wards == null

            if (spell_level == 1) then
                    set dmg = 35
                
                elseif (spell_level == 2) then
                    set dmg = 45

                elseif (spell_level == 3) then
                    set dmg = 55

                elseif (spell_level == 4) then
                    set dmg = 65

                elseif (spell_level == 5) then
                    set dmg = 75
            endif

                //Removed a lot of multiples in your code, added these here because they were all the same
                //Except for the damages, which are now stored in a local so that you only use Unit Damage Target once.
                call UnitDamageTarget(ward, target, dmg, true, false, ATTACK_TYPE_MAGIC, DAMAGE_TYPE_LIGHTNING, null)
                call IssueTargetOrder(ward, "fingerofdeath", target)
                call GroupRemoveUnit(wards,ward)
            endloop
        endif
        call GroupRemoveUnit(wards,ward)
    endif

    //Cleaning Leaks
    call DestroyGroup(wards)
    call DestroyFilter(WardC)
    set target = null
    set caster = null
    set ward = null
    set dmg = 0
    set spell_level = 0
endfunction

EDIT: By the way, I added a few blank lines to break things up, it makes the code easier to read when you have like-actions together. Just a helpful tip that hopefully you understand.

EDIT 2: I just realised I made a mistake in the other trigger, in the leak cleaning section instead of setting b = null it should be call DestroyGroup(b), I edited the other post and fixed it.

 

[Element of Water]

2 small optimization points:
1) You should use boolexpr instead of filterfunc, because the function takes a boolexpr, not a filterfunc, so it would need to be converted into a boolexpr for the function (not sure if the converting to boolexpr when it's created makes up for time gained here though)

2) NEVER use locations unless absolutely necessary (GetLocationZ and a few others). Coordinates, coordinates, coordinates!

Other than that, I see no other way to optimize.

 

[Dr Super Good]

Use a formula for spell damage, it ends up faster than multiple selection.
Destroying filters is stupid and a waste of time unless the code will only ever fire once as they do not leak otherwise so you just waste quite a bit of time for a few bytes of memory.

 

[OneKlicKill]

Use a formula for spell damage, it ends up faster than multiple selection.

good point i'm going to use (25+10*spell_level)

and Element of Water it dose ask for a boolexpr but i'm not sure how to do that here is the filter for what units i'm trying to get

function WardCheck takes nothing returns boolean
     local unit ward = GetFilterUnit()
     return GetUnitTypeId(ward) == 'o001' 
endfunction

also i'm not sure how not to use a location in this situation there are no GetUnitsInRange that uses coordinates as far as i could find

 

[Element of Water]

1. GetFilterUnit() works whether you use a boolexpr or filterfunc, it doesn't matter.

2.

native GroupEnumUnitsInRange takes group whichGroup, real x, real y, real radius, boolexpr filter returns nothing

This enumeration function does the trick. The other one you used is a BJ, designed for GUI, therefore they didn't do one for coordinates. Enumeration functions just put every unit in the range into your group. It's faster too since its native.

 

[Dr Super Good]

However, never pass them a null boolexpr, as that leaks which is best avoided. If you want to enum all units, you have to use a dummy boolexpr which returns true all the time.

 

[Element of Water]

But he doesn't - he's using a filter on GetUnitsInRangeOfLoc anyway.

Another thing - don't loop through the whole group after you've enumerated them - loop through the group in the enum filter.

 

[Need_O2]

PolledWait is better than TSA, TSAs can easily desync

you cannot possibly be serius about that
PolledWait itself uses TSA ever checked ?
it isnt something so "pro" at all (Wait Game Time in GUI is PolledWait)

 

[xAbysSx]

Yes, I know it uses TSA. However its more accurate, dont ask me how, its just what I was told by the person that I believes knows JASS better than 95% of the people on this website.

 

[Deaod]

When will you guys learn that destroying groups is senseless as there are always better alternatives (GroupUtils)?

            if (spell_level == 1) then
                    set dmg = 35

                elseif (spell_level == 2) then
                    set dmg = 45

                elseif (spell_level == 3) then
                    set dmg = 55

                elseif (spell_level == 4) then
                    set dmg = 65

                elseif (spell_level == 5) then
                    set dmg = 75
            endif

And this is a good example for what you should NOT do.
Make a constant function and place that function in the UnitDamageTarget() call. Saves you a declaration of a local and a bunch of lines.

And destroying boolexprs not created by And() or Or() can fuck up your code since youre destroying every instance of a boolexpr derived from that function.

As others already pointed out, using locations is the Blizzard way to approach this. And its not a good way. Use x-y-coordinates instead.

If TSA desyncs, then so does PolledWait. PolledWait probably even more easily. But to be honest, ive never seen TSA desyncing a game. Did you? If you have a proof for that claim, reveal it to us.

 

[Element of Water]

PolledWait won't desync as a timer controls the TSA used within it. But I agree, when has TSA ever desynced a game? Its only problem is its inaccuracy.

 

[Need_O2]

Yes, I know it uses TSA. However its more accurate, dont ask me how, its just what I was told by the person that I believes knows JASS better than 95% of the people on this website.

I ment the desync thing not accurate
I know its more accurate

 

[Element of Water]

Need_O2, read my post.

 

[Need_O2]

Ive never seen that TSA desync'd anyway...
Why did you say "Read my post"
Im talking to abyss guy not you

 

[Element of Water]

I said "read my post", because in it I explained why PolledWait might be better at not causing desyncs than TSA. But I agree, I've never encountered TSA desyncs before either.

 

[Dr Super Good]

The closest to a desync I had was in a multiplayer game where all game time failed. So both trigger sleep action and timers were running 100-1000 times faster than they were meant to so the map kind of got very buggy and fell appart.

 

[Element of Water]

What map was that? It would be great to be able to recreate that for super accurate TSA.