A game crashing error

06-05-2010, 08:26 PM

I have a spell that deals damage every half a second for 4 seconds, but there's something in the code that is causing Warcraft to exit. There's no fatal error message or anything. Warcraft just... exits.

It looks fine to me, but I'm obviously overlooking something. Can someone look it over? I've tried to find it, but to no avail. It's been a few days since the spell went haywire. What's weird is that it used to work perfectly fine.

JASS:

scope Arc initializer Init

globals
    private unit c
    private real x
    private real y
    private integer Count = 0
    private integer Limit = 8
endglobals

private function Conditions takes nothing returns boolean
    return GetSpellAbilityId() == 'A0H3'
endfunction

private function Filterer takes nothing returns boolean
    return IsUnitEnemy(GetFilterUnit(), GetOwningPlayer(c)) == true and GetWidgetLife(GetFilterUnit()) > .0204
endfunction

private function dcallback takes nothing returns nothing
    local unit t = GetEnumUnit()
    local real d = ((30. + (GetHeroInt(c, true)))
    call UnitDamageTargetEx(c, t, "spell", d, ATTACK_TYPE_SIEGE, DAMAGE_TYPE_COLD)
endfunction

private function callback takes nothing returns nothing
    local timer t = GetExpiredTimer()
    local group g = CreateGroup()
    call GroupEnumUnitsInRange(g, x, y, 300., Filter(function Filterer))
    call ForGroup(g, function callback)
    call GroupClear(g)
    set Count = Count + 1
    if Count == Limit then
        call PauseTimer(t)
        set Count = 0
        call DestroyTimer(t)
        set c  = null
        set g = null
        call DestroyGroup(g)
    endif
endfunction

private function SpellActions takes nothing returns nothing
    local timer t = CreateTimer()
    local integer Percent
    set c = GetTriggerUnit()
    set x = GetLocationX(GetSpellTargetLoc())
    set y = GetLocationY(GetSpellTargetLoc())
    call TimerStart(t, .5, true, function callback)
endfunction

//===========================================================================
private function Init takes nothing returns nothing
    local trigger AB = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ(AB, EVENT_PLAYER_UNIT_SPELL_EFFECT)
    call TriggerAddCondition(AB, Condition(function Conditions))
    call TriggerAddAction(AB, function SpellActions)
endfunction

endscope

I thought I was using the timer wrong, but all of the other timer spells are doing fine...? I also noticed that the trigger exits Warcraft right when damage is being applied.

06-05-2010, 08:56 PM	#2
Bribe	JASS: set g = null call DestroyGroup(g) Flip those around, then let me see if I can find something else, too. Ok, the crash is caused by this: JASS: private function callback takes nothing returns nothing call ForGroup(g, function callback) Notice the group is doing its callback to the exact same function that started it? This gives an infinite loop, which is a crash.

06-06-2010, 12:20 PM

Tot

Quote:

Originally Posted by Bribe

[jass]
Ok, the crash is caused by this:

JASS:

    private function callback takes nothing returns nothing
        call ForGroup(g, function callback)

Notice the group is doing its callback to the exact same function that started it? This gives an infinite loop, which is a crash.

the bare loop wont be a problem, the problem is the memory allocation for the locals, which won't be freed, cause the function running endlessly. Because of the jass-VM having a memory-limit which causes upon exceed the vm to crash, what's again exiting the game the same way as via the task-manager (thread-termination).

06-06-2010, 12:38 PM	#4
Ammorth	Last time I checked, an inifinite loop causes issues. Even if he doesn't null his locals (yes, they will leak), his main concern is the infinite loop.

06-06-2010, 12:48 PM

Tot

Quote:

Originally Posted by Ammorth

Last time I checked, an inifinite loop causes issues. Even if he doesn't null his locals (yes, they will leak), his main concern is the infinite loop.

the problem is not not-nulling them, the problem is the local-block

@ignited:

improved code (still not mui):

scope Arc initializer Init

globals
    private unit c
    private real x
    private real y
    private integer Count = 0
    private integer Limit = 8

    private player p //contains c's owner (for filterer)
    private group g = CreateGroup() //recycle them, it's faster
    private timer t = CreateTimer()
    private boolexpr filter //wont change, so give him a global
endglobals

private function Conditions takes nothing returns boolean
    return GetSpellAbilityId() == 'A0H3'
endfunction

private function Filterer takes nothing returns boolean
    local unit u = GetFilterUnit() //locals are faster than function-calls
    set b = (IsUnitEnemy(u,p) and GetWidgetLife(u) > .0204)
    set u = null //don't leak
    return b
endfunction

private function callback takes nothing returns nothing
    local unit u
    local real d = 30. + GetHeroInt(c, true) //store it so save some function-calls
    call GroupClear(g)
    set p = GetOwningPlayer(c)
    call GroupEnumUnitsInRange(g, x, y, 300., filter)
    loop  //this way of looping is possible cause there won't be any shadows insode g, cause g is clear and filled in the lines straight above in the same func
        set u = FirstOfGroup(g)
        exitwhen u == null //groop is empty, exit loop
        call UnitDamageTargetEx(c, u, "spell", d, ATTACK_TYPE_SIEGE, DAMAGE_TYPE_COLD)
        call GroupRemoveUnit(g,u)
    endloop
    set u = null //Don't leak
    set Count = Count + 1
    if Count == Limit then
        call PauseTimer(t)
        set Count = 0
        set c  = null
    endif
endfunction

private function SpellActions takes nothing returns nothing
    set c = GetTriggerUnit()
    set x = GetSpellTargetX() //only useable in 1.24+, but is faster and doesn't leak a location
    set y = GetSpellTargetY()
    call PauseTimer(t) //bypass some strange timer thingies
    call TimerStart(t, .5, true, function callback)
endfunction

//===========================================================================
private function Init takes nothing returns nothing
    local trigger AB = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ(AB, EVENT_PLAYER_UNIT_SPELL_EFFECT)
    call TriggerAddCondition(AB, Condition(function Conditions))
    call TriggerAddAction(AB, function SpellActions)
    set AB=null //don't leak
    set filter=Condition(funtion Filterer)
endfunction

endscope

06-06-2010, 02:16 PM	#6
Bribe	JASS: set AB=null //don't leak Wasteful, nulling a spell trigger is no better than nulling a player handle.

06-06-2010, 11:02 PM

Ignitedstar

Quote:

Originally Posted by Bribe

JASS:

            set g = null
            call DestroyGroup(g)

Flip those around, then let me see if I can find something else, too.

Ok, the crash is caused by this:

JASS:

    private function callback takes nothing returns nothing
        call ForGroup(g, function callback)

Notice the group is doing its callback to the exact same function that started it? This gives an infinite loop, which is a crash.

... Man. Well, duh. No wonder why it crashes. An infinite loop. That's what I get for naming functions near identical save the small 'd' at the beginning. lol, and it's funny because I've done tiny things like this all of the time and stuff breaks badly.

Thanks for helping me with something so tiny. Haha.

Well, I might as well bring it up, since Tot is talking about it. It's not that I'm aware that it is better to recycle groups than to remove them and make new ones. However, I don't understand how we recycle them when whenever I make a group, it's going to be a local or a private global and thus can't be used by anything else. How does this work?

The next thing I'd be worried about are leaking variables. I thought I did a good job of cleaning up, but it looks like it's more complicated than I thought...

Oh, is it better to make a loop to do damage against multiple units via FirstOfGroup? And the reason why I didn't use GetSpellTargetX/Y is because I thought those only worked for units, not points. I can see how it could because the name of the native is sort of vague as there's potential to get the coordinates of a spot or something on that spot.

06-06-2010, 11:32 PM

Bribe

SpellTargetX/Y returns:

If "no target" --> the coordinates of the caster (I think).
If "unit target" --> the coordinates of the target.
If "point target" --> the coordinates of the point.

Group Handling:

Usually you never even need to use "ForGroup". You can almost always stick it all in the "Filter". If you don't need to reference those units later on, then you never should even bother adding units to that group, either (filter should return "false" for any circumstance for no units to be added).

Thus, in that case, you can use a single global group for all of your triggers (GroupUtils labels this ENUM_GROUP, though bj_lastCreatedGroup works too, as it's never destroyed).

06-07-2010, 05:53 AM

Tot

Quote:

Originally Posted by Bribe

if no target, it sometimes returns crap, but you should know weather your spell is targeted or not, so it's n big deal.

Quote:

Usually you never even need to use "ForGroup"

that's wrong. cause of blizz beeing stupid you shall use it for "storage"-groups, cause shadow units cause FirstOfGroup() to returns null, but aren't the last one inside the group --> you're missing some units

yea simply use a single global, but remember never to "store" anything in there for longer than a function call (causes wired bugs; I know what I'm talking about) and never forget to clear it before useage.

06-07-2010, 06:23 AM	#10
Bribe	GroupEnum$Something$ clears the group before adding new units, so is there really a point to clearing it first? And what I meant by hardly needing ForGroup... I don't know about you but 90% of the time when I use a group it's just a simple "grab units, do a few things, done with group", not for storage, so I use a global group for everything except special cases.

06-07-2010, 07:42 AM

#11

Tot

Quote:

Originally Posted by Bribe

GroupEnum$Something$ clears the group before adding new units, so is there really a point to clearing it first?

i think it doesn't clear the group before; it simply adds the new units to it

06-07-2010, 11:00 AM	#12
Bribe	It does remove all units from the group automatically before enumerating. I wanted to combine enumeration and group-storage to one group for efficiency, but each time I did GroupEnum it wiped out what units were in it prior. I'm just not sure if doing it like this removes ghost reference the same way GroupClear does (likely).

06-07-2010, 10:55 PM	#13
Ignitedstar	Huh. And group recycling won't cause any problems when the group being recycled is called simultaneously by different functions?

06-08-2010, 02:50 AM

#14

Bribe

To eyes and imagination, it may seem that they are running "simultaneously", but in the game's internal engine it is only running one batch at a time. You might not be able to guess which "simultaneous" process it chooses first, but no two events in WC3 truly happen at the same time.

Using a global group for enumerations that don't stick past the nanosecond duration of the function is perfectly safe.