[JASS] Making safe and effective code

08-01-2008, 01:10 AM

About

This tutorial is beyond beginner level. I assume that you feel confident in the basic syntax of JASS and vJASS before proceeding. Also, you should know what leaks and what does not.

I won't add any sources for my claims (since I don't feel for scanning wc3campaigns for hours to find them). If you have links to relevant test results or the like, please post them here.

Index

Brief Safety Info
Unit Groups
- Creating / Destroying
- Looping
Dynamic Triggers
- Safety Concerns
- Action vs. Condition

.....................: Brief Safety Info :.....................

Using UnitDamagePoint or UnitDamagePointLoc will desync mac users.
The 8192:th slot in all arrays will be forgotten if the game is saved and loaded. This means we can safely use array[0] to array[8190].
TriggerSleepAction (normal Wait) doesn't care if the game is paused by Waiting for players-window.
Destroying a timer and then setting its variable to null might sometimes cause a crash. But you know this, right? :)
Functions returning nothing shouldn't be passed to Filters/Conditions because that can cause desyncs.

.....................: Unit Groups :.....................

-- Creating / Destroying --

We've all learned that when dealing with local unit groups, you have to destroy them to prevent leaks. That's true. The problem is that creating a unit group takes time a small amount of time. This matters if you, for instance, want to use a lot of groups periodically.

Also, destroying unit groups in certain situations might cause leaks. It's better if you use a global group instead:

JASS:

scope GroupEx

globals
    private group tempGroup
endglobals

function DoSomething takes nothing returns nothing
// Empty group before usage
    call ClearGroup(tempGroup)

// Use tempGroup for whatever you need!
// *** EXAMPLE ***
    call GroupEnumUnitsInRange(tempGroup,X,Y,500,null)
    call GroupIssuePointOrder(tempGroup,0,0,"attack")
endfunction

public function InitTrig takes nothing returns nothing
    //....
    set tempGroup=CreateGroup()
endfunction

endscope

Sometimes, just 1 group might not be enough. You can then implement your own group-stack with the help of whatever struct you might be using:

JASS:

 // Instead of destroying the group,
// just empty it, and leave it for the next struct.

struct Data
    group g

    method create takes nothing returns Data
        local Data d=Data.allocate()
        if d.g==null then
            set d.g=CreateGroup()
        endif
        return d
    endmethod

    method onDestroy takes nothing returns nothing
        call GroupClear(.g)
    endmethod
endstruct

-- Looping --

The equivalent of Pick all units in Group... is ForGroupBJ(), which is a wrapper around ForGroup(). If you already have a group, the fastest way to loop through it is indeed ForGroup(). Use temporary globals if you need to.

JASS:

globals
    private player temp
endglobals

private function Loop takes nothing returns nothing
    call SetUnitOwner(GetEnumUnit(),temp,true)
endfunction

function GroupChangeOwner takes group g, player newOwner returns nothing
    set temp=newOwner
    call ForGroup(g,function Loop)
endfunction

If you only want to loop through a group once, you don't have to use ForGroup. This is very common for spell-making. You can do the loop directly in the GroupEnum-call:

JASS:

globals
    private unit tempUnit
    private integer tempInt
    private group tempGroup
    private boolexpr cond
endglobals
//======================================================

private function Loop takes nothing returns boolean
    if IsUnitEnemy(GetFilterUnit(),GetOwningPlayer(tempUnit)) then
        call UnitDamageTarget(tempUnit, GetFilterUnit(), tempInt, true, false, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_NORMAL, WEAPON_TYPE_WHOKNOWS)
    endif
    return false
endfunction

function AOEDamage takes unit attacker, real x, real y, integer damage returns nothing
    set tempUnit = attacker
    set tempInt = damage
    call GroupEnumUnitsInRange(tempGroup,x,y,RADIUS,cond)
endfunction

//======================================================
private function Init takes nothing returns nothing
    set tempGroup=CreateGroup()
    set cond=Condition(function Loop)
endfunction

.....................: Dynamic Triggers :.....................

-- Safety Concerns --

A trigger which has it's attributes (event/condition/action) modified by a script not running at map init is called a dynamic trigger. If you do not know about triggers in JASS, I'd recommend Vexorian's tutorial.

Since you can create triggers on-the-fly, you will also want to destroy the triggers, in order to prevent memory leaks. This is bad because destroying the trigger under certain conditions (having waits in any code used by, or called by the trigger) may cause two handles to get the same id.

Usually you can work your way around using dynamic triggers in your scripts. For Damage Detection, you have to use a dynamic trigger. The solution here is to never destroy the trigger, and accept the fact that it will leak a bit. After all, you would rather have a leaking trigger than a corrupted game. For auras and other enter range events, you can use periodical GroupEnums.

-- Conditions vs. Actions --

Trigger Conditions are faster than Trigger Actions, but you cannot use waits in conditions.

JASS:

// Custom Charm.

private function Actions takes nothing returns boolean
    if GetSpellAbilityId() == 'char' then
       call SetUnitOwner(GetSpellTargetUnit(),GetOwningPlayer(GetSpellAbilityUnit()),true)
    endif
    return false
enfunction

public function InitTrig takes nothing returns nothing
    local trigger trg=CreateTrigger()
    call TriggerRegisterAnyUnitEvent(trg,PLAYER_UNIT_SPELL_EFFECT)
    call TriggerAddCondition(trg, Condition(function Actions))
    set trg=null
endfunction

08-01-2008, 06:12 AM

darkwulfv

Quote:

The problem is that creating a unit group takes time. Also, destroying unit groups in certain situations might cause leaks. It's better if you use a global group instead:

Like, .0045 seconds? If that? Only crucial if every single nanosecond counts for something. Using a global can overcomplicate things since you'll sometimes need to use special systems just to make things MUI (in some situations)
And could you explain how destroying a group leaks?

That bit of information about performing actions on units from groups directly in the filter is pretty nifty, though.

08-01-2008, 02:35 PM

Themerion

Quote:

It's important if you want to have, for instance, something periodically creating and destroying a lot of groups. I'll add that mention...

Quote:

And could you explain how destroying a group leaks?

I cannot. But I've learned to trust in what ever Vexorian says ^^

CSSafety Intro:

//* CSSafety 15.2
//* ¯¯¯¯¯¯¯¯
//* [.......]
//*
//*  Eventually, we also added a group stack, the justification is different though, there
//* are just memory leak related issues when using DestroyGroup on groups that were handled
//* by certain natives, thus it is good to recycle them. Replace CreateGroup with NewGroup
//* and DestroyGroup with ReleaseGroup.

09-17-2008, 07:30 AM

xyzie

I wish I had seen this thread sooner :D
It has covered about all the uncertainties I have about JASS in the last couple weeks and some new information too.

Quote:

The 8191:th slot in all arrays will be forgotten if the game is saved and loaded.

Isn't that the 8192:th slot that is trimmed? So to be on the absolute safe side we should only use some_array[0]...some_array[8190]?

Thanks for the thread. Really helpful!

09-18-2008, 06:12 AM

Pyrogasm

Quote:

Originally Posted by xyzie

Isn't that the 8192:th slot that is trimmed? So to be on the absolute safe side we should only use some_array[0]...some_array[8190]?

That is correct.

10-08-2008, 05:29 PM

Anitarf

Destroying triggers won't cause the game to crash, it just may cause two handles to get the same handle id for some reason. Sure, dynamic triggers are still "evil", but there's no need to exaggerate how bad they really are.

Otherwise, this is a nice tutorials, it covers many useful coding details, but you really should provide links to threads, you can probably find most of them in the Known bugs in Jass sticky in the T&S forum, as for the others invest some effort and look for them.

12-05-2008, 01:53 PM	#7
PitzerMike	Yes this is great. I have an addition to the Safety Info section. Functions returning nothing shouldn't be passed to Filters/Conditions because that can cause desyncs. The latest pjass should catch those however. Approved!

12-18-2008, 12:06 AM	#8
MaD[Lion]	very useful summary :) Eventho i knew those things already but sometime u forget when u do much of only 1 problem for long time, so nice to refresh the other problems :)

12-27-2008, 02:27 PM

Themerion

Quote:

Originally Posted by xyzie

Isn't that the 8192:th slot that is trimmed? So to be on the absolute safe side we should only use some_array[0]...some_array[8190]?

You are right, of course. With array[0] being the first slot, and array[8190] being the 8191:th...

Quote:

Originally Posted by Anitarf

Aye. I'll change that.

Quote:

Otherwise, this is a nice tutorials, it covers many useful coding details, but you really should provide links to threads, you can probably find most of them in the Known bugs in Jass sticky in the T&S forum, as for the others invest some effort and look for them.

Muuh... *grumble* it'd take me more time to find links than it took to write the tutorial itself...

Quote:

Originally Posted by PitzerMike

I have an addition to the Safety Info section.
Functions returning nothing shouldn't be passed to Filters/Conditions because that can cause desyncs. The latest pjass should catch those however.

Hum, but why would somebody pass a return nothing function to a filter?

Quote:

Approved!

\O/