[JASS] Reoccuring group problem

[Shyhalu]

As of now, the spell will pick the spell ability level -1 targets and cast entangling roots on them, but will not pick another group and do it again.

Its suppose to pick X targets and cast the root spell on them every 3 seconds, where X is the hero's ability level. It iterates every 3 seconds, but will not pick another group.

udg_HelpInts[7] is just a global slot holding the value of the ability level.

Any help is appreciated.

function KoSC takes nothing returns boolean
    return GetSpellAbilityId() == 'A03U'
endfunction

function isroot takes nothing returns boolean
    return GetOwningPlayer(GetEnumUnit()) != GetOwningPlayer(GetSpellAbilityUnit())
endfunction

function root takes nothing returns nothing
    local unit u = GetSpellAbilityUnit()
    local unit n = GetEnumUnit()    
    local unit z

  if(udg_HelpInts[7] > 0) then
    if(GetOwningPlayer(n) != GetOwningPlayer(u)) then
        set z = CreateUnit(GetOwningPlayer(u), 'nbda', GetUnitX(n), GetUnitY(n), 0)         
        call IssueTargetOrder(z, "entanglingroots", n)
        call UnitApplyTimedLife(z,'BTLF', 3)
        set z = null
        set udg_HelpInts[7] = udg_HelpInts[7] - 1
    endif
  endif

    set u = null
    set n = null
endfunction

function KoSA takes nothing returns nothing
   local unit u = GetSpellAbilityUnit()
   local location l
   local group g
   local integer j = 30
   local integer c = GetUnitAbilityLevel(u,'A03U')

   loop
      exitwhen j <= 0 or GetUnitState(u, UNIT_STATE_LIFE) <= 0
        set l = GetUnitLoc(u)
        set g = GetUnitsInRangeOfLocMatching(800,l, Condition(function isroot))
        set udg_HelpInts[7] = c

        call DisplayTextToPlayer(Player(0),0,0,"start funct")

           call ForGroupBJ(g, function root)

        call DisplayTextToPlayer(Player(0),0,0,"end funct")
      
        call RemoveLocation(l)
        call DestroyGroup(g)
  
        call DisplayTextToPlayer(Player(0),0,0,I2S(j))

        call TriggerSleepAction(3)
      set j = j-3
   endloop

   call RemoveLocation(l)
   call DestroyGroup(g)

   set u = null
   set l = null
   set g = null
endfunction

//===========================================================================
function InitTrig_Ko_Spir takes nothing returns nothing
    set gg_trg_Ko_Spir = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( gg_trg_Ko_Spir, EVENT_PLAYER_UNIT_SPELL_CAST )
    call TriggerAddCondition( gg_trg_Ko_Spir, Condition( function KoSC ) )
    call TriggerAddAction( gg_trg_Ko_Spir, function KoSA )
endfunction

 

[GhostWolf]

You destroyed the group g, thus g refers to nothing, and you need to CreateGroup() it.
If you don't want to allocate a new group every iteration, ClearGroup() it instead of destroying it.

 

[Shyhalu]

I got the same result when I did

set g = CreateGroup() inside the loop right avove the part where it picks units in range.

function GetUnitsInRangeOfLocMatching takes real radius, location whichLocation, boolexpr filter returns group
    local group g = CreateGroup()
    call GroupEnumUnitsInRangeOfLoc(g, whichLocation, radius, filter)
    call DestroyBoolExpr(filter)
    return g
endfunction

This is also the inner code for the function, it creates a group itself too.

 

[Shyhalu]

Still no dice

function KoSA takes nothing returns nothing
   local unit u = GetSpellAbilityUnit()
   local location l
   local group g = CreateGroup()
   local integer j = 30
   local integer c = GetUnitAbilityLevel(u,'A03U')

   loop
      exitwhen j <= 0 or GetUnitState(u, UNIT_STATE_LIFE) <= 0
        set l = GetUnitLoc(u)
        set g = GetUnitsInRangeOfLocMatching(800,l, Condition(function isroot))
        set udg_HelpInts[7] = 6

        call DisplayTextToPlayer(Player(0),0,0,"start funct")

           call ForGroupBJ(g, function root)

        call DisplayTextToPlayer(Player(0),0,0,"end funct")
      
        call RemoveLocation(l)
        call GroupClear(g)
  
        call DisplayTextToPlayer(Player(0),0,0,I2S(j))

        call TriggerSleepAction(3)
      set j = j-3
   endloop

   call RemoveLocation(l)
   call DestroyGroup(g)

   set u = null
   set l = null
   set g = null
endfunction

 

[GhostWolf]

In that case, is GetSpellAbilityUnit() the same unit after 3 seconds? You could use GetTriggerUnit() which will technically make this MUI.

If that's not the cause then I have no idea what it is (I suppose there is no need to ask if "end funct" is shown as it should be, right?).

Aside from that, you could inline that ForGroupBJ inside the loop itself - you don't need the other function in the first place, and since it only bothers you (with the global integer), why not just remove it?

 

[Shyhalu]

After some debugging, I think I've narrowed it down to it not running the function at all.

C is 7 at the start, then it will start the function
J goes to 27 and C goes to -1

The next iteration
C is 7, it starts the function
Then J is 24, but C is 7 again, not -1.

 

[Shyhalu]

In that case, is GetSpellAbilityUnit() the same unit after 3 seconds? You could use GetTriggerUnit() which will technically make this MUI.

If that's not the cause then I have no idea what it is (I suppose there is no need to ask if "end funct" is shown as it should be, right?).

Aside from that, you could inline that ForGroupBJ inside the loop itself - you don't need the other function in the first place, and since it only bothers you (with the global integer), why not just remove it?

I don't know how to iterate a group of units without the forgroupBJ
Its suppose to pick all units within 800 distance, filter out allies, then root X of them, and do that every 3 seconds until the spell is over or the caster is dead.

if the other functions were not right, it wouldn't hit any of them with entangling roots.
The endfunct and startfunct are both shown with each iteration, J declines properly, but C/the global int does not, only works once for some reason.

(The previous posted code I set the global to 6 to see if it was not updating, right now its set to C)

Is there a specific keyword for inlining in jass?

 

[GhostWolf]

Just put your ForGroupBJ inside a nested loop.
Something like this:

loop
    ...

    loop

        set temp_unit = FirstOfGroup(g)
        exitwhen temp_unit == null // will happen when the group in empty
        call GroupRemoveUnit(temp_unit)
        
        if your_conditions then
        
            // your ForGroupBJ actions
        
        endif
    
    endloop

    ...
endloop

 

[Shyhalu]

Ok....its GetTriggerUnit(), I switched it to that and it works fine..

but what the hell? Why doesn't getspellabilityunit() work?

ty for the inlining, I'll apply it when I overhaul all my heros' trigger spells, I imagine thats a ton faster than using the BJ.

 

[GhostWolf]

GetSpellAbilityUnit() and basically ALL the other versions that get event-callback units are just returning the value of a global unit.

For example, GetSpellAbilityUnit() returns bj_SpellAbilityUnit (this isn't the real name, if you want the real one open blizzard.j and find it).

Therefore, every time a trigger runs which has an event that has anything to do with casting spells - it overwrites bj_SpellAbilityUnit with the new casting unit.

GetTriggerUnit() works in a different way, in which there is a reference to a Triggering Unit in each trigger that is run in the game (if there is indeed a unit that triggered it, of course), so there are no overwriting problems.

 

[Shyhalu]

It will cast it mutliple times now....but it doesn't want to get a new group now

function KoSA takes nothing returns nothing
   local unit u = GetTriggerUnit()
   local unit z
   local unit q
   local location l
   local group g
   local player p = GetOwningPlayer(u)
   local integer j = 30
   local integer c = GetUnitAbilityLevel(u,'A03U')
   local integer k = c

   loop
      exitwhen j <= 0 or GetUnitState(u, UNIT_STATE_LIFE) <= 0
        set l = GetUnitLoc(u)
        set g = CreateGroup()
        set g = GetUnitsInRangeOfLocMatching(800,l, null)

        call DisplayTextToPlayer(Player(0),0,0,I2S(k))

          loop
              set z = FirstOfGroup(g)
              exitwhen z == null or k == 0

               if(IsUnitEnemy(z,p)) then
                   set q = CreateUnit(p, 'nbda', GetUnitX(z), GetUnitY(z), 0)         
                   call IssueTargetOrder(q, "entanglingroots", z)
                   call UnitApplyTimedLife(q,'BTLF', 3)
                   set k = k - 1
               endif
 
              call GroupRemoveUnit(g,z)
          endloop         

        call DestroyGroup(g)
        call RemoveLocation(l)
        call TriggerSleepAction(3)
      set k = c
      set j = j-3
   endloop

   call RemoveLocation(l)
   call DestroyGroup(g)

   set q = null
   set z = null
   set u = null
   set l = null
   set g = null
endfunction

 

[GhostWolf]

I can't think of anything else that can bug beside the group.

Try using a native (GroupEnumUnitsInRangeOfLoc / GroupEnumUnitsInRange or the Counted versions which you'd maybe prefer in this case) and see if it works.

For example:

local real x = GetUnitX(u)
local real y = GetUnitY(u)
local group g = CreateGroup()
...

loop

    call GroupEnumUnitsInRange(g,x,y,800,null)
    ...

endloop

call DestroyGroup(g)
set g = null

 

[Shyhalu]

No dice, although atleast its faster now.

function KoSA takes nothing returns nothing
   local unit u = GetTriggerUnit()
   local unit z
   local unit q
   local group g
   local player p = GetOwningPlayer(u)
   local integer j = 30
   local integer c = GetUnitAbilityLevel(u,'A03U')
   local integer k = c

   loop
      exitwhen j <= 0 or GetUnitState(u, UNIT_STATE_LIFE) <= 0
        set g = CreateGroup()
        call GroupEnumUnitsInRange(g, GetUnitX(u), GetUnitY(u), 800, null)

        call DisplayTextToPlayer(Player(0),0,0,I2S(k))

          loop
              set z = FirstOfGroup(g)
              exitwhen z == null or k == 0

               if(IsUnitEnemy(z,p)) then
                   set q = CreateUnit(p, 'nbda', GetUnitX(z), GetUnitY(z), 0)         
                   call IssueTargetOrder(q, "entanglingroots", z)
                   call UnitApplyTimedLife(q,'BTLF', 3)
                   set k = k - 1
               endif
 
              call GroupRemoveUnit(g,z)
          endloop         

        call DestroyGroup(g)
        call TriggerSleepAction(3)
      set k = c
      set j = j-3
   endloop

   call DestroyGroup(g)

   set q = null
   set z = null
   set u = null
   set g = null

 

[GhostWolf]

I didn't show the example for no reason - create the group at the start and destroy it at the end, not in the loop.

 

[Shyhalu]

Same results

On a side note, since GetSpellAbilityUnit is a bj variable, I take it that would cause a lot of crashes and concurrency issues?

function KoSA takes nothing returns nothing
   local unit u = GetTriggerUnit()
   local unit z
   local unit q
   local group g = CreateGroup()
   local player p = GetOwningPlayer(u)
   local integer j = 30
   local integer c = GetUnitAbilityLevel(u,'A03U')
   local integer k = c

   loop
      exitwhen j <= 0 or GetUnitState(u, UNIT_STATE_LIFE) <= 0     

        call GroupEnumUnitsInRange(g, GetUnitX(u), GetUnitY(u), 800, null)
        call DisplayTextToPlayer(Player(0),0,0,I2S(k))

          loop
              set z = FirstOfGroup(g)
              exitwhen z == null or k == 0

               if(IsUnitEnemy(z,p)) then
                   set q = CreateUnit(p, 'nbda', GetUnitX(z), GetUnitY(z), 0)         
                   call IssueTargetOrder(q, "entanglingroots", z)
                   call UnitApplyTimedLife(q,'BTLF', 3)
                   set k = k - 1
               endif
 
              call GroupRemoveUnit(g,z)
          endloop         

        call TriggerSleepAction(3)
      set k = c
      set j = j-3
   endloop

   call DestroyGroup(g)

   set q = null
   set z = null
   set u = null
   set g = null
endfunction

 

[Deaod]

...since GetSpellAbilityUnit is a bj variable,...

no, its not, its a native function returning the caster of the ability being cast.

Edit: I think there are some problems associated with that function. If memory serves, the value it returns gets reset when you call certain functions.

 

[Shyhalu]

Well, it switched groups when i moved the hero far away with some chasing

I think the problem is that is selects the same units from the group every time, the same exact way
I'm gonna add a buff check to it

 

[Shyhalu]

no, its not, its a native function returning the caster of the ability being cast.

Edit: I think there are some problems associated with that function. If memory serves, the value it returns gets reset when you call certain functions.

Well, my map crashes somewhat consistently so Im gonna change it to triggerunits for the hell of it.

 

[Shyhalu]

Yeah, that was the problem, it was picking the units in the same order every time. Ty much for the help.

Here is the finished code for anyone who cares.

Its linked to the locust swarm ability, it will cast entangling roots on nearby enemy units

function KoSA takes nothing returns nothing
   local unit u = GetTriggerUnit()
   local unit z
   local unit q
   local group g = CreateGroup()
   local player p = GetOwningPlayer(u)
   local integer j = 30
   local integer c = GetUnitAbilityLevel(u,'A03U')
   local integer k = c

   loop
      exitwhen j <= 0 or GetUnitState(u, UNIT_STATE_LIFE) <= 0     
         call GroupEnumUnitsInRange(g, GetUnitX(u), GetUnitY(u), 800, null)

          loop
              set z = FirstOfGroup(g)
              exitwhen z == null or k == 0

               if(IsUnitEnemy(z,p) and GetUnitAbilityLevel(z, 'BEer') < 1) then
                   set q = CreateUnit(p, 'nbda', GetUnitX(z), GetUnitY(z), 0)         
                   call IssueTargetOrder(q, "entanglingroots", z)
                   call UnitApplyTimedLife(q,'BTLF', 3)
                   set k = k - 1
               endif
 
              call GroupRemoveUnit(g,z)
          endloop         

        call TriggerSleepAction(3)
      set k = c
      set j = j-3
   endloop

   call DestroyGroup(g)

   set q = null
   set z = null
   set u = null
   set g = null
endfunction

Edit: But I need to spam to get post count up and increase epeen size!

 

[Deaod]

Tip: Edit your posts, instead of triple-posting.

 

[GhostWolf]

For whatever it's worth, put GetUnitAbilityLevel(z, 'BEer') in an integer and put that in the condition (or just put it straight away in a boolean and put that in the condition).

It's always faster to put a value in a variable if it removes the need to call a function which returns the same thing, multiple times.

I also hope you learn from this thread how to design code better, as you've probably seen a new way of looking at things.
Happy coding.

 

[Shyhalu]

For whatever it's worth, put GetUnitAbilityLevel(z, 'BEer') in an integer and put that in the condition (or just put it straight away in a boolean and put that in the condition).

It's always faster to put a value in a variable if it removes the need to call a function which returns the same thing, multiple times.

I also hope you learn from this thread how to design code better, as you've probably seen a new way of looking at things.
Happy coding.

I know its faster to pop it in a variable, but in this case it wouldn't be referencing the same thing. It checks the current unit to see if its already been hit with entangling roots, then removes it from the group. Storing it in a variable would just have it referencing the same unit for all units picked.

Anyway, yeah the inlining helps me eliminate a bunch of function calls and ForGroupBJs for hero abilities and unit spawns. Real glad to learn how to do it.