[Solved] Help with fire spread trigger

[wc3neverdies]

Hello as title says i want to make fire spread on my map. The fire effect is a unit with immolation so it burns nearby enemies and negative HP regen so it destroy itslef.
I used OE filed to achieve this becasue i think it's better than trigger these effects.

The fire spread is made by creating a fire unit next to another if the terrain type is grass.

The first tirgger is the one that start the effect, and is activated whenever a fire unit enters the map.

globals

   group array FireUnitGroup
   effect treeburnfx
   integer spreadint
endglobals

function DestAoeDmgFireDOT takes nothing returns nothing
  
  

   if IsDestructableAlive(GetEnumDestructable()) and IsDestructableTree(GetEnumDestructable()) then
     //set treeburnfx = AddSpecialEffectLocBJ( GetDestructableLoc(GetEnumDestructable()), "Environment\\LargeBuildingFire\\LargeBuildingFire2.mdl" )
     //call BlzSetSpecialEffectZ( treeburnfx, ( BlzGetLocalSpecialEffectZ(treeburnfx) + 200.00 ) )  

    call SetDestructableAnimation( GetEnumDestructable(), " stand hit")  
  
   call SetDestructableLife( GetEnumDestructable(), ( GetDestructableLife(GetEnumDestructable()) - 10) )

   endif

endfunction


function fireunitactions takes nothing returns nothing

    local unit pfu = GetEnumUnit()
    local real frx = GetUnitX(GetEnumUnit())
    local real fry = GetUnitY(GetEnumUnit())
    local real frz = BlzGetUnitZ(GetEnumUnit())
 local integer i = spreadint //GetPlayerId(GetOwningPlayer(GetEnumUnit()))

  local real angle = 0
  local real nfrx
  local real nfry
  local group tfg

      

    loop
    exitwhen(angle > 360)



        if angle == 0 or angle == 90 or angle == 180 or angle == 270 then
          set nfrx= frx + 129*Cos(angle)
          set nfry= fry + 129*Sin(angle)
        endif
        if angle == 45 or angle == 135 or angle == 225 or angle == 315 then
          set nfrx= frx + 181*Cos(angle*bj_DEGTORAD)
          set nfry= fry + 181*Sin(angle*bj_DEGTORAD)
        endif

      
  
        set tfg = GetUnitsInRangeOfLocMatching(100, Location(nfrx,nfry), Condition( function fireunitbool)  )
          

           if CountUnitsInGroup(tfg) == 0 and GetTerrainType(nfrx, nfry) == 'Fgrd' then
              call CreateUnitAtLoc( Player(i), 'n005', Location(nfrx, nfry), 0. )
           endif

           call SetUnitLifeBJ(pfu, ( GetUnitStateSwap(UNIT_STATE_LIFE, pfu) - 2 ) )
          
           call DestroyGroup(tfg)

    set angle = angle + 45
    endloop
    
       // D E S T R   F I R E   DOT   D A M A G E
    call EnumDestructablesInCircleBJ( 100.00, Location(frx,fry), function DestAoeDmgFireDOT  )



endfunction

function fireunitpick1 takes nothing returns boolean
    
    local integer i = spreadint

    return ( GetOwningPlayer(GetFilterUnit()) == Player(i) )
endfunction


function FireSpreadLoop takes nothing returns nothing

    local integer i = spreadint
  
    set FireUnitGroup[i] = GetUnitsInRectMatching(GetPlayableMapRect(), And(Condition(function fireunitpick1), Condition(function fireunitbool)) )  
  
      
     if CountUnitsInGroup(FireUnitGroup[i]) > 0 then  
      
       call ForGroupBJ(FireUnitGroup[i],function fireunitactions)

     endif


     if CountUnitsInGroup(FireUnitGroup[i]) == 0 then
      call PauseTimer(udg_FireSpreadTimer[i])
      call DisplayTextToForce( GetPlayersAll(), "SpreadFireOFF  " + I2S(i) )
      //call DestroyEffect(treeburnfx)
     endif

   call DestroyGroup(FireUnitGroup[i])

endfunction



function Trig_FirespreadJASS_Actions takes nothing returns nothing


    local integer i = GetPlayerId(GetOwningPlayer(GetEnteringUnit()))
    local unit eu = GetEnteringUnit()
 
    set spreadint = i

if GetUnitTypeId(eu) == 'n005' and GetOwningPlayer(eu) == Player(i)  then
    call DisplayTextToForce( GetPlayersAll(), "SpreadFireON  " + I2S(i) )

    call GroupAddUnit(FireUnitGroup[i], eu)
  
    //call EnableTrigger( gg_trg_FireSpreadonYellowGrass )
    //call DisableTrigger( GetTriggeringTrigger() )
    call TimerStart( udg_FireSpreadTimer[i], 0.60, true,function FireSpreadLoop)
  
endif

endfunction

//===========================================================================
function InitTrig_FirespreadJASS takes nothing returns nothing
    set gg_trg_FirespreadJASS = CreateTrigger(  )
      
    //call DisableTrigger( gg_trg_FirespreadJASS )
    call TriggerRegisterEnterRectSimple( gg_trg_FirespreadJASS, GetPlayableMapRect() )
  
  
    call TriggerAddAction( gg_trg_FirespreadJASS,function Trig_FirespreadJASS_Actions )
endfunction

When a fire unit dies it changes the terrain to another type of grass(burned grass).

function Trig_FireUnitDie_Actions takes nothing returns nothing

   local real dfux = GetUnitX(GetDyingUnit())
   local real dfuy = GetUnitY(GetDyingUnit())

    if GetUnitTypeId(GetDyingUnit()) == 'n005' then
  
        if GetTerrainType(dfux, dfuy) == 'Fgrd' or GetTerrainType(dfux, dfuy) == 'Lgrd' then
              
                call SetTerrainType(dfux, dfuy, 'Jgsb', -1, 1, 0)
         endif
    endif

endfunction

//===========================================================================
function InitTrig_FireUnitDie takes nothing returns nothing
    set gg_trg_FireUnitDie = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( gg_trg_FireUnitDie, EVENT_PLAYER_UNIT_DEATH )
  
    call TriggerAddAction( gg_trg_FireUnitDie, function Trig_FireUnitDie_Actions )
endfunction

The problem is that the timer does not paused when there are no fire units in the map.

Also i don't know why only 7 fire units are created instead of 8.


Thanks in advance fo the help.

EDIT: I fixed the deg to rad thing. Now my concern is that i keep getting the fire spread off message if i test it with player 2,3,4....

 

[Pyrogasm]

• You're never initializing the unit groups. They need to be set = CreateGroup() on map init. You're also destroying them the first time the periodic function runs, not sure why you thought you needed to do that.
  
• Where is function fireunitbool? I don't see it in your code.
• The last time you posted about this fire spreading you had given these fire units locust and (presumably) learned that 'units in region' will not detect locusted units. Here you're using GetUnitsInRectMatching so it won't find them if they have locust....
• There's no reason to do things like using local integer i = spreadint in FireSpreadLoop. Just use spreadint directly.
• The way you are using spreadint, the spreading fire can only be owned by one player at a time. If a new player spawns fire while the first player's fire still exists, all newly created fire (around both fires) will belong to the new player.
• You're still using a few BJ functions. Pretty much don't use them except things like TriggerRegisterAnyUnitEventBJ().
  
• You can use BJDebugMsg("text here") to display messages for debugging instead of DisplayTextToForce(...)
  
• There's no reason to do something like this:
  

       if CountUnitsInGroup(FireUnitGroup[i]) > 0 then  
        
         call ForGroupBJ(FireUnitGroup[i],function fireunitactions)
  
       endif

  If the group is empty nothing will be run in the ForGroup anyway. Additionally it may be worth it to do a FirstOfGroup loop instead of a ForGroup callback for some groups.
• Your formatting is still a little weird but that's just my opinion.
• You should use encapsulation (libraries and scopes with initializers) and global variables liberally instead of hardcoding things like 'Fgrd', 'n005', and 'Jgsb':
  

  library firespread initializer init
  globals
    private constant integer FIRE_ID = 'n005'
    private constant integer GRASS_1 = 'Fgrd'
    private constant integer GRASS_2=  'Lgrd'
    private constant real SPREAD_INTERVAL = 0.60
    //etc.
  endglobals
  //...
  private function init takes nothing returns nothing
    //sets up the global group array
  endfunction
  endlibrary

• This line leaks a location: set tfg = GetUnitsInRangeOfLocMatching(100, Location(nfrx,nfry), Condition( function fireunitbool) ). You are leaking other locations in similar ways using Location(...).
  
• You can and should use GetTriggerUnit() wherever you can instead of things like GetDyingUnit(), GetEnteringUnit(). Most of the time the only other unit event response I ever need is GetSpellTargetUnit().
• This condition: CountUnitsInGroup(FireUnitGroup[i]) > 0 and this condition: CountUnitsInGroup(FireUnitGroup[i]) == 0 are opposites so you can just put the second one inside an else instead of checking the condition again for its inverse.
• Stop using locations. Use XY coordinates directly. This removes possible leaks, are generally faster, and don't require the game engine to allocate resources for a handle.

•         if angle == 0 or angle == 90 or angle == 180 or angle == 270 then
            set nfrx= frx + 129*Cos(angle)
            set nfry= fry + 129*Sin(angle)
          endif
          if angle == 45 or angle == 135 or angle == 225 or angle == 315 then
            set nfrx= frx + 181*Cos(angle*bj_DEGTORAD)
            set nfry= fry + 181*Sin(angle*bj_DEGTORAD)
          endif

  You forgot a DEGTORAD but I think that's what your edit's about. This can be simplified using a trick with integer division since it truncates. If angle/90 == (angle + 45)/90 then it's a cardinal direction; if they're not equal it's a diagonal (think about it or do the math yourself).
  

  if angle/90 == (angle+45)/90 then
    set d = 128. //not 129.
  else
    set d = 181 //128*sqrt(2)
  endif
  set nfrx = frx + d*Cos(angle*bj_DEGTORAD)
  set nfry = fry + d*Sin(angle*bj_DEGTORAD)

There's probably more but that's it for a first pass from me. I think you misusing the unit groups is the issue, so try fixing that first then tackle the other things I listed.

 

[wc3neverdies]

Hello, first of all, thank you for the answer.

• You're never initializing the unit groups. They need to be set = CreateGroup() on map init. You're also destroying them the first time the periodic function runs, not sure why you thought you needed to do that.

• Where is function fireunitbool? I don't see it in your code.

• The last time you posted about this fire spreading you had given these fire units locust and (presumably) learned that 'units in region' will not detect locusted units. Here you're using GetUnitsInRectMatching so it won't find them if they have locust....

• There's no reason to do things like using local integer i = spreadint in FireSpreadLoop. Just use spreadint directly.

• The way you are using spreadint, the spreading fire can only be owned by one player at a time. If a new player spawns fire while the first player's fire still exists, all newly created fire (around both fires) will belong to the new player.

• You're still using a few BJ functions. Pretty much don't use them except things like TriggerRegisterAnyUnitEventBJ().

• You can use BJDebugMsg("text here") to display messages for debugging instead of DisplayTextToForce(...)

1. Ahh, I see. Since this is a udg variable i thought it wasnt necessary. Will do.

2. It's in another trigger that i made it before this one. I converted some GUI actions too see how i can add functions with argument to group function.

function fireunitbool takes nothing returns boolean
    return ( GetUnitTypeId(GetFilterUnit()) == 'n005' )
endfunction

3.Yeah i removed the locust ability from the units.

4. I know. I recently started to use jass, so instead of deleting a line that i might need to write back i just changed the value there.

5. Yeah, that's my main problem, 'spreadint' variable reaches 27, i don't know how since the loop is supposed to stop at 23.

6. I know but i find them useful in term of less copy paste. I use BJ's that are more lines of normal code that i would need to copy or write myself. How can i avoid using these? Like: PolledWait, two point distance or some boolean.

7. I do when i can, the only problem with that is the duration of the text. It's set to 60, too long for me, since i have miltiple triggers debug messages.

• If the group is empty nothing will be run in the ForGroup anyway. Additionally it may be worth it to do a FirstOfGroup loop instead of a ForGroup callback for some groups.

• Your formatting is still a little weird but that's just my opinion.

• You should use encapsulation (libraries and scopes with initializers) and global variables liberally instead of hardcoding things like 'Fgrd', 'n005', and 'Jgsb':

1. Ahh thank you. I am a JASS noob so i will check others toutorials about groups in JASS.

2. I know, i program in the WE since i can run syntax checker and jass helper every time i finish to write a line . And | write a line - copy paste - run check | takes more time that write and check directly( also the checker speed slows down after every line written.) I make a lot of spaces and empty lines between blocks of codes so it's easier for me to navigate through. Also at teh beginning of a new line i use TAB key instead of space because it's difficult to select the very first character in the editor, it sometimes just scrolls down and make me go insane.

3. I am not familiar with this. I tried to create a library once ( just after i downloaded the mouse snippet you suggested me) but i got lots of errors and problems and got scared. Also the mouse snippet is disabled because in one of my tests my Map Initialization trigger (i still have it in GUI, for now) didnt work, it was like it was disabled but it wasn't. Disabling the mouse snippet solved the issue.

What if instead of create a library and globals i create udg variables? I already use plenty of those since some part fo the map script are still in GUI. Also for fast debugging i always use GUI and i can't have access to global variables with GUI.

• This line leaks a location: set tfg = GetUnitsInRangeOfLocMatching(100, Location(nfrx,nfry), Condition( function fireunitbool) ). You are leaking other locations in similar ways using Location(...).

• You can and should use GetTriggerUnit() wherever you can instead of things like GetDyingUnit(), GetEnteringUnit(). Most of the time the only other unit event response I ever need is GetSpellTargetUnit().

• This condition: CountUnitsInGroup(FireUnitGroup) > 0 and this condition: CountUnitsInGroup(FireUnitGroup) == 0 are opposites so you can just put the second one inside an else instead of checking the condition again for its inverse.
  

1. How can i destroy this? I was told the location was leaking but then when i tried to figure how to do so, the only way i found is to create a local location varialbe and destroy it.

• Stop using locations. Use XY coordinates directly. This removes possible leaks, are generally faster, and don't require the game engine to allocate resources for a handle.

• 
  

I hope you mean "try to avoid using location where you don't have to" because i can't imagine to get rid of locations completely. Also using X and Y feels more tricky than a location for me but I will try.

2. I will. Even thouh i developed the habit of using those in GUI, it may takes a while.

3. That's right.

You forgot a DEGTORAD but I think that's what your edit's about. This can be simplified using a trick with integer division since it truncates. If angle/90 == (angle + 45)/90 then it's a cardinal direction; if they're not equal it's a diagonal (think about it or do the math yourself).

I am terrible at math and i tried to understand this but assuming angle is in degrees :


angle = 0

0/90 = (0+45)/90
0 = 0.5

angle = 45

45/90 = (45+45)/90
0.5 = 1

angle = 90

90/90 = (90+45)/90
1 = 0.7


For me this "angle/90 == (angle + 45)/90" it's the same of this "X == X +1" and it will be always false.
So i think im too dumb to get it, would you like to explain more precisely?

 

[Pyrogasm]

Will give a detailed response later but as for the arithmetic the reason it works is that integer division truncates and never rounds. As long as all numbers/variables involved in a computation are integers then it will follow those rules. In int-land:

1/2 = 1/10 = 7/8 = 17472/222043 = 0

2/2 = 3/2 = 11/10 = 14/8 = 1

So you have:
(45+45)/90 = 90/90 = 1
45/90 = 0
1 != 0 so it’s a diagonal
——
(90+45)/90 = 135/90 = 1
90/90 = 1
1 = 1 so it’s a cardinal

This only works if angle is an integer variable, not a real. If angle is a real then the check will fail because as you said x+n != x for any value of x or n that isn't 0.

 

[Pyrogasm]

To expand on my comment about the CountUnitsInGroup check: if you don't actually need to know the number of units in the group but just want to see if it has any units, you can check FirstOfGroup(g) != null.

I know, i program in the WE since i can run syntax checker and jass helper every time i finish to write a line .

Fair enough, I won't comment on it again. Everyone has their own style.

it's difficult to select the very first character in the editor, it sometimes just scrolls down and make me go insane

Yeah that shit drives me insane too. Here is the JASSHelper manual, which explains how to use pretty much everything it can do: JassHelper 0.A.0.0 Check out the libraries and global declaration freedom sections.

How can i destroy this? I was told the location was leaking but then when i tried to figure how to do so, the only way i found is to create a local location varialbe and destroy it.

That's the correct way to do it if you must use a location:

local location l = Location(x,y)
call DoSomethingWithLoc(l)
call RemoveLocation(l)
set l = null

I do, however, mean to stop using locations entirely. If you are using GUI often then yeah it's pretty much impossible to eliminate locations, but in JASS nearly everything takes XY coordinates directly. The only function I am aware of that requires you to use a location is GetLocationZ(); everything else has both a location and xy variant of the native. It may be slightly more complex initially but the freedom it affords you is significant, and since coordinates are reals you don't ever have to clean them up! If you want to see how the coordinates work for polar projection (for example), you can always look at the function in Blizzard.j.

Since this is a udg variable i thought it wasnt necessary. ... What if instead of create a library and globals i create udg variables? I already use plenty of those since some part fo the map script are still in GUI. Also for fast debugging i always use GUI and i can't have access to global variables with GUI.

I think you understand this but the globals created in GUI just have a udg_ text prefix; they aren't different in any way. The functional 'difference' is just that GUI can't see/interact with freely declared globals in your JASS code, so yes if you want a GUI trigger to be able to use it you must create it in the variable editor. The easiest way around this is to use custom script lines in your GUI triggers which should be able to see your declared globals, and you can do something like this to access their values by assigning them to other GUI-accessible variables.

• Custom script: set udg_GUI_Int = SomeGlobalIntVariableInJASS
• Game - Display to (All players) the text String(GUI_Int)
• -------- --------
• Custom script: set udg_GUI_Unit = SomeGlobalUnit
• Unit - Kill GUI_Unit

Now, as for how the 'size' value of GUI arrays works: say you give an int array called Test a size N and default value of V. This means that the first N indicies of Test will be assigned the value V upon map startup. Say N = 5 and V = -1, you will have this:
Test[0] = 0 //GUI starts arrays at index 1 and ignores the 0th index
Test[1] = -13
Test[2] = -13
Test[3] = -13
Test[4] = -13
Test[5] = -13
Test[6] = 0
Test[7] = 0
...
Test[32767] = 0 //max array index currently

For most array variables you will be setting the value of the array before you attempt to use it, or you are okay with using the 'base' value of 0, 0.0, false, "", null (depending on variable type). In this instance you can leave N = 1 and it doesn't matter what V is. For handle-type variables, the value of N sometimes matters (and V is usually set based on the variable type and can't be changed). It doesn't matter for units. It does matter for groups and timers. If you attempt to add units to an uninitialized group nothing will happen. If you attempt to start an uninitialized timer nothing will run. In these cases you either need to set the size variable appropriately so that the first N groups are allocated as 'Empty Group', or set them yourself before attempting to add units to them (if they have not yet been initialized).

Yeah, that's my main problem, 'spreadint' variable reaches 27, i don't know how since the loop is supposed to stop at 23.

I don't see where spreadint is ever incremented. It's only assigned once in Trig_FirespreadJASS_Actions as GetPlayerId(GetOwningPlayer(GetEnteringUnit())). How is it getting up to 27? Where is the loop that's supposed to stop at 23?

 

[wc3neverdies]

I don't see where spreadint is ever incremented. It's only assigned once in Trig_FirespreadJASS_Actions as
GetPlayerId(GetOwningPlayer(GetEnteringUnit()))
. How is it getting up to 27? Where is the loop that's supposed to stop at 23?

Yeah, i am confused too.

In this video i recorded you can see the string TimeSpreadOff 27 as soon as i used the fire element. I sincerely don't know how this can be possible

 

[Pyrogasm]

There has to be another trigger that is setting the value of spreadint that you haven't posted. The reason I know this is the case is that "SpreadFireON 0" is visible at 0:14 and then the next time a fire message appears a second later it shows "SpreadFire OFF 27". Something changed the value from 0 (player # of player red, so that was correct) to 27. Can you post the map if you can't find the trigger that is changing it?

 

[wc3neverdies]

Sure, but my code and triggers are quite messy. I think the issue is in the triggers that spawn a fire unit in the map, PointAttackMissileJASS and TargetAttackMissileJASS

 

[wc3neverdies]

Okay i solved it, i remove the spreadint variable and managed to solve the player index issue.

Thanks for the help, patience and time