[vJASS] Crit Strike

[RupTure_QS]

So I asked for a bit of advice in the World Editor Help Section about a damage detection system. Here is what I created, now is there anything I could do to make it more efficient or better? It works as I want it to but I asked for the purpose to learn not to work with whatever. No I do not want to use an already created system, does not allow me to learn.

scope CriticalStrike initializer CS_onInit
    globals
        trigger damageEvent
        texttag array critTag
        integer tagIndex = 0
    endglobals
    
    struct CS_Variables extends array
        static unit source
        static unit target
    endstruct
    
    private function CS_onDamage takes nothing returns boolean
        local real damageAmount
        local integer i = 0
        local real Pi = 3.14159
        local real vel = 0.0355
        local real xVel = vel * Cos(Pi/2)
        local real yVel = vel * Sin(Pi/2)
        
        set CS_Variables.source = GetEventDamageSource()
        if GetUnitAbilityLevel(CS_Variables.source, 'CS00') >= 1 then
            set damageAmount = GetEventDamage()
            if damageAmount == 0.00 then
                return false
            endif
            
            set CS_Variables.target = GetTriggerUnit()
            set i = GetRandomInt(1, 4)
            if i == 1 then
                set tagIndex = tagIndex + 1
                set critTag[tagIndex] = CreateTextTag()
                call DisableTrigger(damageEvent)
                call UnitDamageTarget(CS_Variables.source, CS_Variables.target, damageAmount, true, false, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_NORMAL, WEAPON_TYPE_WHOKNOWS)
                call EnableTrigger(damageEvent)
                call SetTextTagText(critTag[tagIndex], R2S(damageAmount * 2), 0.0345)
                call SetTextTagPosUnit(critTag[tagIndex], CS_Variables.target, -64.00)
                call SetTextTagColor(critTag[tagIndex], 255, 0, 0, 255)
                call SetTextTagPermanent(critTag[tagIndex], false)
                call SetTextTagVelocity(critTag[tagIndex], xVel, yVel)
                call SetTextTagFadepoint(critTag[tagIndex], 0.00)
                call SetTextTagLifespan(critTag[tagIndex], 3.00)
            endif
            return true
        endif
        return false
    endfunction
    
    private function CS_restoreTriggers takes nothing returns nothing
        local unit enumUnit = GetEnumUnit()
        
        if GetWidgetLife(enumUnit) > 0.406 then
            call TriggerRegisterUnitEvent(damageEvent, enumUnit, EVENT_UNIT_DAMAGED)
        endif
        
        set enumUnit = null
    endfunction
    
    private function CS_clearMemory takes nothing returns nothing
        local group g = CreateGroup()
        local code c_onDamage = function CS_onDamage
        local integer i = 1
        
        call BJDebugMsg("Running cleanup")
        
        call GroupEnumUnitsInRect(g, GetWorldBounds(), null)
        call ResetTrigger(damageEvent)
        call DestroyTrigger(damageEvent)
        set damageEvent = null
        
        set damageEvent = CreateTrigger()
        call TriggerAddCondition(damageEvent, Condition(c_onDamage))
        call ForGroup(g, function CS_restoreTriggers)
        
        loop
            exitwhen critTag[i] == null
            call DestroyTextTag(critTag[i])
            set critTag[i] = null
            call BJDebugMsg("critTag[" + I2S(i) + "] nulled and destroyed")
            set i = i + 1
        endloop
        
        call DestroyGroup(g)
        set c_onDamage = null
        set g = null
    endfunction
    
    private function CS_indexEnteringUnit takes nothing returns nothing
        local unit triggerUnit = GetTriggerUnit()
        
        call TriggerRegisterUnitEvent(damageEvent, triggerUnit, EVENT_UNIT_DAMAGED)
        call BJDebugMsg(GetUnitName(triggerUnit) + " registered")
        
        set triggerUnit = null
    endfunction
    
    private function CS_mapInit takes nothing returns nothing
        local unit enumUnit = GetEnumUnit()
        
        call TriggerRegisterUnitEvent(damageEvent, enumUnit, EVENT_UNIT_DAMAGED)
        call BJDebugMsg(GetUnitName(enumUnit) + " registered")
        
        set enumUnit = null
    endfunction
    
    private function CS_onInit takes nothing returns nothing
        local group g = CreateGroup()
        local region r = CreateRegion()
        local trigger unitEnters = CreateTrigger()
        local trigger clearMemory = CreateTrigger()
        local code c_onDamage = function CS_onDamage
        local code c_unitEnters = function CS_indexEnteringUnit
        local code c_clearMemory = function CS_clearMemory
        
        set damageEvent = CreateTrigger()
        
        call TriggerAddCondition(damageEvent, Condition(c_onDamage))
        call GroupEnumUnitsInRect(g, GetWorldBounds(), null)
        call ForGroup(g, function CS_mapInit)
        
        call RegionAddRect(r, GetWorldBounds())
        call TriggerRegisterEnterRegion(unitEnters, r, null)
        call TriggerAddAction(unitEnters, c_unitEnters)
        
        call TriggerRegisterTimerEvent(clearMemory, 60.00, true)
        call TriggerAddAction(clearMemory, c_clearMemory)
        
        call PauseUnit(gg_unit_hfoo_0006, true)
        
        call DestroyGroup(g)
        set c_clearMemory = null
        set c_unitEnters = null
        set c_onDamage = null
        set clearMemory = null
        set unitEnters = null
        set r = null
        set g = null
    endfunction
endscope

 

[Cokemonkey11]

I suggest reading this -

http://www.hiveworkshop.com/forums/...on-using-first-group-loop-enumeration-223140/

Edit:

You can also replace your timer trigger with this

call TimerStart(CreateTimer(), 60., true, function refresh)

That way there's no need for creating a trigger object.

Lastly, you do not need to destroy texttag manually - this is one of the few datatypes with dynamic memory management built in

 

[RupTure_QS]

Doesn't that leak a timer because you're not setting it to a variable? Thank you though, I thought that texttags destroyed themselves, but I wasn't sure. I will also try to implement the FirstOfGroup() loop.

 

[Cokemonkey11]

Doesn't that leak a timer because you're not setting it to a variable? Thank you though, I thought that texttags destroyed themselves, but I wasn't sure. I will also try to implement the FirstOfGroup() loop.

If you create an object and don't destroy it, it's a leak.

But if you create an object and never want to stop using it, what is it then?

(The same holds for triggers - triggers can leak as well)

 

[RupTure_QS]

I see, danke schön.

 

[RupTure_QS]

scope CriticalStrike initializer CS_onInit
    globals
        trigger damageEvent
        texttag array critTag
        integer tagIndex = 0
    endglobals
    
    struct CS_Variables extends array
        static unit source
        static unit target
    endstruct
    
    private function CS_onDamage takes nothing returns boolean
        local real damageAmount
        local integer i = 0
        local real Pi = 3.14159
        local real vel = 0.0355
        local real xVel = vel * Cos(Pi/2)
        local real yVel = vel * Sin(Pi/2)
        
        set CS_Variables.source = GetEventDamageSource()
        if GetUnitAbilityLevel(CS_Variables.source, 'CS00') >= 1 then
            set damageAmount = GetEventDamage()
            if damageAmount == 0.00 then
                return false
            endif
            
            set CS_Variables.target = GetTriggerUnit()
            set i = GetRandomInt(1, 1)
            if i == 1 then
                set tagIndex = tagIndex + 1
                set critTag[tagIndex] = CreateTextTag()
                call DisableTrigger(damageEvent)
                call UnitDamageTarget(CS_Variables.source, CS_Variables.target, damageAmount, true, false, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_NORMAL, WEAPON_TYPE_WHOKNOWS)
                call EnableTrigger(damageEvent)
                call SetTextTagText(critTag[tagIndex], R2S(damageAmount * 2), 0.0345)
                call SetTextTagPosUnit(critTag[tagIndex], CS_Variables.target, -64.00)
                call SetTextTagColor(critTag[tagIndex], 255, 0, 0, 255)
                call SetTextTagPermanent(critTag[tagIndex], false)
                call SetTextTagVelocity(critTag[tagIndex], xVel, yVel)
                call SetTextTagFadepoint(critTag[tagIndex], 0.00)
                call SetTextTagLifespan(critTag[tagIndex], 3.00)
            endif
            return true
        endif
        return false
    endfunction
    
    private function CS_clearMemory takes nothing returns nothing
        local group g = CreateGroup()
        local code c_onDamage = function CS_onDamage
        local unit firstOfGroup
        local integer i = 1
        
        call BJDebugMsg("Running cleanup")
        
        call GroupEnumUnitsInRect(g, GetWorldBounds(), null)
        call ResetTrigger(damageEvent)
        call DestroyTrigger(damageEvent)
        set damageEvent = null
        
        set damageEvent = CreateTrigger()
        call TriggerAddCondition(damageEvent, Condition(c_onDamage))
        loop
            set firstOfGroup = FirstOfGroup(g)
            exitwhen firstOfGroup == null
            call GroupRemoveUnit(g, firstOfGroup)
            if GetWidgetLife(firstOfGroup) > 0.406 then
                call TriggerRegisterUnitEvent(damageEvent, firstOfGroup, EVENT_UNIT_DAMAGED)
            endif
        endloop
        
        set tagIndex = 0
        loop
            exitwhen critTag[i] == null
            set critTag[i] = null
            call BJDebugMsg("critTag[" + I2S(i) + "] nulled")
            set i = i + 1
        endloop
        
        call DestroyGroup(g)
        set c_onDamage = null
        set g = null
    endfunction
    
    private function CS_indexEnteringUnit takes nothing returns nothing
        local unit triggerUnit = GetTriggerUnit()
        
        call TriggerRegisterUnitEvent(damageEvent, triggerUnit, EVENT_UNIT_DAMAGED)
        call BJDebugMsg(GetUnitName(triggerUnit) + " registered")
        
        set triggerUnit = null
    endfunction
    
    private function CS_onInit takes nothing returns nothing
        local group g = CreateGroup()
        local region r = CreateRegion()
        local unit firstOfGroup
        local trigger unitEnters = CreateTrigger()
        local code c_onDamage = function CS_onDamage
        local code c_unitEnters = function CS_indexEnteringUnit
        local code c_clearMemory = function CS_clearMemory
        
        set damageEvent = CreateTrigger()
        
        call TriggerAddCondition(damageEvent, Condition(c_onDamage))
        call GroupEnumUnitsInRect(g, GetWorldBounds(), null)
        loop
            set firstOfGroup = FirstOfGroup(g)
            exitwhen firstOfGroup == null
            call GroupRemoveUnit(g, firstOfGroup)
            call TriggerRegisterUnitEvent(damageEvent, firstOfGroup, EVENT_UNIT_DAMAGED)
            call BJDebugMsg(GetUnitName(firstOfGroup) + " registered")
        endloop
        
        call RegionAddRect(r, GetWorldBounds())
        call TriggerRegisterEnterRegion(unitEnters, r, null)
        call TriggerAddAction(unitEnters, c_unitEnters)
        
        call TimerStart(CreateTimer(), 30.00, true, c_clearMemory)
        
        call DestroyGroup(g)
        set c_clearMemory = null
        set c_unitEnters = null
        set c_onDamage = null
        set unitEnters = null
        set r = null
        set g = null
    endfunction
endscope

How's this?

 

[Wietlol]

You use an action as Trigger response.
Conditions are slightly faster. (Just make sure to return false at all cost.)
In this case you can add a filter to the event... I think that using the filter instead of the condition is even more efficient... ain't it?

You also add critical strike to damage done by spells... but I assume that that is intentional.

You can start arrays at 0... and just to make it make more sense, you should use it.

PI is a constant "bj_PI". I suggest you to use those instead of using magic numbers.