phyrex1an
Staff Member and irregular helper
- Reaction score
- 447
Why do I need this?
So you just read your first jass tutorial and starded to learn jass, actualy you are pretty good at it.
Your own codes works as planned and you can eliminate the few errors you find on your own.
Everything is nice, or?
To become a true jass user you must not only be able to write jass code, you must make the code as effective as possible. If your only goal is to make the script work you will not make better code than a good GUI user that uses some lines of 'Custom Script' to remove leaks.
[SIZE=+1]Lesson 1: Remove unnecessary function calls[/SIZE]
Introduction
When coding you should always try to get as few function calls as possible.
The number that counts is the TOTAL amount, not the amount used in your function.
If you call a function that itself calls 3 functions that is a total of 4.
By calling the 3 functions yourself you get a total of 3. That is one less.
Also if you use GetTriggerUnit() 3 times in your script, why don't use a varaible that holds
the value of GetTriggerUnit()? If you do you remove 2 function calls.
This is of course a balance between making short scripts and making few function calls.
If you have a function calls a function that have 40 function calls, inline the 40 functions will
make the main function so much longer that it is not worth the one less function call.
Use Natives
Most of the BJ (blizzard.j) functions are just fluff functions that does nothing but calling a native function
If you call a BJ of that type you have 100% more function calls that if you call the native direct.
So use natives before BJ's. That is the biggest rule.
Find unnecessary functions
The easiest way to find unnecessary functions is to look them up somewhere.
I suggest that you get a Jass writing program that has all functions in it. You can find programs like this at www.wc3sear.ch.
If you don't want to use a program or you are unsure what a function does look it up at www.wc3jass.com.
An easy way to say if the function call is unneeded and you can inline the function instead is to look at the name.
If the name ends with an BJ it is most likely unnecessary. There is exceptions, some BJ functions does a real work.
Some of the blizzard.j function also don't have BJ in there name.
Example
So since we have the a perfect example of inefficient code in the word editor itself
we can start there.
This is an average GUI revive script. As a Jass user you can probebly se that there is some memory leaks.
We convert this to 'Custom Scrip' and fix the leaks.
You will get something that looks like this:
This is an example of an non leaking but inefficient jass script.
This trigger works perfect and it doesn't lagg at all. But we all want to get better, right?
We start from the bottom of the trigger.
The InitTrig_Unit_Dies is as good as it will get, if we want we can inline the TriggerRegisterAnyUnitEventBJ function in the trigger.
But somewhere you have to draw the line between 'Function Length' and 'Native Usage'. And I think that add the whole TriggerRegisterAnyUnitEventBJ (7 lines)
just because we want 100% natives is unnecessary.
However you have to make choices like this by your own so here is the Trig_Unit_Dies_Actions with an inlined TriggerRegisterAnyUnitEventBJ.
So now we are done with the first function, now to the Trig_Unit_Dies_Actions.
Since the Trig_Unit_Dies_Func004C belongs to this function we can take them both.
Here is is plenty to do. First of all notice how much we use GetDyingUnit() and GetKillingUnitBJ().
That seems like a total waste.
Declare some locals at the beginning at the function and set their value to these functions. And use these variables instead.
After this the function looks like this.
The IF is a little trickier than the BJ so I explain that little more.
You se, ifs (and elseif’s and exitwhens) is just boolean values.
A boolean value is either true or false. It doesn't matter how you get that value.
You can write 'true == true' as well as true only.
Or you can write 'FunctionThatReturnsBoolean() == true' as well as 'FunctionThatReturnsBoolean()'.
If FunctionThatReturnsBoolean() is true the outcome will be true for both of them, but the second one looks a lot simpler, right?
So since the whole Trig_Unit_Dies_Func004C function just returns if the Player(0) (which is Player 1 in GUI) is an ally of GetDyingUnit() we can replace everything with 'IsUnitAlly(dying, Player(0))'
Togheter with the other thing we now have this.
So this is starting to look better and better.
Now we se another thing, the only difference between the if and the else is the value of the variable l.
So we can move everything but the l out of the 'if'/'else' block. This will make the code shorter.
Since we already got a little speed in this there is 4 things more we can do:
With this we minimize the total amount of function calls and that is what we are we want.
This is what we have after we done everything I mentioned.
You may notice two things:
So only two things left at this stage:
Now we have this.
So we leave that function for now, but we will come back later when we are going to learn new stuff.
The last function is easy, but there is one thing to remember. This is actually a bug from blizzards side and only applies on this specific case.
So we can simplify this.
If NOT IsUnitType(GetDyingUnit(), UNIT_TYPE_HERO) == true then it returns false.
Else it returns true.
Think about that, it simply returns the value of IsUnitType(GetDyingUnit(), UNIT_TYPE_HERO), right?
So why don’t write that instead of all that mess?
So to the bug. This is a condition, i.e. it is a function used in a trigger that returns a boolean value.
It is not used as a stand-alone function as the other condition we had.
In the condition we use the IsUnitType() function, and it is here the bug comes.
IsUnitType() MUST be used togheter with a comparison. I.E. IsUnitType() == true.
Else it will give bugs. Now UNIT_TYPE_HERO and RANGED_ATTACKER are the only values that don’t give bugs with this. (Strange isn't it)
But for good practice I always use the bug proof way when using this function.
We rewrite the function to.
Now we are done with the first optimizing.
This is what we have:
End of Lesson 1
When you get more experienced you can jump over everything and optimize the script direct.
When you are at this stage you can write jass functions without the GUI as base and just use the GUI as
an way to generate new 'triggers', blocks where you can put your code.
This is another reson to get a jass editor. Since you have an easy acces to all functions until you know the most important ones.
[SIZE=+1]Lesson 2: Use x, y coordinates instead of locations (Points).[/SIZE]
Introduction
X and Y coordinates have one big advantage before locations.
There is no messy cleanup when you are done.
Locations are faster when if you remove the fact that you have to create and destroy them. But you always have to do that.
If you already have a location, work with that.
If you don't have one, don't create on. Use the x, y coordinates instead.
The only time you are forced to use locations is when you want to use the GetSpellTargetLoc and GetLocationZ, there is no x, y equivalence for those.
Also, most of the Location functions uses the reals anyway, but after extracting them from the location.
Look for example at GetItemLoc.
It is clear that using the GetItemX/Y direct are better, this way we also get less function calls
as we talked about in Lesson 1.
Example
We turn back to our old Trig_Unit_Dies_Actions and make that use real instead.
Since a location is a X,Y coordinate (it also has a Z value but we don't need that) we need 2 reals.
There is 3 things we have to change in the function.
This is what we get
[SIZE=+1]Summary[/SIZE]
DO use as few functions calls as possible. DO use natives. DON'T inline to big functions in your script.
DO set the value of a variable when you declare it rather that after the local block.
DO use x, y coordinates when it is possible. DON'T assume that a function is a native just because it doesn't have BJ in the name.
[SIZE=+1]Useful links & backup for statements[/SIZE]
Advanced JASS Tips - Same stuff with different name
Memory Leaks and Custom Scripts - Remove leaks
Memory Leaks - Remove more leaks
Locations > Reals ( in speed ) - Discussion about locations speed
The use of BJ functions - BJ functions
The Lamest Bug ever - The IsUnitType bug.
So you just read your first jass tutorial and starded to learn jass, actualy you are pretty good at it.
Your own codes works as planned and you can eliminate the few errors you find on your own.
Everything is nice, or?
To become a true jass user you must not only be able to write jass code, you must make the code as effective as possible. If your only goal is to make the script work you will not make better code than a good GUI user that uses some lines of 'Custom Script' to remove leaks.
[SIZE=+1]Lesson 1: Remove unnecessary function calls[/SIZE]
Introduction
When coding you should always try to get as few function calls as possible.
The number that counts is the TOTAL amount, not the amount used in your function.
If you call a function that itself calls 3 functions that is a total of 4.
By calling the 3 functions yourself you get a total of 3. That is one less.
Also if you use GetTriggerUnit() 3 times in your script, why don't use a varaible that holds
the value of GetTriggerUnit()? If you do you remove 2 function calls.
This is of course a balance between making short scripts and making few function calls.
If you have a function calls a function that have 40 function calls, inline the 40 functions will
make the main function so much longer that it is not worth the one less function call.
Use Natives
Most of the BJ (blizzard.j) functions are just fluff functions that does nothing but calling a native function
If you call a BJ of that type you have 100% more function calls that if you call the native direct.
So use natives before BJ's. That is the biggest rule.
Find unnecessary functions
The easiest way to find unnecessary functions is to look them up somewhere.
I suggest that you get a Jass writing program that has all functions in it. You can find programs like this at www.wc3sear.ch.
If you don't want to use a program or you are unsure what a function does look it up at www.wc3jass.com.
An easy way to say if the function call is unneeded and you can inline the function instead is to look at the name.
If the name ends with an BJ it is most likely unnecessary. There is exceptions, some BJ functions does a real work.
Some of the blizzard.j function also don't have BJ in there name.
Example
So since we have the a perfect example of inefficient code in the word editor itself
we can start there.
Code:
Unit Dies
Events
Unit - A unit Dies
Conditions
((Dying unit) is A Hero) Equal to True
Actions
Game - Display to (All players) for 30.00 seconds the text: ((Name of (Owner of (Dying unit))) + ( was killed by + (Name of (Owner of (Killing unit)))))
Player - Add ((Level of (Dying unit)) x 50) to (Owner of (Killing unit)) Current gold
Wait ((Real((Level of (Dying unit)))) x 10.00) seconds
If (All Conditions are True) then do (Then Actions) else do (Else Actions)
If - Conditions
((Dying unit) belongs to an ally of Player 1 (Red)) Equal to True
Then - Actions
Hero - Instantly revive (Dying unit) at (Center of Team 1 Base <gen>), Show revival graphics
Camera - Pan camera for (Owner of (Dying unit)) to (Position of (Dying unit)) over 2.00 seconds
Else - Actions
Hero - Instantly revive (Dying unit) at (Center of Team 2 Base <gen>), Show revival graphics
Camera - Pan camera for (Owner of (Dying unit)) to (Position of (Dying unit)) over 2.00 secondsWe convert this to 'Custom Scrip' and fix the leaks.
You will get something that looks like this:
This is an example of an non leaking but inefficient jass script.
JASS:
function Trig_Unit_Dies_Conditions takes nothing returns boolean
if ( not ( IsUnitType(GetDyingUnit(), UNIT_TYPE_HERO) == true ) ) then
return false
endif
return true
endfunction
function Trig_Unit_Dies_Func004C takes nothing returns boolean
if ( not ( IsUnitAlly(GetDyingUnit(), Player(0)) == true ) ) then
return false
endif
return true
endfunction
function Trig_Unit_Dies_Actions takes nothing returns nothing
local location l
call DisplayTimedTextToForce( GetPlayersAll(), 30, ( GetPlayerName(GetOwningPlayer(GetDyingUnit())) + ( " was killed by " + GetPlayerName(GetOwningPlayer(GetKillingUnitBJ())) ) ) )
call AdjustPlayerStateBJ( ( GetUnitLevel(GetDyingUnit()) * 50 ), GetOwningPlayer(GetKillingUnitBJ()), PLAYER_STATE_RESOURCE_GOLD )
call TriggerSleepAction( ( I2R(GetUnitLevel(GetDyingUnit())) * 10.00 ) )
if ( Trig_Unit_Dies_Func004C() ) then
set l = GetRectCenter(gg_rct_Team_1_Base)
call ReviveHeroLoc( GetDyingUnit(), l, true )
call PanCameraToTimedLocForPlayer( GetOwningPlayer(GetDyingUnit()), l, 2 )
else
set l = GetRectCenter(gg_rct_Team_2_Base)
call ReviveHeroLoc( GetDyingUnit(), l, true )
call PanCameraToTimedLocForPlayer( GetOwningPlayer(GetDyingUnit()), l, 2 )
endif
call RemoveLocation(l)
set l = null
endfunction
//===========================================================================
function InitTrig_Unit_Dies takes nothing returns nothing
set gg_trg_Unit_Dies = CreateTrigger( )
call TriggerRegisterAnyUnitEventBJ( gg_trg_Unit_Dies, EVENT_PLAYER_UNIT_DEATH )
call TriggerAddCondition( gg_trg_Unit_Dies, Condition( function Trig_Unit_Dies_Conditions ) )
call TriggerAddAction( gg_trg_Unit_Dies, function Trig_Unit_Dies_Actions )
endfunctionThis trigger works perfect and it doesn't lagg at all. But we all want to get better, right?
We start from the bottom of the trigger.
The InitTrig_Unit_Dies is as good as it will get, if we want we can inline the TriggerRegisterAnyUnitEventBJ function in the trigger.
But somewhere you have to draw the line between 'Function Length' and 'Native Usage'. And I think that add the whole TriggerRegisterAnyUnitEventBJ (7 lines)
just because we want 100% natives is unnecessary.
However you have to make choices like this by your own so here is the Trig_Unit_Dies_Actions with an inlined TriggerRegisterAnyUnitEventBJ.
JASS:
function InitTrig_Unit_Dies takes nothing returns nothing
local integer index = 0
set gg_trg_Unit_Dies = CreateTrigger( )
loop
call TriggerRegisterPlayerUnitEvent(gg_trg_Unit_Dies, Player(index), EVENT_PLAYER_UNIT_DEATH, null)
set index = index + 1
exitwhen index == bj_MAX_PLAYER_SLOTS
endloop
call TriggerAddCondition( gg_trg_Unit_Dies, Condition( function Trig_Unit_Dies_Conditions ) )
call TriggerAddAction( gg_trg_Unit_Dies, function Trig_Unit_Dies_Actions )
endfunctionNOTICE: I set a vaule at the declaration of the index varabile.
Some of you may be used not to do this and use a 'set index = 0' after the local block.
Don't do like that unless you realy must to. It is always better to se a vaule at the declaration stage than right after the declaration.
Some of you may be used not to do this and use a 'set index = 0' after the local block.
Don't do like that unless you realy must to. It is always better to se a vaule at the declaration stage than right after the declaration.
So now we are done with the first function, now to the Trig_Unit_Dies_Actions.
Since the Trig_Unit_Dies_Func004C belongs to this function we can take them both.
JASS:
function Trig_Unit_Dies_Func004C takes nothing returns boolean
if ( not ( IsUnitAlly(GetDyingUnit(), Player(0)) == true ) ) then
return false
endif
return true
endfunction
function Trig_Unit_Dies_Actions takes nothing returns nothing
local location l
call DisplayTimedTextToForce( GetPlayersAll(), 30, ( GetPlayerName(GetOwningPlayer(GetDyingUnit())) + ( " was killed by " + GetPlayerName(GetOwningPlayer(GetKillingUnitBJ())) ) ) )
call AdjustPlayerStateBJ( ( GetUnitLevel(GetDyingUnit()) * 50 ), GetOwningPlayer(GetKillingUnitBJ()), PLAYER_STATE_RESOURCE_GOLD )
call TriggerSleepAction( ( I2R(GetUnitLevel(GetDyingUnit())) * 10.00 ) )
if ( Trig_Unit_Dies_Func004C() ) then
set l = GetRectCenter(gg_rct_Team_1_Base)
call ReviveHeroLoc( GetDyingUnit(), l, true )
call PanCameraToTimedLocForPlayer( GetOwningPlayer(GetDyingUnit()), l, 2 )
else
set l = GetRectCenter(gg_rct_Team_2_Base)
call ReviveHeroLoc( GetDyingUnit(), l, true )
call PanCameraToTimedLocForPlayer( GetOwningPlayer(GetDyingUnit()), l, 2 )
endif
call RemoveLocation(l)
set l = null
endfunctionHere is is plenty to do. First of all notice how much we use GetDyingUnit() and GetKillingUnitBJ().
That seems like a total waste.
Declare some locals at the beginning at the function and set their value to these functions. And use these variables instead.
REMEMBER: If possible, set the value of a variable at the declaration stage.
After this the function looks like this.
JASS:
function Trig_Unit_Dies_Func004C takes nothing returns boolean
if ( not ( IsUnitAlly(GetDyingUnit(), Player(0)) == true ) ) then
return false
endif
return true
endfunction
function Trig_Unit_Dies_Actions takes nothing returns nothing
local unit dying = GetDyingUnit()
local unit killing = GetKillingUnitBJ()
local location l
call DisplayTimedTextToForce( GetPlayersAll(), 30, ( GetPlayerName(GetOwningPlayer(dying)) + ( " was killed by " + GetPlayerName(GetOwningPlayer(killing)) ) ) )
call AdjustPlayerStateBJ( ( GetUnitLevel(dying) * 50 ), GetOwningPlayer(killing), PLAYER_STATE_RESOURCE_GOLD )
call TriggerSleepAction( ( I2R(GetUnitLevel(dying)) * 10.00 ) )
if ( Trig_Unit_Dies_Func004C() ) then
set l = GetRectCenter(gg_rct_Team_1_Base)
call ReviveHeroLoc( dying, l, true )
call PanCameraToTimedLocForPlayer( GetOwningPlayer(dying), l, 2 )
else
set l = GetRectCenter(gg_rct_Team_2_Base)
call ReviveHeroLoc( dying, l, true )
call PanCameraToTimedLocForPlayer( GetOwningPlayer(dying), l, 2 )
endif
call RemoveLocation(l)
set l = null
endfunctionNOTICE 1: Since the 'if' is in another function, I can't use the local there. This is bad and since there is no really reason for having another function for the 'if' we inline this in the main function.
NOTICE 2: GetKillingUnitBJ() is just a blizzard.j fluff function without any real purpose. It just returns GetKillingUnit(), so we switch to that one.
NOTICE 2: GetKillingUnitBJ() is just a blizzard.j fluff function without any real purpose. It just returns GetKillingUnit(), so we switch to that one.
The IF is a little trickier than the BJ so I explain that little more.
You se, ifs (and elseif’s and exitwhens) is just boolean values.
A boolean value is either true or false. It doesn't matter how you get that value.
You can write 'true == true' as well as true only.
Or you can write 'FunctionThatReturnsBoolean() == true' as well as 'FunctionThatReturnsBoolean()'.
If FunctionThatReturnsBoolean() is true the outcome will be true for both of them, but the second one looks a lot simpler, right?
So since the whole Trig_Unit_Dies_Func004C function just returns if the Player(0) (which is Player 1 in GUI) is an ally of GetDyingUnit() we can replace everything with 'IsUnitAlly(dying, Player(0))'
Togheter with the other thing we now have this.
JASS:
function Trig_Unit_Dies_Actions takes nothing returns nothing
local unit dying = GetDyingUnit()
local unit killing = GetKillingUnit()
local location l
call DisplayTimedTextToForce( GetPlayersAll(), 30, ( GetPlayerName(GetOwningPlayer(dying)) + ( " was killed by " + GetPlayerName(GetOwningPlayer(killing)) ) ) )
call AdjustPlayerStateBJ( ( GetUnitLevel(dying) * 50 ), GetOwningPlayer(killing), PLAYER_STATE_RESOURCE_GOLD )
call TriggerSleepAction( ( I2R(GetUnitLevel(dying)) * 10.00 ) )
if IsUnitAlly(dying, Player(0)) then
set l = GetRectCenter(gg_rct_Team_1_Base)
call ReviveHeroLoc( dying, l, true )
call PanCameraToTimedLocForPlayer( GetOwningPlayer(dying), l, 2 )
else
set l = GetRectCenter(gg_rct_Team_2_Base)
call ReviveHeroLoc( dying, l, true )
call PanCameraToTimedLocForPlayer( GetOwningPlayer(dying), l, 2 )
endif
call RemoveLocation(l)
set l = null
endfunctionNOTICE: The function Trig_Unit_Dies_Func004C are now usless, so we removed it.
So this is starting to look better and better.
Now we se another thing, the only difference between the if and the else is the value of the variable l.
So we can move everything but the l out of the 'if'/'else' block. This will make the code shorter.
Since we already got a little speed in this there is 4 things more we can do:
- Make a variable with the value GetOwningPlayer(dying) and use that.
- Make a variable with the value GetOwningPlayer(killing) and use that.
- Make a variable with the value GetUnitLevel(dying) and use that.
- Remove the unnecessary R2I. Integers can be reals without any function calls, it is the opposite that gives problems.
With this we minimize the total amount of function calls and that is what we are we want.
This is what we have after we done everything I mentioned.
JASS:
function Trig_Unit_Dies_Actions takes nothing returns nothing
local unit dying = GetDyingUnit()
local unit killing = GetKillingUnit()
local player ownerDying = GetOwningPlayer(dying)
local player ownerKilling = GetOwningPlayer(killing)
local integer level = GetUnitLevel(dying)
local location l
call DisplayTimedTextToForce( GetPlayersAll(), 30, GetPlayerName(ownerDying) + " was killed by " + GetPlayerName(ownerKilling) )
call AdjustPlayerStateBJ( level * 50 , ownerKilling, PLAYER_STATE_RESOURCE_GOLD )
call TriggerSleepAction( level * 10.00 )
if IsUnitAlly(dying, Player(0)) then
set l = GetRectCenter(gg_rct_Team_1_Base)
else
set l = GetRectCenter(gg_rct_Team_2_Base)
endif
call ReviveHeroLoc( dying, l, true )
call PanCameraToTimedLocForPlayer( ownerDying, l, 2 )
call RemoveLocation(l)
set dying = null
set killing = null
set ownerDying = null
set ownerKilling = null
set l = null
endfunctionYou may notice two things:
NOTICE 1: I removed some '(' and ')', only use those when you want to calculated some things before other things. Like 3*(2+4), not for (3*2)+4 since 3*2
allredy is calculated before +4. Be sure that you don't remove the '(' ')' used for function calls.
NOTICE 2: This function is longer that the one before, don't worry. It isn't the length that is important, it is the number of function calls.
allredy is calculated before +4. Be sure that you don't remove the '(' ')' used for function calls.
NOTICE 2: This function is longer that the one before, don't worry. It isn't the length that is important, it is the number of function calls.
So only two things left at this stage:
- The DisplayTimedTextToForce is a little unnecessary. Since we want to display the text for all why do we need to check if they are in a force first.
HINT: Look at the DisplayTimedTextToForce function below.
JASS:function DisplayTimedTextToForce takes force toForce, real duration, string message returns nothing if (IsPlayerInForce(GetLocalPlayer(), toForce)) then // Use only local code (no net traffic) within this block to avoid desyncs. call DisplayTimedTextToPlayer(GetLocalPlayer(), 0, 0, duration, message) endif endfunction
What is does is to se if the GetLocalPlayer is in a force and then display a text for that player. Since we want to diplay for all
players we can use 'call DisplayTimedTextToPlayer(GetLocalPlayer(), 0, 0, duration, message)' direct. - AdjustPlayerStateBJ. This is a BJ that actually has a purpose. It also updates the Total Gathered amount. But only if the change is positive.
and since we allredy know that the change is positive there is a lot of things we can change. We add a new varaible that holds the gold gained. And then add this to the 2 Player Stats Current Gold and Gold Gathered.
Now we have this.
JASS:
function Trig_Unit_Dies_Actions takes nothing returns nothing
local unit dying = GetDyingUnit()
local unit killing = GetKillingUnit()
local player ownerDying = GetOwningPlayer(dying)
local player ownerKilling = GetOwningPlayer(killing)
local integer level = GetUnitLevel(dying)
local location l
local integer gold = level * 50
call DisplayTimedTextToPlayer( GetLocalPlayer(), 0, 0, 30, GetPlayerName(ownerDying) + " was killed by " + GetPlayerName(ownerKilling) )
call SetPlayerState(ownerKilling, PLAYER_STATE_RESOURCE_GOLD , GetPlayerState(ownerKilling, PLAYER_STATE_RESOURCE_GOLD ) + gold)
call SetPlayerState(ownerKilling, PLAYER_STATE_GOLD_GATHERED, GetPlayerState(ownerKilling, PLAYER_STATE_GOLD_GATHERED) + gold)
call TriggerSleepAction( level * 10.00 )
if IsUnitAlly(dying, Player(0)) then
set l = GetRectCenter(gg_rct_Team_1_Base)
else
set l = GetRectCenter(gg_rct_Team_2_Base)
endif
call ReviveHeroLoc( dying, l, true )
call PanCameraToTimedLocForPlayer( ownerDying, l, 2 )
call RemoveLocation(l)
set dying = null
set killing = null
set ownerDying = null
set ownerKilling = null
set l = null
endfunctionSo we leave that function for now, but we will come back later when we are going to learn new stuff.
The last function is easy, but there is one thing to remember. This is actually a bug from blizzards side and only applies on this specific case.
JASS:
function Trig_Unit_Dies_Conditions takes nothing returns boolean
if ( not ( IsUnitType(GetDyingUnit(), UNIT_TYPE_HERO) == true ) ) then
return false
endif
return true
endfunctionSo we can simplify this.
If NOT IsUnitType(GetDyingUnit(), UNIT_TYPE_HERO) == true then it returns false.
Else it returns true.
Think about that, it simply returns the value of IsUnitType(GetDyingUnit(), UNIT_TYPE_HERO), right?
So why don’t write that instead of all that mess?
So to the bug. This is a condition, i.e. it is a function used in a trigger that returns a boolean value.
It is not used as a stand-alone function as the other condition we had.
In the condition we use the IsUnitType() function, and it is here the bug comes.
IsUnitType() MUST be used togheter with a comparison. I.E. IsUnitType() == true.
Else it will give bugs. Now UNIT_TYPE_HERO and RANGED_ATTACKER are the only values that don’t give bugs with this. (Strange isn't it)
But for good practice I always use the bug proof way when using this function.
We rewrite the function to.
JASS:
function Trig_Unit_Dies_Conditions takes nothing returns boolean
return IsUnitType(GetDyingUnit(), UNIT_TYPE_HERO) == true
endfunctionNow we are done with the first optimizing.
This is what we have:
JASS:
function Trig_Unit_Dies_Conditions takes nothing returns boolean
return IsUnitType(GetDyingUnit(), UNIT_TYPE_HERO) == true
endfunction
function Trig_Unit_Dies_Actions takes nothing returns nothing
local unit dying = GetDyingUnit()
local unit killing = GetKillingUnit()
local player ownerDying = GetOwningPlayer(dying)
local player ownerKilling = GetOwningPlayer(killing)
local integer level = GetUnitLevel(dying)
local location l
local integer gold = level * 50
call DisplayTimedTextToPlayer( GetLocalPlayer(), 0, 0, 30, GetPlayerName(ownerDying) + " was killed by " + GetPlayerName(ownerKilling) )
call SetPlayerState(ownerKilling, PLAYER_STATE_RESOURCE_GOLD , GetPlayerState(ownerKilling, PLAYER_STATE_RESOURCE_GOLD ) + gold)
call SetPlayerState(ownerKilling, PLAYER_STATE_GOLD_GATHERED, GetPlayerState(ownerKilling, PLAYER_STATE_GOLD_GATHERED) + gold)
call TriggerSleepAction( level * 10.00 )
if IsUnitAlly(dying, Player(0)) then
set l = GetRectCenter(gg_rct_Team_1_Base)
else
set l = GetRectCenter(gg_rct_Team_2_Base)
endif
call ReviveHeroLoc( dying, l, true )
call PanCameraToTimedLocForPlayer( ownerDying, l, 2 )
call RemoveLocation(l)
set dying = null
set killing = null
set ownerDying = null
set ownerKilling = null
set l = null
endfunction
function InitTrig_Unit_Dies takes nothing returns nothing
local integer index = 0
set gg_trg_Unit_Dies = CreateTrigger( )
loop
call TriggerRegisterPlayerUnitEvent(gg_trg_Unit_Dies, Player(index), EVENT_PLAYER_UNIT_DEATH, null)
set index = index + 1
exitwhen index == bj_MAX_PLAYER_SLOTS
endloop
call TriggerAddCondition( gg_trg_Unit_Dies, Condition( function Trig_Unit_Dies_Conditions ) )
call TriggerAddAction( gg_trg_Unit_Dies, function Trig_Unit_Dies_Actions )
endfunctionEnd of Lesson 1
When you get more experienced you can jump over everything and optimize the script direct.
When you are at this stage you can write jass functions without the GUI as base and just use the GUI as
an way to generate new 'triggers', blocks where you can put your code.
This is another reson to get a jass editor. Since you have an easy acces to all functions until you know the most important ones.
[SIZE=+1]Lesson 2: Use x, y coordinates instead of locations (Points).[/SIZE]
Introduction
X and Y coordinates have one big advantage before locations.
There is no messy cleanup when you are done.
Locations are faster when if you remove the fact that you have to create and destroy them. But you always have to do that.
If you already have a location, work with that.
If you don't have one, don't create on. Use the x, y coordinates instead.
The only time you are forced to use locations is when you want to use the GetSpellTargetLoc and GetLocationZ, there is no x, y equivalence for those.
Also, most of the Location functions uses the reals anyway, but after extracting them from the location.
Look for example at GetItemLoc.
JASS:
It is clear that using the GetItemX/Y direct are better, this way we also get less function calls
as we talked about in Lesson 1.
Example
We turn back to our old Trig_Unit_Dies_Actions and make that use real instead.
JASS:
function Trig_Unit_Dies_Actions takes nothing returns nothing
local unit dying = GetDyingUnit()
local unit killing = GetKillingUnit()
local player ownerDying = GetOwningPlayer(dying)
local player ownerKilling = GetOwningPlayer(killing)
local integer level = GetUnitLevel(dying)
local location l
local integer gold = level * 50
call DisplayTimedTextToPlayer( GetLocalPlayer(), 0, 0, 30, GetPlayerName(ownerDying) + " was killed by " + GetPlayerName(ownerKilling) )
call SetPlayerState(ownerKilling, PLAYER_STATE_RESOURCE_GOLD , GetPlayerState(ownerKilling, PLAYER_STATE_RESOURCE_GOLD ) + gold)
call SetPlayerState(ownerKilling, PLAYER_STATE_GOLD_GATHERED, GetPlayerState(ownerKilling, PLAYER_STATE_GOLD_GATHERED) + gold)
call TriggerSleepAction( level * 10.00 )
if IsUnitAlly(dying, Player(0)) then
set l = GetRectCenter(gg_rct_Team_1_Base)
else
set l = GetRectCenter(gg_rct_Team_2_Base)
endif
call ReviveHeroLoc( dying, l, true )
call PanCameraToTimedLocForPlayer( ownerDying, l, 2 )
call RemoveLocation(l)
set dying = null
set killing = null
set ownerDying = null
set ownerKilling = null
set l = null
endfunctionSince a location is a X,Y coordinate (it also has a Z value but we don't need that) we need 2 reals.
There is 3 things we have to change in the function.
- GetRectCenter. It just calls the 2 functions GetRectCenterX/Y. So we use them insteed.
- ReviveHeroLoc. This is a real native but we can use ReviveHero that takes x and y vaules insteed.
- PanCameraToTimedLocForPlayer. Look what we found here. it seems that we missed some optimization in the
Lesson 1. This takes us to a important thing: Look up the functions you are using. even if the not use BJ in there name they may be blizzard.j functions. So we inline this and use the x, y values direct.
This is what we get
JASS:
function Trig_Unit_Dies_Actions takes nothing returns nothing
local unit dying = GetDyingUnit()
local unit killing = GetKillingUnit()
local player ownerDying = GetOwningPlayer(dying)
local player ownerKilling = GetOwningPlayer(killing)
local integer level = GetUnitLevel(dying)
local real x
local real y
local integer gold = level * 50
call DisplayTimedTextToPlayer( GetLocalPlayer(), 0, 0, 30, GetPlayerName(ownerDying) + " was killed by " + GetPlayerName(ownerKilling) )
call SetPlayerState(ownerKilling, PLAYER_STATE_RESOURCE_GOLD , GetPlayerState(ownerKilling, PLAYER_STATE_RESOURCE_GOLD ) + gold)
call SetPlayerState(ownerKilling, PLAYER_STATE_GOLD_GATHERED, GetPlayerState(ownerKilling, PLAYER_STATE_GOLD_GATHERED) + gold)
call TriggerSleepAction( level * 10.00 )
if IsUnitAlly(dying, Player(0)) then
set x = GetRectCenterX(gg_rct_Team_1_Base)
set y = GetRectCenterY(gg_rct_Team_1_Base)
else
set x = GetRectCenterX(gg_rct_Team_2_Base)
set y = GetRectCenterY(gg_rct_Team_2_Base)
endif
call ReviveHero( dying, x, y, true )
if (GetLocalPlayer() == ownerDying) then
// Use only local code (no net traffic) within this block to avoid desyncs.
call PanCameraToTimed( x, y, 2)
endif
set dying = null
set killing = null
set ownerDying = null
set ownerKilling = null
endfunction[SIZE=+1]Summary[/SIZE]
DO use as few functions calls as possible. DO use natives. DON'T inline to big functions in your script.
DO set the value of a variable when you declare it rather that after the local block.
DO use x, y coordinates when it is possible. DON'T assume that a function is a native just because it doesn't have BJ in the name.
[SIZE=+1]Useful links & backup for statements[/SIZE]
Advanced JASS Tips - Same stuff with different name
Memory Leaks and Custom Scripts - Remove leaks
Memory Leaks - Remove more leaks
Locations > Reals ( in speed ) - Discussion about locations speed
The use of BJ functions - BJ functions
The Lamest Bug ever - The IsUnitType bug.
[SIZE=-2]By phyrex1an 2005-12-28[/SIZE]
