vJass unit recycling

08-20-2014, 11:00 PM

i'm making a first attempt at unit recycling for my map. it's be pretty standard, probably the only non-vanilla bit is the use of a hashtable to store the various recyclers

JASS:

library Recycle
    globals
        private constant integer MAX_SINGLE_INSTANCE_COUNT = 100
        public constant real SAFE_X = -15555
        public constant real SAFE_Y = 15555
    endglobals
    
    public struct UnitRecycler
        private integer uID
        private unit array uStack[MAX_SINGLE_INSTANCE_COUNT]
        private integer count
        private real facing
        private static hashtable Recyclers = InitHashtable()
        //private static unit MostRecent
                
        public static method Make takes integer uID, real x, real y returns unit
            local thistype r = UnitRecycler(LoadInteger(.Recyclers, 0, uID))
            
            if r.count == 0 then //no units in stack
                return CreateUnit(Player(10), r.uID, x, y, r.facing)
            endif
            
            set r.count = r.count - 1 //pop the top unit off stack
            call ShowUnit(r.uStack[r.count], true)
            call SetUnitX(r.uStack[r.count], x)
            call SetUnitY(r.uStack[r.count], y)
            return r.uStack[r.count]
        endmethod
        
        public method Release takes unit u returns nothing
            local thistype r = UnitRecycler(LoadInteger(.Recyclers, 0, GetUnitTypeId(u)))
            if r.count == MAX_SINGLE_INSTANCE_COUNT then
                call RemoveUnit(u)
            else
                set r.uStack[r.count] = u
                call SetUnitX(u, SAFE_X)
                call SetUnitY(u, SAFE_Y)
                call ShowUnit(u, false)
                set r.count = r.count + 1
            endif
        endmethod
        
        private static method onInit takes nothing returns nothing
            thistype.create(LGUARD, 0)
            thistype.create(UBOUNCE, 90)
        endmethod
        
        public static method create takes integer UID, real DefaultFacing returns thistype
            local thistype new = thistype.allocate()
            
            set new.count = 0
            set new.uID = UID
            set new.facing = DefaultFacing
            call SaveInteger(.Recyclers, 0, UID, new)
            
            return new
        endmethod
    endstruct
endlibrary

the thing is, whenever i create a new unit i want to immediately run the locust trick on it 100% of the time. because Make does not indicate if the resulting unit is recycled or fresh, it would be easiest if it happened inside of Make. also, it just makes sense for them to be encapsulated together

but if i do it in Make i leak a handle per new unit

JASS:

public static method Make takes integer uID, real x, real y returns unit
            local thistype r = UnitRecycler(LoadInteger(.Recyclers, 0, uID))
            local unit u
            
            if r.count == 0 then //no units in stack
                set u = CreateUnit(Player(10), r.uID, x, y, r.facing)
                call UnitAddAbility(u, 'Aloc')
                call ShowUnit(u, false)
                call ShowUnit(u, true)
                return u
                //set u = null
            endif
            
            set r.count = r.count - 1 //pop the top unit off stack
            call ShowUnit(r.uStack[r.count], true)
            call SetUnitX(r.uStack[r.count], x)
            call SetUnitY(r.uStack[r.count], y)
            return r.uStack[r.count]
        endmethod

which aint great for a recycler...

the other idea i tried was a LastCreated approach

JASS:

public static method Make takes integer uID, real x, real y returns unit
            local thistype r = UnitRecycler(LoadInteger(.Recyclers, 0, uID))
            
            if r.count == 0 then //no units in stack
                set .LastCreated = CreateUnit(Player(10), r.uID, x, y, r.facing)
                call UnitAddAbility(.LastCreated, 'Aloc')
                call ShowUnit(.LastCreated, false)
                call ShowUnit(.LastCreated, true)
                return .LastCreated
            endif
            
            set r.count = r.count - 1 //pop the top unit off stack
            call ShowUnit(r.uStack[r.count], true)
            call SetUnitX(r.uStack[r.count], x)
            call SetUnitY(r.uStack[r.count], y)
            return r.uStack[r.count]
        endmethod

this at least either leaks only one handle or one handle per recycler, depending on if LastCreated is static or not, but this was running into problems when i tried to recycle multiple units in the same enum group call. either the code was buggy (after a lot of testing i didnt find anything) or .LastCreated was having the usual race condition. if someone could guess which, that would be appreciated -- i still have no idea when i need to worry about functions racing

any suggestions for doing this / advice based on experience with recycling is much appreciated!

08-21-2014, 05:17 PM

Anitarf

The only way using a global unit variable can bug is if one of the calls between set .LastCreated = and return .LastCreated triggers another trigger. I don't think the locust trick can do that, so your only other possible culprit is the CreateUnit call itself.

Do you have an indexing system in your map? That could possibly cause an error, but you'd have to call Make again from one of the indexing system's callbacks, or do something else in the callback that triggers another trigger which then calls Make.

You said you got errors when you tried recycling units in a GroupEnum callback? By that you mean you were removing existing units or creating new units? Because if there is a problem due to using a global, it's with unit creation, not unit removal.

You can always use a hybrid solution to avoid both the leak and the possibility of an error:

JASS:

                set u = CreateUnit(Player(10), r.uID, x, y, r.facing)
                call UnitAddAbility(u, 'Aloc')
                call ShowUnit(u, false)
                call ShowUnit(u, true)
                set .lastCreated = u
                set u = null
                return .lastCreated
                //

Or you can use another wrapper function/static method since arguments don't leak:

JASS:

    static method setupUnit takes unit u returns unit
        call UnitAddAbility(u, 'Aloc')
        call ShowUnit(u, false)
        call ShowUnit(u, true)
        return u
    endmethod

    ...

                return .setupUnit( CreateUnit(Player(10), r.uID, x, y, r.facing) )

The code you posted first wasn't that big of a problem either. The "not setting local variables to null" thing only becomes a problem once you destroy the object the variables were pointing to, since that makes the game engine unable to recycle that object's id, but in this case the units are never destroyed since they get recycled by your system, so you're not actually leaking anything.