[Solved] Need help with a spell

[TotalClusterLuck]

The spell is about the Thrall's feral spirit.

I made a Devotion aura based spell and gave it to the summoned wolves. It has a very small radius (Single target) and has 4 levels. The rawcode of the spell is 'W001'.

Plan was to check every 2s of gametime the number of Wolves in each of the Wolves' range of 400 and set the aura level equal to the number of wolves close by. (simulating a wolf pack, the more the merrier.)

Thing is, only one of the wolves gets updated, the rest have max aura no matter the distance from other wolves because they are getting summoned clumped up. Thrall's spell summons 4 wolves btw, vanilla stats.

Also (thanks to the BJDebugMsg) I know that the WolfPack function never gets to the end. I do not know why since (I think) there are no endless loops. I tried one single group with constant cleaning after each wolf check, and I tried 4 different groups (present code). I also tried using TriggerSleep with 1s wait in a loop simulating a periodic timer of 1s.

I am new to this site and Jass coding in general and I would appreciate some help
If I overstepped some rules, please let me know I will adjust the post.

Anyways, here's the code (it's messy I know, but I am a beginner, be gentle )
Also used TimerUtilsEx library to my absolute confusion (timers are weird).

scope WolfPack initializer Init

globals
private unit Wolf1
private unit Wolf2
private unit Wolf3
private unit Wolf4
private boolexpr b
private integer c
endglobals

private function WolfCheck takes unit u returns boolean
return (GetUnitName(u) == "Shadow Wolf" and GetWidgetLife(u) > 0.405)
endfunction

private function Filterb takes nothing returns boolean
return WolfCheck(GetFilterUnit())
endfunction

private function WolfPack takes nothing returns nothing
local group g_1
local group g_2
local group g_3
local group g_4
local integer w = 0
local unit u
//First Wolf
set g_1 = GetUnitsInRangeOfLocMatching(400, GetUnitLoc(Wolf1), b)
loop
    set u = FirstOfGroup (g_1)
    exitwhen (u==null)
    set w=w+1
    call GroupRemoveUnit (g_1, u)
endloop
call SetUnitAbilityLevel (Wolf1, 'W001', w)
call SetUnitAbilityLevel (Wolf1, 'W002', w)
set w=0
// Second Wolf
set g_2 = GetUnitsInRangeOfLocMatching(400, GetUnitLoc(Wolf2), b)
loop
set u = FirstOfGroup (g_2)
    exitwhen (u==null)
    set w=w+1
    call GroupRemoveUnit(g_2,u)
endloop
call SetUnitAbilityLevel (Wolf2, 'W001', w)
call SetUnitAbilityLevel (Wolf2, 'W002', w)
set w=0
// Third Wolf
set g_3 = GetUnitsInRangeOfLocMatching(400, GetUnitLoc(Wolf3), b)
loop
set u = FirstOfGroup (g_3)
    exitwhen (u==null)
    set w=w+1
    call GroupRemoveUnit(g_3,u)
endloop
call SetUnitAbilityLevel (Wolf3, 'W001', w)
call SetUnitAbilityLevel (Wolf3, 'W002', w)
set w=0
// Fourth Wolf
set g_4 = GetUnitsInRangeOfLocMatching(400, GetUnitLoc(Wolf4), b)
loop
set u = FirstOfGroup (g_4)
    exitwhen (u==null)
    set w=w+1
    call GroupRemoveUnit(g_4,u)
endloop
call SetUnitAbilityLevel (Wolf4, 'W001', w)
call SetUnitAbilityLevel (Wolf4, 'W002', w)
set w=0
set c=c+1
call DestroyGroup (g_1)
call DestroyGroup (g_2)
call DestroyGroup (g_3)
call DestroyGroup (g_4)
set g_1=null
set g_2=null
set g_3=null
set g_4=null
call BJDebugMsg ("WolfPack done")
if c>29 then
set c=0
set Wolf1=null
set Wolf2=null
set Wolf3=null
set Wolf4=null
endif
endfunction

private function Actions takes nothing returns nothing
local group g
local unit u
local integer c = 0
local timer t
call TriggerSleepAction (1)
set t = NewTimer()
call BJDebugMsg ("Actions starting")
set g = GetUnitsInRangeOfLocMatching(1000, GetUnitLoc(GetTriggerUnit()), b)
set Wolf1 = FirstOfGroup(g)
call GroupRemoveUnit(g,Wolf1)
set Wolf2 = FirstOfGroup(g)
call GroupRemoveUnit(g,Wolf2)
set Wolf3 = FirstOfGroup(g)
call GroupRemoveUnit(g,Wolf3)
set Wolf4 = FirstOfGroup(g)
call GroupRemoveUnit(g,Wolf4)
call SetWidgetLife (Wolf1, 200)
call SetWidgetLife (Wolf2, 200)
call SetWidgetLife (Wolf3, 200)
call SetWidgetLife (Wolf4, 200)
call BJDebugMsg("B4 calling the functions")
call TimerStart (t, 2, true, function WolfPack )
call DestroyGroup (g)
set g=null
call BJDebugMsg ("Actions worked")
endfunction

private function Conditions takes nothing returns boolean
return GetSpellAbilityId() == 'T003'
endfunction

private function Init takes nothing returns nothing
local trigger t = CreateTrigger()
        call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_SPELL_EFFECT )
        call TriggerAddCondition (t,Condition(function Conditions))
        call TriggerAddAction( t, function Actions )
        set Wolf1= null
        set Wolf2= null
        set Wolf3= null
        set Wolf4= null
        set b= Condition (function Filterb)
endfunction
endscope

 

[TotalClusterLuck]

Bumpy Bump.

 

[Pyrogasm]

• In general try to avoid BJ functions (they appear here as red), which are defined non-native functions Blizzard wrote that usually just do the stuff the natives do with slightly different arguments. Stuff like TriggerRegisterAnyUnitEventBJ is fine because it only runs once but GetUnitsInRangeOfLocMatching is less so because it runs often. Some BJs are useful, like BJDebugMsg.
  
• You're leaking a location when using GetUnitLoc(). The rules are the same: Things That Leak
• If at all possible avoid locations entirely. JASS makes it much easier to work directly with XY coordinates than GUI does and you don't have to worry about forgetting to remove them. Most of the natives take XY coordinates rather than locations.
• Timers just run a function periodically (or only once after a delay). That's all they do.
• I wouldn't check for the right units by their name with GetUnitName(), I would recommend using GetUnitTypeId() and checking against its rawcode.
• Globals don't have to be set to null in init threads.
• Triggers and timers are handle-type variables and as such locals need to be nulled at then end of the function like groups, units, etc..
• Instead of waiting a second after the spell is cast you could use the "unit enters map" event instead. There's also the "unit spawns a summoned unit" event: EVENT_PLAYER_UNIT_SUMMON.
• Instead of using ForGroup() you can loop through the group using FirstOfGroup to do it within the same function. This does, however, clear all units from the group in the process but it removes the need for the globals. Here it will actually be simplest to use ForGroup because we want to keep the units in the group.
  
• A few configuration globals would be beneficial so it's easy to modify in the future.
• Your code would benefit most from removing its reliance on hardcoding in lieu of running sections repeatedly automatically. This will significantly simplify your code. To this end we'll restructure how we do the checking: instead of looking around each wolf to see if there are other wolves, we just compare each wolf to every other wolf. This could be more but is likely fewer operations than the other way. Distance involves a square root check which can be slow, so instead we will compare the squares of the distance which works just as fine.

scope WolfPack initializer Init
globals
  private constant integer WOLF_ID = 'WOLF'
  private constant integer SUMMON_SPELL = 'T003'
  private constant integer AURA_SPELL = 'W001'
  private constant integer OTHER_SPELL = 'W002'

  private constant integer SPAWN_LEVEL = 4 //default level of the spell wolves are given when summoned
  private constant boolean ALLIED_PACKS_ONLY = true //wolves must be allied with each other to gain pack bonus
  private real PACK_DISTANCE = 400.00 //can't be constant because it gets squared at init.

  private timer WolfUpdate = null
  private group Wolves = null

  private player up
  private real ux
  private real uy
  private integer c
endglobals


private function WolfPackSubGroup takes nothing returns nothing
  local unit w = Get
  local real wx
  local real wy

  if (ALLIED_PACKS_ONLY and IsPlayerAlly(up, GetOwningPlayer(w))) or not ALLIED_PACKS_ONLY /*
*/   and (wx-ux)*(wx-ux) + (wy-uy)*(wy-uy) <= PACK_DISTANCE  /*comparing the square of the distance to avoid using SquareRoot()
*/   and GetWidgetLife(w) > 0.405 then
    set c = c+1
  endif

  set w = null
endfunction


private function WolfPackGroup takes nothing returns nothing
  local unit u = GetEnumUnit()

  set ux = GetUnitX(u)
  setl uy = GetUnitY(u)
  set up = GetOwningPlayer(up)
  set c = 0

  if GetWidgetLife(u) > 0.405 then
    call ForGroup(Wolves, function WolfPackSubGroup)
    call SetUnitAbilityLevel(u, AURA_SPELL, c)
    call SetUnitAbilityLevel(u, OTHER_SPELL, c)
  else
    call GroupRemoveUnit(Wolves, u)
  endif

  set u = null
endfunction

private function WolfPack takes nothing returns nothing
  call ForGroup(Wolves, function WolfPackGroup)
endfunction

private function Actions takes nothing returns nothing
  local unit u = GetSummonedUnit()

  call SetUnitAbilityLevel(u, AURA_SPELL, SPAWN_LEVEL)
  call SetUnitAbilityLevel(u, OTHER_SPELL, SPAWN_LEVEL)
  call GroupAddUnit(Wolves, u)

  set u = null
endfunction

private function Conditions takes nothing returns boolean
  return GetSpellAbilityId() == SUMMON_SPELL
endfunction

private function Init takes nothing returns nothing
  local trigger t = CreateTrigger()
  call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_SUMMON)
  call TriggerAddCondition (t,Condition(function Conditions))
  call TriggerAddAction(t, function Actions)
  set t = null
  
  set PACK_DISTANCE = PACK_DISTANCE*PACK_DISTANCE
  set Wolves = CreateGroup()
  set WolfUpdate = NewTimer()
  call TimerStart(WolfUpdate, WOLF_UPDATE_PERIOD, true, function WolfPack)
endfunction
endscope

 

[TotalClusterLuck]

Thanks you very much! I learned more from this post than the past 7 days worth of tutorials. I got a couple of questions tho

The wolfSubgroup compares the 2 positions of the wolves, right? So

local unit w = Get

GetEmunUnit ()?

Also I noticed you never Paused/removed the timer. In this case, what happens with the timer when the wolves expire with all of them being alive beforehand? Does it stop? Or does it just keep ticking with no functions to work with cuz all of the units being gone?

Thank you once again for answering, you saved me so much time and showed how versatile ForGroup can be.

 

[Pyrogasm]

Yes I meant to write Enum there, oops. I also forgot to write in the WOLF_UPDATE_PERIOD global. The timer goes indefinitely because it was started with true in the periodic argument. Every time it expires it runs the Wolves function which will do nothing if the group is empty. You could pause the timer when there are no wolves but since the period isn't particularly low there's not much to be gained by doing so.