Big spell help (JASS)

[Silvenon]

Hello, I'm sorry this is one of those "can u pls look at my code" threads. This spell crashes when it's supposed to finish! Maybe related to lightnings, can u pls look at my code?

constant function FireDrain_SpellId takes nothing returns integer
    return 'A008'
endfunction

constant function FireDrain_FireOrbId takes nothing returns integer
    return 'h007'
endfunction

constant function FireDrain_LightningId takes nothing returns string
    return "DRAL"
endfunction

constant function FireDrain_FireOrbHeight takes nothing returns real
    return 200.0
endfunction

constant function FireDrain_DrainHeight takes nothing returns real
    return 50.0
endfunction

constant function FireDrain_TargetCount takes integer lev returns integer
    return 1 + lev
endfunction

constant function FireDrain_Duration takes nothing returns real
    return 15.0
endfunction

constant function FireDrain_AOE takes real lev returns real
    return 450.0 + 50.0 * lev
endfunction

constant function FireDrain_LifeDrained takes real lev returns real
    return 100.0 + 100 * lev
endfunction

constant function FireDrain_LightningEstablishInterval takes nothing returns real
    return 0.5
endfunction

constant function FireDrain_DrainInterval takes nothing returns real
    return 0.5
endfunction

constant function FireDrain_SFX takes nothing returns string
    return "Abilities\\Weapons\\FireBallMissile\\FireBallMissile.mdl"
endfunction

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

function Trig_Fire_Drain_Conditions takes nothing returns boolean
    return GetSpellAbilityId() == FireDrain_SpellId()
endfunction

function FireDrain_Drain takes nothing returns nothing
    local timer t = GetExpiredTimer()
    local unit c = GetHandleUnit(t, "c")
    local group g = GetHandleGroup(t, "g")
    local real lev = I2R(GetHandleInt(t, "lev"))
    local integer i = GetHandleInt(t, "i")
    local integer q = R2I((FireDrain_Duration() + FireDrain_LightningEstablishInterval() * FireDrain_TargetCount(R2I(lev)))/FireDrain_DrainInterval())
    local group g2 = CreateGroup()
    local unit u
    local unit b
    local real x
    local real y
    call GroupAddGroup(g, g2)
    loop
        set u = FirstOfGroup(g2)
        exitwhen u == null
        call GroupRemoveUnit(g2, u)
        set x = GetUnitX(u)
        set y = GetUnitY(u)
        call DestroyEffect(AddSpecialEffect(FireDrain_SFX(), x, y))
        call UnitDamageTarget(c, u, FireDrain_LifeDrained(lev)/I2R(q), true, false, ATTACK_TYPE_CHAOS, DAMAGE_TYPE_FIRE, null)
    endloop
    if i >= q - 1 then
        call FlushHandleLocals(t)
        call DestroyTimer(t)
    else
        call SetHandleInt(t, "i", i + 1)
    endif
    call DestroyGroup(g2)
    set t = null
    set c = null
    set g = null
    set g2 = null
    set b = null
endfunction

function FireDrain_SetLightnings takes nothing returns nothing
    local timer t = GetExpiredTimer()
    local unit c = GetHandleUnit(t, "c")
    local group g = GetHandleGroup(t, "g")
    local group g2 = GetHandleGroup(t, "g2")
    local integer lev = GetHandleInt(t, "lev")
    local unit u = FirstOfGroup(g2)
    local lightning l = GetHandleLightning(u, "l")
    local integer i = GetHandleInt(t, "i")
    call GroupRemoveUnit(g2, u)
    if l != null then
        call SetLightningColor(l, 1, 0, 0, 1)
    endif
    if i >= CountUnitsInGroup(g) - 1 then
        call DestroyGroup(g2)
        call PauseTimer(t)
        call TimerStart(t, FireDrain_DrainInterval(), true, function FireDrain_Drain)
    else
        call SetHandleInt(t, "i", i + 1)
    endif
    set t = null
    set c = null
    set g = null
    set g2 = null
    set l = null
endfunction

function FireDrain_SetPositions takes nothing returns nothing
    local timer t = GetExpiredTimer()
    local unit c = GetHandleUnit(t, "c")
    local unit o = GetHandleUnit(t, "o")
    local group g = GetHandleGroup(t, "g")
    local integer lev = GetHandleInt(t, "lev")
    local group g2 = CreateGroup()
    local unit u
    local integer i = GetHandleInt(t, "i")
    local real x = GetUnitX(c)
    local real y = GetUnitY(c)
    local location p
    local lightning l
    call SetUnitX(o, x)
    call SetUnitY(o, y)
    call GroupAddGroup(g, g2)
    loop
        set u = FirstOfGroup(g2)
        exitwhen u == null
        call GroupRemoveUnit(g2, u)
        set l = GetHandleLightning(u, "l")
        if l != null then
            call MoveLightningEx(l, true, x, y, FireDrain_FireOrbHeight() + FireDrain_DrainHeight(), GetUnitX(u), GetUnitY(u), FireDrain_DrainHeight())
        endif
        if (((x-GetUnitX(u))*(x-GetUnitX(u))+(y-GetUnitY(u))*(y-GetUnitY(u))) >= (FireDrain_AOE(I2R(lev)) + 100)*(FireDrain_AOE(I2R(lev)) + 100) and l != null) or (IsUnitType(u, UNIT_TYPE_DEAD) and l != null) then
            call DestroyLightning(l)
            call SetHandleHandle(u, "l", null)
            call GroupRemoveUnit(g, u)
        endif
    endloop
    if i * Frequency() >= FireDrain_Duration() + FireDrain_LightningEstablishInterval() * FireDrain_TargetCount(lev) then
        loop
            set u = FirstOfGroup(g)
            exitwhen u == null
            call GroupRemoveUnit(g, u)
            call SetHandleHandle(u, "l", null)
        endloop
        call DestroyGroup(g)
        set p = Location(x, y)
        call RemoveUnit(o)
        call DestroyEffect(AddSpecialEffectZ(FireDrain_SFX(), x, y, FireDrain_FireOrbHeight() - GetLocationZ(p)))
        call RemoveLocation(p)
        call FlushHandleLocals(t)
        call DestroyTimer(t)
    else
        call SetHandleHandle(t, "g", g)
        call SetHandleInt(t, "i", i + 1)
    endif
    call DestroyGroup(g2)
    set t = null
    set c = null
    set o = null
    set p = null
    set l = null
    set g = null
    set g2 = null
endfunction

function Trig_Fire_Drain_Actions takes nothing returns nothing
    local timer t1 = CreateTimer()
    local timer t2 = CreateTimer()
    local unit c = GetTriggerUnit()
    local real lev = I2R(GetUnitAbilityLevel(c, FireDrain_SpellId()))
    local real x = GetUnitX(c)
    local real y = GetUnitY(c)
    local unit o = CreateUnit(GetOwningPlayer(c), FireDrain_FireOrbId(), x, y, 0)
    local location p = Location(x, y)
    local real h = FireDrain_FireOrbHeight() - GetLocationZ(p)
    local real r = FireDrain_FireOrbHeight()
    local group g = CreateGroup()
    local group g2 = CreateGroup()
    local boolexpr b
    local unit u
    local integer i = 0
    local integer j
    local lightning l
    call RemoveLocation(p)
    call SetUnitFlyHeight(o, h, r)
    set bj_ghoul[100] = c
    set b = Condition(function TargCond)
    call GroupEnumUnitsInRange(g, x, y, FireDrain_AOE(lev), b)
    call GroupAddGroup(g, g2)
    set j = CountUnitsInGroup(g)
    if j >= FireDrain_TargetCount(R2I(lev)) then
        set j = FireDrain_TargetCount(R2I(lev))
    endif
    call GroupClear(g)
    loop
        exitwhen i == j
        set u = FirstOfGroup(g2)
        call GroupRemoveUnit(g2, u)
        call GroupAddUnit(g, u)
        set l = AddLightningEx(FireDrain_LightningId(), true, x, y, FireDrain_FireOrbHeight() + FireDrain_DrainHeight(), GetUnitX(u), GetUnitY(u), FireDrain_DrainHeight())
        call LightningApplyTimedLife(l, FireDrain_Duration() + FireDrain_LightningEstablishInterval() * FireDrain_TargetCount(R2I(lev)))
        call SetLightningColor(l, 1, 0, 0, 0)
        call SetHandleHandle(u, "l", l)
        set i = i + 1
    endloop
    call SetHandleHandle(t1, "c", c)
    call SetHandleHandle(t1, "o", o)
    call SetHandleHandle(t1, "g", g)
    call SetHandleInt(t1, "lev", R2I(lev))
    call TimerStart(t1, Frequency(), true, function FireDrain_SetPositions)
    call GroupAddGroup(g, g2)
    call SetHandleHandle(t2, "c", c)
    call SetHandleHandle(t2, "g", g)
    call SetHandleHandle(t2, "g2", g2)
    call SetHandleInt(t2, "lev", R2I(lev))
    call TimerStart(t2, FireDrain_LightningEstablishInterval(), true, function FireDrain_SetLightnings)
    call DestroyBoolExpr(b)
    set t1 = null
    set t2 = null
    set c = null
    set o = null
    set p = null
    set g = null
    set g2 = null
    set b = null
    set l = null
endfunction

Sorry for lot of constants, I would do better if I could use vJass (I can't because of certain reasons)

Well anyways, +rep to the one who finds the problem (if you have some code optimization suggestions, feel free to tell me)

 

[Diablo-dk]

Could you tell us what the spell is supposed to do and whereabout it crashes?

Try to remove some of the code or place debug messages around in the script.

btw, no need to destroy boolexpr's or even use them in a variable.

 

[edge[d1]]

Put one call BJDebugMsg("line 0: spell start!") at start of your actions function (last one), below declarations.

if it is displayed on spell-cast, then spread 10 more all over the trigger to see how far it gets. have those print out some variables important to thei location.

if the first message is not displayed on spell cast then it is one of these things:
1: spell is not A008. find it in object editor and predd ctrl+d to see...
2: trigger has no event (not likely, but it happens. people forget.)
3: IsUnitType bug: in TargCond function replace every and not IsUnitType(GetFilterUnit(), UNIT_TYPE_XXX) with
and not ( IsUnitType(GetFilterUnit(), UNIT_TYPE_XXX) == true )... it shouldn't be the case since those "and"s should convert it to "normal true" but who knows...
4: bj_ghoul[100] may be unassigned - disable the condition and print out name of that unit at the spell start. if it is empty-string, it is unassigned...

good luck finding the bug.
by the way, would you tell us why you named a variable bj_ghoul?

 

[Diablo-dk]

bj_ghoul is a global blizzard.j variable, some people use them for conditions.

 

[Silvenon]

if the first message is not displayed on spell cast then it is one of these things:
1: spell is not A008. find it in object editor and predd ctrl+d to see...
2: trigger has no event (not likely, but it happens. people forget.)
3: IsUnitType bug: in TargCond function replace every and not IsUnitType(GetFilterUnit(), UNIT_TYPE_XXX) with
and not ( IsUnitType(GetFilterUnit(), UNIT_TYPE_XXX) == true )... it shouldn't be the case since those "and"s should convert it to "normal true" but who knows...
4: bj_ghoul[100] may be unassigned - disable the condition and print out name of that unit at the spell start. if it is empty-string, it is unassigned...

This spell crashes when it's supposed to finish!

Ok, now the description: this spell creates lightnings ("DRAL") on number of targets depending on the level of the ability (it links targets to the caster). It's links targets one by one, then when the linking is over, it's draining in an interval of 0.5, a fire effect spawns on each target on every pulse (though units aren't damaging and I think the caster isn't getting any bonus life!! You may check that also....), then when the spell is over, the lightnings should be destroyed and the orb that was created above the caster (I forgot to mention that) should be destroyed to (with a fire effect), but instead of that, it crashes.

Thanks for trying

 

[Daelin]

Boolean expressions are objects. It's false to assume that once they are created they don't need to be destroyed. Atleast at the point at which I quit modding, they were being destroyed as they leaked! If further studies revealed the fact that BoolExprs don't leak that find. But as far as I know, as long as an object is created and stored in a local variable, if not destroyed before any link to it is lost we have a leak.

-Mihai

 

[Dr Super Good]

I would destroy it to be on the safe side, methods of detecting leaks arnt really reliable and so it might still leak but not increase the demand on the CPU to preform a comparision using them.

 

[PurplePoot]

According to a few people who tested it, BoolExprs (except those generated with And() or Or()) are treated like Strings in many regards.

 

[Silvenon]

Tnx guys, I didn't know that about BoolExprs. But what about the crash in my spell?

 

[Daelin]

The destruction native, I believe it works! It was made to destroy a boolean expression. There is no such function for strings... Therefore, I presume in the case boolexpr, it actually works. These are my 2 cents, the function call doesn't eat that much processing, so it doesn't harm using it.

-Mihai

 

[Silvenon]

Ok, I'll use DestroyBoolExpr just in case, bc it doesn't do any harm. Tnx Daelin

Does anybody have a clue why does my spell crash??

 

[PurplePoot]

Sorry, nope. I'll have a more thourough look when I can find some more time.

Daelin -- I meant in the fact that they're reused

 

[PurgeandFire]

Ok, I'll use DestroyBoolExpr just in case, bc it doesn't do any harm. Tnx Daelin

Does anybody have a clue why does my spell crash??

Vexorian said that it's dangerous to destroy boolexprs. In fact, I think it is more harm to destroy it than not too.

As for your spell, I'll look into it.

 

[Daelin]

Vexorian said that? Apparently things changed... I based my statement upon this post: http://www.wc3campaigns.net/showpost.php?p=823513&postcount=8

-Mihai

 

[Dr Super Good]

I see no harm in destroying them, and I do it alot.

 

[PurplePoot]

http://www.wc3campaigns.net/showthread.php?t=93168

There's where I'm getting it from.

Oh, and silvenon, I highly doubt this is the cause of your problems, but you forgot to pause your periodic timers before destroying them.

 

[Silvenon]

Tnx guys, in my spells I always destroy boolexprs like that, and nothing crashes.

Oh, and silvenon, I highly doubt this is the cause of your problems, but you forgot to pause your periodic timers before destroying them.

Really??? I never do that and nothing bad happens, but you're the man
I'll do that from now on.

 

[PurplePoot]

Apparently the timers will sometimes keep running if you don't pause them before destroying them. Never tested it myself, but it doesn't really hurt to do it anyways X.x

 

[Dr Super Good]

The timer runs 1 more time after its destruction which can result in bugs, leaks or worse.

 

[Silvenon]

This is not the problem, bc it crashes every time.......but tnx for the advice. Grrr stupid spell........I'll have to look at that stupid code for 2 hours.....

EDIT1: I found the damage problem, I forgot to do attach the caster to that other timer, now the damage works, and I'm almost positive the problem is not in FireDrain_Drain function. I'll narrow the checking field for you:

if i * Frequency() >= FireDrain_Duration() + FireDrain_LightningEstablishInterval() * FireDrain_TargetCount(lev) then
    loop
        set u = FirstOfGroup(g)
        exitwhen u == null
        call GroupRemoveUnit(g, u)
        call FlushHandleLocals(u)
    endloop
    call DestroyGroup(g)
    set p = Location(x, y)
    call RemoveUnit(o)
    call DestroyEffect(AddSpecialEffectZ(FireDrain_SFX(), x, y, FireDrain_FireOrbHeight() - GetLocationZ(p)))
    call RemoveLocation(p)
    call FlushHandleLocals(t)
    call DestroyTimer(t)
else
    call SetHandleHandle(t, "g", g)
    call SetHandleInt(t, "i", i + 1)
endif

I'm almost positive the problem is in there.....Because that's what happens when the spell ends.

EDIT2: Hmm the crash reason must be destroying lightnings, when I removed the line with LightningApplyTimedLife, it didn't crash. It is possible that in that function lies the problem, but maybe not:

function LightningApplyTimedLife_child takes nothing returns nothing
    local timer t = GetExpiredTimer()
    local lightning l = GetHandleLightning(t, "l")
    if l != null then
        call DestroyLightning(l)
    endif
    call SetHandleHandle(t, "l", null)
    call DestroyTimer(t)
    set t = null
    set l = null
endfunction

function LightningApplyTimedLife takes lightning l, real w returns nothing
    local timer t = CreateTimer()
    call SetHandleHandle(t, "l", l)
    call TimerStart(t, w, false, function LightningApplyTimedLife_child)
    set t = null
endfunction

EDIT3: Ok, I updated the code. The problem is in LightningApplyTimedLife, when I remove it, nothing crashes. But If I try to destroy them differently, with putting destroy lightning in the if/then/else of FireDrain_SetPosition, but it didn't work. Please guys, give it one more shot, I think you will find the problem.

EDIT4: Oh, no need, I found the problem