[JASS] Dummy doesn't cast abilities

[FaceJaraxxus]

Hi! I have a really bizarre problem with my code and I'm hoping that someone might see an issue.

This function simply activates when an ability casts, then finds a target. The unit variable is returned and a dummy is created to cast a sleep spell on the target. It correctly finds a target, so the debugging properly triggers (e.g. target unit: 'generic hero'), and "dummy created".

function DoomLordSleep_Actions takes nothing returns nothing
    local unit caster = GetTriggerUnit()
    local unit dammy
    local unit target
    local real x
    local real y

    set x = GetUnitX(caster)
    set y = GetUnitY(caster)
    set target = FindClosestEnemy(caster)

    if target != null then
            call DisplayTextToForce(GetPlayersAll(), "Target unit: " + GetUnitName(target))
            set dammy = CreateUnit(GetOwningPlayer(caster), 'n00S', x, y, GetUnitFacing(target))

            if dammy != null then
                  call DisplayTextToForce( GetPlayersAll(), ( "Dummy created" ) )
            endif

             call UnitAddAbility(dammy,'A00J')
             call IssueTargetOrder(dammy,"sleep",target)
             call UnitApplyTimedLifeBJ(3.00, 'BTLF', dammy)
    endif

    set caster = null
    set target = null
    set dammy = null
endfunction

Here is the ability:

Here's the FindClosestEnemy function in case that's the problem:
,

function FindClosestEnemy takes unit caster returns unit
    local unit target
    local unit result
    local group g = CreateGroup()
    local real x = GetUnitX(caster)
    local real y = GetUnitY(caster)
    local real minDistance = 9999.0
    local real distance 
    call GroupEnumUnitsInRange(g, x, y, 6000, function AGRO)
    loop
        set target = FirstOfGroup(g)
        exitwhen target == null
            set distance = SquareRoot(Pow(x - GetUnitX(target), 2) + Pow(y - GetUnitY(target), 2))
            if distance < minDistance then
                set minDistance = distance
                set result = target
            endif
        call GroupRemoveUnit(g, target)
    endloop
    call DestroyGroup(g)
   if result != null then
   call DisplayTextToForce( GetPlayersAll(), ( "There is a target" ) )
   endif
    return result
endfunction

There is nothing wrong with the dummy either, it has 3.0 turn rate and works perfectly for everything else. I have also tried some other abilities for the dummy to cast but nothing has worked yet. Help is appreciated!

 

[Pyrogasm]

Does your AGRO function properly return only enemies? I don’t really see anywhere it could be doing that because you never store the caster/owner in a global that AGRO could check. It’s probably being told to target the caster with that sleep, which obviously won’t work. Also what did you change in Targets Allowed?

The easiest way to test for issues like this is just to give the ability to some unit on the map and attempt to manually cast it on targets. Does it cast? Cool, it’s not an issue with the ability.

Then I’d try using some other known working spell to see if it will cast that successfully. It seems you’ve done this and have learned an issue is with either the dummy, target, or order. You can narrow it down.

Your dummy does not need to have 3.0 turn rate. In order to cast instantly in any direction the dummy should have: movement type NONE, speed base 0, cast backswing 0, and cast point 0.

Finally, there is no reason to use Pow(_,2) to square numbers, and taking the square root is also unnecessary. Both functions are not fast to call and can cause a bottleneck in situations where many such calls are made per second. Here it won’t matter but if GetClosestEnemy is constantly being used it could.

Instead of this:

mindist = SomeNumber
ux = GetUnitX(u)
uy = GetUnitY(u)
if sqareroot(Pow(ux-x,2)+Pow(uy-y,2)) < mindist …

Do this:

mindist = SomeNumber
ux = GetUnitX(u)
uy = GetUnitY(u)
if (ux-x)*(ux-u)+(uy-y)*(uy-y) < mindist*mindist

 

[FaceJaraxxus]

Does your AGRO function properly return only enemies? I don’t really see anywhere it could be doing that because you never store the caster/owner in a global that AGRO could check. It’s probably being told to target the caster with that sleep, which obviously won’t work. Also what did you change in Targets Allowed?

The easiest way to test for issues like this is just to give the ability to some unit on the map and attempt to manually cast it on targets. Does it cast? Cool, it’s not an issue with the ability.

Then I’d try using some other known working spell to see if it will cast that successfully. It seems you’ve done this and have learned an issue is with either the dummy, target, or order. You can narrow it down.

Your dummy does not need to have 3.0 turn rate. In order to cast instantly in any direction the dummy should have: movement type NONE, speed base 0, cast backswing 0, and cast point 0.

Finally, there is no reason to use Pow(_,2) to square numbers, and taking the square root is also unnecessary. Both functions are not fast to call and can cause a bottleneck in situations where many such calls are made per second. Here it won’t matter but if GetClosestEnemy is constantly being used it could.

Instead of this:

mindist = SomeNumber
ux = GetUnitX(u)
uy = GetUnitY(u)
if sqareroot(Pow(ux-x,2)+Pow(uy-y,2)) < mindist …

Do this:

mindist = SomeNumber
ux = GetUnitX(u)
uy = GetUnitY(u)
if (ux-x)*(ux-u)+(uy-y)*(uy-y) < mindist*mindist

Thanks a lot for the pointers and troubleshooting! Really helpful.

Oh I didn't even think to just try to manually try out the ability. I'll remember it!

The problem turned out to be the AGRO function not properly returning the correct target, despite me being sure that it would. So what happens now is that the closest unit is put to sleep by the dummy. It also seems to find the correct target smoothly even at +10 heroes to filter through. I did try to optimize according to your recommendation but after some frustration with testing I reverted. I still have a lot to learn. Also, the AI would use it instantly during testing, messing with the intialization and freezing the game. I should test in more proper ways from now on.

Here's the entire script at a "working" state:

function DoomLordSleep_Conditions takes nothing returns boolean
    return GetSpellAbilityId() == '(dummyAbilityID)'
endfunction

function FindClosestEnemy takes unit caster returns unit
    local unit target
    local unit result
    local group g = CreateGroup()
    local real x = GetUnitX(caster)
    local real y = GetUnitY(caster)
    local real minDistance = 6000.0
    local real distance 
    call GroupEnumUnitsInRange(g, x, y, 6000, function FilterUnitFunction()
    loop
        set target = FirstOfGroup(g)
        exitwhen target == null
            set distance = SquareRoot(Pow(x - GetUnitX(target), 2) + Pow(y - GetUnitY(target), 2))
            if distance < minDistance then
                set minDistance = distance
                set result = target
            endif
        call GroupRemoveUnit(g, target)
    endloop
    call DestroyGroup(g)
    return result
endfunction

function DoomLordSleep_Actions takes nothing returns nothing
    local unit caster = GetTriggerUnit()
    local unit dammy
    local unit target
    local real x
    local real y
    set x = GetUnitX(caster)
    set y = GetUnitY(caster)
    set target = FindClosestEnemy(caster)
    if target != null then
    set dammy = CreateUnit(GetOwningPlayer(caster), '(dummyID)', x, y, GetUnitFacing(target))
        call UnitAddAbility(dammy,'(abilityID)')
        call IssueTargetOrder(dammy,"sleep",target)
        call UnitApplyTimedLifeBJ(3.00, 'BTLF', dammy)
    endif
    set caster = null
    set target = null
    set dammy = null
endfunction

//===========================================================================
function InitTrig_DoomLordSleep takes nothing returns nothing
    set gg_trg_DoomLordSleep = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ(gg_trg_DoomLordSleep, EVENT_PLAYER_UNIT_SPELL_EFFECT)
    call TriggerAddCondition(gg_trg_DoomLordSleep, Condition(function DoomLordSleep_Conditions))
    call TriggerAddAction(gg_trg_DoomLordSleep, function DoomLordSleep_Actions)
endfunction

 

[Pyrogasm]

That still doesn't show the AGRO function. You really didn't have to change much to make it work; all you need to do is store the player that owns the caster, then only look for units hostile to that player. One global player variable is all that's required:

globals
    player AGRO_Player
endglobals

function AGRO takes nothing returns boolean
    local unit f = GetFilterUnit()
    local boolean b = false
    set b = IsUnitEnemy(f, AGRO_Player) and /*
         */ IsUnitType(f, UNIT_TYPE_STRUCTURE) and /*
         */ GetWidgetLife(f) > 0.405

    //This is just a multiline comment to better organize each different condition onto a 'new' line without actually having them be on separate lines.
    //Everything between /* and */ is treated as a comment, including the linebreak
    //If you have other conditions (or don't need the structure check) you can add them, just don't forget the ANDs
    //
    //The reason a boolean is being used is that if you just returned the value, the unit variable f would not be nulled before the return and would
    //become a sort of reference memory leak. b does not have this issue because it is a primitive type variable.

    set f = null
    return b
endfunction

function FindClosestEnemy takes unit caster returns unit
    local unit target
    local unit result
    local group g = CreateGroup()
    local real x = GetUnitX(caster)
    local real y = GetUnitY(caster)
    local real minDistance = 6000.0
    local real distance
    local real ux
    local real uy
    set AGRO_Player = GetOwningPlayer(caster)
    call GroupEnumUnitsInRange(g, x, y, minDistance, function AGRO) //this line uses minDistance as it should now
    set minDistance = minDistance * minDistance //square it after we've used it for search radius
    loop
        set target = FirstOfGroup(g)
        exitwhen target == null
            set ux = GetUnitX(target)
            set uy = GetUnitY(target)
            set distance = (x-ux)*(x-ux) + (y-uy)*(y-uy)  //this is actually the distance squared
            if distance < minDistance then
                set minDistance = distance
                set result = target
            endif
        call GroupRemoveUnit(g, target)
    endloop
    call DestroyGroup(g)
    set g = null //forgot this
    return result
endfunction

There are a few other odd things I see.

call GroupEnumUnitsInRange(g, x, y, 6000, function FilterUnitFunction() has an erroneous ( near the end. The last argument to that function is actually a function itself, and when you give a function as an argument you aren't calling the function directly so you don't need the () at the end. I'd assume JASSHelper just ignored the ( and didn't throw an error, but it is unnecessary. The other ) actually closes the parenthesis containing the arguments, so it isn't 'trying' to call FilterUnitFunction either.

'(dummyID)' is not the value of dummyID, it is the number (dummyID) converted from base-64 to base-10. That includes the parenthesis as part of the number. If you are inputting a rawcode directly you will use the apostrophes, as they are what does the conversion: 'BTLF' is the base-10 converstion of BTLF in base-64. When using a variable you just need to put the variable there directly: dummyID, though when that variable is assigned a value in your configuration of course it needs to use the proper 'A001' form. If using Lua instead you would use FourCC("A001").

 

[FaceJaraxxus]

That still doesn't show the AGRO function. You really didn't have to change much to make it work; all you need to do is store the player that owns the caster, then only look for units hostile to that player. One global player variable is all that's required:

globals
    player AGRO_Player
endglobals

function AGRO takes nothing returns boolean
    local unit f = GetFilterUnit()
    local boolean b = false
    set b = IsUnitEnemy(f, AGRO_Player) and /*
         */ IsUnitType(f, UNIT_TYPE_STRUCTURE) and /*
         */ GetWidgetLife(f) > 0.405

    //This is just a multiline comment to better organize each different condition onto a 'new' line without actually having them be on separate lines.
    //Everything between /* and */ is treated as a comment, including the linebreak
    //If you have other conditions (or don't need the structure check) you can add them, just don't forget the ANDs
    //
    //The reason a boolean is being used is that if you just returned the value, the unit variable f would not be nulled before the return and would
    //become a sort of reference memory leak. b does not have this issue because it is a primitive type variable.

    set f = null
    return b
endfunction

function FindClosestEnemy takes unit caster returns unit
    local unit target
    local unit result
    local group g = CreateGroup()
    local real x = GetUnitX(caster)
    local real y = GetUnitY(caster)
    local real minDistance = 6000.0
    local real distance
    local real ux
    local real uy
    call GroupEnumUnitsInRange(g, x, y, minDistance, function AGRO) //this line uses minDistance as it should now
    set minDistance = minDistance * minDistance //square it after we've used it for search radius
    loop
        set target = FirstOfGroup(g)
        exitwhen target == null
            set ux = GetUnitX(target)
            set uy = GetUnitY(target)
            set distance = (x-ux)*(x-ux) + (y-uy)*(y-uy)  //this is actually the distance squared
            if distance < minDistance then
                set minDistance = distance
                set result = target
            endif
        call GroupRemoveUnit(g, target)
    endloop
    call DestroyGroup(g)
    set g = null //forgot this
    return result
endfunction

There are a few other odd things I see.

call GroupEnumUnitsInRange(g, x, y, 6000, function FilterUnitFunction() has an erroneous ( near the end. The last argument to that function is actually a function itself, and when you give a function as an argument you aren't calling the function directly so you don't need the () at the end. I'd assume JASSHelper just ignored the ( and didn't throw an error, but it is unnecessary. The other ) actually closes the parenthesis containing the arguments, so it isn't 'trying' to call FilterUnitFunction either.

'(dummyID)' is not the value of dummyID, it is the number (dummyID) converted from base-64 to base-10. That includes the parenthesis as part of the number. If you are inputting a rawcode directly you will use the apostrophes, as they are what does the conversion: 'BTLF' is the base-10 converstion of BTLF in base-64. When using a variable you just need to put the variable there directly: dummyID, though when that variable is assigned a value in your configuration of course it needs to use the proper 'A001' form. If using Lua instead you would use FourCC("A001").

Ah, my apologies. The final script I posted here was to clarify what variable/function types goes where for people that might stumble upon the code and be interested. Thanks for watching out for me! Here's the actual one I've used:

function DoomLordSleep_Conditions takes nothing returns boolean
    return GetSpellAbilityId() == 'A0BP'
endfunction

function FindClosestEnemy takes unit caster returns unit
    local unit target
    local unit result
    local group g = CreateGroup()
    local real x = GetUnitX(caster)
    local real y = GetUnitY(caster)
    local real minDistance = 6000.0
    local real distance 
    call GroupEnumUnitsInRange(g, x, y, 6000, function AGRO2)
    loop
        set target = FirstOfGroup(g)
        exitwhen target == null
            set distance = SquareRoot(Pow(x - GetUnitX(target), 2) + Pow(y - GetUnitY(target), 2))
            if distance < minDistance then
                set minDistance = distance
                set result = target
            endif
        call GroupRemoveUnit(g, target)
    endloop
    call DestroyGroup(g)
    set g = null
    return result
endfunction

function DoomLordSleep_Actions takes nothing returns nothing
    local unit caster = GetTriggerUnit()
    local unit dammy
    local unit target
    local real x
    local real y
    set x = GetUnitX(caster)
    set y = GetUnitY(caster)
    set target = FindClosestEnemy(caster)
    if target != null then
    set dammy = CreateUnit(GetOwningPlayer(caster), 'n00S', x, y, GetUnitFacing(target))
        call UnitAddAbility(dammy,'A05Q')
        call IssueTargetOrder(dammy,"sleep",target)
        call UnitApplyTimedLifeBJ(3.00, 'BTLF', dammy)
    endif
    set caster = null
    set target = null
    set dammy = null
endfunction

//===========================================================================
function InitTrig_DoomLordSleep takes nothing returns nothing
    set gg_trg_DoomLordSleep = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ(gg_trg_DoomLordSleep, EVENT_PLAYER_UNIT_SPELL_EFFECT)
    call TriggerAddCondition(gg_trg_DoomLordSleep, Condition(function DoomLordSleep_Conditions))
    call TriggerAddAction(gg_trg_DoomLordSleep, function DoomLordSleep_Actions)
endfunction

And the bool function for anyone interested:

function AGRO2 takes nothing returns boolean
return IsPlayerAlly(Player(11),GetOwningPlayer(GetFilterUnit()))==false and GetUnitAbilityLevel(GetFilterUnit(),'Aloc')==0 and IsUnitPaused(GetFilterUnit())==false and IsUnitType(GetFilterUnit(),UNIT_TYPE_MECHANICAL)==false and GetUnitState(GetFilterUnit(),UNIT_STATE_LIFE)>1.0 and IsUnitVisible(GetFilterUnit(),Player(11))==true and GetUnitAbilityLevel(GetFilterUnit(),'Avul')==0
endfunction

I added the null for g, thanks! I'll definitely improve it with your suggestions, the practical application and comments is very helpful! It's currently as you've pointed out: laggy if it occurs often or if multiple units cast it.

 

[Pyrogasm]

• Units with Locust will never appear in area-based group enums, so you do not need to filter that manually. They can only be 'found' by GroupEnumUnitsOfPlayer.
• Checking if the unit is visible to Player(11) might be another performance bottleneck; I'm not familiar with exactly how that works but my guess is it's not very efficient. You want to prevent the Sleep being cast on any units that are close but not visible to the caster?
• Why check to make sure a potential target isn't paused? Are there reasons random nearby units might be paused such that you'll need to account for them?
• Checking if the owner of a unit is the ally of another player is more convoluted than checking if a unit is hostile to a specific player: IsUnitEnemy(GetFilterUnit(), Player(11))
• Units die when they reach 0.405 life or less, not at 1 as you have checked. Very minor edge case, but that's something you should be aware of. A unit with 0.787 hp is 'alive' according to the game and would show 1 hp. You can also use the simpler function native GetWidgetLife takes widget whichWidget returns real instead of the more complex unit state to get the current life of a unit. Checking UNIT_TYPE_DEAD is not a good solution, either, though you're not doing that here. The UnitAlive function from common.ai works fine, but it does need to be declared manually once somewhere in your map (and multiple declarations might cause a JASSHelper error) in order to be callable.
• You're not matching the level of the dummy-cast Sleep to that of the original spell. This only matters if the sleep behavior/timing/targeting/something should be different for different levels rather than always being the same.
• IMO hardcoding in something like Player(11) for hostility check is a weird choice and could produce some strange behavior later on if a unit hostile to Player(11) can cast this spell or you attempt to do so for debug purposes and get confused.

Finally, there's no real need to compare boolean values like this: if FunctionThatReturnsABooleanValue() == true then. That function returns either true or false so you can imagine replacing that function call with both values to see what that looks like: if true == true then or if false == true then. You could just write if true then instead, and comparing the boolean == true is the same thing as just asking for the value of the boolean. You can use the not keyword to invert boolean statements.

You can reduce the text in your filtering this way and make it a little easier to read. It won't change anything, just good practice to get familiar with.