TriggerRegisterAnyUnitEventBJ Question

12-14-2008, 04:04 PM	#1
dead_or_alivex	Is it true that TriggerRegisterAnyUnitEventBJ() leaks each time a trigger using it is fired, due to the null filter? I just read that it was okay to use the function, but then read about it leaking somewhere else, so I was wondering if the issues with it (if any) were serious enough to warrant a replacement function.

12-14-2008, 04:56 PM	#2
Here-b-Trollz	You shouldn't have to worry about leaks, since you aren't creating dynamic triggers with this - and if you are, you're crazy. And really, I wouldn't think null would leak. I don't know where you read this, but a lot of people are misinformed about... lots of things.

12-14-2008, 05:22 PM	#3
chobibo	You should be worrying more about the triggeraction accompanied by the trigger instead of the event registration, most of the time, that is where most leaks come from.

12-14-2008, 05:49 PM	#4
Zerzax	Just use it. In my opinion it is the most useful BJ out there. The null filter is negligible.

12-14-2008, 06:08 PM	#5
DioD	you can "fix" this bj function, by adding non null condition if you like.

12-15-2008, 11:20 AM	#6
saw792	It doesn't leak every time the event is fired. It leaks once, when you call it.

12-15-2008, 12:37 PM

Karawasa

Quote:

Originally Posted by Here-b-Trollz

you aren't creating dynamic triggers with this - and if you are, you're crazy...a lot of people are misinformed about...lots of things.

I'd be interested in learning more about why creating dynamic triggers with this is crazy (and why dynamic triggers in general are crazy). Anyone care to enlighten with a post or thread (I tried searching but failed, and that flood protection makes it a useless function somewhat).

12-15-2008, 12:50 PM

Here-b-Trollz

Quote:

Originally Posted by Karawasa

Dynamic trigger + RegisterAnyUnitEvent = crazy
Dynamic trigger + RegisterUnitEvent = stupid

That you'd randomly be creating an destroying triggers registered to every unit makes no sense, when you could just be using a boolean variable or something.

That'd you'd be using dynamic triggers for stuff is also stupid. Other than RegisterUnitInRange, there's no reason. Though it might actually faster to just do a group enum every .03 ~ .1 seconds.

12-15-2008, 03:43 PM	#9
chobibo	Plenty of problem arises when one tries to destroy triggers, it includes memory leaks, corrupts handles, and invokes fatal errors, there's more, I just don't know. Maybe Captain Griffen could give you a link to the thread discussing this.

12-15-2008, 11:01 PM

#10

Karawasa

Don't mean to hijack the thread, but his question has already been answered and I don't want to start a new one...

I've been using this type of code block in a couple different triggers:

JASS:

//===========================================================================
public function Register takes unit whichUnit returns nothing
    local integer index  = GetUnitIndex(whichUnit)
   
    if PUI_Trigger[index] != null then
        call DestroyTrigger(PUI_Trigger[index])
    endif
    set PUI_Trigger[index] = CreateTrigger()

    call TriggerRegisterUnitStateEvent(PUI_Trigger[index], whichUnit, UNIT_STATE_LIFE, GREATER_THAN_OR_EQUAL, 0.405)
    call TriggerAddCondition(PUI_Trigger[index], Condition(function Revive))
endfunction

It fires a lot, and a lot of games have been played. That is to say, it has been tested more than most code here. Yet no problems that I can tell...

What should I be looking for?

12-15-2008, 11:20 PM

#11

moyack

The problem is with the DestroyTrigger function which in some special cases screw up the handle count, and this is the beginning of a huge failure in your map. This post is the key: http://www.wc3campaigns.net/showpost...4&postcount=19

the idea is to avoid destroying the trigger while the action is running, and it can happen if you have waits in that function. So a solution is to develop the code in such way that it ensures it will destroy the triggers when they have run or when they are not really needed.

12-15-2008, 11:23 PM	#12
PandaMine	Well if what Vex is saying in that post that Moyack linked is correct, you just need to use Destroy Trigger at the end of the function (i.e. when every thread finishes) and not at the start. Only issue is finding out if the thread that the trigger created has finished before the function ends its execution

12-15-2008, 11:28 PM

#13

moyack

Quote:

Originally Posted by PandaMine

Well if what Vex is saying in that post that Moyack linked is correct, you just need to use Destroy Trigger at the end of the function (i.e. when every thread finishes) and not at the start

Not only at the end, but ensuring that it won't have any chances of being called anymore.

12-15-2008, 11:37 PM	#14
PandaMine	Yeah I just edited (finished reading the whole thread). In any way, this just means that you shouldn't use DestroyTriggers on threads that are very long (such as heavy loops or waits or whatnot). Remember that triggers do not leak at every execution, they only leak when they are "activated"

12-15-2008, 11:38 PM

#15

cohadar

TriggerRegisterAnyUnitEventBJ is not a BJ function and it does not leak when it fires event.

Using TriggerAddAction with dynamic triggers equals random map crashes.

A lot of other stuff people say: "don't use that" actually means "don't use that because you are not experienced enough in jass to avoid problems with it"

99% of bugs are people who write code not thinking about all options.
One classic example are spells that attach something to unit.
Most common mistake is to forget that same spell can be used on same unit twice.

bad:

JASS:

local unit target = GetSpellTargetUnit()
call AttachSomething(target, data)
// do other stuff

good:

JASS:

local unit target = GetSpellTargetUnit()
local Data data = GetSomeAttachedData(target)
if data == 0 then  // is data already attached to unit?
    // new data
    set data = Data.create(target)
    call AttachSomething(target, data)
else 
    // reload new info in already attached data
    call data.reload(target)
endif
// do other stuff

Ye the second one is much more complicated but it is the price of being MUI and not crashing.