[vJASS] Issue with Instancing?

[ArpatronEvilDrac]

[Fixed]Issue with Instancing?

Ahoy. I uh, mostly got together timer usage in JASS but am now starting to hit a bit of a pickle. When there are multiple instances of the ability (which has various text commands to check what's going up,) the current instance of the ability overrides the previous, finishing up, then the last instance finishes, etc.

I used abomination model files for the dummies just to visually check what was going on, and yeah, sometimes they spawn on dead creeps because the target died after a current usage of the ability happened.

The ability is supposed to periodically spawn dummies which mana burn the target.

Could someone point out where I went wrong in terms of keeping it multi-use?

function Trig_Zap_Test_Conditions takes nothing returns boolean//GUI setup
    if ( not ( GetSpellAbilityId() == 'Awfb' ) ) then
        return false
    endif
    return true
endfunction

globals
    timer burnTimer=CreateTimer() //Timer for our stuff
    real INTERVAL=0.50 //Half-second intervals between burns
    target array targetArray //Array of the target struct
    integer instancesOfBurn=0 //Necessary to avoid bad things, keeps track of whether stuff is happening
endglobals

struct target
    unit t
    unit caster
    integer burnsNumber
endstruct

function BurnInitiate takes nothing returns nothing //Actual part that does something

    local target targetUnit
    local integer i=0
    local integer burns
    local unit dummy
    
    call DisplayTextToForce( GetPlayersAll(),"Timer elapsed"+R2S(INTERVAL))
    
    loop
    
        exitwhen i>=instancesOfBurn
        set i=i+1
        
    endloop
    
    set targetUnit=targetArray[i]
    set burns=targetArray[i].burnsNumber
    set dummy=CreateUnitAtLoc(GetOwningPlayer(targetUnit.caster),'o000',GetUnitLoc(targetUnit.t),GetUnitFacing(targetUnit.t))
    call UnitApplyTimedLife(dummy, 'B001', 2.0)
    call IssueTargetOrder(dummy, "manaburn",targetUnit.t)
    set targetArray[i].burnsNumber=targetArray[i].burnsNumber-1
        
    call DisplayTextToForce( GetPlayersAll(),"loop, (burns left, iteration) "+I2S(burns)+" "+I2S(i))
    
    if burns<=0 then
        set instancesOfBurn=instancesOfBurn-1
        set targetArray[i]=targetArray[instancesOfBurn-1]
        call targetUnit.destroy()
    endif
    
    if instancesOfBurn==0 then
        call PauseTimer(burnTimer)
    endif
    
    call DisplayTextToForce( GetPlayersAll(),"Instances after:"+" "+I2S(instancesOfBurn))
    
endfunction

function Trig_Zap_Test_Actions takes nothing returns nothing //GUI converted

    local target targetUnit=target.create()
    
    set targetUnit.t=GetSpellTargetUnit()
    set targetUnit.caster=GetTriggerUnit()
    set targetUnit.burnsNumber=12
    
    call DisplayTextToForce( GetPlayersAll(),"Triggered")
    
    if instancesOfBurn<=0 then
        call TimerStart(burnTimer, INTERVAL, true, function BurnInitiate)
    endif
    
    set instancesOfBurn=instancesOfBurn+1
    set targetArray[instancesOfBurn]=targetUnit
    
    call DisplayTextToForce( GetPlayersAll(),"Instances, burns:"+" "+I2S(instancesOfBurn)+" "+I2S(targetUnit.burnsNumber))
    
endfunction

//===========================================================================
function InitTrig_Zap_Test takes nothing returns nothing//GUI setup
    set gg_trg_Zap_Test = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( gg_trg_Zap_Test, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddCondition( gg_trg_Zap_Test, Condition( function Trig_Zap_Test_Conditions ) )
    call TriggerAddAction( gg_trg_Zap_Test, function Trig_Zap_Test_Actions )
endfunction

 

[watermelon_1234]

I'm pretty sure this part should be in the above loop.

    set targetUnit=targetArray[i]
    set burns=targetArray[i].burnsNumber
    set dummy=CreateUnitAtLoc(GetOwningPlayer(targetUnit.caster),'o000',GetUnitLoc(targetUnit.t),GetUnitFacing(targetUnit.t))
    call UnitApplyTimedLife(dummy, 'B001', 2.0)
    call IssueTargetOrder(dummy, "manaburn",targetUnit.t)
    set targetArray[i].burnsNumber=targetArray[i].burnsNumber-1
       
    call DisplayTextToForce( GetPlayersAll(),"loop, (burns left, iteration) "+I2S(burns)+" "+I2S(i))
   
    if burns<=0 then
        set instancesOfBurn=instancesOfBurn-1
        set targetArray[i]=targetArray[instancesOfBurn-1]
        call targetUnit.destroy()
    endif

You also have to be careful with this part:

    if burns<=0 then
        set instancesOfBurn=instancesOfBurn-1
        set targetArray[i]=targetArray[instancesOfBurn-1]
        call targetUnit.destroy()
    endif

If you don't decrement i within that if, then the last instance won't get looped over.

Some other pointers:

• Stick with zero-based indexing for arrays since that's less confusing. I'm saying this since you don't actually use targetArray[0] since you increment instancesOfBurn first.
• Don't start with GUI converted Jass, it almost always ends up complicating things more than they should.
  For instance,
  

  function Trig_Zap_Test_Conditions takes nothing returns boolean//GUI setup
      if ( not ( GetSpellAbilityId() == 'Awfb' ) ) then
          return false
      endif
      return true
  endfunction

  can be simplified to
  

  function Trig_Zap_Test_Conditions takes nothing returns boolean//GUI setup
      return GetSpellAbilityId() == 'Awfb' )
  endfunction

• Don't use locations unless you're using GetLocationZ. Stick to using coordinates like CreateUnit and GetUnitX/Y.

 

[deathismyfriend]

• Don't start with GUI converted Jass, it almost always ends up complicating things more than they should.

This is ok to do as many here have done it. Even me. I have a tutorial in my sig for converting GUI to efficient jass. It will show you how to make the old inefficient GUI into efficient jass.
Then you can do the same with vJass.

 

[ArpatronEvilDrac]

Although the using the i-1 to stick at 0 first was something for the most part, there's also the fact that I set it up to find i and it takes the value of the last instance of burn.

As for moving everything to the loop, there's a problem:

I need it to no just go on forever, and not just do it once. The rest there's a loop is so that
i iterates to the last bit of saved struct in the global struct array for the info, then the rest of the code should just do it periodically with the timer's repeat expiration.

As said that makes it work well when it's only one instance ever, but multiple get screwy.

When it's inside the loop, it only happens once, or infinitely, both of which are not effects I'm going for.

Ideally, when timer expires every .5, if there's at least one instance of burn, it runs through the stuff for every single instance. I mean, I was thinking that couldn't I just set up the loop to iterate to the latest instance, then fire off a separate function with it's own locals, and that would avoid it "stacking" as it does now?

 

[deathismyfriend]

As watermelon said everything should be in the loop.
You are only doing things for the last index

 

[ArpatronEvilDrac]

I had a bit of a block and for the most part figured it out. It's functioning, except for the burn integer and it's interaction-occasionally it'll still finish off the current instance by burning the last target (and displaying burn: -1) / do one extra

function Trig_Zap_Test_Conditions takes nothing returns boolean//GUI setup
    return GetSpellAbilityId() == 'Awfb'
endfunction

globals
    timer burnTimer=CreateTimer() //Timer for our stuff
    real INTERVAL=0.50 //Half-second intervals between burns
    target array targetArray //Array of the target struct
    integer instancesOfBurn=0 //Necessary to avoid bad things, keeps track of whether stuff is happening
endglobals

struct target
    unit t
    unit caster
    integer burnsNumber
endstruct

function BurnEffects takes unit casterTarget, player casterOwner returns nothing
    local unit dummy
    set dummy=CreateUnitAtLoc(casterOwner,'o000',GetUnitLoc(casterTarget),GetUnitFacing(casterTarget))
    call UnitApplyTimedLife(dummy, 'B001', 2.0)
    call IssueTargetOrder(dummy, "manaburn",casterTarget)
endfunction

function BurnInitiate takes nothing returns nothing

    local target targetUnit
    local integer i=0
    local integer burns
    
    call DisplayTextToForce( GetPlayersAll(),"Timer elapsed"+R2S(INTERVAL))
    
    loop
        exitwhen i>=instancesOfBurn
        set targetUnit=targetArray[i]
        set burns=targetArray[i].burnsNumber
    call DisplayTextToForce( GetPlayersAll(),"Burns:"+I2S(burns))
        call BurnEffects(targetUnit.t, GetOwningPlayer(targetUnit.caster))
        set targetArray[i].burnsNumber=targetArray[i].burnsNumber-1
            if burns<=0 then
                set instancesOfBurn=instancesOfBurn-1
                set targetArray[i]=targetArray[instancesOfBurn-1]
                call targetUnit.destroy()
            endif
        set i=i+1
        
    endloop
    
    call DisplayTextToForce( GetPlayersAll(),"loop, (burns left, iteration) "+I2S(burns)+" "+I2S(i))
    
    if instancesOfBurn==0 then
        call PauseTimer(burnTimer)
    endif
    
    call DisplayTextToForce( GetPlayersAll(),"Instances after:"+" "+I2S(instancesOfBurn))
    
endfunction

function Trig_Zap_Test_Actions takes nothing returns nothing

    local target targetUnit=target.create()
    
    set targetUnit.t=GetSpellTargetUnit()
    set targetUnit.caster=GetTriggerUnit()
    set targetUnit.burnsNumber=12
    
    call DisplayTextToForce( GetPlayersAll(),"Triggered")
    set targetArray[instancesOfBurn]=targetUnit
    
    if instancesOfBurn<=0 then
        call TimerStart(burnTimer, INTERVAL, true, function BurnInitiate)
    endif
    
    set instancesOfBurn=instancesOfBurn+1
    call DisplayTextToForce( GetPlayersAll(),"Instances, burns:"+" "+I2S(instancesOfBurn)+" "+I2S(targetUnit.burnsNumber))
    
endfunction

//===========================================================================
function InitTrig_Zap_Test takes nothing returns nothing
    set gg_trg_Zap_Test = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( gg_trg_Zap_Test, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddCondition( gg_trg_Zap_Test, Condition( function Trig_Zap_Test_Conditions ) )
    call TriggerAddAction( gg_trg_Zap_Test, function Trig_Zap_Test_Actions )
endfunction

EDIT: Think I've figured it out. If there's some catastrophic issue with this, lemme know:

function Trig_Zap_Test_Conditions takes nothing returns boolean//GUI setup
    return GetSpellAbilityId() == 'Awfb'
endfunction

globals
    timer burnTimer=CreateTimer() //Timer for our stuff
    real INTERVAL=0.50 //Half-second intervals between burns
    target array targetArray //Array of the target struct
    integer instancesOfBurn=0 //Necessary to avoid bad things, keeps track of whether stuff is happening
endglobals

struct target
    unit t
    unit caster
    integer burnsNumber
endstruct

function BurnEffects takes unit casterTarget, player casterOwner returns nothing
    local unit dummy
    set dummy=CreateUnitAtLoc(casterOwner,'o000',GetUnitLoc(casterTarget),GetUnitFacing(casterTarget))
    call UnitApplyTimedLife(dummy, 'B001', 2.0)
    call IssueTargetOrder(dummy, "manaburn",casterTarget)
endfunction

function BurnInitiate takes nothing returns nothing

    local target targetUnit
    local integer i=0
    local integer burns
   
    call DisplayTextToForce( GetPlayersAll(),"Timer elapsed"+R2S(INTERVAL))
   
    loop
        exitwhen i>=instancesOfBurn
        set targetUnit=targetArray[i]
        set burns=targetArray[i].burnsNumber
    call DisplayTextToForce( GetPlayersAll(),"Burns:"+I2S(burns))
        if burns!=0 then
            call BurnEffects(targetUnit.t, GetOwningPlayer(targetUnit.caster))
            set targetArray[i].burnsNumber=targetArray[i].burnsNumber-1
        endif
        if burns<=0 then
            set instancesOfBurn=instancesOfBurn-1
            set targetArray[i]=targetArray[instancesOfBurn]
            call targetUnit.destroy()
        endif
        set i=i+1
       
    endloop
   
    call DisplayTextToForce( GetPlayersAll(),"loop, (burns left, iteration) "+I2S(burns)+" "+I2S(i))
   
    if instancesOfBurn==0 then
        call PauseTimer(burnTimer)
    endif
   
    call DisplayTextToForce( GetPlayersAll(),"Instances after:"+" "+I2S(instancesOfBurn))
   
endfunction

function Trig_Zap_Test_Actions takes nothing returns nothing

    local target targetUnit=target.create()
   
    set targetUnit.t=GetSpellTargetUnit()
    set targetUnit.caster=GetTriggerUnit()
    set targetUnit.burnsNumber=12
   
    call DisplayTextToForce( GetPlayersAll(),"Triggered")
    set targetArray[instancesOfBurn]=targetUnit
   
    if instancesOfBurn<=0 then
        call TimerStart(burnTimer, INTERVAL, true, function BurnInitiate)
    endif
   
    set instancesOfBurn=instancesOfBurn+1
    call DisplayTextToForce( GetPlayersAll(),"Instances, burns:"+" "+I2S(instancesOfBurn)+" "+I2S(targetUnit.burnsNumber))
   
endfunction

//===========================================================================
function InitTrig_Zap_Test takes nothing returns nothing
    set gg_trg_Zap_Test = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( gg_trg_Zap_Test, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddCondition( gg_trg_Zap_Test, Condition( function Trig_Zap_Test_Conditions ) )
    call TriggerAddAction( gg_trg_Zap_Test, function Trig_Zap_Test_Actions )
endfunction

At least almost. It seems to be going through an extra loop when I change the parameters a bit though in another trigger. The ability seems to work fine as is... Hmm. Nevermind, figured it out!

 

[deathismyfriend]

don't use this.

CreateUnitAtLoc(casterOwner,'o000',GetUnitLoc(casterTarget)

do call CreateUnit()

You should make this whole spell using structs if you're going to use them.
I think the way you have it that the variables in the struct will get overwritten like a global variable.
You need to allocate and deallocate struct instances to keep the spell running.

Don't use TriggerAddAction only use TriggerAddCondition.

Check out some of my spells in vJass.

 

[ArpatronEvilDrac]

don't use this.

CreateUnitAtLoc(casterOwner,'o000',GetUnitLoc(casterTarget)

do call CreateUnit()

You should make this whole spell using structs if you're going to use them.
I think the way you have it that the variables in the struct will get overwritten like a global variable.
You need to allocate and deallocate struct instances to keep the spell running.

Don't use TriggerAddAction only use TriggerAddCondition.

Check out some of my spells in vJass.

The createUnit func versus the createUnitAtLoc func... The createUnitAtLoc creates a leak because it creates a loc? What's up?

It works fine for simultaneous instances now. There's a part the clears the current instance already.

I have a remedial understanding of the "TriggerAdd" etc. stuff. For the most part, why are you suggesting I do this besides cleaning up code? Can I have the condition if true run the function that does what I need it to do?

"Check out some of my spells" doesn't really do anything besides promote yourself. I'm not sure what or why you want me to do so.

 

[PurgeandFire]

He is just mentioning some improvements for efficiency purposes.

As far as functionality goes, you've got the indexing right. There is nothing catastrophic in the code. Nice job!

 

[FUJI]

This is the efficiency deathismyfriend said.

local real x = GetUnitX(casterTarget)
local real y = GetUnitY(casterTarget)
local unit dummy
set dummy=CreateUnit(casterOwner,'o000',x,y,GetUnitFacing(casterTarget))

 

[Malhorne]

Don't use GetPlayersAll() but bj_FORCE_ALL_PLAYERS
Then

function InitTrig_Zap_Test takes nothing returns nothing
    local trigger t = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( t, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddCondition( t, Condition( function Trig_Zap_Test_Conditions ) )
    call TriggerAddAction( t, function Trig_Zap_Test_Actions )
    set t = null
endfunction

Then don't forget to nullify every handle locals :

local unit u
//Do some stuff...
set u = null

And finally since you're in vJASS why don't you use the keyword private instead of those Trig_Zap_Test_Actions