[Solved] RegisterUnitInRangeEvent fails after some time

[l46kok]

Demo Map reproducing the bug: https://www.dropbox.com/s/tlgw0rrnctrjowy/BugDemo.w3x?dl=0

Ok, here's the situation:

I'm trying to dynamically attach RegisterUnitInRangeEvent to created units. These dynamically created units disappear after 10 seconds.

Problem is, when this code has been run for multiple times, event fails to register. Printing out the contents of linked list in UnitInRangeEvent source code seems to show that the links between nodes starts to get messed up.

So my suspicion about this problem is,

1. There's a problem with my code (Likely, this is the case)
2. There's a bug with RegisterUnitInRangeEvent / UnitIndexer / Missile Library

You can check out the bug in the demo map linked above. Moving your footman nearby the generated runes is supposed to destroy them, but it stops working after a while.

Relevant Libraries:
https://github.com/nestharus/JASS/blob/master/jass/Systems/UnitInRangeEvent/script.j
https://github.com/nestharus/JASS/blob/5e0b5672a8a3a68df1829902e6334525af768b06/jass/Systems/Unit%20Indexer/script.j
http://www.hiveworkshop.com/forums/jass-resources-412/missile-265370/

library Runes initializer onInit requires TimerUtils, UnitInRangeEvent, Missile
    globals    
        private constant real SPELL_DURATION = 1.0
        private constant real RUNE_DURATION = 10.0
        private constant real RUNE_STEP_AOE = 100.0
        
        private constant integer RUNE_COUNT = 9 //Preferably a square number. n each for stun/healing.
        private constant integer STUN_RUNE_UNIT_ID = 'hspt'
        private constant integer HEALING_RUNE_UNIT_ID = 'hsor'
        
        private constant real MAP_BOUND_MIN_X = -1000
        private constant real MAP_BOUND_MIN_Y = -1000
        private constant real MAP_BOUND_MAX_X = 1000
        private constant real MAP_BOUND_MAX_Y = 1000
    endglobals
    
    private function Runes_OnRuneStepped takes nothing returns nothing
        local unit enteredUnit = GetTriggerUnit()
        local unit sourceUnit = GetEventSourceUnit()
        
        if GetUnitTypeId(enteredUnit) == 'hfoo' then
            call RemoveUnit(sourceUnit)
        endif
        
        set enteredUnit = null
        set sourceUnit = null
    endfunction
    
    private struct Runes_HealingRuneMissile extends array        
        private static method onRemove takes Missile this returns boolean
            call DestroyEffect(AddSpecialEffect(HOLY_LIGHT_FX,0,0))
            return true
        endmethod
        
        implement MissileStruct
    endstruct
    
    private function Runes_OnHealingRuneDeath takes nothing returns nothing
        local unit rune = GetTriggerUnit()
        local Missile m
        if GetUnitTypeId(rune) == STUN_RUNE_UNIT_ID then
            call RemoveUnit(rune)
            set rune = null
            return
        elseif GetUnitTypeId(rune) != HEALING_RUNE_UNIT_ID then
            set rune = null
            return
        endif
        set m           = Missile.createXYZ(GetUnitX(rune), GetUnitY(rune), 0, 0, 0, 66.)
        set m.speed     = 40.
        set m.model     = LICH_MISSILE_FX
        set m.collision = 32.
        set m.arc       = 1
        set m.turn      = 3.5*Missile_TIMER_TIMEOUT// turn rate per tick.
        //set m.target    = System.currentBoss
        call Runes_HealingRuneMissile.launch(m)
        set rune = null
    endfunction

    private struct Runes_Missile extends array        
        private static method onRemove takes Missile this returns boolean
            local unit u = CreateUnit(Player(PLAYER_NEUTRAL_AGGRESSIVE),this.data,this.x,this.y,0.00)
            local integer id = -99
            call UnitApplyTimedLife(u,'BTLF',RUNE_DURATION)
            set id = RegisterUnitInRangeEvent(function Runes_OnRuneStepped, u, RUNE_STEP_AOE)
            set u = null
            return true
        endmethod
        
        implement MissileStruct
    endstruct
    
    private function Runes_LaunchRune takes integer unitId, real sectMinX, real sectMinY, real sectMaxX, real sectMaxY returns nothing
        local real randomRuneLocX = GetRandomReal(sectMinX, sectMaxX)
        local real randomRuneLocY = GetRandomReal(sectMinY, sectMaxY)
        
        local Missile m 
        set m           = Missile.createXYZ(0, 0, 50, randomRuneLocX, randomRuneLocY, 0)
        set m.speed     = 30.
        set m.model     = ANNIHILATION_MISSILE_FX
        set m.arc       = 1
        set m.collision = 32
        set m.data = unitId
        call Runes_Missile.launch(m)
    endfunction
    
    private function Runes_CreateRunes takes nothing returns nothing
        //Divide up the map in 9 sections (3x3 matrix)
        //Pseudo-guarantees a randomized uniform distribution of runes in field
        local integer i = 0
        local integer j = 0
        local integer squareCount = R2I(SquareRoot(RUNE_COUNT))
        local real currSectMinX = MAP_BOUND_MIN_X
        local real currSectMinY = MAP_BOUND_MIN_Y
        local real sectOffsetX = (MAP_BOUND_MAX_X - MAP_BOUND_MIN_X) / squareCount
        local real sectOffsetY = (MAP_BOUND_MAX_Y - MAP_BOUND_MIN_Y) / squareCount
        local real currSectMaxX = currSectMinX + sectOffsetX
        local real currSectMaxY = currSectMinY + sectOffsetY

        for i = 0 to squareCount -  1
            set currSectMinX = MAP_BOUND_MIN_X
            set currSectMaxX = currSectMinX + sectOffsetX
            for j = 0 to squareCount - 1
                call Runes_LaunchRune(STUN_RUNE_UNIT_ID,currSectMinX,currSectMinY,currSectMaxX,currSectMaxY)
                call Runes_LaunchRune(HEALING_RUNE_UNIT_ID,currSectMinX,currSectMinY,currSectMaxX,currSectMaxY)
                set currSectMinX = currSectMinX + sectOffsetX
                set currSectMaxX = currSectMaxX + sectOffsetX
            endfor
            set currSectMinY = currSectMinY + sectOffsetY
            set currSectMaxY = currSectMaxY + sectOffsetY
        endfor
    endfunction
    
    private function Runes_CastFinished takes nothing returns nothing
        call Runes_CreateRunes()
    endfunction
    
    function Runes_Cast takes nothing returns nothing
        call TimerStart(NewTimer(), SPELL_DURATION,true, function Runes_CastFinished)
    endfunction
    
    private function onInit takes nothing returns nothing
        call RegisterPlayerUnitEvent(EVENT_PLAYER_UNIT_DEATH, function Runes_OnHealingRuneDeath)
        call Runes_Cast()
    endfunction
endlibrary

 

[Anitarf]

Can you post links to the libraries you're using?

 

[BPower]

I would answer offhand that the problem is in library UnitInRangeEvent.

I just checked it quickly, so someone could confirm my guess.

first[unitIndex] is not set to 0 on deindex event.
If the index is re-used by the unit indexer the following fails.

            set head = first[sourceUnit]
            if (0 == head) then
                set first[sourceUnit] = this
                set next[this] = this
                set prev[this] = this
            else
                set prev[this] = prev[head]
                set next[this] = head
                set next[prev[head]] = this
                set prev[head] = this
            endif

 

[l46kok]

Can you post links to the libraries you're using?

Posted. See edit of OP

 

[Anitarf]

I think BPower already found the culprit. The system wouldn't break immediately since the trigger would still get created but it would get inserted into an incorrect list (or even the recycle queue), which could lead to it getting prematurely removed.

Edit: In case it was not clear, here's the fix:

        private static method onDeindex takes nothing returns boolean
            local integer sourceUnit = GetIndexedUnitId()
            local integer node = first[sourceUnit]
            
            //if there are any events on the unit
            if (0 != node) then
                //clean all events
                set next[prev[node]] = 0
                loop
                    call DestroyTrigger(trig[node])
                    set trig[node] = null
                    
                    call rangeEvent.remove(handleId[node])
                    
                    debug set Debug.allocated[node] = false
                    
                    set node = next[node]
                    exitwhen 0 == node
                endloop
                
                //deallocate list
                set node = first[sourceUnit]
                set next[prev[node]] = next[0]
                set next[0] = node
                set first[sourceUnit] = 0
            endif
            
            return false
        endmethod

 

[l46kok]

I think BPower already found the culprit. The system wouldn't break immediately since the trigger would still get created but it would get inserted into an incorrect list (or even the recycle queue), which could lead to it getting prematurely removed.

Edit: In case it was not clear, here's the fix:

        private static method onDeindex takes nothing returns boolean
            local integer sourceUnit = GetIndexedUnitId()
            local integer node = first[sourceUnit]
            
            //if there are any events on the unit
            if (0 != node) then
                //clean all events
                set next[prev[node]] = 0
                loop
                    call DestroyTrigger(trig[node])
                    set trig[node] = null
                    
                    call rangeEvent.remove(handleId[node])
                    
                    debug set Debug.allocated[node] = false
                    
                    set node = next[node]
                    exitwhen 0 == node
                endloop
                
                //deallocate list
                set node = first[sourceUnit]
                set next[prev[node]] = next[0]
                set next[0] = node
                set first[sourceUnit] = 0
            endif
            
            return false
        endmethod

Thanks, I had a suspicion that it was a problem related to the list implementation in the library but wasn't sure. Your fix worked excellently!