HomeUser Control Panel (unavailable in archive)ForumsTutorialsArt GalleryResourcesMaps

Questions about Leaks & Conditions and whatever

03-27-2008, 02:58 AM#1
Burning Rose
Well I'm seriously entering JASS, I got the New Gen Pack and it's working (Sorta, there's another post about that). I'm wondering a few things about Memory Leaks and Conditions.
First, are functions like "GetTriggeringUnit" and whatnot, functions that return handles, bad to use in the middle of a function? Like is doing this:
Collapse JASS:
function Trig_Untitled_Trigger_005_Actions takes nothing returns nothing
    local unit u = GetTriggerUnit()
    call KillUnit(u)
    set u = null
endfunction
neccisarily better for memory leaks than this?:
Collapse JASS:
function Trig_Untitled_Trigger_005_Actions takes nothing returns nothing
    call KillUnit( GetTriggerUnit() )
endfunction

And about Conditions, I noticed that the way Conditions are in JASS if they were converted from GUI seems wierd. They do an IF/THEN/ELSE to see if a function is wrong, and then return false, else they return true, among a lot of other stuff. Like these:
Collapse JASS:
function Trig_Taunt_Cast_Func005C takes nothing returns boolean
    if ( ( GetSpellAbilityId() == 'ANta' ) ) then
        return true
    endif
    if ( ( GetSpellAbilityId() == 'ACsi' ) ) then
        return true
    endif
    if ( ( GetSpellAbilityId() == 'A075' ) ) then
        return true
    endif
    if ( ( GetSpellAbilityId() == 'A076' ) ) then
        return true
    endif
    if ( ( GetSpellAbilityId() == 'A074' ) ) then
        return true
    endif
    return false
endfunction

function Trig_Taunt_Cast_Conditions takes nothing returns boolean
    if ( not Trig_Taunt_Cast_Func005C() ) then //Why does it Reference the other condition instead of just doing what 005C does? 
        return false
    endif
    return true
endfunction

And why does it do this:
Collapse JASS:
function Trig_Taunt_Cast_Func002C takes nothing returns boolean
    if ( not ( GetUnitUserData(GetTriggerUnit()) == 1 ) ) then
        return false
    endif
    return true
endfunction

Instead of this (If this works, which is my real question, does this work?):
Collapse JASS:
function Trig_Taunt_Cast_Func002C takes nothing returns boolean
    return GetUnitUserData(GetTriggerUnit()) == 1
endfunction

So in other words, is what I *think* can be changed and made more efficient truly as simple as it looks, or is it fine, or is there a different way?
(Sorry for the Noobness, but I am learning >_>)
03-27-2008, 03:05 AM#2
Ammorth
Quote:
Originally Posted by Burning Rose
First, are functions like "GetTriggeringUnit" and whatnot, functions that return handles, bad to use in the middle of a function? Like is doing this:
Collapse JASS:
function Trig_Untitled_Trigger_005_Actions takes nothing returns nothing
    local unit u = GetTriggerUnit()
    call KillUnit(u)
    set u = null
endfunction
neccisarily better for memory leaks than this?:
Collapse JASS:
function Trig_Untitled_Trigger_005_Actions takes nothing returns nothing
    call KillUnit( GetTriggerUnit() )
endfunction

The second version is more efficient if that is all you are doing. Using GetTriggerUnit() doesn't create a new handle, it just references the handle.

If your function was going to do more things with GetTriggerUnit(), it might be more efficient to set GetTriggerUnit() to a local variable and then use the local (as functions are "expensive" ). The threshold where it becomes faster varies with the function. I'm not too sure myself either, but usually more than 4 references and I use a local.

Quote:
Originally Posted by Burning Rose
And about Conditions, I noticed that the way Conditions are in JASS if they were converted from GUI seems wierd. They do an IF/THEN/ELSE to see if a function is wrong, and then return false, else they return true, among a lot of other stuff. Like these:
Collapse JASS:
function Trig_Taunt_Cast_Func005C takes nothing returns boolean
    if ( ( GetSpellAbilityId() == 'ANta' ) ) then
        return true
    endif
    if ( ( GetSpellAbilityId() == 'ACsi' ) ) then
        return true
    endif
    if ( ( GetSpellAbilityId() == 'A075' ) ) then
        return true
    endif
    if ( ( GetSpellAbilityId() == 'A076' ) ) then
        return true
    endif
    if ( ( GetSpellAbilityId() == 'A074' ) ) then
        return true
    endif
    return false
endfunction

function Trig_Taunt_Cast_Conditions takes nothing returns boolean
    if ( not Trig_Taunt_Cast_Func005C() ) then //Why does it Reference the other condition instead of just doing what 005C does? 
        return false
    endif
    return true
endfunction

And why does it do this:
Collapse JASS:
function Trig_Taunt_Cast_Func002C takes nothing returns boolean
    if ( not ( GetUnitUserData(GetTriggerUnit()) == 1 ) ) then
        return false
    endif
    return true
endfunction

Instead of this (If this works, which is my real question, does this work?):
Collapse JASS:
function Trig_Taunt_Cast_Func002C takes nothing returns boolean
    return GetUnitUserData(GetTriggerUnit()) == 1
endfunction

So in other words, is what I *think* can be changed and made more efficient truly as simple as it looks, or is it fine, or is there a different way?
(Sorry for the Noobness, but I am learning >_>)

The reason the script is so fucked up when you convert from GUI to Jass was that it was easier to convert the GUI in that manor than it was to do it properly (either blizzard was lazy, or they thought it didn't matter).

Your second condition is as fast as it's going to get, but it is till better to in-line the conditions:

Collapse JASS:
function Condition001 takes integer i returns boolean
    return i == 5
endfunction

function SomeFunc takes nothing returns nothing
    local integer f = 3
    if Condition001(f) then
        // do stuff
    endif
endfunction

function SomeFuncOptimized takes nothing returns nothing
    local integer f = 3
    if f == 5 then // the condition is part of the main function and doesn't need to be called (saving a function call and shortening the code)
        // do stuff
    endif
endfunction
03-27-2008, 03:16 AM#3
Burning Rose
Ah. I see. Well, I got my syntax Checker Working and now it's telling me that apparently the preplaced Units, and the Regions on my maps are not valid variables. What do I do about that? =_=
If it helps:
Hidden information:
Collapse JASS:
function Trig_Red_Units_Actions takes nothing returns nothing
    local unit first = GetTrainedUnit()
    local integer SpawnTypez = GetUnitTypeId(first)
    local location red = GetRectCenter(gg_rct_Red_Barracks)
    local location grey = GetRectCenter(gg_rct_Grey_attack_move)
    local location brown = GetRectCenter(gg_rct_Brown_Attack_move)
    local unit second = CreateUnitAtLoc( Player(0), SpawnTypez, red, 0 )
    call SetUnitOwner( first, Player(0), true )
    call IssuePointOrderLocBJ( first, "attack", grey )
    call SetUnitUserData( first, 1 )
    call IssuePointOrderLocBJ( second, "attack", brown )
    call SetUnitUserData( second, 2 )
    set first = null
    set red = null
    set grey = null
    set brown = null
    set second = null
endfunction

//===========================================================================
function InitTrig_Red_Units takes nothing returns nothing
    local trigger Red_Units = CreateTrigger(  )
    call TriggerRegisterUnitEvent( Red_Units, gg_unit_h001_0019, EVENT_UNIT_TRAIN_FINISH )
    call TriggerRegisterUnitEvent( Red_Units, gg_unit_h001_0020, EVENT_UNIT_TRAIN_FINISH )
    call TriggerRegisterUnitEvent( Red_Units, gg_unit_h001_0018, EVENT_UNIT_TRAIN_FINISH )
    call TriggerAddAction( Red_Units, function Trig_Red_Units_Actions )
    set Red_Units = null
endfunction


Oh and the "GetTriggerUnit" was just an example. I mean anything that returns a handle, like "GetRectCenter", or whatever.
03-27-2008, 10:20 AM#4
Captain Griffen
The syntax checker doesn't check the whole script. Anything not in that trigger won't be included in the syntax check. Main reason why I never use that syntax checker...I just save my map.
03-27-2008, 09:54 PM#5
Burning Rose
Well it seems to work when I saved it... Alright then.

Basically what I wanted to know is: My map is currently entirely GUI, and I'm transitioning into JASS now, and was wondering what sort of things (Like certain kinds of functions, certain actions, etc.) I should watch for memory leaks. I figure it's important, seeing as my map can get kinda choppy.
03-30-2008, 11:00 PM#6
Pyrogasm
Well, you have 3 location leaks in that code you recently posted, so...

I would also watch for group leaks.
03-31-2008, 08:11 PM#7
Burning Rose
~_~ But i set them to null. Do you have to do something else for locations? If so, what others do that? :(
04-01-2008, 12:13 AM#8
Ammorth
Almost all handles have to be removed or destroyed with their appropriate destruction function.

call RemoveLocation(loc)
04-01-2008, 01:02 AM#9
Anitarf
Also, one of the first things you should learn when switching to Jass is that most things are just easier to do without locations, by using coordinates instead.
04-01-2008, 08:06 PM#10
Burning Rose
Ahh, alright then.