[vJASS] how to improve ...

[mateuspv]

library ThundergodsWrath initializer onInit uses SpellEffectEvent
globals
private constant integer    ABIL_CODE             = 'A03Z'
private constant integer    ABIL_CODE_DUMMY       = 'A03W'
private constant integer    ABIL_FLY              = 'Amrf'
private constant integer    DUMMY                 = 'n00V'
private constant string     EFFECT_1              = "Abilities\\Weapons\\ChimaeraLightningMissile\\ChimaeraLightningMissile.mdl"
private constant string     EFFECT_2              = "Abilities\\Weapons\\Bolt\\BoltImpact.mdl"
private constant real       REAL                  = 1000
endglobals

private function TargetFilter takes nothing returns boolean
    return IsUnitType(GetFilterUnit(),UNIT_TYPE_HERO) and IsUnitEnemy(GetFilterUnit(),GetOwningPlayer(GetTriggerUnit())) and GetUnitAbilityLevel(GetFilterUnit(),'Aloc') == 0
endfunction

private function Thundergods_Wrath_Bolt takes nothing returns nothing
    local unit u = GetTriggerUnit()
    local unit e = GetEnumUnit()
    local unit d = CreateUnit(GetOwningPlayer(u),DUMMY,GetUnitX(e),GetUnitY(e),0)
    call UnitAddAbility(d,ABIL_FLY)
    call UnitAddAbility(d,ABIL_CODE_DUMMY)
    call SetUnitAbilityLevel(d,ABIL_CODE_DUMMY,GetUnitAbilityLevel(u,ABIL_CODE)+4)
    call SetUnitFlyHeight(d,REAL,0)
    call IssueTargetOrder(d,"chainlightning",e)
    call UnitApplyTimedLife(d,'BTLF',3)
    set e = null
    set d = null
endfunction

private function onCast takes nothing returns nothing
    local group g = CreateGroup()
    local unit u = GetTriggerUnit()
    local unit p
    local unit d
    local boolexpr b = Condition(function TargetFilter)
    call DestroyEffect(AddSpecialEffectTarget(EFFECT_1,u,"hand right"))
    call DestroyEffect(AddSpecialEffectTarget(EFFECT_2,u,"origin"))
    call GroupEnumUnitsInRect(g,GetPlayableMapRect(),b)
    call ForGroup(g,function Thundergods_Wrath_Bolt)
    call DestroyBoolExpr(b)
    call DestroyGroup(g)
    set u = null
    set p = null
    set d = null
    set g = null
    set b = null
endfunction

private function onInit takes nothing returns nothing
    call RegisterSpellEffectEvent(ABIL_CODE, function onCast)
endfunction
endlibrary

How to improve it
-GetPlayableMapRect
-TargetFilter
-ForGroup and GroupEnumUnitsInRect(I have printout that Magtheridon96 said something to me once ...

let your opinion, tip, tell about a mistake, all is well accepted
I have a printout that already created a topic like this...
thanks

 

[dudeim]

This is a little optimized might be possible to optimize more, not sure though.
I changed your ForGroup() call to a FirstOfGroup() loop as it's slightly faster and easier to code with. Also removed some unused locals.

library ThundergodsWrath initializer onInit uses SpellEffectEvent
globals
private constant integer    ABIL_CODE             = 'A03Z'
private constant integer    ABIL_CODE_DUMMY       = 'A03W'
private constant integer    ABIL_FLY              = 'Amrf'
private constant integer    DUMMY                 = 'n00V'
private constant string     EFFECT_1              = "Abilities\\Weapons\\ChimaeraLightningMissile\\ChimaeraLightningMissile.mdl"
private constant string     EFFECT_2              = "Abilities\\Weapons\\Bolt\\BoltImpact.mdl"
private constant real       REAL                  = 1000
endglobals

private function TargetFilter takes nothing returns boolean
    return IsUnitType(GetFilterUnit(),UNIT_TYPE_HERO) and IsUnitEnemy(GetFilterUnit(),GetOwningPlayer(GetTriggerUnit())) and GetUnitAbilityLevel(GetFilterUnit(),'Aloc') == 0
endfunction

/*private function Thundergods_Wrath_Bolt takes nothing returns nothing
    local unit u = GetTriggerUnit()
    local unit e = GetEnumUnit()
    local unit d = CreateUnit(GetOwningPlayer(u),DUMMY,GetUnitX(e),GetUnitY(e),0)
    call UnitAddAbility(d,ABIL_FLY)
    call UnitAddAbility(d,ABIL_CODE_DUMMY)
    call SetUnitAbilityLevel(d,ABIL_CODE_DUMMY,GetUnitAbilityLevel(u,ABIL_CODE)+4)
    call SetUnitFlyHeight(d,REAL,0)
    call IssueTargetOrder(d,"chainlightning",e)
    call UnitApplyTimedLife(d,'BTLF',3)
    set e = null
    set d = null
endfunction*/

private function onCast takes nothing returns nothing
    local group g = CreateGroup()
    local unit u = GetTriggerUnit()
    //local unit p //unused
    local unit d
    local unit l //the looping unit
    local boolexpr b = Condition(function TargetFilter)
    call DestroyEffect(AddSpecialEffectTarget(EFFECT_1,u,"hand right"))
    call DestroyEffect(AddSpecialEffectTarget(EFFECT_2,u,"origin"))
    call GroupEnumUnitsInRect(g,bj_mapInitialPlayableArea,b) //changing the function call to a variable read (bj_mapInitialPlayableArea)
    //call ForGroup(g,function Thundergods_Wrath_Bolt) //making a FOG loop of it
    loop
        set l = FirstOfGroup(g)
        exitwhen l == null
        set d = CreateUnit(GetOwningPlayer(u),DUMMY,GetUnitX(l),GetUnitY(l),0)
        call UnitAddAbility(d,ABIL_FLY)
        call UnitAddAbility(d,ABIL_CODE_DUMMY)
        call SetUnitAbilityLevel(d,ABIL_CODE_DUMMY,GetUnitAbilityLevel(u,ABIL_CODE)+4)
        call SetUnitFlyHeight(d,REAL,0)
        call IssueTargetOrder(d,"chainlightning",l)
        call UnitApplyTimedLife(d,'BTLF',3)
        set l = null //nulling the looped unit not sure if needed but thought it was:P
        set d = null
    endloop
        
    call DestroyBoolExpr(b)
    call DestroyGroup(g)
    set u = null
    //set p = null
    //set d = null // this one is not needed anymore as we null in inside the loop
    set g = null
    set b = null
endfunction

private function onInit takes nothing returns nothing
    call RegisterSpellEffectEvent(ABIL_CODE, Filter(function onCast))
endfunction
endlibrary

 

[Dr Super Good]

set l = null //nulling the looped unit not sure if needed but thought it was

This is not needed.
FirstOfGroup(g) returns null when the group is empty.

However, your code is an infinite loop. You fail to remove unit l from group g. Thus it will loop infinitly until the op limit is reached.

 

[mateuspv]

private function onInit takes nothing returns nothing
    call RegisterSpellEffectEvent(ABIL_CODE, Filter(function onCast))
endfunction
//
private function onInit takes nothing returns nothing
    call RegisterSpellEffectEvent(ABIL_CODE, function onCast)
endfunction

if I use the first I get an error

 

[dudeim]

This is not needed.
FirstOfGroup(g) returns null when the group is empty.

However, your code is an infinite loop. You fail to remove unit l from group g. Thus it will loop infinitly until the op limit is reached.

Ooh frick forgot that I created a system that does that in my map to ease the progress so forgot about that.

@mateuspv
Hmm probably the system doesn't allow conditions then well then use your version.

library ThundergodsWrath initializer onInit uses SpellEffectEvent
globals
private constant integer    ABIL_CODE             = 'A03Z'
private constant integer    ABIL_CODE_DUMMY       = 'A03W'
private constant integer    ABIL_FLY              = 'Amrf'
private constant integer    DUMMY                 = 'n00V'
private constant string     EFFECT_1              = "Abilities\\Weapons\\ChimaeraLightningMissile\\ChimaeraLightningMissile.mdl"
private constant string     EFFECT_2              = "Abilities\\Weapons\\Bolt\\BoltImpact.mdl"
private constant real       REAL                  = 1000
endglobals

private function TargetFilter takes nothing returns boolean
    return IsUnitType(GetFilterUnit(),UNIT_TYPE_HERO) and IsUnitEnemy(GetFilterUnit(),GetOwningPlayer(GetTriggerUnit())) and GetUnitAbilityLevel(GetFilterUnit(),'Aloc') == 0
endfunction

/*private function Thundergods_Wrath_Bolt takes nothing returns nothing
    local unit u = GetTriggerUnit()
    local unit e = GetEnumUnit()
    local unit d = CreateUnit(GetOwningPlayer(u),DUMMY,GetUnitX(e),GetUnitY(e),0)
    call UnitAddAbility(d,ABIL_FLY)
    call UnitAddAbility(d,ABIL_CODE_DUMMY)
    call SetUnitAbilityLevel(d,ABIL_CODE_DUMMY,GetUnitAbilityLevel(u,ABIL_CODE)+4)
    call SetUnitFlyHeight(d,REAL,0)
    call IssueTargetOrder(d,"chainlightning",e)
    call UnitApplyTimedLife(d,'BTLF',3)
    set e = null
    set d = null
endfunction*/

private function onCast takes nothing returns nothing
    local group g = CreateGroup()
    local unit u = GetTriggerUnit()
    //local unit p //unused
    local unit d
    local unit l //the looping unit
    local boolexpr b = Condition(function TargetFilter)
    call DestroyEffect(AddSpecialEffectTarget(EFFECT_1,u,"hand right"))
    call DestroyEffect(AddSpecialEffectTarget(EFFECT_2,u,"origin"))
    call GroupEnumUnitsInRect(g,bj_mapInitialPlayableArea,b) //changing the function call to a variable read (bj_mapInitialPlayableArea)
    //call ForGroup(g,function Thundergods_Wrath_Bolt) //making a FOG loop of it
    loop
        set l = FirstOfGroup(g)
        exitwhen l == null
        set d = CreateUnit(GetOwningPlayer(u),DUMMY,GetUnitX(l),GetUnitY(l),0)
        call UnitAddAbility(d,ABIL_FLY)
        call UnitAddAbility(d,ABIL_CODE_DUMMY)
        call SetUnitAbilityLevel(d,ABIL_CODE_DUMMY,GetUnitAbilityLevel(u,ABIL_CODE)+4)
        call SetUnitFlyHeight(d,REAL,0)
        call IssueTargetOrder(d,"chainlightning",l)
        call UnitApplyTimedLife(d,'BTLF',3)
        call GroupRemoveUnit(g,l)
        set d = null
    endloop
        
    call DestroyBoolExpr(b)
    call DestroyGroup(g)
    set u = null
    //set p = null
    //set d = null // this one is not needed anymore as we null in inside the loop
    set g = null
    set b = null
endfunction

private function onInit takes nothing returns nothing
    call RegisterSpellEffectEvent(ABIL_CODE, function onCast)
endfunction
endlibrary

 

[mateuspv]

thanks, was enlightening
one more question
I really need this since we use only once?

private constant real       REAL                  = 1000

 

[Maker]

^You don't need it, but it is good to have since it is easier to configure then.

You don't have to null d inside the loop, null it at the end of the trigger.

Also, local boolexp is not needed, -> call GroupEnumUnitsInRect(g,bj_mapInitialPlayableArea, function TargetFilter)

 

[mateuspv]

yes yes it's true
thanks maker