[Spell] Need explanation on a simple spell mechanics

[Roflan]

I am learning JASS and I made two versions of one spell (GUI and JASS version) and I do not understand why the former doesn't work properly.

My spell reads like this:

'Choose a unit. In ten seconds the chosen unit will die'

First version:
===================================

Event:
- Unit casts a spell
Condition:
- Ability being cast is KILL
Actions:
- Set VICTIM = TargetUnitOfAbilityBeingCast
- Wait 10 seconds
- Kill VICTIM

====================================

So clearly when you cast the spell two times on two different units within the 10 seconds period, only the second target dies because the value VICTIM is being overwritten every time you cast the spell.

But this code in JASS with basically the same contents works fine:

Second version:

function Trig_Kill_Conditions takes nothing returns boolean

    return GetSpellAbilityId() == 'A000'

endfunction

function Trig_Kill_Actions takes nothing returns nothing

    local unit u = GetSpellTargetUnit()
    call TriggerSleepAction(10)
    call KillUnit(u)

endfunction
//===========================================================================
function InitTrig_Kill takes nothing returns nothing
    set gg_trg_Kill = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ(gg_trg_Kill, EVENT_PLAYER_UNIT_SPELL_EFFECT)
    call TriggerAddCondition (gg_trg_Kill, Condition (function Trig_Kill_Conditions))
    call TriggerAddAction( gg_trg_Kill, function Trig_Kill_Actions )
endfunction

Even if I cast this on two units, both die in10 seconds. For some reason the local 'u' does not overwrite itself and I don't know why it works like this. Why is that so?
Do I have to also add set u = null in the end of my function to avoid leaks?

Also, I rarely see the "Wait" command in other people's spells however I find it useful and I cannot do without it especially when making a consequence of special effects. Is "wait" bad or not?


// P.S.:
How do I make different triggers with each having its own event and condition within a single document? Like in GUI you had one document per trigger, can I put them all in one with JASS?

// P.P.S:
Can I refer to other documents when using JASS? I mean do they share variables (at least globals)? Can I turn on or off other 'trigger documents' or call funcitons from them? Or it works some other way?

 

[Jampion]

It's a local variable so it is unique everytime the function runs. So basically it's a different variable everytime you use that ability.
GUI only uses global variables, so they will overwrite each other.

Do I have to also add set u = null in the end of my function to avoid leaks?

yes

Is "wait" bad or not?

It's bad. Especially for spells. Waits are inaccurate and accuracy is extremely important for spells. If you just show a quest message with a delay accuracy is not important, but in a spell it is.
Fortunately in JASS you have a very good alternative: timer

// P.S.:
How do I make different triggers with each having its own event and condition within a single document? Like in GUI you had one document per trigger, can I put them all in one with JASS?

function InitTrig_Kill takes nothing returns nothing
    local trigger trg = CreateTrigger()
    // first trigger
    call TriggerRegisterAnyUnitEventBJ(trg , EVENT_PLAYER_UNIT_SPELL_EFFECT)
    call TriggerAddCondition (trg, Condition (function Trig_Kill_Conditions))
    call TriggerAddAction( trg, function Trig_Kill_Actions )
    // second trigger
    set trg = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ(trg , EVENT_PLAYER_UNIT_SPELL_EFFECT)
    call TriggerAddCondition (trg, Condition (function secondCondition))
    call TriggerAddAction( trg, function secondFunction)
    // third trigger
    set trg = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ(trg , EVENT_PLAYER_UNIT_SPELL_EFFECT)
    call TriggerAddCondition (trg, Condition (function thirdCondition))
    call TriggerAddAction( trg, function thirdFunction)
    ....
endfunction

// P.P.S:
Can I refer to other documents when using JASS? I mean do they share variables (at least globals)? Can I turn on or off other 'trigger documents' or call funcitons from them? Or it works some other way?

When you save the map, all JASS and GUI code is combined into 1 JASS file. The documents are just a help for you to structure the code. Having everything in one document would be quite messy.
You can refer to other document's function, but you can only refer to documents that come "above" your current document. By above I mean above in that 1 JASS file I mentioned.
In JASS you can't really decide which document comes first in that one file, that's why referencing functions from other documents can sometimes be annyoing.
In vJASS you have methods to change the order the code is put together.

Globals are shared for the whole map. Locals are obviously only accessable within one function.

You cannot turn on or off trigger documents. As I said trigger documents are just for you and their structure is lost when they are combined into this one file. You can however turn on/off triggers.

You can use [code=jass]...[/code] tags to have syntax highlighting when you post jass code.

 

[Roflan]

thanks that helped a lot!

Should I use timer instead of wait in this spell or it's fine? What would the code look like then?

 

[Jampion]

Should I use timer instead of wait in this spell or it's fine?

It's up to you to decide.
I would use a timer.

What would the code look like then?

function Trig_Banish_Kill takes nothing returns nothing
    local timer t = GetExpiredTimer()
    local integer h = GetHandleId(t)
    local unit u = LoadUnitHandle(udg_HashTable, h, 0)
    call KillUnit(u)
    call DestroyTimer(t)
    call FlushChildHashtable(udg_HashTable, h)
    set t = null
    set u = null
endfunction


function Trig_Banish_Actions takes nothing returns nothing
    local timer t
    local integer h
    if GetSpellAbilityId() == 'AHbn' then
        set t = CreateTimer()
        set h = GetHandleId(t)
        call SaveUnitHandle(udg_HashTable, h, 0, GetSpellTargetUnit())
        call TimerStart(t, 10, false, function Trig_Banish_Kill)
        set t = null
    endif
endfunction

//===========================================================================
function InitTrig_Banish takes nothing returns nothing
    set gg_trg_Banish = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( gg_trg_Banish, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddAction( gg_trg_Banish, function Trig_Banish_Actions )
    set udg_HashTable = InitHashtable()
endfunction

You attach the unit to the timer using a hash table. Just make sure the hash table is created (I did it in the Init function).
The hash table can be reused for everything you want to attach to certain handles.

It is quite a bit more to write, but you can shorten it, if you write functions for it. You probably gonna need this stuff more often.

function ReadTimerUnit takes nothing returns unit
    local timer t = GetExpiredTimer()
    local integer h = GetHandleId(t)
    set udg_ReturnUnit = LoadUnitHandle(udg_HashTable, h, 0)
    call DestroyTimer(t)
    call FlushChildHashtable(udg_HashTable, h)
    set t = null
    return udg_ReturnUnit
endfunction

function TimerStartUnit takes real time, code func, unit u returns nothing
    local timer t = CreateTimer()
    local integer h = GetHandleId(t)
    call SaveUnitHandle(udg_HashTable, h, 0, u)
    call TimerStart(t, time, false, func)
    set t = null
endfunction

function Trig_Banish_Kill takes nothing returns nothing
    call KillUnit(ReadTimerUnit())
endfunction


function Trig_Banish_Actions takes nothing returns nothing
    local timer t
    local integer h
    if GetSpellAbilityId() == 'AHbn' then
        call TimerStartUnit(2, function Trig_Banish_Kill, GetSpellTargetUnit())
    endif
endfunction

//===========================================================================
function InitTrig_Banish2 takes nothing returns nothing
    set gg_trg_Banish2 = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( gg_trg_Banish2, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddAction( gg_trg_Banish2, function Trig_Banish_Actions )
    set udg_HashTable = InitHashtable()
endfunction

Here I have two new functions, that I can use whenever I need to attach a unit to a timer. With that the code of this specific spell is very small.
In the ReadTimerUnit function I use a global variable to return the unit, because local handle variables should be nulled, which is not possible if you want to return them.

A very common system that is used with timers is TimerUtils. If you use vJASS and don't know it already you should check it out.

 

[Wrda]

you forgot to remove local timer in the banish_actions since you're just not using it.

Doesn't this have a leak?

function TimerStartUnit takes real time, code func, unit u returns nothing
    local timer t = CreateTimer()
    local integer h = GetHandleId(t)
    call SaveUnitHandle(udg_HashTable, h, 0, u)
    call TimerStart(t, time, false, func)
    set t = null
    //leak before destroying timer? then null it?
endfunction

To prevent it, this would have more coding i suppose.
Anyway, Jampions' approach works, just note that waits are not bad if used correctly.

 

[Jampion]

you forgot to remove local timer in the banish_actions since you're just not using it.

Yes true, both locals are not needed. I guess I still had them there from the original function.

Doesn't this have a leak?

The timer must not be destroyed here. Otherwise the timer would never finish and the function will not be executed. The timer will be destroyed when it expires:

function ReadTimerUnit takes nothing returns unit
    local timer t = GetExpiredTimer()
    local integer h = GetHandleId(t)
    set udg_ReturnUnit = LoadUnitHandle(udg_HashTable, h, 0)
    call DestroyTimer(t)
    call FlushChildHashtable(udg_HashTable, h)
    set t = null
    return udg_ReturnUnit
endfunction

That is of course only the case, if the function TimerStartUnit uses destroys the timer.
In this case the function is Trig_Banish_Kill, which calls ReadTimerUnit, which in the end destroys the timer.

That's kind of a special thing about timers. Even if you have a local timer and you null the variable you can still get the timer by using GetExpiredTimer() in the timer function.