can someone help to clean up this jass code

[grundy]

Im making this spell that creates 3/4/5 illusions of the caster and positions them around the target and then each illusion attacks the target 1 after the other 1 at a time and after each one makes an attack it is removed and after all the illusions have attacked the caster takes the position where the last standing illusion was at... so it gets pretty messy.

i have the first trigger, the main trigger that goes off when the spell is cast. that trigger creates a local trigger that positions the illusions around the target when the illusions get summoned. after all of the illusions have been summoned and positioned that trigger creates another local trigger that goes off every time the target takes damage, if the damage source was one of the illusions that illusion is removed and the next illusion attacks. when all the illusions are gone the caster is reappeared where the last illusion was standing.

but because the spell works in this way there are a few conditions i need to take care of, like what if the caster dies before all of the illusions have attacked? i have that taken care of in the trigger but it just looks so messy. can anyone help make this trigger better?

constant function Shadow_Stab_SpellId takes nothing returns integer
    return 'A01Y'
endfunction

constant function Shadow_Stab_BuffId takes nothing returns integer
    return 'B00H'
endfunction

constant function Shadow_Stab_Create_Illusion_SpellId takes nothing returns integer
    return 'A01Z'
endfunction

constant function Shadow_Stab_Illusion_BuffId takes nothing returns integer
    return 'B00G'
endfunction

constant function Shadow_Stab_Illusion_Count takes integer L returns integer
    return L+2
endfunction

constant function Create_Illusion_Order takes nothing returns integer
    return 852274
endfunction

function Trig_Shadow_Stab_Conditions takes nothing returns boolean
    return GetSpellAbilityId() == Shadow_Stab_SpellId()
endfunction

function Shadow_Stab_Illusion_Has_Buff takes nothing returns boolean
    return GetUnitAbilityLevel(GetSummonedUnit(), Shadow_Stab_Illusion_BuffId()) > 0
endfunction

//function Shadow_Stab_Damage_Condition takes nothing returns boolean
//    local trigger Trig = GetTriggeringTrigger()
//    local group G = H2G(GetHandleHandle(Trig, "G"))
//    local unit I = GetEventDamageSource()
//    local boolean B = IsUnitInGroup(I, G)
//    set Trig = null
//    set G = null
//    set I = null
//    return B
//endfunction

function Shadow_Stab_Damage_Action takes nothing returns nothing
    local trigger Trig = GetTriggeringTrigger()
    local unit I = GetEventDamageSource()
    local unit C = H2U(GetHandleHandle(Trig, "C"))
    local unit T = H2U(GetHandleHandle(Trig, "T"))
    local group G = H2G(GetHandleHandle(Trig, "G"))
    local real X = GetUnitX(I)
    local real Y = GetUnitY(I)
    
    if GetUnitState(T, UNIT_STATE_LIFE)-GetEventDamage() < 0.5 then
        set I = FirstOfGroup(G)
        loop
            exitwhen I == null
            call GroupRemoveUnit(G, I)
            set X = GetUnitX(I)
            set Y = GetUnitY(I)
            call RemoveUnit(I)
            set I = FirstOfGroup(G)
        endloop
        call SetUnitX(C, X)
        call SetUnitY(C, Y)
        call PauseUnit(T, false)
        call ShowUnit(C, true)
        call SelectUnitAddForPlayer(C, GetOwningPlayer(C))
        call FlushHandleLocals(Trig)
        call DestroyTrigger(Trig)
        call DestroyGroup(G)
        
    elseif IsUnitInGroup(I, G) then
    
        call TriggerSleepAction(0.4)
    
        call GroupRemoveUnit(G, I)
        call RemoveUnit(I)
    
        if CountUnitsInGroup(G) > 0 then
            set I = GroupPickRandomUnit(G)
            call PauseUnit(I, false)
            call IssueTargetOrder(I, "attack", T)
        else
            call ShowUnit(C, true)
            call SetUnitX(C, X)
            call SetUnitY(C, Y)
            call SelectUnitAddForPlayer(C, GetOwningPlayer(C))
            call IssueTargetOrder(C, "attack", T)
            call FlushHandleLocals(Trig)
            call DestroyTrigger(Trig)
            call DestroyGroup(G)
            call PauseUnit(T, false)
            call UnitRemoveAbility(T, Shadow_Stab_BuffId())
        endif
        if CountUnitsInGroup(G) == 1 then
            set I = FirstOfGroup(G)
            call SetUnitFacing(C, GetUnitFacing(I))
        endif
    
    endif
    set Trig = null
    set I = null
    set C = null
    set T = null
    set G = null
endfunction

function Summon_Illusion_Function takes nothing returns nothing
    local trigger Trig = GetTriggeringTrigger()
    local unit C = H2U(GetHandleHandle(Trig, "C"))
    local unit T = H2U(GetHandleHandle(Trig, "T"))
    local integer L = GetHandleInt(Trig, "L")
    local group G = H2G(GetHandleHandle(Trig, "G"))
    local unit I = GetSummonedUnit()
    local integer N = Shadow_Stab_Illusion_Count(L)
    local integer InG
    local real D = 150.0
    local real A = 360.0/N
    local real X = GetUnitX(T)
    local real Y = GetUnitY(T)
    local real X2
    local real Y2
    local trigger Trig2
    
    if G == null then
        set G = CreateGroup()
        call SetHandleHandle(Trig, "G", G)
    endif
    
    call GroupAddUnit(G, I)
    set InG = CountUnitsInGroup(G)
    call SetUnitPathing(I, false)
    
    set X2 = X + D * Cos(A*InG * bj_DEGTORAD)
    set Y2 = Y + D * Sin(A*InG * bj_DEGTORAD)
    call SetUnitX(I, X2)
    call SetUnitY(I, Y2)
    call SetUnitFacing(I, A*InG+180)
    call PauseUnit(I, true)
    
    if InG == N then
        call FlushHandleLocals( Trig )
        call DestroyTrigger( Trig )
        call ShowUnit(C, false)
        set Trig2 = CreateTrigger()
        call SetHandleHandle(Trig2, "C", C)
        call SetHandleHandle(Trig2, "T", T)
        call SetHandleHandle(Trig2, "G", G)
        call TriggerRegisterUnitEvent(Trig2, T, EVENT_UNIT_DAMAGED)
//        call TriggerAddCondition(Trig2, Condition( function Shadow_Stab_Damage_Condition ))
        call TriggerAddAction(Trig2, function Shadow_Stab_Damage_Action)
        set I = GroupPickRandomUnit(G)
        call PauseUnit(I, false)
        call IssueTargetOrder(I, "attack", T)
    else
        call DisplayTextToForce( GetPlayersAll(), "Units in group = "+I2S(InG) )
    endif
    set Trig = null
    set Trig2 = null
    set C = null
    set T = null
    set G = null
endfunction

function Trig_Shadow_Stab_Actions takes nothing returns nothing
    local unit C = GetSpellAbilityUnit()
    local unit T = GetSpellTargetUnit()
    local trigger Trig = CreateTrigger()
    local unit Temp
    local real X = GetUnitX(C)
    local real Y = GetUnitY(C)
    local player P = GetOwningPlayer(C)
    local integer N = 0
    local integer L = GetUnitAbilityLevel(C, Shadow_Stab_SpellId())
    call PauseUnit(T, true)
    call TriggerSleepAction(0.0)
    loop
        set N = N+1
        set Temp = CreateUnit(P, Dummy_Unit_TypeId(), X, Y, 0)
        call SetUnitPathing(Temp, false)
        call UnitAddAbility(Temp, Shadow_Stab_Create_Illusion_SpellId())
        call IssueTargetOrderById(Temp, Create_Illusion_Order(), C)
        call UnitApplyTimedLife(Temp, 'BTLF', 2.0)
        exitwhen N >= Shadow_Stab_Illusion_Count(L)
    endloop
    call TriggerRegisterPlayerUnitEvent( Trig, P, EVENT_PLAYER_UNIT_SUMMON, null )
    call TriggerAddCondition( Trig, Condition( function Shadow_Stab_Illusion_Has_Buff ) )
    call TriggerAddAction( Trig, function Summon_Illusion_Function )
    call SetHandleInt( Trig, "L", L )
    call SetHandleHandle( Trig, "C", C )
    call SetHandleHandle( Trig, "T", T )
    
    set C = null
    set T = null
    set Trig = null
    set Temp = null
    set P = null    
endfunction

//===========================================================================
function InitTrig_Shadow_Stab takes nothing returns nothing
    set gg_trg_Shadow_Stab = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( gg_trg_Shadow_Stab, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddCondition( gg_trg_Shadow_Stab, Condition( function Trig_Shadow_Stab_Conditions ) )
    call TriggerAddAction( gg_trg_Shadow_Stab, function Trig_Shadow_Stab_Actions )
endfunction

oh yea and some times, rarely, the triggers dont even work. the caster gets hidden and the illusions are created, but the illusions do not get positioned around the caster and they dont get paused and they just attack whatever is closest to them inside their acquisition range. i have not figured out why that happens because its not an easily replicated bug. there is no apparent reason why it happens - most of the time it works perfectly though.

sorry its not in JESP and there are no comments, this wasn't meant for anyone to see but myself...
and i don't understand how to use scopes and structs and shit so thats why im using game cache

 

[Earth-Fury]

from reading your code, this is all i have to say:

• Single letter variable names, by convention, are lower case.
• The common convention is to use i j and k for iterator variables. You use N.
• You use H2U(GetHandleHandle(Trig, "C")) when you could use GetHandleUnit(Trig, "C")
• You use handle vars in the first place, which is just wrong in this day and age. Other, better, more stable and less error prone solutions to the problem of attaching data to handles exist.
• You should look in to vJASS. Its an extension to the JASS language which is precompiled down to normal JASS on save. Check out the JASS NewGen pack, link in my sig.

As for cleaning up the actual code itself... I'm way too sleepy and lazy.

 

[wd40bomber7]

Remember 'FlushHandleLocals' Destroys all gamecache. This might remove the MUI of your trigger.

call TriggerSleepAction(0.0)

Whats the point of that? It'll wait approx .3 seconds before continuing.

 

[grundy]

from reading your code, this is all i have to say:

• Single letter variable names, by convention, are lower case.
• The common convention is to use i j and k for iterator variables. You use N.
• You use H2U(GetHandleHandle(Trig, "C")) when you could use GetHandleUnit(Trig, "C")
• You use handle vars in the first place, which is just wrong in this day and age. Other, better, more stable and less error prone solutions to the problem of attaching data to handles exist.
• You should look in to vJASS. Its an extension to the JASS language which is precompiled down to normal JASS on save. Check out the JASS NewGen pack, link in my sig.

As for cleaning up the actual code itself... I'm way too sleepy and lazy.

i have newgen but theres nothing that i have found that really shows how to use structs and what not in a good way so i just said fuck it. theres nothing wrong with using game cache at all. everyone says its slower but so what its still fast enough that you can't notice any delay at all and as long as you destroy/remove the handles flush the localvars then theres no leaks either its incredibly easy to use and learn and i have no problems with it at all.

i found the problem with this trigger and it was the event of the trigger. sometimes the dummy units cast item illusion so fast that the illusions spawned BEFORE the event was registered to the local trigger. i just moved the register event function before the loop and it works perfectly.

 

[Earth-Fury]

Do people even realize that NewGen comes with the JASSHelper manual? (in Jass NewGen Pack\jasshelper\jasshelpermanual.html) Its also online, linked in the JASSHelper thread... http://wc3campaigns.net/vexorian/jasshelpermanual.html

As for gamecache, there are unavoidable leaks. Namely string leaks. Strings in JASS are immutable. There is barely any garbage cleaning, so when you create a new string, it stays around for a while. I2S() is the issue, of course. As for speed, its not a requirement for many things. But it is a requirement for some. And, as for the general buggyness of I2H(), i guess you'll just have to end up learning the hard way with some of those weird and ever annoying errors. And really, its just a shitty solution for the problem when there are such better and more flexible alternatives.

 

[wd40bomber7]

i guess you'll just have to end up learning the hard way with some of those weird and ever annoying errors. And really, its just a shitty solution for the problem when there are such better and more flexible alternatives.

I'd love to hear those possibilities. While I've thought of a few, I'm open to more. (Like instead of attaching the unit to the struct, attach the struct to a unit!)

 

[grundy]

Do people even realize that NewGen comes with the JASSHelper manual? (in Jass NewGen Pack\jasshelper\jasshelpermanual.html) Its also online, linked in the JASSHelper thread... http://wc3campaigns.net/vexorian/jasshelpermanual.html

yea i know stayed up all night reading it. all the manual does is tell you what the keywords do it doesnt tell you how to use the stuff in a trigger. every time i type the word scope i get a syntax error "expected end of line"

As for gamecache, there are unavoidable leaks. Namely string leaks. Strings in JASS are immutable. There is barely any garbage cleaning, so when you create a new string, it stays around for a while. I2S() is the issue, of course. As for speed, its not a requirement for many things. But it is a requirement for some. And, as for the general buggyness of I2H(), i guess you'll just have to end up learning the hard way with some of those weird and ever annoying errors. And really, its just a shitty solution for the problem when there are such better and more flexible alternatives.

well strings are natives, why do people say natives don't leak if they actually do?

 

[wd40bomber7]

Strings are an exception.
All other natives are 100% superclean.

Besides theres almost no point in knowing about a problem you can't fix or deal with.

 

[Earth-Fury]

yea i know stayed up all night reading it. all the manual does is tell you what the keywords do it doesnt tell you how to use the stuff in a trigger. every time i type the word scope i get a syntax error "expected end of line"


well strings are natives, why do people say natives don't leak if they actually do?

whats so hard about

scope ScopeName
endscope

As for... "natives don't leak"... errr... handles and strings in JASS are pointers. All other data types are not, and thus do not allocate memory (beyond the memory usage of the variable). Strings and handles, when created, do allocate additional memory, holding a pointer in the variable.

 

[grundy]

whats so hard about

scope ScopeName
endscope

its not hard at all. that is exactly what i typed and it highlights that first line and says error expected end of line

 

[Earth-Fury]

its not hard at all. that is exactly what i typed and it highlights that first line and says error expected end of line

Do you have JASS Helper enabled in the JASS Helper Menu?

 

[Need_O2]

did you disable WE syntax checking

(noobz )

(I still wonder why didnt they ban me yet)

 

[grundy]

yes... and yes...

 

[grundy]

hrmmm all of a sudden it works...

ok so i just converted it over to ABC and structs. first spell ever using ABC and first spell ever using structs...

scope ShadowAssault

//Made by: THe PeNDuLuM BRiNGeR

//Instructions:
//
// 1. Copy the "Shadow Assault" spell ability the Object Editor.
// 2. Copy the "Shadow Assault Create Illusion" ability from the Object Editor.
// 3. Copy the "Shadow Assault Illusion" buff from the Object Editor.
// 4. Copy the "Dummy Unit" unit from the Object Editor
//    *If you alredy have a dummy unit for casting spells you can skip this
//
// 5. If you don't already have ABC v5.1 implemented in
//    your map, then copy the "ABC" trigger.
// 6. Copy this trigger.
// 7. Make sure that the rawcode inside this spell's Configuration Menu
//    matches the rawcode of your previously copied "Shadow Assault" spell,
//    and "Shadow Assault Create Illusion" spell.
//
// ***Don't forget to fix the Shadow Assault Create Illusuion's buff if the ID chagnes***
//            ***Don't forget to fix tooltip tags if the ability ID changes***

// ====================================================================================================================
//CONFIGURATION MENU
    globals
        //Rawcode of the spell.
        private constant integer SHADOWASSAULTID = 'A01Y'
        //Rawcode of the create illusion spell.
        private constant integer SHADOWASSAULTCREATEILLUSIONID = 'A01Z'
        //Order ID of the create illusion spell
        private constant integer SHADOWASSAULTCREATEILLUSIONORDERID = 852274
        //Path of the illusion spawn effect
        private constant string ILLUSIONSPAWNEFFECT = "Abilities\\Spells\\Orc\\MirrorImage\\MirrorImageCaster.mdl"
        //path of the illusion death effect
        private constant string ILLUSIONDEATHEFFECT = "Abilities\\Spells\\Orc\\MirrorImage\\MirrorImageDeathCaster.mdl"
        //Distance between illusions and the target
        private constant real ILLUSIONDISTANCE = 150.0
        //Dummy unit type
        private constant integer DUMMYUNITID = 'h002'
        //Generic timed life buff
        private constant integer TIMEDLIFEBUFF = 'BTLF'
        //Magic immune spell id
        private constant integer MAGICIMMUNITY = 'Amim'
    endglobals
    
    private constant function ShadowAssaultIllusionCount takes integer L returns integer
        //How many illusions to create
        return L+3
    endfunction
//END OF CONFIGURATION MENU
//=====================================================================================================================



private struct data
    unit caster
    unit target
    unit attacker
    real targetX
    real targetY
    integer level
    group illusions = CreateGroup()
    integer inGroup
    
    static method create takes unit C, unit T returns data
        local data dat = data.allocate()
        set dat.caster = C
        set dat.target = T
        set dat.targetX = GetUnitX(T)
        set dat.targetY = GetUnitY(T)
        set dat.level = GetUnitAbilityLevel(C, SHADOWASSAULTID)
        set dat.inGroup = 0
        return dat
    endmethod
    
    method onDestroy takes nothing returns nothing
        call DestroyGroup(this.illusions)
    endmethod
    
    method updateCasterPos takes nothing returns nothing
        call SetUnitX(this.caster, GetUnitX(this.attacker))
        call SetUnitY(this.caster, GetUnitY(this.attacker))
        call SetUnitFacing(this.caster, GetUnitFacing(this.attacker))
    endmethod
        
    
    method newAttacker takes nothing returns nothing
        set this.attacker = GroupPickRandomUnit(this.illusions)
        call PauseUnit(this.attacker, false)
        call IssueTargetOrder(this.attacker, "attack", this.target)
        
        call this.updateCasterPos()
    endmethod
    
    method endSpell takes nothing returns nothing
        //call SetUnitX(this.caster, this.targetX)
        //call SetUnitY(this.caster, this.targetY)
        call PauseUnit(this.target, false)
        call ShowUnit(this.caster, true)
        call PauseUnit(this.caster, false)
        call SelectUnitAddForPlayer(this.caster, GetOwningPlayer(this.caster))
        if not IsUnitType(this.target, UNIT_TYPE_DEAD) then
            call IssueTargetOrder(this.caster, "attack", this.target)
        endif
        call this.destroy()
    endmethod
    
    method removeIllusion takes nothing returns nothing
        local effect E = AddSpecialEffect(ILLUSIONDEATHEFFECT, GetUnitX(this.attacker), GetUnitY(this.attacker))
        call DestroyEffect(E)
        
        call GroupRemoveUnit(this.illusions, this.attacker)
        set this.inGroup = this.inGroup-1
        call RemoveUnit(this.attacker)
        set this.attacker = null
        
        if not IsUnitType(this.target, UNIT_TYPE_DEAD) then
            if this.inGroup == 0 then
                call this.endSpell()
            endif
        endif
        
        set E = null
    endmethod
    
    method prematureDeath takes nothing returns nothing
        if this.attacker == null then
            set this.attacker = GroupPickRandomUnit(this.illusions)
        endif
        loop
            exitwhen this.attacker == null
            call this.updateCasterPos()
            call TriggerSleepAction(0.4)
            call this.removeIllusion()
            set this.attacker = GroupPickRandomUnit(this.illusions)
        endloop
        call this.endSpell()
    endmethod
endstruct

private function DoNext takes nothing returns nothing
    local timer T = GetExpiredTimer()
    local data dat = GetTimerStructA(T)
    call dat.removeIllusion()
    if dat.inGroup > 0 then
        call dat.newAttacker()
    else
        call dat.endSpell()
    endif
    
    call ClearTimerStructA(T)
    call DestroyTimer(T)
    set T = null
endfunction

private function ShadowAssaultDamage_Actions takes nothing returns nothing
    local trigger Trig = GetTriggeringTrigger()
    local unit I = GetEventDamageSource()
    local data dat = GetTriggerStructA(Trig)
    local real X
    local real Y
    local timer T
    
    if GetUnitState(dat.target, UNIT_STATE_LIFE)-GetEventDamage() < 0.5 then
        call dat.prematureDeath()
        call ClearTriggerStructA(Trig)
        call DestroyTrigger(Trig)
    elseif IsUnitInGroup(I, dat.illusions) then
        set T = CreateTimer()
        call SetTimerStructA(T, dat)
        call TimerStart(T, 0.3, false, function DoNext)
        if dat.inGroup == 1 then
            call ClearTriggerStructA(Trig)
            call DestroyTrigger(Trig)
        endif
    endif
    
    set Trig = null
    set I = null
    set T = null    
endfunction

private function ShadowAssaultIllusions_Actions takes nothing returns nothing
    local trigger Trig = GetTriggeringTrigger()
    local unit I = GetSummonedUnit()
    local data dat = GetTriggerStructA(Trig)
    local integer N = ShadowAssaultIllusionCount(dat.level)
    local real Angle = 360.0/N
    local real X = dat.targetX + ILLUSIONDISTANCE*Cos(Angle*dat.inGroup*bj_DEGTORAD)
    local real Y = dat.targetY + ILLUSIONDISTANCE*Sin(Angle*dat.inGroup*bj_DEGTORAD)
    local trigger Trig2
    
    local effect E = AddSpecialEffectTarget(ILLUSIONSPAWNEFFECT, I, "origin")
    call DestroyEffect(E)
    
    call GroupAddUnit(dat.illusions, I)
    
    call SetUnitPathing(I, false)
    call UnitAddType(I, UNIT_TYPE_MAGIC_IMMUNE)
    call SetUnitX(I, X)
    call SetUnitY(I, Y)
    call SetUnitTurnSpeed(I, 1.0)
    call SetUnitFacing(I, Angle*dat.inGroup + 180.0)
    call PauseUnit(I, true)
    call UnitAddAbility(I, MAGICIMMUNITY)
    
    set dat.inGroup = dat.inGroup+1
    
    if dat.inGroup == N then
        call ClearTriggerStructA(Trig)
        call DestroyTrigger(Trig)
        call ShowUnit(dat.caster, false)
        set Trig2 = CreateTrigger()
        call SetTriggerStructA(Trig2, dat)
        call TriggerRegisterUnitEvent(Trig2, dat.target, EVENT_UNIT_DAMAGED)
        call TriggerAddAction(Trig2, function ShadowAssaultDamage_Actions)
        call dat.newAttacker()
    endif

    set Trig = null
    set Trig2 = null
    set I = null
endfunction
    

private function ShadowAssault_Conditions takes nothing returns boolean
    return GetSpellAbilityId() == SHADOWASSAULTID
endfunction

private function ShadowAssault_Actions takes nothing returns nothing
    local unit C = GetSpellAbilityUnit()
    local unit T = GetSpellTargetUnit()
    local unit Temp
    local real cX = GetUnitX(C)
    local real cY = GetUnitY(C)
    local player P = GetOwningPlayer(C)
    local integer N = 0
    local trigger Trig = CreateTrigger()
    local data dat = data.create(C, T)
    
    call SetTriggerStructA(Trig, dat)
    call PauseUnit(T, true)
    call PauseUnit(C, true)
    call TriggerSleepAction(0.0)
    call TriggerAddAction(Trig, function ShadowAssaultIllusions_Actions)
    
    loop
        set N = N+1
        set Temp = CreateUnit(P, DUMMYUNITID, cX, cY, 0)
        call UnitAddAbility(Temp, SHADOWASSAULTCREATEILLUSIONID)
        call TriggerRegisterUnitEvent(Trig, Temp, EVENT_UNIT_SUMMON)
        call IssueTargetOrderById(Temp, SHADOWASSAULTCREATEILLUSIONORDERID, C)
        call UnitApplyTimedLife(Temp, TIMEDLIFEBUFF, 2.0)
        exitwhen N >= ShadowAssaultIllusionCount(dat.level)        
    endloop
    
    set C = null
    set T = null
    set Temp = null
    set P = null
    set Trig = null
endfunction

public function InitTrig takes nothing returns nothing
    local trigger Shadow_Assault_Trig = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( Shadow_Assault_Trig, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddCondition( Shadow_Assault_Trig, Condition( function ShadowAssault_Conditions ) )
    call TriggerAddAction( Shadow_Assault_Trig, function ShadowAssault_Actions )
    set Shadow_Assault_Trig = null
endfunction

endscope