Simple vJass trigger

[GIMLI_2]

Hi, im new to vJass and i got some problems
I wanna do an ability, that gives nearby enemies a 100% miss chance for a certain time. for that, i want to create a dummy that casts the curse ability on all enemies in range.

Code

function Trig_Sand_Pulse_Conditions takes nothing returns boolean
    if ( GetSpellAbilityId() == 'A019' ) then
        return true
    endif
    return false
endfunction

function IsUnitAlive takes nothing returns boolean
    return GetWidgetLife(GetFilterUnit()) > 0.405
endfunction

function IsUnitHostile takes nothing returns boolean
    return (IsUnitEnemy(GetFilterUnit(), GetFilterPlayer()) == true)
endfunction

function UseAbility takes nothing returns nothing
    call IssueTargetOrder(bj_lastCreatedUnit, "curse", GetEnumUnit())
endfunction


function Trig_Sand_Pulse_Actions takes nothing returns nothing
    local unit u = GetTriggerUnit()
    local player p = GetOwningPlayer(u)
    local location l = GetUnitLoc(u)
    local group g = CreateGroup()
    call CreateUnitAtLoc(p, 'h00B', l, 0)
    call UnitApplyTimedLife(bj_lastCreatedUnit, 'BTLF', 1.00)
    call GroupEnumUnitsInRange(g, 0, 0, 325.00, Filter(function IsUnitAlive))
    call ForGroup(g, function UseAbility)
    set u = null
    set p = null
    set l = null
    set g = null
endfunction

//===========================================================================
function InitTrig_Sand_Pulse takes nothing returns nothing
    set gg_trg_Sand_Pulse = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( gg_trg_Sand_Pulse, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddCondition( gg_trg_Sand_Pulse, Condition( function Trig_Sand_Pulse_Conditions ) )
    call TriggerAddAction( gg_trg_Sand_Pulse, function Trig_Sand_Pulse_Actions )
endfunction

I encountered some problems:

1. "TriggerRegisterAnyUnitEventBJ" how do i create that without the bj, because the TriggerRegisterPlayerUnitEvent works only for 1 selected player (maybe with a loop?)

2. The dummy unit wont cast curse on enemies

3.

call GroupEnumUnitsInRange(g, 0, 0, 325.00, Filter(function IsUnitAlive))

i would need to also check the IsUnitHostile function, how do i connect them with "AND"?

4. Any other improvements?


thanks

 

[deathismyfriend]

This is not vJass it is jass.
You should look at my tutorial Converting GUI to efficient jass. It has a lot of pointers.

This BJ is ok to use. TriggerRegisterAnyUnitEventBJ
If you have JNGP then hold control and left click that. It will show you what it does.

 

[PurgeandFire]

1. It is fine as the BJ. But for future reference, if you wanted to inline it, you would use a loop for all 16 players.

2. You have to set the dummy to a unit variable. CreateUnitAtLoc won't set bj_lastCreatedUnit. Also, you are grouping the units around the coordinates (0, 0) rather than the caster's coordinates.

3. You will usually just connect them, like so:

function IsUnitAlive takes nothing returns boolean
    return GetWidgetLife(GetFilterUnit()) > 0.405
endfunction

function IsUnitHostile takes nothing returns boolean
    return (IsUnitEnemy(GetFilterUnit(), GetFilterPlayer()) == true)
endfunction

->

function IsUnitAliveAndHostile takes nothing returns boolean
    return GetWidgetLife(GetFilterUnit()) > 0.405 and IsUnitEnemy(GetFilterUnit(), GetTriggerPlayer())
endfunction

(Note that I changed GetFilterPlayer() to GetTriggerPlayer(). iirc, then GetFilterPlayer() is used for forces (Player Groups), rather than units)

EDIT: Here is the code with some updates.

function Trig_Sand_Pulse_Conditions takes nothing returns boolean
    /*
         Condition was merged into one boolean expression 
    */
    return GetSpellAbilityId() == 'A019'
endfunction

function Trig_Sand_Pulse_Actions takes nothing returns nothing
    local unit u = GetTriggerUnit()
    local real x = GetUnitX(u)
    local real y = GetUnitY(u)
    local group g = CreateGroup()
    local player p = GetTriggerPlayer()
    local unit dummy = CreateUnit(p, 'h00B', x, y, 0)
    local unit fog

    call UnitApplyTimedLife(dummy, 'BTLF', 1.00)
    call GroupEnumUnitsInRange(g, x, y, 325, null)

    /* 
        Use the convenient first-of-group loop method
    */
    loop
        set fog = FirstOfGroup(g)
        exitwhen fog == null
        if GetWidgetLife(fog) > 0.405 and IsUnitEnemy(fog, p) then
            call IssueTargetOrder(dummy, "curse", fog)
        endif
        call GroupRemoveUnit(g, fog)
    endloop

    call DestroyGroup(g)
    set g = null
    set u = null
    set dummy = null
endfunction

//===========================================================================
function InitTrig_Sand_Pulse takes nothing returns nothing
    set gg_trg_Sand_Pulse = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( gg_trg_Sand_Pulse, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddCondition( gg_trg_Sand_Pulse, Condition( function Trig_Sand_Pulse_Conditions ) )
    call TriggerAddAction( gg_trg_Sand_Pulse, function Trig_Sand_Pulse_Actions )
endfunction

There could be more improvements, but those are some of the main points. The first-of-group loop avoids delegating things to other functions (which is convenient so that you don't have to pass data into globals, and stuff like that). You can just directly check the conditions within the loop. Hopefully the rest is self explanatory. If not, then feel free to ask.

I assigned the dummy to a local because units that are created in JASS aren't stored in bj_lastCreatedUnit. That only applies if you are using the BJ. Otherwise, you'll have to assign some variable to it upon creation to refer to it later.

 

[GIMLI_2]

first thing, thanks for helping me!

now i got some other questions

1. i created a function IsUnitAlive in general because i will need it more often

function IsUnitAlive takes unit u returns boolean
    if GetWidgetLife(u) > 0.405 then
    return true
    endif
    return false
endfunction

is that ok?

2. should i always use IF CONDITON AND CONDITION instead of creating an own function?

3. about the TriggerRegisterAnyUnitEventBJ, couldnt i write a function for that, that only loops through the players that i need?




EDIT: here is my current trigger:

Initialization

function IsUnitAlive takes unit u returns boolean
    if GetWidgetLife(u) > 0.405 then
    return true
    endif
    return false
endfunction

Spell

function Trig_Sand_Pulse_Conditions takes nothing returns boolean
    return GetSpellAbilityId() =='A019'
endfunction


function Trig_Sand_Pulse_Actions takes nothing returns nothing
    local unit u = GetTriggerUnit()
    local real x = GetUnitX(u)
    local real y = GetUnitY(u)
    local group g = CreateGroup()
    local player p = GetTriggerPlayer()
    local unit d = CreateUnit(p, 'h00B', x, y, 0)
    local unit fog
    
    call UnitApplyTimedLife(d, 'BTLF', 1.00)
    call GroupEnumUnitsInRange(g, x, y, 325.00, null)
    
    loop
        set fog = FirstOfGroup(g)
        exitwhen fog == null
        if IsUnitAlive(fog) and IsUnitEnemy(fog, p) then
            call IssueTargetOrder(d, "curse", fog)
        endif
        call GroupRemoveUnit(g, fog)
    endloop
    
    call DestroyGroup(g)
    set g = null
    set u = null
    set d = null
endfunction

//===========================================================================
function InitTrig_Sand_Pulse takes nothing returns nothing
    local trigger t = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( t, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddCondition( t, Condition( function Trig_Sand_Pulse_Conditions ) )
    call TriggerAddAction( t, function Trig_Sand_Pulse_Actions )
    set t = null
endfunction

EDIT 2: i also nulled the player variable and the fog variable

 

[deathismyfriend]

1) The problem is that is not a good way to detect if a unit is dead since you can heal dead units.
To properly check do this.

function UnitAlive takes unit u returns boolean
    return GetWidgetLife(u) > 0.405 and not IsUnitType( u, UNIT_TYPE_DEAD) and GetUnitTypeId( u) != 0
endfunction

Also notice how I got rid of the unneeded if block.

2) It is faster and more efficient to use the If condition instead of a function call. But for readability it can help a lot.

3) Yes you can.


Never use TriggerAddAction. Use TriggerAddCondition and run everything from there.

You should also create a global group. That way you only create it once and add units and remove them as necessary. Creating and destroying groups is costly. So when you create it once it is much better.

 

[GIMLI_2]

You should also create a global group. That way you only create it once and add units and remove them as necessary. Creating and destroying groups is costly. So when you create it once it is much better.

i should also use it in my example?

 

[edo494]

didnt read the responses, so sorry if this was mentioned already

never name your function IsUnitAlive, because there already is existing native called that, if you have vJass, you should use that by including native IsUnitAlive takes unit u returns boolean

Also the TriggerRegisterAnyUnitEventBJ BJ is one of the more useful ones, since it does what you would do otherwise(loop throught all players and add event to the player for any unit)

Yes, DestroyGroup is very costly function, it can actually lag the game when used 2 or 3 times in 0.02 - 0.01 second periodic event/timer. Just use global one and clear it at the end of the function

 

[PurgeandFire]

first thing, thanks for helping me!

now i got some other questions

1. i created a function IsUnitAlive in general because i will need it more often

function IsUnitAlive takes unit u returns boolean
    if GetWidgetLife(u) > 0.405 then
    return true
    endif
    return false
endfunction

is that ok?

It works well functionally, so nice job. However, it can be reduced to a single expression, like so:

function IsUnitAlive takes unit u returns boolean
    return GetWidgetLife(u) > 0.405
endfunction

Which is pretty useful to know. If you think about it, when you pass a unit into the function, it'll check if his life is greater than 0.405. If it is greater, then you can just imagine the expression evaluating to: return true, right? If it is less than 0.405, then it would evaluate to: return false.

Keep this in mind, for it is always useful to cut down the number of if's and such.

Apart from that, there is a slight issue. As death said, simply checking if the unit's hp is greater than 0.405 won't handle all cases. There are some cases where it won't work, such as when a unit dies, but you set its life (via triggers) above 0.405. Then it would fail, even though the unit is dead. How do you fix that? Like so:

function IsUnitAlive takes unit u returns boolean
    return not IsUnitType(u, UNIT_TYPE_DEAD) and GetUnitTypeId(u) != 0
endfunction

If I recall correctly, the GetUnitTypeId() check is in the case where the unit is removed or a ghost reference (no longer in game). You don't need to focus too much on the details though, just know that this will handle more, if not all, cases.

As for the "not IsUnitType(u, UNIT_TYPE_DEAD)", units are flagged as DEAD when they die (probably for rezzing, raise dead, etc.). So that is a nice built-in way of checking for death.

2. should i always use IF CONDITON AND CONDITION instead of creating an own function?

As death said. If it makes it more readable, then putting it in a separate function will usually be better for you. The speed difference is rather negligible, so just use whatever you're most comfortable with.

3. about the TriggerRegisterAnyUnitEventBJ, couldnt i write a function for that, that only loops through the players that i need?

Yes, you can. It can actually save you a decent number of handles that way. For example, if your map only uses 5 players and the creeps, then you only need to register it for 5 players, and neutral aggressive (Player(12)).

-----

As for your current trigger, it has definitely improved. Do you use vJASS (included in JassNewGenPack)? If so, you may be able to clean-up your code a bit.

As for the conditions thing that death mentioned, it is essentially changing this:

Code

function Conditions takes nothing returns boolean
    return <Some Condition>
endfunction

function Actions takes nothing returns nothing
    // some actions
endfunction

function Init takes nothing returns nothing
    local trigger t = CreateTrigger()
    call TriggerRegister...<Some Event>
    call TriggerAddCondition(t, Condition(function Conditions))
    call TriggerAddAction(t, function Actions)
endfunction

To this ->

Code

function ConditionsAndActions takes nothing returns boolean
    if <Some Condition> then
        // some actions
    endif
    return false
endfunction

function Init takes nothing returns nothing 
    local trigger t = CreateTrigger()
    call TriggerRegister...<Some Event>
    call TriggerAddCondition(t, Condition(function ConditionsAndActions))
endfunction

But if it is confusing for you, then just ignore it for now. It is sometimes even a pain to implement if you have a lot of locals that you want to initialize, because then you have to defer setting the locals into the if-block. Just work on: (1) cleaning your code (2) optimizing where it is convenient/easy (3) fix any critical issues/leaks. Your code has already addressed that.

@eubz: The native is called UnitAlive, not IsUnitAlive, so his name is fine.

 

[GIMLI_2]

thanks so much for helping me!

As for your current trigger, it has definitely improved. Do you use vJASS (included in JassNewGenPack)? If so, you may be able to clean-up your code a bit.

yes i use the NewGen Editor

i understood everything except of the last part
what i can see there is that the TriggerAddCondition calls the Actions? a bit confusing^^

 

[edo494]

in vJass, we dont usually use actions, because its rumoured(dont know if that is actually true tho) that conditions are slightly faster than actions, and conditions can do about anything that actions can do except pausing the running thread with things like GUI's Wait