• 🏆 Texturing Contest #33 is OPEN! Contestants must re-texture a SD unit model found in-game (Warcraft 3 Classic), recreating the unit into a peaceful NPC version. 🔗Click here to enter!
  • It's time for the first HD Modeling Contest of 2024. Join the theme discussion for Hive's HD Modeling Contest #6! Click here to post your idea!

Rects or Regions leaks

Status
Not open for further replies.
Level 12
Joined
Sep 4, 2007
Messages
407
Ok, requestioning: deos this leaks?
JASS:
private function init takes nothing returns nothing
    local trigger BarTool = CreateTrigger(  )
    local region enterbar = CreateRegion()
    local rect r = Rect(1408.0, 2144.0, 1888.0, 2368.0)
    call RegionAddRect(enterbar, r)
    call TriggerRegisterEnterRegion(BarTool, enterbar, null)
    call TriggerAddCondition( BarTool, Condition( function BarEnter ) )
    call TriggerAddAction( BarTool, function BarEnterNice )
    set BarTool = null
endfunction
 

Bribe

Code Moderator
Level 50
Joined
Sep 26, 2009
Messages
9,464
Yes, it leaks. You should recycle rects because they allocate at least twice the memory leak as locations do. Crazed_seal told you mostly incorrect information. Technically (and I mean strictly technically) regions you click-and-drag in the GUI region editor do not leak, but they still have all the effects of any other leak if you do not remove them.

The only reason they are not technically a "leak" is because it only leaks if you lose the ability to access that rect to remove it. But if you forget to remove it then you might as well call it a leak.

Regions absolutely leak.

JASS:
globals
    //Recycle TempRect each trigger.  When you do "RegionAddRect" the region permanently
    //remembers the rect so you are free to set TempRect to a new rect each time.
    rect TempRect = Rect(0.,0.,0.,0.)
endglobals
private function init takes nothing returns nothing
    local trigger t = CreateTrigger()
    local region r = CreateRegion()
    //notice the native command "SetRect".  this is helpful.
    call SetRect(TempRect,1408., 2144., 1888., 2368.)
    call RegionAddRect(r, TempRect)
    call TriggerRegisterEnterRegion(t, r, null)
    //make sure that you do RemoveRegion(GetTriggeringRegion()), TriggerClearActions(GetTriggeringTrigger()
    //and DestroyTrigger(GetTriggeringTrigger()) when/if you are done with this trigger.
    //if you never destroy the trigger/region, you don't have to set these next two to null:
    set r = null
    set t = null
    //Combine trigger conditions with trigger actions because you're leaking 3 handles
    //per trigger.  This should be:
    call TriggerAddAction(t,function BarEnter)
    //and within function BarEnter you type:
    if(BarEnter Conditions)then
        call BarEnterNice()
    endif
    //That way you only need an action handle per trigger instead of 2 condition handles
    //and an action handle.
endfunction
 
@lelyanra: You should remove the rect.

@Bribe: Yeah, you can recycle rects, but that is usually only with dynamic triggers. But it is perfectly fine to do so, but just not as necessary since you (except for dynamic triggers) will only create/remove them once. =)

//Combine trigger conditions with trigger actions because you're leaking 3 handles
//per trigger. This should be:

Actually, it should be trigger actions merged into conditions. ;D For some reasons:
  • For dynamic triggers, you need to manually TriggerRemoveAction() (and null them, and TriggerClearActions() doesn't help with the handle leak as far as I know) whereas you don't need to remove trigger conditions.
  • You can just use TriggerEvaluate() which is much faster than TriggerExecute(). Plus, most systems use:
    JASS:
    if TriggerEvaluate(t) then
        call TriggerExecute(t)
    endif
    So TriggerEvaluate(t) is more direct. (And then TriggerExecute() is a wasted function call, which is why the functions we use for conditions return false)
  • There is also something about waits with trig actions and trig conditions. Since you can't wait in trigger conditions, you are forced to use a timer. For dynamic triggers, this is good because if two waits occur after the trigger was destroyed it has a chance to double-free the trigger (something along those lines). This is generally just due to carelessness, since with proper coding this can be avoided even if you are using trigger actions, but it still shows that trigger conditions are a bit safer.

So it is mostly conditional (lol pun) but using trigger conditions is a generally safer method.
 

Bribe

Code Moderator
Level 50
Joined
Sep 26, 2009
Messages
9,464
whereas you don't need to remove trigger conditions.

If you need to remove triggeraction, why don't you need to remove triggercondition?

If there are no conditions, TriggerEvaluate defaults true, anway. And if more people started using triggeraction instead of triggercondition then those systems could be updated without the evaluater.

If using TriggerEvaluate is faster than TriggerExecute, is the native event-driven execution of the trigger faster, as well? And wouldn'tTriggerExecute be faster if replaced with ExecuteFunc? But this is me thinking rationally, something blizzard clearly skipped on with many of their codes.
 
I found the thread that I got my info from. It is this one:
http://www.thehelper.net/forums/showthread.php?t=149771

I questioned the same things but I assume it is generally safety issues. But you might want to direct your questions to Jesus4Lyf because I don't know the specifics (I haven't tested it much myself)

If there are no conditions, TriggerEvaluate defaults true, anway. And if more people started using triggeraction instead of triggercondition then those systems could be updated without the evaluater.

Yeah, but you must always consider that some people might implement it into an old map. =P The systems could also be updated to just use TriggerEvaluate() but they don't since they can't expect everyone to follow the correct format. =)

If using TriggerEvaluate is faster than TriggerExecute , is the native event-driven execution of the trigger faster, as well? And wouldn't TriggerExecute be faster if replaced with ExecuteFunc ? But this is me thinking rationally, something blizzard clearly skipped on with many of their codes.

The first, I don't know. I've heard people say conditions were generally faster but I am not sure how reliable they are.

TriggerExecute could be replaced with ExecuteFunc() but people generally lead to not using ExecuteFunc since it is crash-prone if not used correctly. Also, TriggerExecute() factors in multiple actions (lol, for whatever reason) and is useful for systems in general where you don't always know the name of the trigger's actions. =)

But at least in sc2 they fixed this problem. =D
 

Bribe

Code Moderator
Level 50
Joined
Sep 26, 2009
Messages
9,464
From what I've read about the Galaxy Editor you can't make a timer go below 0.4 or something, so SC2 looks like a game I will never waste money on. If "everything" is "done for you" in the "Data Editor" then I don't see it as an opportunity to improve my understanding of programming and apply total creativity like I can with JASS.
 
A bit off-topic, but still on the point. Bribe, don't underestimate Starcraft's Editor. There is a reason why everyone is in a frenzy state. Just because the Beta were a bit disappointing in some features, it doesn't mean that the final product will be a let-down. Warcraft III might not die, but it can handle 48% of what Galaxy Editor can. It's a must for your modding experience.
 

Bribe

Code Moderator
Level 50
Joined
Sep 26, 2009
Messages
9,464
Yeah, I noticed that went offtopic, my bad. To further the offtopic experience, I don't even play this game anymore, I just enjoy coding. So I don't consider myself a modder, I consider myself a programmer. Essentially, there would be no point in doing SC2 if I have so little to gain by typing my own raw codes. I'm not going to waste my time if you can just do it in their GUI system.
 
Level 12
Joined
Sep 4, 2007
Messages
407
this trigger will be used during the entire match, so i should recycle the rect.

JASS:
globals
    rect BarEnter = Rect(0.,0.,0.,0.)
endglobals

scope BarEnter initializer init

    private function enter takes nothing returns nothing
        local player p = GetOwningPlayer(GetTriggerUnit())
        call BJDebugMsg("entering: The Drunken Panda tavern")
        call SetLightIndoors(p)
        set p = null
    endfunction
    

    private function cond takes nothing returns boolean
        local integer LoopA = 1

        loop
            exitwhen LoopA > 12
            if GetTriggerUnit() == PlayerHero[LoopA] then
                if isIndoor[LoopA] == false then
                    call enter()
                    return false
                endif
                return false
            endif
            set LoopA = LoopA + 1    
        endloop
        return false
    endfunction

private function init takes nothing returns nothing
    local trigger BarTool = CreateTrigger(  )
    local region enterbar = CreateRegion()
    call SetRect(BarEnter, 1408., 2200., 1880., 2210.)
    call RegionAddRect(enterbar, BarEnter)
    call TriggerRegisterEnterRegion(BarTool, enterbar, null)
    call TriggerAddCondition( BarTool, Condition( function cond ) )
    set BarTool = null
    set BarEnter = null
endfunction
endscope

so, about this?
 
Last edited:
Level 40
Joined
Dec 14, 2005
Messages
10,532
TriggerExecute is slower than TriggerEvaluate, but I assume it is faster than ExecuteFunc. Why? TriggerExecute and ExecuteFunc both make threads, hence they are slower than TriggerEvaluate. ExecuteFunc needs to search the script, hence it is probably slower (barring major fuckups on Blizzard's part) than TriggerExecute.

--

You don't need to remove triggerconditions because they don't leak; that's all there is to it.
 

Bribe

Code Moderator
Level 50
Joined
Sep 26, 2009
Messages
9,464
You don't need to remove triggerconditions because they don't leak; that's all there is to it.
Ok, so the code mechanism for registering actions causes a leak because each new code reference is like creating a new code object? I get that once you create a Condition any further attempts to create that condition would just reference the same one you already made, but codes not? That would be problematic for people using timers or ForGroup calls, wouldn't it?
 

Bribe

Code Moderator
Level 50
Joined
Sep 26, 2009
Messages
9,464
JASS:
globals
    rect BarEnter = Rect(0.,0.,0.,0.)
endglobals
 
scope BarEnter initializer init
    private function enter takes nothing returns nothing
        local player p = GetTriggerPlayer() //returns the same player as owner of triggering unit.
        call BJDebugMsg("entering: The Drunken Panda tavern")
        call SetLightIndoors(p) //Why are you setting a variable if you're only going to use it once?  Just inline it to SetLightIndoors(GetTriggerPlayer())
        //set p = null <!-- nulling players is a waste of time.  They are never destroyed, so you can have a million pointers to them. -->
    endfunction
 
    private function cond takes nothing returns boolean
        local integer LoopA = 1
        local unit trigUnit = GetTriggerUnit() //don't call GetTriggerUnit() so many times, just make it a variable.
        loop
            //Why do you have the players assigned to 1-12?  It's always easier to zap Player(0) to array[0].
            exitwhen LoopA > 12
            if trigUnit == PlayerHero[LoopA] then
                if not isIndoor[LoopA] then
                    call enter()
                    //return false <!-- no need for this -->
                endif
                exitwhen(true) //return false <!-- don't do a return.  Just exit the loop so that the function can finish and set trigUnit to null. -->
            endif
            set LoopA = LoopA + 1
        endloop
        set trigUnit = null
        return false
    endfunction
 
private function init takes nothing returns nothing
    local trigger BarTool = CreateTrigger(  )
    local region enterbar = CreateRegion()
    call SetRect(BarEnter, 1408., 2200., 1880., 2210.)
    call RegionAddRect(enterbar, BarEnter)
    call TriggerRegisterEnterRegion(BarTool, enterbar, null)
    call TriggerAddCondition( BarTool, Condition( function cond ) )
    //set BarTool = null <!-- Unless you plan to destroy the trigger, don't set it to null. -->
    //set BarEnter = null <!-- Same as above. If the trigger doesn't need destroying, this region doesn't need nulling. -->
endfunction
endscope
 
Status
Not open for further replies.
Top