• 🏆 Texturing Contest #33 is OPEN! Contestants must re-texture a SD unit model found in-game (Warcraft 3 Classic), recreating the unit into a peaceful NPC version. 🔗Click here to enter!
  • It's time for the first HD Modeling Contest of 2024. Join the theme discussion for Hive's HD Modeling Contest #6! Click here to post your idea!

[vJASS] Safe Recycler System + Dummy System

Level 3
Joined
Feb 12, 2018
Messages
12
Why a unit recycler is important?
A unit recycler is important because CreateUnit is a computationally heavy operation. If you could, instead of creating an unit, reuse an already created unit, it would greatly improve your map performance, reducing drastically the lag. Also, every time you call CreateUnit, the engine leaks 0.04KB, which can be a significant amount if you are creating a lot of units during the game duration.

What's wrong with already existing unit recyclers?
I had an already very complex map, which I needed to optimize very badly.
I decided then to use AGD's Unit Recycler ([Snippet] Unit Recycler). However, I found many limitations, such as:
- It uses bj_lastCreatedUnit at some parts of the code, which can break my code (e.g., if I call GetRecycledUnit while bj_lastCreatedUnit is pointing to another unit, it will now point to the recycled unit);
- It creates a new timer every time I call RecycleUnitDelayed. It will be very inefficient if I am recycling units a lot;
- It is not very safe: Auto-recycle dead units is a desired feature, but it will recycle any 'valid unit', whereas I want to auto-recycle only units which were created with GetRecycledUnit (it is safer and can coexist with existing maps as mine). Likewise, it'd be safer if I could only recycle a unit if the unit was created with GetRecycledUnit, otherwise, it should only remove the unit.
- It does not support caster dummies naturally. It would be desirable if the recycler could remove a unit's abilities before recycling, otherwise the following case scenario could happen: Create a caster dummy with thunderclap ability, recycle it, get previously recycled caster dummy, add another thunderclap ability, order it to cast thunderclap (an order id conflict will happen!).
- It uses PurgeandFire's Revive Unit ([Snippet] ReviveUnit), which can be bugged if, for example, a unit was killed by an event which is triggered by entering a region (when the revive dummy moves to the unit location, it will also be dead and thus won't revive the unit. I solved it by adding instant Reincarnation to the revive dummy).
- Units with Locust cannot be revived by PurgeandFire's Revive Unit, thus making the recycle system useless for dummies units (I solved it by removing locust before reviving and then adding it again after).

What I propose
In face of those limitations, I decided to create my own Unit Recycler, which I present below, intended to be used for modders that want to optimize a map in an already advanced stage but don't want to change much in their codes.
The three mains philosophies behind this unit recycler:
- Safety
It must coexist with already existing maps harmlessly until you start calling the library API;
- Efficiency
It must be fast, otherwise, a recycler system would be pointless;
- Flexibility
One shouldn't modify their code entirely to be able to benefit from the library.

Plus (full support for dummies):
- UnitAddAbilityEx which enables a unit's added abilities to be removed before recycling.
- Dummy units are also recycled, even if they are killed somehow. Also, if they are hidden and then killed, the system detects it and recycle them anyway, displaying and reviving them in the safe area (special care was taken for locust units, so the locust will be re-added after the unit is shown).
- A method to create dummies unit which has a special effect attached which will be destroyed when the unit is recycled.

Requirements
The same notes from AGD's Unit Recycle apply here:
- This only supports non-hero units and units which leave a corpse (those units whose death type is "Can raise, does decay"). So if you want to make a unit recyclable, set its death type in the Object Editor to "Can raise, does decay".
- When retrieving/recycling a unit, this does not fire a unit index/deindex event

How to use?
- Import all the library requirements to your map
- Make an empty script in your map
- Paste the library script below
- Set the desired configurations at the beginning of the script, as indicated in the code
- Import the attached dummy model
- Replace all occurrences of UnitAddAbility to UnitAddAbilityEx in your code (the system will still work if you don't, but the feature of removing abilities before recycling a unit will be disabled.)
- That's it!

Script

JASS:
library SafeUnitRecycler requires Alloc, UnitDex, Table
    /**
    Safe Unit Recycler - v1.1
    Created by PicoleDeLimao 10/02/18
    Last update 16/02/18
    [email protected]

    Requirements:
        - Alloc => https://www.hiveworkshop.com/threads/snippet-alloc.192348/
        - UnitDex => https://www.hiveworkshop.com/threads/system-unitdex-unit-indexer.248209/
        - Table => https://www.hiveworkshop.com/threads/snippet-new-table.188084/

    Installation:
        1) Import Veroxian's dummy model (https://www.hiveworkshop.com/attachments/dummy-rar.68985/)
        2) Replace all occurrencies of UnitAddAbility to UnitAddAbilityEx.
            (The system will still work if you don't, but the feature of removing added abilities before recycling will be disabled.)
        3) Copy the library dependencies, then remove the ! from //! below after saving and restarting world editor.
    */
    //! external ObjectMerger w3a AHre URiz anam "Dummy Resurrection" aher 0 acat "" atat "" Hre1 1 1 aare 1 0 aran 1 0 acdn 1 0 amcs 1 0 atar 1 "Air,Dead,Enemy,Friend,Neutral,Ground"
    //! external ObjectMerger w3a AOre URiv anam "Dummy Revive" aeat "" acas 1 0 acdn 1 0 aher 0 Ore1 1 0
    //! external ObjectMerger w3u nech eRiz unam "Dummy" uabi "Aloc,Avul" ucbs 0 ucpt 0 umdl "war3mapImported\dummy.mdx" usca "1.0" ushu "None" umvh 0 umvs 0 ufoo 0 uhpr 1000 umpi 100000 umpm 100000 umpr 1000 umvt "Fly" ucol 0 umxp 0 umxr 0 umvr 3 uble 0

    /*
    Description:
        This is my unit recycler.
        It is efficient because it only uses a single timer for delayed recycles.
        Also it is made to be very safe. It will be totally harmless to already existing maps,
        until you start calling the library API (i.e., creating units with GetUnit).

        What does it support?
        - Recycle considering facing angle (but you can turn it off for some units - e.g., caster dummies -);
        - Safe recycles (only recycle units created with GetUnit);
        - Delayed recycles;
        - Auto recycle dead units (but only the ones that were created with GetUnit);
        - Added abilities will be removed when the unit is recycled (useful for caster dummies);

    API:
        GetUnit takes player p, integer raw, real x, real y, real facing returns unit
            It is meant to be a substitute to 'CreateUnit´ for units that you want to recycle later.

        GetDummy takes player p, string sfx, real x, real y, real facing returns unit
            Create a recyclable dummy which has sfx attached

        GetInvisibleDummy takes player p, real x, real y returns unit
            Create a recyclable invisible dummy

        RecycleUnit takes unit u returns boolean
            Recycle a unit (it will only work if the unit was created with GetUnit, otherwise it will only remove it).
            It will return true case the recycle was successful.

        RecycleUnitDelayed takes unit u, real delay returns nothing
            Recycle a unit after some duration.

        AddUnitsToStock takes integer raw, real angle, integer number returns nothing
            Stock units to recycle later.
          
        UnitAddAbilityEx takes unit u, integer raw returns boolean
            It is meant to be a substitute to 'UnitAddAbility' so units can have their added abilities removed before recycling.
    **/

    //=============================================================================================
    // INIT CONFIGURATIONS
    //=============================================================================================
    globals
        private real SAFE_AREA_X = -1 // X position of the location where recycled units will be sent to
        private real SAFE_AREA_Y = -1 // Y position of the location where recycled units will be sent to
        // If you set SAFE_AREA_X or SAFE_AREA_Y to -1, it will be set to a location besides the camera bounds

        private constant real FACING_THRESHOLD_DEGREES = 20 // Max angle difference which will be tolerated between the requested facing angle and a recycled unit's facing angle
        private constant boolean AUTO_RECYCLE_DEAD_UNITS = true // it will only recycle units which were created with GetUnit
        private constant real DELAY_AFTER_DEATH = 3.0 // time after death for the unit to be recycled (don't set it higher to the game decay time - Check game constants -)

        private constant integer DUMMY_ID = 'eRiz'
        private constant integer REVIVE_DUMMY_RES_ABILITY = 'URiz'
        private constant integer REVIVE_DUMMY_SELF_RES_ABILITY = 'URiv'
    endglobals

    // Units which can be recycled
    private function ValidUnitFilter takes unit u returns boolean
        return u != null and not IsUnitIllusion(u) and IsUnitType(u, UNIT_TYPE_HERO) == false and IsUnitType(u, UNIT_TYPE_STRUCTURE) == false
    endfunction

    // Units which does not need to be facing any particular angle when recycled
    private function IgnoreFacingFilter takes unit u returns boolean
        return false
    endfunction

    // Actions which will be called before recycling a unit
    private function BeforeRecycling takes unit u returns nothing
        call PauseUnit(u, true)
        call UnitRemoveBuffs(u, true, true)
        call SetUnitState(u, UNIT_STATE_LIFE, GetUnitState(u, UNIT_STATE_MAX_LIFE))
        call SetUnitState(u, UNIT_STATE_MANA, GetUnitState(u, UNIT_STATE_MAX_MANA))
        call SetUnitInvulnerable(u, true)
        call SetUnitTimeScale(u, 0)
        call UnitResetCooldown(u)
    endfunction

    // Actions which will be called after a unit is recycled
    private function AfterRecycled takes unit u returns nothing
        call PauseUnit(u, false)
        call SetUnitTimeScale(u, 1.0)
        call SetUnitAnimation(u, "stand")
        call SetUnitInvulnerable(u, false)
    endfunction
    //=============================================================================================
    // END CONFIGURATIONS
    //=============================================================================================

    private struct LinkedList extends array
        implement Alloc
        thistype next
        thistype previous
        thistype tail
        integer data
        static method create takes nothing returns thistype
            local thistype this = thistype.allocate()
            set this.next = this
            set this.previous = this
            set this.tail = this
            return this
        endmethod
        method add takes integer value returns thistype
            local thistype temp = thistype.create()
            set this.tail.data = value
            set this.tail.next = temp
            set temp.previous = this.tail
            set this.tail = temp
            return this
        endmethod
        method remove takes nothing returns thistype
            local thistype temp
            if this != this.next then
                if this.previous == this then
                    set temp = this.next
                    if temp == this.tail then
                        set temp.data = 0
                        call temp.deallocate()
                        set this.tail = this
                        set this.next = this
                    else
                        set this.data = temp.data
                        set this.next = temp.next
                        set this.next.previous = this
                        set temp.data = 0
                        call temp.deallocate()
                    endif
                    return this
                else
                    set temp = this.next
                    set this.previous.next = temp
                    set temp.previous = this.previous
                    set this.data = 0
                    call this.deallocate()
                    return temp
                endif
            endif
            return this
        endmethod
        method destroy takes nothing returns nothing
            local thistype node = this.tail
            local thistype temp
            loop
                exitwhen node == this
                set temp = node.previous
                call node.deallocate()
                set node = temp
            endloop
            call this.deallocate()
        endmethod
    endstruct

    private struct SpecialEffect extends array
        implement Alloc
        private effect e
        static method create takes unit u, string sfx returns thistype
            local thistype this = thistype.allocate()
            set this.e = AddSpecialEffectTarget(sfx, u, "origin")
            return this
        endmethod
        method destroy takes nothing returns nothing
            call DestroyEffect(this.e)
            set this.e = null
            call this.deallocate()
        endmethod
    endstruct

    private struct TimedUnit extends array
        implement Alloc
        unit u
        real duration
        static method create takes unit u, real duration returns thistype
            local thistype this = thistype.allocate()
            set this.u = u
            set this.duration = duration
            return this
        endmethod
        method destroy takes nothing returns nothing
            set this.u = null
            call this.deallocate()
        endmethod
    endstruct

    globals
        private Table AvailableUnits // table which has as key the unit type id and as value a linked list for units of this unit type id facing different angles
        private Table RecyclableUnits // table which has as key the unit id and as value a linked list for the added abilities
        private Table LocustUnits // units which have locust (it is necessary as a workaround for revive ability)
        private Table DummyUnits // units which are dummies
        private LinkedList UnitsToRecycle // units which will be recycle delayed
        private unit ReviveDummy // unit which is used to revive dead units
    endglobals

    private module InitGlobalVariablesM
        private static method onInit takes nothing returns nothing
            local rect bounds = GetWorldBounds()
            set AvailableUnits = Table.create()
            set RecyclableUnits = Table.create()
            set LocustUnits = Table.create()
            set DummyUnits = Table.create()
            set UnitsToRecycle = LinkedList.create()
            if SAFE_AREA_X == -1 or SAFE_AREA_Y == -1 then
                set SAFE_AREA_X = 0
                set SAFE_AREA_Y = GetRectMaxY(bounds) + 1000
            endif
            set ReviveDummy = CreateUnit(Player(PLAYER_NEUTRAL_PASSIVE), DUMMY_ID, SAFE_AREA_X, SAFE_AREA_Y, 0)
            call UnitAddAbility(ReviveDummy, REVIVE_DUMMY_RES_ABILITY)
            call UnitAddAbility(ReviveDummy, REVIVE_DUMMY_SELF_RES_ABILITY)
            call SetUnitPathing(ReviveDummy, false)
            call RemoveRect(bounds)
            set bounds = null
        endmethod
    endmodule
      
     private struct InitGlobalVariables extends array
        implement InitGlobalVariablesM
    endstruct

    private function ReviveUnitEx takes unit u returns boolean
        local boolean success
        call UnitResetCooldown(ReviveDummy)
        call SetUnitX(ReviveDummy, GetUnitX(u))
        call SetUnitY(ReviveDummy, GetUnitY(u))
        set success = IssueImmediateOrderById(ReviveDummy, 852094)
        call SetUnitX(ReviveDummy, SAFE_AREA_X)
        call SetUnitY(ReviveDummy, SAFE_AREA_Y)
        return success
    endfunction

    private function RemoveRecyclableUnit takes unit u returns nothing
        local integer id = GetUnitId(u)
        local LinkedList node
        if RecyclableUnits.has(id) then
            set node = RecyclableUnits[id]
            call node.destroy()
            call RecyclableUnits.remove(id)
        endif
    endfunction

    private function MinDistAngles takes real a1, real a2 returns real
        local real diff = a1 - a2
        return RAbsBJ(ModuloReal(diff + 180, 360) - 180)
    endfunction

    function GetUnit takes player p, integer raw, real x, real y, real angle returns unit
        local LinkedList node
        local unit u
        if AvailableUnits.has(raw) then
            set node = AvailableUnits[raw]
            loop
                exitwhen node == node.next
                set u = GetUnitById(node.data)
                if GetWidgetLife(u) > 0.405 and (angle < 0 or IgnoreFacingFilter(u) or MinDistAngles(GetUnitFacing(u), angle) <= FACING_THRESHOLD_DEGREES) then
                    if angle >= 0 then
                        call SetUnitFacing(u, angle)
                    endif
                    call SetUnitOwner(u, p, true)
                    call AfterRecycled(u)
                    if GetUnitAbilityLevel(u, 'Aloc') == 0 then
                        call SetUnitPosition(u, x, y)
                    else
                        call SetUnitX(u, x)
                        call SetUnitY(u, y)
                    endif
                    if LocustUnits.has(node.data) then
                        call UnitAddAbility(u, 'Aloc')
                        call LocustUnits.remove(node.data)
                    endif
                    //debug call BJDebugMsg("Found recycled unit " + GetUnitName(u) + "(" + I2S(node.data) + ") with facing = " + I2S(R2I(GetUnitFacing(u))) + ".")
                    call node.remove()
                    return u
                endif
                set node = node.next
            endloop
        endif
        set u = CreateUnit(p, raw, x, y, angle)
        if ValidUnitFilter(u) then
            set RecyclableUnits[GetUnitId(u)] = LinkedList.create()
        endif
        debug call BJDebugMsg("Creating new unit " + GetUnitName(u) + "(" + I2S(GetUnitId(u)) + ")  with facing = " + I2S(R2I(angle)) + ".")
        return u
    endfunction

    function GetDummy takes player p, string sfx, real x, real y, real facing returns unit
        local unit u = GetUnit(p, DUMMY_ID, x, y, facing)
        local SpecialEffect e = SpecialEffect.create(u, sfx)
        set DummyUnits[GetUnitId(u)] = e
        call SetUnitScale(u, 1, 1, 1)
        call SetUnitVertexColor(u, 255, 255, 255, 255)
        call SetUnitFlyHeight(u, 0, 0)
        return u
    endfunction

    function GetInvisibleDummy takes player p, real x, real y returns unit
        local unit u = GetUnit(p, DUMMY_ID, x, y, -1)
        call SetUnitScale(u, 1, 1, 1)
        call SetUnitVertexColor(u, 255, 255, 255, 255)
        call SetUnitFlyHeight(u, 0, 0)
        return u
    endfunction

    function UnitAddAbilityEx takes unit u, integer abilityId returns boolean
        local integer id = GetUnitId(u)
        local LinkedList node
        if abilityId != 'Aloc' and RecyclableUnits.has(id) then
            set node = RecyclableUnits[id]
            call node.add(abilityId)
        endif
        return UnitAddAbility(u, abilityId)
    endfunction
  
    private function RemoveUnitAbilities takes unit u, integer id returns nothing
        local LinkedList node
        if RecyclableUnits.has(id) then
            set node = RecyclableUnits[id]
            loop
                exitwhen node == node.next
                call UnitRemoveAbility(u, node.data)
                set node = node.remove()
            endloop
        endif
    endfunction

    function RecycleUnit takes unit u returns boolean
        local integer id = GetUnitId(u)
        local SpecialEffect e
        local LinkedList node
        if DummyUnits.has(id) then
            set e = DummyUnits[id]
            call e.destroy()
            call DummyUnits.remove(id)
        endif
        if not ValidUnitFilter(u) then
            call RemoveUnit(u)
            debug call BJDebugMsg("Tring to recycle an invalid unit " + GetUnitName(u) + "(" + I2S(id) + ").")
        elseif not RecyclableUnits.has(id) then
            call RemoveUnit(u)
            debug call BJDebugMsg("Trying to recycle a non-recyclable unit " + GetUnitName(u) + "(" + I2S(id) + ").")
        elseif AvailableUnits.has(id) then
            debug call BJDebugMsg("Attempting to recycle an already recycled unit " + GetUnitName(u) + "(" + I2S(id) + ").")
        else
            call SetUnitOwner(u, Player(PLAYER_NEUTRAL_PASSIVE), false)
            call SetUnitPosition(u, SAFE_AREA_X, SAFE_AREA_Y)
            call ShowUnit(u, false)
            if GetUnitAbilityLevel(u, 'Aloc') > 0 then
                call UnitRemoveAbility(u, 'Aloc')
                set LocustUnits[id] = 1
            endif
            call ShowUnit(u, true)
            call SetUnitInvulnerable(u, false)
            if GetWidgetLife(u) < 0.405 and not ReviveUnitEx(u) then
                debug call BJDebugMsg("Could not revive dead unit to recycle " + GetUnitName(u) + "(" + I2S(id) + ").")
                call RemoveRecyclableUnit(u)
                call RemoveUnit(u)
                return false
            endif
            call BeforeRecycling(u)
            call RemoveUnitAbilities(u, id)
            call SetUnitX(u, SAFE_AREA_X)
            call SetUnitY(u, SAFE_AREA_Y)
            if AvailableUnits.has(GetUnitTypeId(u)) then
                set node = AvailableUnits[GetUnitTypeId(u)]
            else
                set node = LinkedList.create()
                set AvailableUnits[GetUnitTypeId(u)] = node
            endif
            call node.add(id)
            //debug call BJDebugMsg("Recycled " + GetUnitName(u) + "(" + I2S(id) + ").")
            return true
        endif
        return false
    endfunction

    function RecycleUnitDelayed takes unit u, real delay returns nothing
        local TimedUnit t = TimedUnit.create(u, delay)
        call UnitsToRecycle.add(t)
    endfunction

    private struct RecycleDelayedUnits extends array
        private static method checkUnits takes nothing returns nothing
            local TimedUnit t
            local LinkedList node = UnitsToRecycle
            loop
                exitwhen node == node.next
                set t = node.data
                set t.duration = t.duration - 0.05
                if t.duration <= 0 then
                    if t.u != null then
                        call RecycleUnit(t.u)
                    endif
                    call t.destroy()
                    set node = node.remove()
                else
                    set node = node.next
                endif
            endloop
        endmethod
        private static method onInit takes nothing returns nothing
            call TimerStart(CreateTimer(), 0.05, true, function thistype.checkUnits)
        endmethod
    endstruct

    function AddUnitsToStock takes integer raw, real angle, integer number returns nothing
        local integer i = 0
        loop
            set i = i + 1
            exitwhen i > number
            call RecycleUnit(GetUnit(Player(PLAYER_NEUTRAL_PASSIVE), raw, SAFE_AREA_X, SAFE_AREA_Y, angle))
        endloop
    endfunction

    static if AUTO_RECYCLE_DEAD_UNITS then
        private struct RecycleDeadUnits extends array
            private static LinkedList DeadUnitsToRecycle
            private static method checkUnits takes nothing returns nothing
                local TimedUnit t
                local LinkedList node = DeadUnitsToRecycle
                loop
                    exitwhen node == node.next
                    set t = node.data
                    set t.duration = t.duration - 0.05
                    if t.duration <= 0 then
                        if t.u != null then
                            call RecycleUnit(t.u)
                        endif
                        call t.destroy()
                        set node = node.remove()
                    else
                        set node = node.next
                    endif
                endloop
            endmethod
            private static method addDeadUnitCondition takes nothing returns boolean
                local integer id
                local SpecialEffect e
                if ValidUnitFilter(GetTriggerUnit()) then
                    set id = GetUnitId(GetTriggerUnit())
                    if DummyUnits.has(id) then
                        set e = DummyUnits[id]
                        call e.destroy()
                        call DummyUnits.remove(id)
                    endif
                    if RecyclableUnits.has(id) then
                        return true
                    endif
                endif
                return false
            endmethod
            private static method addDeadUnitAction takes nothing returns nothing
                local TimedUnit t = TimedUnit.create(GetTriggerUnit(), DELAY_AFTER_DEATH)
                call DeadUnitsToRecycle.add(t)
            endmethod
            private static method onInit takes nothing returns nothing
                local trigger triggerAddDeadUnit = CreateTrigger()
                call TriggerRegisterAnyUnitEventBJ(triggerAddDeadUnit, EVENT_PLAYER_UNIT_DEATH)
                call TriggerAddCondition(triggerAddDeadUnit, function thistype.addDeadUnitCondition)
                call TriggerAddAction(triggerAddDeadUnit, function thistype.addDeadUnitAction)
                set DeadUnitsToRecycle = LinkedList.create()
                call TimerStart(CreateTimer(), 0.05, true, function thistype.checkUnits)
            endmethod
        endstruct
    endif

endlibrary

Sample usage 1 (Spawn units each 30s)
JASS:
struct SpawnUnits extends array

    private static constant integer UNIT_TO_SPAWN = 'hsor'
    private static constant integer NUM_UNITS_TO_SPAWN = 5
    private static constant integer SPAWN_LOCATION_X = 0
    private static constant integer SPAWN_LOCATION_Y = 0

    private static method spawnUnits takes nothing returns nothing
        local unit u
        local integer i = 0
        loop
            set i = i + 1
            exitwhen i > NUM_UNITS_TO_SPAWN
            set u = GetUnit(Player(PLAYER_NEUTRAL_AGGRESSIVE), UNIT_TO_SPAWN, SPAWN_LOCATION_X, SPAWN_LOCATION_Y, 270.0)
            call IssuePointOrder(u, "attack", SPAWN_LOCATION_X, SPAWN_LOCATION_Y)
        endloop
        set u = null
    endmethod

    private static method onInit takes nothing returns nothing
        call AddUnitsToStock(UNIT_TO_SPAWN, 270.0, NUM_UNITS_TO_SPAWN)
        call TimerStart(CreateTimer(), 30.0, true, function thistype.spawnUnits)
    endmethod

endstruct

Sample Usage 2 (spell with dummies for special effects and caster dummy)
JASS:
struct DeepImpact extends array
    implement Alloc
    private static method condition takes nothing returns boolean
        return GetSpellAbilityId() == 'AHfs'
    endmethod
    private static method action takes nothing returns nothing
        local integer i = 0
        local real x
        local real y
        loop
            exitwhen i >= 10
            set x = GetSpellTargetX() + Cos(Deg2Rad(36 * i)) * 250
            set y = GetSpellTargetY() + Sin(Deg2Rad(36 * i)) * 250
            set bj_lastCreatedUnit = GetDummy(GetTriggerPlayer(), "Objects\\Spawnmodels\\Naga\\NagaDeath\\NagaDeath.mdl", x, y, 36 * i)
            call UnitApplyTimedLife(bj_lastCreatedUnit, 'BTLF', 1.0 + GetRandomReal(0, 1))
            set i = i + 1
        endloop
        set bj_lastCreatedGroup = CreateGroup()
        call GroupEnumUnitsInRange(bj_lastCreatedGroup, GetSpellTargetX(), GetSpellTargetY(), 300, null)
        loop
            set bj_lastCreatedUnit = FirstOfGroup(bj_lastCreatedGroup)
            exitwhen bj_lastCreatedUnit == null
            if GetWidgetLife(bj_lastCreatedUnit) > 0.405 and IsUnitEnemy(bj_lastCreatedUnit, GetTriggerPlayer()) then
                call UnitDamageTarget(GetTriggerUnit(), bj_lastCreatedUnit, 100 * GetUnitAbilityLevel(GetTriggerUnit(), GetSpellAbilityId()), true, false, ATTACK_TYPE_MAGIC, DAMAGE_TYPE_MAGIC, null)
            endif
            call GroupRemoveUnit(bj_lastCreatedGroup, bj_lastCreatedUnit)
        endloop
        call DestroyGroup(bj_lastCreatedGroup)
        set bj_lastCreatedUnit = GetInvisibleDummy(GetTriggerPlayer(), GetSpellTargetX(), GetSpellTargetY())
        call UnitAddAbilityEx(bj_lastCreatedUnit, 'A000')
        call IssueImmediateOrder(bj_lastCreatedUnit, "thunderclap")
        call RecycleUnitDelayed(bj_lastCreatedUnit, 1.0)
    endmethod
    private static method onInit takes nothing returns nothing
        local trigger t = CreateTrigger()
        call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_SPELL_EFFECT)
        call TriggerAddCondition(t, function thistype.condition)
        call TriggerAddAction(t, function thistype.action)
    endmethod
endstruct

Changelog:
1.1:
- Removed hooks
- Struct initialization changed to module initialization
- Changed global variables naming convenction
 

Attachments

  • dummy.mdx
    34.2 KB · Views: 81
  • UnitRecycler.w3x
    63.9 KB · Views: 101
Last edited:
I think Flux's Dummy Recycling System already fits the niche here, with dummies as the primary focus. However, seeing what your script does, I see that this tries to improve upon AGD's Unit Recycler.

I'll see with what I can notice from the script:

JASS:
//! external ObjectMerger w3a AHre URiz anam "Dummy Resurrection" aher 0 acat "" atat "" Hre1 1 1 aare 1 0 aran 1 0 acdn 1 0 amcs 1 0 atar 1 "Air,Dead,Enemy,Friend,Neutral,Ground"
//! external ObjectMerger w3a AOre URiv anam "Dummy Revive" aeat "" acas 1 0 acdn 1 0 aher 0 Ore1 1 0
//! external ObjectMerger w3u nech eRiz unam "Dummy" uabi "Aloc,Avul" ucbs 0 ucpt 0 umdl "war3mapImported\dummy.mdx" usca "1.0" ushu "None" umvh 0 umvs 0 ufoo 0 uhpr 1000 umpi 100000 umpm 100000 umpr 1000 umvt "Fly" ucol 0 umxp 0 umxr 0 umvr 3 uble 0

I'm not sure if those parts will work in the most recent version of Warcraft 3 (v.1.28.2+, I guess), due to some issue with the access of the Meta Data. You ought to add a disclaimer that this should be for the versions below it.

JASS:
struct LinkedList extends array
        implement Alloc
        public thistype next
        public thistype previous
        public thistype tail
        public integer data
        public static method create takes nothing returns thistype
            local thistype this = thistype.allocate()
            set this.next = this
            set this.previous = this
            set this.tail = this
            set this.data = 0
            return this
        endmethod
        public method add takes integer value returns thistype
            local thistype temp = thistype.create()
            set this.tail.data = value
            set this.tail.next = temp
            set temp.previous = this.tail
            set this.tail = temp
            return temp.previous
        endmethod
        public method remove takes nothing returns thistype
            local thistype temp
            if this != this.next then
                if this.previous == this then
                    set temp = this.next
                    if temp == this.tail then
                        call temp.deallocate()
                        set this.tail = this
                        set this.next = this
                    else
                        set this.data = temp.data
                        set this.next = temp.next
                        set this.next.previous = this
                        call temp.deallocate()
                    endif
                    return this
                else
                    set temp = this.next
                    set this.previous.next = temp
                    set temp.previous = this.previous
                    call this.deallocate()
                    return temp
                endif
            endif
            return this
        endmethod
        public method destroy takes nothing returns nothing
            local thistype node = this.tail
            local thistype temp
            loop
                exitwhen node == this
                set temp = node.previous
                call node.deallocate()
                set node = temp
            endloop
            call this.deallocate()
        endmethod
        public method contains takes integer value returns boolean
            local thistype node = this
            loop
                exitwhen node == node.next
                if node.data == value then
                    return true
                endif
                set node = node.next
            endloop
            return false
        endmethod
        public method size takes nothing returns integer
            local integer count = 0
            local thistype node = this
            loop
                exitwhen node == node.next
                set count = count + 1
                set node = node.next
            endloop
            return count
        endmethod
    endstruct

You might want to outsource the linked list struct from other sources, and I think the tail variable is not really needed. The keyword public is also unnecessary, since methods per se are public by default in vJASS.



I'm currently parsing the code below to the way I understand it
JASS:
struct LinkedList extends array
        implement Alloc

        thistype next
        thistype previous
        thistype tail
        integer data
        
        static method create takes nothing returns thistype
            local thistype this = allocate()
            set next = this
            set previous = this
            set tail = this
            
            //set data = 0                   ... not really necessary.
            return this
        endmethod

        method add takes integer value returns thistype
            local thistype temp = create()

            // Why write to the previous node and not to current node?
            set tail.data = value
            set tail.next = temp
            set temp.previous = tail
            set tail = temp

            // By review of process in logic, this will always return the next to the last instance and not the main instance itself.
            return temp.previous
        endmethod

        /*
            LinkedList.create()..add(2)..add(5)
            
            Result:
            LinkedList.create()
                next[1] = 1
                prev[1] = 1
                tail[1] = 1
                
                return 1 castTo LinkedList
            
            ..add(value 2)
                let temp = LinkedList.create()
                    next[2] = 2
                    prev[2] = 2
                    tail[2] = 2

                    return 2 castTo LinkedList
               
                data[tail[1]] -> data[1] = {value 2}
                next[tail[1]] -> next[1] = {2}
                prev[2] = tail[1] -> 1
                tail[1] = 2
                
                return prev[2] castTo LinkedList   -> 1 castTo LinkedList

            ..add(value 5)
                let temp = LinkedList.create()
                    next[3] = 3
                    prev[3] = 3
                    tail[3] = 3

                    return 3 castTo LinkedList
               
                data[tail[1]] -> data[2] = {value 5}
                next[tail[1]] -> next[2] = {3}
                prev[3] = tail[1] -> 2
                tail[1] = 3
               
                return prev[3] castTo LinkedList   -> 2 castTo LinkedList
        */

        method remove takes nothing returns thistype
            local thistype temp
            if this != next then
                if previous == this then
                    set temp = next
                    if temp == tail then
                        call temp.deallocate()
                        set tail = this
                        set next = this
                    else
                        set data = temp.data
                        // set temp.data = 0        ... You could null it here, wouldn't hurt.
                        set next = temp.next
                        set next.previous = this
                        call temp.deallocate()
                    endif
                    return this
                else
                    set temp = next
                    set previous.next = temp
                    set temp.previous = this.previous
                    // set data = 0
                    call deallocate()
                    return temp
                endif
            endif
            return this
        endmethod

        method destroy takes nothing returns nothing
            local thistype node = this.tail
            local thistype temp
            
            // On one side, I think this can go horribly wrong.
            loop
                exitwhen node == this
                set temp = node.previous

                static if OPTION_ONE then
                    set node.data = 0
                    call node.deallocate()
                else
                    // You already have the remove method, why not use it?
                    call node.remove()
                endif
                set node = temp
            endloop
            call this.deallocate()
        endmethod

        method contains takes integer value returns boolean
            local thistype node = this
            loop
                exitwhen node == node.next
                if node.data == value then
                    return true
                endif
                set node = node.next
            endloop
            return false
        endmethod

        /*
        Based on what type of linked list is being used, I assume it is not a circular double-linked list.

            1 castTo LinkedList.contains(5)
            Suppose list is: 1, 2, 3, 4, 5
            
            1 != 5
            2 != 5
            3 != 5
            4 != 5

            return false
            Error, since 5 is still there.

            Therefore:

            method contains takes integer value returns boolean
                local thistype node = this
                loop
                    if node.data == value then
                        return true
                    endif
           
                    exitwhen node == node.next
                    set node = node.next
                endloop
                return false
            endmethod
        */

        method size takes nothing returns integer
            local integer count = 1          //0     .. Assume that you already have the first instance.
            local thistype node = this
            loop
                exitwhen node == node.next
                set count = count + 1
                set node = node.next
            endloop
            return count
        endmethod
    endstruct

From there, I could assume that the script may be prone to errors such as the node not being the head node.


This part should be initialized via modules from

JASS:
private struct InitGlobalVariables extends array
    private static method onInit takes nothing returns nothing
        local rect bounds = GetWorldBounds()
        set AVAILABLE_UNITS = Table.create()
        set RECYCLABLE_UNITS = Table.create()
        set LOCUST_UNITS = Table.create()
        set DUMMY_UNITS = Table.create()
        if SAFE_AREA_X == -1 or SAFE_AREA_Y == -1 then
            set SAFE_AREA_X = 0
            set SAFE_AREA_Y = GetRectMaxY(bounds) + 1000
        endif
        set REVIVE_DUMMY = CreateUnit(Player(PLAYER_NEUTRAL_PASSIVE), DUMMY_ID, SAFE_AREA_X, SAFE_AREA_Y, 0)
        call UnitAddAbility(REVIVE_DUMMY, REVIVE_DUMMY_RES_ABILITY)
        call UnitAddAbility(REVIVE_DUMMY, REVIVE_DUMMY_SELF_RES_ABILITY)
        call SetUnitPathing(REVIVE_DUMMY, false)
        set bounds = null
    endmethod
endstruct

to

JASS:
private module InitGlobalVariablesM
    private static method onInit takes nothing returns nothing
        local rect bounds = GetWorldBounds()
        set AVAILABLE_UNITS = Table.create()
        set RECYCLABLE_UNITS = Table.create()
        set LOCUST_UNITS = Table.create()
        set DUMMY_UNITS = Table.create()
        if SAFE_AREA_X == -1 or SAFE_AREA_Y == -1 then
            set SAFE_AREA_X = 0
            set SAFE_AREA_Y = GetRectMaxY(bounds) + 1000
        endif
        set REVIVE_DUMMY = CreateUnit(Player(PLAYER_NEUTRAL_PASSIVE), DUMMY_ID, SAFE_AREA_X, SAFE_AREA_Y, 0)
        call UnitAddAbility(REVIVE_DUMMY, REVIVE_DUMMY_RES_ABILITY)
        call UnitAddAbility(REVIVE_DUMMY, REVIVE_DUMMY_SELF_RES_ABILITY)
        call SetUnitPathing(REVIVE_DUMMY, false)

        // You might forget to remove the generated rect.
        // call RemoveRect(bounds)
        set bounds = null
    endmethod
endmodule

private struct InitGlobalVariables extends array
    implement InitGlobalVariablesM
endstruct

This one isn't really necessary, but I think it would appear a bit better if it were from this,

JASS:
function GetUnit(player p, int raw, real x, real y, real angle) returns unit
    ...
    if GetWidgetLife(u) > 0.405 ...

to this

JASS:
//Don't forget to declare the native UnitAlive beforehand
// native UnitAlive takes unit id returns boolean

function GetUnit(player p, int raw, real x, real y, real angle) returns unit
    ...
    if UnitAlive(u) ...

Those are just some of what I could see that's somewhat awry.
 
Level 3
Joined
Feb 12, 2018
Messages
12
Thank you for your reply.

I think Flux's Dummy Recycling System already fits the niche here, with dummies as the primary focus. However, seeing what your script does, I see that this tries to improve upon AGD's Unit Recycler.
Flux's Dummy Recycling is a nice system, however it also does not supply my needs. I also propose an improvement over his system, since this does not let you recycle if the unit scale, color or fly height was changed (but you can turn it off for some units). This also remove added abilities before recycling, which I don't think Flux's system does. Also, his delayed recycle also creates a timer for each unit, which I improved by using a single timer. Flux's doesn't auto-recycle dead units, mine does.

I'll see with what I can notice from the script:

JASS:
//! external ObjectMerger w3a AHre URiz anam "Dummy Resurrection" aher 0 acat "" atat "" Hre1 1 1 aare 1 0 aran 1 0 acdn 1 0 amcs 1 0 atar 1 "Air,Dead,Enemy,Friend,Neutral,Ground"
//! external ObjectMerger w3a AOre URiv anam "Dummy Revive" aeat "" acas 1 0 acdn 1 0 aher 0 Ore1 1 0
//! external ObjectMerger w3u nech eRiz unam "Dummy" uabi "Aloc,Avul" ucbs 0 ucpt 0 umdl "war3mapImported\dummy.mdx" usca "1.0" ushu "None" umvh 0 umvs 0 ufoo 0 uhpr 1000 umpi 100000 umpm 100000 umpr 1000 umvt "Fly" ucol 0 umxp 0 umxr 0 umvr 3 uble 0

I'm not sure if those parts will work in the most recent version of Warcraft 3 (v.1.28.2+, I guess), due to some issue with the access of the Meta Data. You ought to add a disclaimer that this should be for the versions below it.
Tested and it does work.

You are using my LinkedList incorrectly. It is not meant to be called as:
linkedlist.add(...).add(...)...
You must always add from the head node. That's why I use a tail variable, to access the last node quickly from the head node. However, I'm going to modify the add method to return the head node to make it more intuitive.
Also, I don't use .remove on the .destroy method because that's inefficient. I just want to deallocate all instances, no need to fix pointers with .remove.

======================
UPDATED:
Changed struct initialization to module.
 
Level 22
Joined
Feb 6, 2014
Messages
2,466
The way I see it, this is like more general purpose recycler that supports any unit and not just Dummy. I haven't tested your map yet (sorry no World Editor at the moment) but won't you end up having too many units waiting to be recycled since each unit type is treated differently? Aside from that, each unit type waiting to be recycled also has different facing angles so you'll need a lot of units waiting to be recycled because if you did not, the hit ratio of actually recycling a unit is miniscule. For example, if I only have 10 peons waiting to be recycled, I wouldn't end up reusing those peons much because the system can only give you 1 peon per 36 degree angle. If I ask for 2 peons at the same angle, the system will end up creating a unit because it can no longer provide it for me. However if I increase that count to 50 peons, it may have a reasonable hit ratio but what if I want to recycle other unit types? 50 peons, 50 peasants, 50 footmen, etc. waiting to be recycled doing nothing will most likely cause a lag at some point.
A Dummy Recycler does not have this problem because it is focused on a single unit type.

I also propose an improvement over his system, since this does not let you recycle if the unit scale, color or fly height was changed (but you can turn it off for some units)
How is this an improvement? (Don't rake this question sarcastically)

This also remove added abilities before recycling, which I don't think Flux's system does
Correct my system does not do that. A user could easily make a AddUnitAbilityTimed though. I didn't put it because in the map I made using my system, I didn't actually require that functionality much and has access when the spell ends thus I just remove the ability there. Since I rarely add an ability to my Dummies, I thought it would just be an overhead to add that functionality to the system.

Also, his delayed recycle also creates a timer for each unit, which I improved by using a single timer.
To me this look like an unnecessary optimization. Is it even an improvement over separate timer per instance? I mean, your timer has to iterate every 0.05 seconds just to update their duration while a separate timer per instance only has one function call which happens upon timer expiration. Besides (in my system at least), you wouldn't end up with more than 10 Dummies Recycle Timer running at the same time unless you like to spam some sfx. Anyways, if one claims an improvement over the other, I suggest it should be backed up with a test like i did in my resource.

Flux's doesn't auto-recycle dead units, mine does.
I don't see a reason why a Dummy Recycler would.

Anyways, I actually support improvement of old/obsolete resources so I if this is better than DummyRecycler or UnitRecycler, then it would be a big help to the community. I didn't thorougly read the code but I did notice that you named some non constants all uppercase which as JPAG goes, is bad. Then there's the use of hook.
 
@PicoleDeLimao

Now, there are some things that might raise eyebrows, hooks being one of them. Hooks are those little cute creatures that hide a monstrosity behind them, and are, in Hive, frowned upon. Also, I might have to raise some current issues,

  • If the user is running in DEBUG_MODE, the user will have a different output than when the user is not in DEBUG_MODE. If you look at Sevion's Alloc, it will behave just like the standard allocation when in DEBUG_MODE, potentially jeopardizing the library. To be more precise, you could probably allocate more than 8190 instances as normal, but be restricted to it otherwise.

  • Is LinkedList supposed to be used externally? By the way it is used, it would be better if it was private.
  • If I recall, module initializers are preferred to struct initializers.

JASS:
/*
..

    RecycleUnit takes unit u returns boolean
            Recycle an unit (it will only work if the unit was created with GetUnit, otherwise it will only remove it).
            It will return true case the recycle was successful.

..
*/

Why would RecycleUnit remove units not created with GetUnit? Wouldn't that defeat the purpose of the Recycler?
 
Level 3
Joined
Feb 12, 2018
Messages
12
@Flux

Thank you for your reply.

You are partially correct. Yes, you can ended up with too many units waiting to be recycled, BUT:
- You can turn off the unit facing check for some listed units;
- When you call GetDummy or GetInvisibleDummy, you always return an unit with the same unit type. Also, GetInvisibleDummy doesn't check for unit facings, which is great for caster dummies.
So it's like a dummy recycler plus a unit recycler. Now the unit recycler it's not meant to be called everywhere. Just for units which you spawn a lot, like, for a Tower Defense map, the creep waves.

About disabling recycle for units which had their scale, color or fly height changed, it's only logical. Let me explain my case scenario to prove it to be useful: I have over 600 custom units in my map, each one with their own default scale, vertex color and fly height. Because of that, I can't just use SetUnitScale(u,1, 0, 0), SetUnitVertexColor(u,255,255,255,255) and SetUnitFlyHeight(u,0,0) as a way to "clear" the unit before being recycled because that wouldn't bring the unit to its defaults! Also, I can't list each unit scale, vertex color and fly height, that would be impractical to be a map as complex as mine. So the only solution for me that would make the system REALLY safe is to disable recycling for units which had those properties changed.
But now, keep reading: Dummy units are not affected by this! I intentionally disabled this feature for dummy units, since when I recycle a dummy unit, I know I can set the dummy properties to scale 1, vertex color 255 and fly height to 0 safely.
AND you can disable this for some listed units and handle the "clearing" process yourself (on BeforeRecycling function).
This is another way to bring generality.

About the added abilities, this is another way to make my system the most general purpose possible. It must attend all case scenarios, because it's meant to be used for already running maps which were not designed to fit any particular system.

About recycling dead units, read my argument above. It may not be useful for dummies, but it is for unit recycling (spawning creep waves).

I will fix the naming and will think about alternatives to hooks.

@MyPad

No, LinkedList is not meant to be used elsewhere. My bad.

No, it does not defeat the purpose of a Recycler, and that's precisely what makes the system safe! I assume the programmer is error-prone, and may call RecycleUnit for an unit which is not meant to be recycled (i.e., was not created using GetUnit). The philosophy behind this system is simple: You want to recycle an unit? Create it through GetUnit first! This is specially true when recycling dead units.

Again, I used hooks because that way the user would need to change its code the least possible to effectively use the system. But I will think in alternatives if it is THAT bad.



======
EDIT:
System updated.
 
Last edited:
Sorry for the nitpicking, but could you add a debug keyword for this line

JASS:
// Insert debug keyword in elseif?
elseif AvailableUnits.has(id) then
    debug call BJDebugMsg("Attempting to recycle an already recycled unit " + GetUnitName(u) + "(" + I2S(id) + ").")

Oh, I think this system will be reviewed and analyzed more thoroughly in the coming days. Perhaps, the self-balancing algorithm (introduced by Nestharus?) (See in Flux's Dummy Recycler) will be mentioned, and something about modularity might come into the picture, but I'm sure if done correctly, this system might be approved. For some cross-compatibility, I would like to write the package for the object generation.
 
Level 22
Joined
Feb 6, 2014
Messages
2,466
- You can turn off the unit facing check for some listed units;
This will be a problem though in some case scenario when the facing angle upon spawn is important.

- When you call GetDummy or GetInvisibleDummy, you always return an unit with the same unit type. Also, GetInvisibleDummy doesn't check for unit facings, which is great for caster dummies.
if GetDummy/GetInvisibleDummy doesn't check for facing angle, then using this system for Dummy casting will lead to varying delay because if you use the Dummy as a caster and it happens to target at the opposite side of its spawning angle (180 degrees), then it will cause the Dummy to turn before it casts it spells thus resulting in a slight delay.

About disabling recycle for units which had their scale, color or fly height changed, it's only logical. Let me explain my case scenario to prove it to be useful: I have over 600 custom units in my map, each one with their own default scale, vertex color and fly height. Because of that, I can't just use SetUnitScale(u,1, 0, 0), SetUnitVertexColor(u,255,255,255,255) and SetUnitFlyHeight(u,0,0) as a way to "clear" the unit before being recycled because that wouldn't bring the unit to its defaults! Also, I can't list each unit scale, vertex color and fly height, that would be impractical to be a map as complex as mine. So the only solution for me that would make the system REALLY safe is to disable recycling for units which had those properties changed.
But now, keep reading: Dummy units are not affected by this! I intentionally disabled this feature for dummy units, since when I recycle a dummy unit, I know I can set the dummy properties to scale 1, vertex color 255 and fly height to 0 safely.
AND you can disable this for some listed units and handle the "clearing" process yourself (on BeforeRecycling function).
This is another way to bring generality.
So there you just said that Dummy units doesn't need this functionality so this
I also propose an improvement over his system, since this does not let you recycle if the unit scale, color or fly height was changed (but you can turn it off for some units).
isn't true because how can it be an improvement if it doesn't need it in the first place. :)
I get what you're saying if it is used for unit recycling though.

About the added abilities, this is another way to make my system the most general purpose possible. It must attend all case scenarios, because it's meant to be used for already running maps which were not designed to fit any particular system.
Sorry I haven't taken a deep look at the code yet but how did you detect if an ability is added to the Dummy so that you can remove it upon readying for recycle? By using a wrapper function like AddUnitAbilityEx?

You also did not reply about the premature timer optimization thingy.

Perhaps, the self-balancing algorithm (introduced by Nestharus?) (See in Flux's Dummy Recycler)
This would be a great addition. This greatly increases the hit ratio of Dummy Recycler.
 
Level 4
Joined
Jan 27, 2016
Messages
89
Would it be possible to add an option to recycle a secondary type of unit? One that would be without locust, but temporary and invulnerable, as I'd need to spawn plenty of them.
 

AGD

AGD

Level 16
Joined
Mar 29, 2016
Messages
688
The way I see it, this is like more general purpose recycler that supports any unit and not just Dummy. I haven't tested your map yet (sorry no World Editor at the moment) but won't you end up having too many units waiting to be recycled since each unit type is treated differently? Aside from that, each unit type waiting to be recycled also has different facing angles so you'll need a lot of units waiting to be recycled because if you did not, the hit ratio of actually recycling a unit is miniscule. For example, if I only have 10 peons waiting to be recycled, I wouldn't end up reusing those peons much because the system can only give you 1 peon per 36 degree angle. If I ask for 2 peons at the same angle, the system will end up creating a unit because it can no longer provide it for me. However if I increase that count to 50 peons, it may have a reasonable hit ratio but what if I want to recycle other unit types? 50 peons, 50 peasants, 50 footmen, etc. waiting to be recycled doing nothing will most likely cause a lag at some point.
A Dummy Recycler does not have this problem because it is focused on a single unit type.
Mine also takes into consideration the facing angle but is designed so you can't have excess recycled units since the angle tolerance is 360 degrees (hardcoded). I agree though, it might not look so good when you need great facing accuracy. But in most maps, such accuracy is not needed for non-dummy units.


- It uses bj_lastCreatedUnit at some parts of the code, which can break my code (e.g., if I call GetRecycledUnit while bj_lastCreatedUnit is pointing to another unit, it will now point to the recycled unit)
The use of the BJ variable is actually intended as also with many other dummy systems. It allows gui users to use a single line custom script and refer to the last recycled unit using GetLastCreatedUnit() as if u are using GetRecycledUnit() as 'the new way' of creating a new unit, which is actually the point of a UnitRecycler.

- It creates a new timer every time I call RecycleUnitDelayed. It will be very inefficient if I am recycling units a lot;
The issue was already discussed by Flux I believe

- It is not very safe: Auto-recycle dead units is a desired feature, but it will recycle any 'valid unit', whereas I want to auto-recycle only units which were created with GetRecycledUnit (it is safer and can coexist with existing maps as mine). Likewise, it'd be safer if I could only recycle a unit if the unit was created with GetRecycledUnit, otherwise, it should only remove the unit.
Makes sense. Yeah, I think this should be the ideal behavior of those functions.

- It does not support caster dummies naturally. It would be desirable if the recycler could remove a unit's abilities before recycling, otherwise the following case scenario could happen: Create a caster dummy with thunderclap ability, recycle it, get previously recycled caster dummy, add another thunderclap ability, order it to cast thunderclap (an order id conflict will happen!).
A better practice I think would be that any modifications or any things done by a certain system or module should be cleared/reset afterwards by that same system or module. Take this for example:
JASS:
function SetUnitPositionEx takes unit u, Vector v returns nothing
    call SetUnitX(u, v.x)
    call SetUnitY(u, v.y)
    call SetUnitFlyHeight(u, v.z)
    // In no way should this function destroy the argument Vector v after using it.
    // The function calling this, which is also where the Vector creation occured,
    // should be the one to destroy the Vector passed as an argument.
    // Doing otherwise might cause some problems to other things outside this function
    // that is using this same Vector
endfunction
In a similar way, the system who added the abilities to units should also be the one to remove those abilities for those units. For this reason, I added a module to add things that are to be run (like resets and all) to be run when a new unit is retrieved.

- It uses PurgeandFire's Revive Unit ([Snippet] ReviveUnit), which can be bugged if, for example, a unit was killed by an event which is triggered by entering a region (when the revive dummy moves to the unit location, it will also be dead and thus won't revive the unit. I solved it by adding instant Reincarnation to the revive dummy).
In this case, you should definitely put a check before killing the entering units. If not, similar buggy behavior could happen to lets say, dummies who are merely used for moving special effects (which is quite common in custom spells). The ideal practice would be to filter out units with locust abilities inside such triggers.

- Units with Locust cannot be revived by PurgeandFire's Revive Unit, thus making the recycle system useless for dummies units (I solved it by removing locust before reviving and then adding it again after).
I see it pointless to recycle dummy units since they should not be killed in the first place, only recycled, hence the dummy recycling systems. I do agree that the said system is useless for non-dummy units with locust ability though.


I'm also in for possible improvements over existing systems, but since the goal of this is to replace mine, I suppose we need a third person opinion/feedback since I might have some bias when comparing two systems with one being mine :).
Also, if the goal is to be a replacement, you should probably simply name this UnitRecycler (SafeUnitRecycler looks ugly imo).
 

Bribe

Code Moderator
Level 50
Joined
Sep 26, 2009
Messages
9,464
NOOOOOOOO, just installed this some days agoooo.
:/.
So what should be the safest alternative to this one?
You can still use graveyarded resources, you just can’t submit resources to the Code section which depend on them (maps can use whatever they want, but spells and snippets should use approved stuff).

As for the uses of this resource, it is much more limited now that special effects can be used instead of units for projectile artwork.
 
Top