HomeUser Control Panel (unavailable in archive)ForumsTutorialsArt GalleryResourcesMaps

Help with TimerUtils debug errors.

12-05-2010, 07:45 AM#1
Titanhex
So I have TimerUtils and recently realized my Timers weren't working properly after a period of time. I enabled debug mode and am going through it. These are some of the problems I've discovered and would love to get fixed, but I don't understand what the problem mentioned is.

Lets start with my hellfire spell. It's a channeled spell that will call down brimstone (Rain of Fire FX) in an area, damaging anything it hits. Each stone does AoE damage upon impact in a small radius.

HellFire

Collapse JASS:
scope HellFire initializer I

globals
    private constant integer SPELLID        = 'HeFi'
endglobals

private struct Data

    unit c
    timer tim
    timer tim1
    integer lvl
    real x
    real y
    real x1
    real y1

    static method Fire takes nothing returns nothing
     local xedamage d=xedamage.create()
     local thistype this = GetTimerData(GetExpiredTimer())
     local integer lvl = GetUnitAbilityLevel(.c, SPELLID)
     local real dam = lvl*8+10+GetHeroInt(.c, true)*(0.4+0.1*lvl)
        call d.factor( UNIT_TYPE_STRUCTURE, 0.2 )
        set d.atype= ATTACK_TYPE_NORMAL
        set d.dtype= DAMAGE_TYPE_MAGIC
        call d.damageAOE(.c, .x1, .y1, 60, dam)
        call ReleaseTimer(.tim1)
        call d.destroy()
        call .destroy()
    endmethod

    static method Timer takes nothing returns nothing
     local thistype this = GetTimerData(GetExpiredTimer())
     local real ang = GetRandomReal(0,2*bj_PI)
     set .x1 = .x+GetRandomReal(50,400)*Cos(ang)
     set .y1 = .y+GetRandomReal(50,400)*Sin(ang)
     set .tim1 = NewTimer()
        if GetUnitCurrentOrder(.c) == OrderId("starfall") then
            call DestroyEffect(AddSpecialEffect(GetAbilityEffectById(SPELLID, EFFECT_TYPE_MISSILE, 0), .x1, .y1))
            call SetTimerData(.tim1, this)
            call TimerStart(.tim1, 0.3, false, function thistype.Fire)
        else
            set .c = null
            call .destroy()
            call ReleaseTimer(.tim)
        endif
    endmethod

    static method create takes unit c, real x, real y, integer i returns thistype
     local thistype this = thistype.allocate()
        set .tim = NewTimer()
        set .c = c
        set .x = x
        set .y = y
        call SetTimerData(.tim,this)
        call TimerStart(.tim, 0.3*i+GetRandomReal(0.1,-0.1), true, function thistype.Timer)
     return this
    endmethod

endstruct

private function Conditions takes nothing returns nothing
 local integer i = 1
 local integer lvl = GetUnitAbilityLevel(GetTriggerUnit(), SPELLID)
 local real x = GetUnitX(GetTriggerUnit())
 local real y = GetUnitY(GetTriggerUnit())
    if GetSpellAbilityId() == SPELLID then
        loop
            call Data.create(GetTriggerUnit(), x, y, i)
        exitwhen i > lvl*10+8
            set i = i + 1
        endloop
    endif
endfunction

//===========================================================================
public function I takes nothing returns nothing
 local trigger t = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( t, EVENT_PLAYER_UNIT_SPELL_CHANNEL )
    call TriggerAddAction( t, function Conditions )
    call XE_PreloadAbility(SPELLID)
 set t = null
endfunction

endscope

the error is: Double free of type: HellFire___Data.

I have a second question too. Would it be better to create multiple instances of the struct or a single Timer within the struct?

I'll be posting other errors I get when I get the chance.
12-05-2010, 08:51 AM#2
Anitarf
It is unfortunate that you didn't have debug mode enabled up to this point, since you would have discovered the bug the moment you introduced it. This is going to be difficult to debug now.

As far as this spell goes, I don't see how you could get a double free with this code alone. There are some other issues, like a missing loop statement in the Conditions function. Considering the spell works for you, you must have had that statement there when testing, so you must have deleted it when copying the code here. Did you delete anything else? We can't really debug your code if you remove the bug.

You're also leaking a timer (.tim doesn't get released in all cases), but unless TimerUtils is giving you an error, this leak hasn't yet caused them to bug. Nevermind, I didn't notice it was a periodic timer. I was expecting it to be a one-shot timer since you create multiple structs, however it seems you create multiple structs and then each one of them creates multiple effects, each one at slightly randomized (+-0.1) increasing (0.3*i) intervals. This was somewhat confusing. Since the Fire method destroys the struct, and it runs multiple times due to the timer tim being periodic, you get double frees.

There are a few other issues, though not really bugs: your method of getting a random point in circle is flawed and you could use more calibration constants, they really help with code maintainability.
12-05-2010, 09:04 AM#3
Titanhex
Yeah, I just caught that after enabling the spell again and saving. Was editing it while it was disabled and testing an alternative method. (The alternative method fails to do damage but does not give the error.) I've updated my first post with the working version of the spell.

How about if I move the ReleaseTimer(.tim) in my Timer method after the endif. Will that fix the leak?

While it's not giving an error, it is driving TimerUtils closer to it's stack limit.

I'm not aware of a better way to get a random point in a circle. Also, what are these calibration methods you speak of?

Unfortunately I only recently discovered debug mode while going through the JASSHelper manual.

EDIT: Set the periodic to false on .tim's TimerStart function. Figured it wouldn't serve any real purpose. This also seems to be what was causing the Double Free like you said, destroying the struct when the timer is still going.
12-05-2010, 10:21 AM#4
Anitarf
Yes, moving ReleaseTimer after the endif would fix the leak. It would also "fix" the bug, since it would no longer matter whether the timer is periodic because it would get released the first time it expired anyway.

The problem with your method for picking a point in circle is that you pick a different random distance for x and y coordinates, so the distribution might be a bit odd, though probably still reasonably random. A secondary problem is that you get a more sparse distribution of points towards the edges of your area of effect. You can use this as a reference for how to avoid that.