[JASS] Smiple JASS trigger (pleas critics)

[Equal]

Hi, i did those 2 simple functions that moves a unit to the left(i guess)
just need to know if it is leakless and MUI.
in other Words just some critics or suggestions to improve it.

function bla takes unit dingo , real r ,real a returns nothing
local  unit U = dingo
local real angle = a
local real D = r
local real uY = GetWidgetY(U) + D * Cos(angle * bj_DEGTORAD)
local real uX = GetWidgetX(U) + D * Sin(angle * bj_DEGTORAD)
call SetUnitPosition (U,uX,uY)
set U = null
endfunction


function F1 takes nothing returns nothing
local unit U = GetTriggerUnit()
call bla(U,100,180)
set U = null
endfunction

Thx for help
~Equal

 

[Need_O2]

so this is a PolarProjectionBJ with cordinates
seems leakless

I didnt get why do you create another locals to use parameters, you can use parameters directly

and you may use SetUnitX and SetUnitY instead of SetUnitPosition

And I suggest GetUnitX istead of GetWidgetX
(dont ask why)

function bla takes unit dingo , real r ,real a returns nothing
SetUnitX(dingo,GetUnitX(dingo) + r * Cos(a *(22/7)/180))
SetUnitY(dingo,GetUnitY(dingo) + r * Sin(a *(22/7)/180))
endfunction

function F1 takes nothing returns nothing
local unit U = GetTriggerUnit()
call bla(U,100,180)
set U = null
endfunction

This is my suggestion
Good jass makings to you
--- End of Reply ---

 

[PurplePoot]

22/7? Use bj_PI. However, using bj_DEGTORAD works just as well...

And you forgot call.

Also also, you don't need the unit local in F1.

function bla takes unit dingo, real r, real a returns nothing
    call SetUnitX(dingo,GetUnitX(dingo) + r * Cos(a * bj_DEGTORAD)
    call SetUnitY(dingo,GetUnitY(dingo) + r * Sin(a * bj_DEGTORAD)
endfunction

function F1 takes nothing returns nothing
    call bla(GetTriggerUnit(),100,180)
endfunction

 

[PurgeandFire]

Yes, yours is leakless and MUI. You can't go wrong with reals.

Anyways, use Poot's optimized version, it is more... um, optimized. (And yes, it is MUI and leakless)

 

[Equal]

THX for the help guys, will learn from this.

EDIT: btw. ao this is how we "pass" locals to other functions?

 

[PurplePoot]

function <name> takes <type> <name>, <type> <name>, <type> <name>, <etc> returns <type>

Eg;

function Hello takes string disp returns boolean

[jass=Then, for the contents of Hello, we could have]function Hello takes string disp returns boolean
if disp != null then
call BJDebugMsg(disp)
return true
endif
return false
endfunction[/code]

[jass=And to call it, for example]local string s = <some string>
local boolean b = Hello(s)
//or
call Hello(s)[/code]

 

[Need_O2]

22/7?
Also also, you don't need the unit local in F1.

1- I think its Pi already, well I like using 22/7 xD it makes me feel good somehow (omg psyco)
2- I know local not needed but he seems beginner in locals etc

 

[PurplePoot]

1- I think its Pi already, well I like using 22/7 xD it makes me feel good somehow (omg psyco)

x.x...

2- I know local not needed but he seems beginner in locals etc

Teaching him to overuse locals only leads the wrong way =/

 

[Need_O2]

true
my bad
have a good day (message was too short for true;my bad xD)

 

[HINDYhat]

Also, remember that inlining functions is also important! So let's say you have your two functions here:

function bla takes unit dingo, real r, real a returns nothing
    call SetUnitX(dingo,GetUnitX(dingo) + r * Cos(a * bj_DEGTORAD)
    call SetUnitY(dingo,GetUnitY(dingo) + r * Sin(a * bj_DEGTORAD)
endfunction
function F1 takes nothing returns nothing
    call bla(GetTriggerUnit(),100,180)
endfunction

You could inline that, to avoid the bla() function call.
As useless as it sounds, calling ANY user-created function (just calling an empty function) is actually slower than calling the Sin() native, which is already considered slow! (I'd link you to the proof of this statement, but I can't seem to find the link... )

So inlining is basically removing the bla() call, and adding the whole called function inside the calling function:

function F1 takes nothing returns nothing
    local unit u=GetTriggerUnit() // We store it in a local because you would otherwise call the GetTriggerUnit() function twice.
    call SetUnitX(u,GetUnitX(u)+100*Cos(180*bj_DEGTORAD) // We could replace 180*bj_DEGTORAD by 3.14159 or bj_PI, but I'll forget that for now.
    call SetUnitY(u,GetUnitY(u)+100*Sin(180*bj_DEGTORAD) // Same as above.
    
    set u=null // Removing memory leaks.
endfunction

If you see the before-last line of that last script, you'd see that I did set u=null. This is because of memory leaks. Now, REALLY basically, memory leaks are caused by objects in wc3 that are not destroyed/nulled. We don't need to destroy the unit, so we will null the variable. Nulling a variable is simply setting it to null.

The only types of variables that we have to set to null are local handles. Handles are every type of variable in warcraft 3 EXCEPT boolean,string,integer,real,code. So let's say you have a local destructable:

function SomeFunctiont takes nothing returns nothing
    local destructable d=CreateDestructable(// Something goes here...
    // Do some actions...
    set d=null
endfunction

All handles can have the "null" value, and it's important to null them at the end of every function where you have a local handle.

There is alot more information on memory leaks in tutorials on this site:
http://www.hiveworkshop.com/forums/showthread.php?t=34049
If you do a simple google search, I'm sure you could find alot of information!

 

[PurplePoot]

Not necessarily, it can be good if you call that a lot and in situations where speed isn't that much of an issue, HINDY.

 

[HINDYhat]

Speed is almost always an issue.

 

[Equal]

Thx for the Link and thx all for the Help and Teaching.

 

[PurplePoot]

Speed is almost always an issue.

Not at all true -.-