• 🏆 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!

[Snippet] UnitLL

Level 17
Joined
Apr 27, 2008
Messages
2,455
Introduction :

For instant enumeration, we have the classic GroupEnum with a null filter, and then the FirstOfGroup/GroupRemoveUnit loop.
Sadly, we can't use that for groups which are kept during time, because of ghost units (units removed of the game but not of the group).
Just because FirstOfGroup can and will return null with a ghost unit.
And even if it wouldn't, at the end of the loop the group will be empty and then couldn't be used again later.
The usual alternative is to use a ForGroup, because ghost units are not enumed with that.
But it's bad because it splits the code and also opens a new thread each time it's called, it's inefficient by all means.

So i've made this library (UnitLL stands for Unit Linked List).
It handles ghost units, there is no way that you will have some inside your UnitLL.
Il also keeps the unit relative order when they were added, you can know how many units you have in there (.count) at any time, the last and the first unit added.
Most of methods returns a boolean (if true the unit was removed/added to the group, whatever, if false not) or an integer.
It works as a group, in the sense that an unit can be added to an UnitLL only one time and no more.

Additional Notes :

- I'm never going to use a struct array and custom allocators, just for silly improvements
- Actually UnitLL is a double not circular linked list, i made it not circular because it makes sense to privilege the iteration over than the add and unit remove.
- I'm not going to make benchmarks, i can't believe that a ForGroup is faster than a not circular LL loop iteration.
Well, maybe i will make this only one if someone still believes it.
But anyway even if it's slower (why the hell ?!), speed is not the main feature here.
Also it's obvious that the custom methods are way slower than the native group functions.

Thx for reading, i'm open to all constructive comments.

JASS:
library UnitLL uses UnitIndexer

    globals
        group GROUP = CreateGroup() // must be used for instant enumeration, use it with a FirstOfGroup/GroupRemoveUnit loop
        // never destroy it
    endglobals
    /*
    ==========================================================================
    UnitLL v2.25
    --------------------------------------------------------------------------
    "Easy" iteration on units, complete. It is designed to be as fast as possible for iteration.
    It handles ghost units (you can't have an unit removed of the game inside an UnitLL)
    But the backward is that is the slowest solution for all other operations.
    UnitLL is basically a double not circular linked list, like groups you can't add an unit several times.
    
    ==========================================================================
    Intro:
    --------------------------------------------------------------------------
    For instant enumeration we have the classic GroupEnum... with a null filter,
    and then a FirstOfGroup/GroupRemoveUnit loop.
    But when you need to keep units during time, you can't safely use a such loop,
    because FirstOfGroup can and will return null with a ghost unit.

    To be clear : this is not for speedfreaks, even if it's probably the fastest solution on loop iterating,
    it's also obvious that is the slowest solution for all other operations.
    
    I would say that the returned values of methods are nice and useful,
    and be able to keep the relative unit order is definetely a plus.

    You have to know how linked list work and especially in vJass to use it, i hope that the demo codes are enough.
    
    Practicals limits (just for completeness reason) :
    
    At any time this must be true :
    
    Number of UnitLL head + Number of UnitLL instances < 8191
    Number of units which are at least in one UnitLL head + Number of UnitLL head where an unit is inside < 8191

    ==========================================================================
    API Guide:
    
    *readonly UnitLL members :
    --------------------------------------------------------------------------
    
    * thistype head
    
        Since most of methods work only with an UnitLL head, and i have the use of it in the internal code.
        An UnitLL head is basically like the "groups", i mean that is the data structure where units are stored.
        
    * thistype first & last
    
        It's the first/last member of an UnitLL head
        
    * thistype next & prev
    
        Classic members of double linked list.
        Note that an UnitLL is a double not circular linked list, so you can't directly use someUnitLLHead.next/prev
        You have to use someUnitLLHead.first/last before starting the loop through the UnitLL
        
    * integer count
    
        It returns the number of unit contained in the UnitLL head
        
    * unit enum
    
        You still can use it, but it's deprecated, since the version 2.1, it's better to use the method getUnit()
        It's simply the unit stored in an UnitLL, you catch them by looping through the UnitLL head instances
        
    * boolean end
    
        You must use it inside a loop, or you can also check if the UnitLL instance == 0, check the demo spell Life Drain.
    
    * static method create takes nothing returns thistype                    Complexity : O(1)
    --------------------------------------------------------------------------

        Create a new UnitLL head. You must create it before anything else.
        An UnitLL head is like a group, it will contain other UnitLL instances (basically units)
        
    * method getUnit takes nothing returns unit                    Complexity : O(1)
    --------------------------------------------------------------------------

        It's simply the unit stored in an UnitLL instance, you catch them by looping through the UnitLL head
        
    * method getRandomUnit takes nothing returns unit                    Complexity : O(N/2) at worst
    --------------------------------------------------------------------------

        returns a random unit from and UnitLL head.
     
    * method addGroup takes group whichGroup, boolean safety returns integer                    Complexity : O(N)
    --------------------------------------------------------------------------
    
        Add a group to an UnitLL head. In case one unit was already inside the UnitLL, it is not added again.
        It returns the number of unit added, useful in some cases.
        Be aware, that if safety == false, it will use a FOG loop (FirstOfGroup/GroupRemoveUnit), so the group will be cleared.
        If safety == true, it will use ForForce, so it will be less efficient but will work if the group has ghost units inside it.
        You should use it only on groups which are instantly enumed, not keep during time, but meh.
        
    * method removeGroup takes group whichGroup, boolean safety returns integer                    Complexity : O(N)
    --------------------------------------------------------------------------
    
        Same as addGroup, except that of course it removes unit from the UnitLL.
        
    * method addUnit takes unit whichUnit returns boolean                    Complexity : O(1)
    --------------------------------------------------------------------------
    
        Add an unit to an UnitLL head. In case one unit was already inside the UnitLL, it is not added again.
        It returns true if the unit is added, and false if not.
        
    * method removeUnit takes unit whichUnit returns boolean                    Complexity : O(1)
    --------------------------------------------------------------------------
    
        Remove an unit to an UnitLL head.
        It returns true if the unit is removed, and false if not.
        
    * method addUnitLL takes UnitLL head returns integer                    Complexity : O(N)
    --------------------------------------------------------------------------
    
        Add an UnitLL head to the other UnitLL head.
        It returns the number of unit added.
        
    * method removeUnitLL takes UnitLL head returns integer                    Complexity : O(N)
    --------------------------------------------------------------------------
    
        Remove an UnitLL head to the other UnitLL head.
        It returns the number of unit removed.
        
    * method isUnitInside takes unit whichUnit returns boolean                    Complexity : O(1)
    --------------------------------------------------------------------------
    
        Returns true if the unit is inside the UnitLL head, and false if not
        
    * method removeThis takes nothing returns nothing                    Complexity : O(1)
    --------------------------------------------------------------------------
    
        Remove an UnitLL instance from the UnitLL head
        
    * method clear takes nothing returns nothing                    Complexity : O(N)
    --------------------------------------------------------------------------
    
        Clear the UnitLL head, but not destroy it.
        
    * method destroy takes nothing returns nothing                    Complexity : O(N)
    --------------------------------------------------------------------------
    
        Clear the UnitLL head, and then destroy it.
        
    * method onDeindex takes code toRegister returns nothing                   Complexity : O(1)
    --------------------------------------------------------------------------
    
        Will fire the code for an UnitLL head if one of its unit is being removed.
        Note that you can use the UnitIndexer API related to the deindex event, such as GetIndexedUnitId() and GetIndexedUnit()
        The registered code will be hopefully removed when the UnitLL will be destroyed (but not just if it's cleared).
        Technically you can register only one code to an UnitLL, if there was already a previous one it will be erased.
        I could allow several codes but i don't see the usefulness of it
        Also, because it uses triggerconditions, don't use GetTriggeringTrigger(), and you're also not allowed to use TriggerSleepAction/PolledWait.
        And some other functions like the sync natives functions and PauseGame.
        
    * static method getDeindexing takes nothing returns thistype                   Complexity : O(1)
    --------------------------------------------------------------------------
    
        UnitLL.getDeindexing() : It must be used inside a code registred with the method onDeindex,
        it returns the UnitLL head where an unit is being removed of the game
     
        Note that if you don't need to do some stuff on unit deindex, these last two methods are useless.
        The unit will still be removed of the UnitLL when it will be removed
     
*/

    globals
        private hashtable Hash_t = InitHashtable()
        private integer array LL_main
    endglobals
    
    private struct LL // Linked list to link units and their UnitLL
        thistype next
        thistype prev
        thistype head
        UnitLL x
        
        static method create takes nothing returns thistype
            local thistype s = thistype.allocate()
            
            set s.head = s
            set s.next = s
            set s.prev = s
            return s
        endmethod
        
        method addUnitLL takes UnitLL x returns thistype
            local thistype s = thistype.allocate()
            
            set this.prev.next = s
            set s.prev = this.prev
            set this.prev = s
            set s.next = this
            set s.head = this.head
            set s.x = x
            return s
        endmethod
        
        method removeThisLL takes nothing returns nothing
            local thistype s = this.head
            
            set this.prev.next = this.next
            set this.next.prev = this.prev
            if s == s.next then // empty
                set LL_main[GetUnitId(this.x.enum)] = 0
                call s.deallocate()
                set s.head = 0
                set s.next = 0
                set s.prev = 0
                set s.x = 0
            endif
            call this.deallocate()
            set this.head = 0
            set this.x = 0
        endmethod
        
    endstruct

    struct UnitLL
    
        readonly unit enum
        readonly thistype next
        readonly thistype prev
        readonly thistype first
        readonly thistype last
        readonly integer count
        
        readonly thistype head
        readonly boolean end
        
        /* used internally (don't care about these comments)
            this , id > whichUnitLL instance, according to the unit id and the UnitLL this
            id ,- this >whichLL instance, according to the unit id and the UnitLL this
        */
        
        /* I've managed to make all the debug stuff not mandatory.
           I mean that it will work as expected in not debug mode even with invalid arguments,
           but the debug mode helps the user to catch errors in his code
        */
        
        static method create takes nothing returns thistype
            local thistype this = thistype.allocate()
            
            set this.head = this
            set this.next = 0
            set this.prev = 0
            set this.count = 0
            set this.last = 0
            set this.first = 0
            set this.enum = null
            return this
        endmethod
        
        method getUnit takes nothing returns unit
            debug if this == 0 then
            debug    call BJDebugMsg("UnitLL.getUnit() : the UnitLL instance is not valid (0)\n\n")
            debug    return null
            debug endif
            debug if this.head == this then
            debug    call BJDebugMsg("UnitLL.getUnit() : you must use an UnitLL instance for this method, not an UnitLL head")
            debug    return null
            debug endif
            return this.enum
        endmethod
        
        method getRandomUnit takes nothing returns unit
            local integer i = GetRandomInt(1,this.count)
            debug if this == 0 then
            debug    call BJDebugMsg("UnitLL.getRandomUnit() : the UnitLL instance is not valid (0)\n\n")
            debug    return null
            debug endif
            debug if this.head != this then
            debug    call BJDebugMsg("UnitLL.getRandomUnit() : you must use an UnitLL head for this method, not an UnitLL instance")
            debug    return null
            debug endif
            if i > this.count / 2 then
                set i = i - this.count/2
                set this = this.last
                loop
                exitwhen i == 0
                    set this = this.prev
                set i = i-1
                endloop
            else
                set this = this.first
                loop
                exitwhen i == 0
                    set this = this.next
                    set i = i-1
                endloop
            endif
            return this.enum
        endmethod
        
        method isUnitInside takes unit whichUnit returns boolean
            debug if this == 0 then
            debug    call BJDebugMsg("UnitLL.isUnitInside() : the UnitLL instance is not valid (0)\n\n")
            debug    return false
            debug endif
            debug if this.head != this then
            debug    call BJDebugMsg("UnitLL.isUnitInside() : you must use an head for this method, not just an UnitLL instance")
            debug    call BJDebugMsg("check also if it's a valid UnitLL head\n\n")
            debug    return false
            debug endif
            debug if not IsUnitIndexed(whichUnit) then
            debug    call BJDebugMsg("UnitLL.isUnitInside() : you must use an indexed unit through UnitIndexer\n\n")
            debug    return false
            debug endif
            return HaveSavedInteger(Hash_t,this,GetUnitId(whichUnit)) and IsUnitIndexed(whichUnit)
        endmethod
        
        method addUnit takes unit whichUnit returns boolean // returns true if the unit is added and false if not
            local thistype id = thistype(GetUnitId(whichUnit))
            local thistype s
            local LL ll
            
            debug if this == 0 then
            debug     call BJDebugMsg("UnitLL.addUnit() : the UnitLL instance is not valid (0)\n\n")
            debug     return false
            debug endif
            debug if not IsUnitIndexed(whichUnit) then
            debug     call BJDebugMsg("UnitLL.addUnit() : you must use an indexed unit by UnitIndexer\n\n")
            debug     return false
            debug endif
            if this.head != this then // will even work with this == 0, check the initializer
                debug call BJDebugMsg("UnitLL.addUnit() : you must use an head for this method, not just an UnitLL instance")
                debug call BJDebugMsg("check also if it's a valid UnitLL head\n\n")
                return false
            endif
            if HaveSavedInteger(Hash_t,this,id) or not IsUnitIndexed(whichUnit) then // the unit is already inside the UnitLL or invalid
                return false
            endif
            set s = thistype.allocate()
            set s.enum = whichUnit
            set s.head = this
            set s.next = 0
            set s.prev = 0
            call SaveInteger(Hash_t,this,id,s) 
            set ll = LL(LL_main[id])
            if ll == 0 then // LL was not already created for the unit
                set ll = LL.create()
                set LL_main[id] = ll
            endif
            if this.count == 0 then
                set this.first = s
            else
                set this.last.next = s
                set s.prev = this.last
            endif
            set this.last = s
            set this.count = this.count + 1
            call SaveInteger(Hash_t,id,-this,ll.addUnitLL(s))
            return true
        endmethod
        
        method removeUnit takes unit whichUnit returns boolean // returns true if the unit is removed and false if not
            local thistype id = thistype(GetUnitId(whichUnit))
            local thistype s = thistype(LoadInteger(Hash_t,this,id))
            local LL ll
            
            debug if this == 0 then
            debug     call BJDebugMsg("UnitLL.removeUnit() : the UnitLL instance is not valid (0)\n\n")
            debug     return false
            debug endif
            debug if not IsUnitIndexed(whichUnit) then
            debug     call BJDebugMsg("UnitLL.removeUnit() : you must use an indexed unit by UnitIndexer\n\n")
            debug     return false
            debug endif
            if this.head != this then // will even work with this == 0, check the initializer
                debug call BJDebugMsg("UnitLL.removeUnit() : you must use an head for this method, not just an UnitLL instance")
                debug call BJDebugMsg("check also if it's a valid UnitLL head\n\n")
                return false
            endif
            if s == 0 or not IsUnitIndexed(whichUnit) then // the unit is not inside the UnitLL or not valid
                return false
            endif
            set this.count = this.count - 1
            if this.count == 0 then
                set this.last = 0
                set this.first = 0
            elseif s == this.last then
                set this.last = s.prev
                set s.prev.next = 0
            elseif s == this.first then
                set this.first = s.next
                set s.next.prev = 0
            else
                set s.next.prev = s.prev
                set s.prev.next = s.next
            endif
            
            set ll = LL(LoadInteger(Hash_t,id,-this))
            if ll != 0 then
                call ll.removeThisLL()
                call RemoveSavedInteger(Hash_t,id,-this)
            endif
            call RemoveSavedInteger(Hash_t,this,id)
            set s.enum = null
            set s.head = 0
            call s.deallocate()            
            return true
        endmethod
        
        private static integer I
        private static thistype This
        private static thistype Temp
    
        private static method safetyAdd takes nothing returns nothing
            if thistype.Temp.addUnit(GetEnumUnit()) then
                set thistype.I = thistype.I + 1
            endif
        endmethod
        
        method addGroup takes group whichGroup, boolean safety returns integer
            local unit u
            
            if this.head != this then // will even work with this == 0, check the initializer
                debug call BJDebugMsg("UnitLL.addGroup() : you must use an head for this method, not just an UnitLL instance")
                debug call BJDebugMsg("check also if it's a valid UnitLL head\n\n")
                return 0
            endif
            set thistype.I = 0
            if safety then
                set thistype.Temp = this
                call ForGroup(whichGroup,function thistype.safetyAdd)
            else
                for u in whichGroup
                    if this.addUnit(u) then
                        set thistype.I = thistype.I + 1
                    endif
                endfor
            endif
            return thistype.I
        endmethod
        
        private static method safetyRemove takes nothing returns nothing
            if thistype.Temp.removeUnit(GetEnumUnit()) then
                set thistype.I = thistype.I + 1
            endif
        endmethod
        
        method removeGroup takes group whichGroup, boolean safety returns integer
            local unit u
            
            if this.head != this then // will even work with this == 0, check the initializer
                debug call BJDebugMsg("UnitLL.removeGroup() : you must use an head for this method, not just an UnitLL instance")
                debug call BJDebugMsg("check also if it's a valid UnitLL head\n\n")
                return 0
            endif
            set thistype.I = 0
            if safety then
                set thistype.Temp = this
                call ForGroup(whichGroup,function thistype.safetyRemove)
            else
                for u in whichGroup
                    if this.removeUnit(u) then
                        set thistype.I = thistype.I + 1
                    endif
                endfor
            endif
            return thistype.I
        endmethod
        
        method addUnitLL takes UnitLL head returns integer
            local thistype s = head.first
            local integer i = 0
            
            if this.head != this then // will even work with this == 0, check the initializer
                debug call BJDebugMsg("UnitLL.addUnitLL() : you must use an head for this method, not just an UnitLL instance")
                debug call BJDebugMsg("check also if it's a valid UnitLL head\n\n")
                return 0
            endif
            loop
            exitwhen s == 0
                if this.addUnit(s.enum) then
                    set i = i + 1
                endif
            set s = s.next
            endloop
            return i
        endmethod
        
        method removeUnitLL takes UnitLL head returns integer
            local thistype s = head.first
            local integer i = 0
            
            if this.head != this then // will even work with this == 0, check the initializer
                debug call BJDebugMsg("UnitLL.removeUnitLL() : you must use an head for this method, not just an UnitLL instance")
                debug call BJDebugMsg("check also if it's a valid UnitLL head\n\n")
                return 0
            endif
            loop
            exitwhen s == 0
                if this.removeUnit(s.enum) then
                    set i = i + 1
                endif
            set s = s.next
            endloop
            return i
        endmethod
        
        method removeThis takes nothing returns nothing
            debug if this.head == this then // will even work with this == 0, check the initializer
            debug    call BJDebugMsg("UnitLL.removeThis() : it is for remove an UnitLL instance from an UnitLL head, don't use it on a head")
            debug    call BJDebugMsg("if you want to clear a whole UnitLL, use the method clear() on an UnitLL head")
            debug    call BJDebugMsg("check also if it's a valid UnitLL instance\n\n")
            debug    return
            debug endif
            call this.head.removeUnit(this.enum)
        endmethod
        
        method clear takes nothing returns nothing
            debug if this.head != this then // will even work with this == 0, check the initializer
            debug     call BJDebugMsg("UnitLL.clear() : you must use an head for this method, not just an UnitLL instance")
            debug     call BJDebugMsg("check also if it's a valid UnitLL head\n\n")
            debug     return
            debug endif
            set this = this.first
            loop
            exitwhen this == 0
                call this.removeThis()
            set this = this.next
            endloop
        endmethod
        
        private trigger trig_deindex
        
        method destroy takes nothing returns nothing
            if this.head != this then // will even work with this == 0, check the initializer
                debug call BJDebugMsg("UnitLL.destroy() : you must use an head for this method, not just an UnitLL instance")
                debug call BJDebugMsg("check also if it's a valid UnitLL head\n\n")
                return
            endif
            if this.trig_deindex != null then
                call DestroyTrigger(this.trig_deindex)
                set this.trig_deindex = null
            endif
            call this.clear()
            call this.deallocate()
            set this.head = 0
            set this.first = 0
            set this.last = 0
        endmethod
        
        method onDeindex takes code toRegister returns nothing
            if this.head != this then // will even work with this == 0, check the initializer
                debug call BJDebugMsg("UnitLL.onDeindex() : you must use an head for this method, not just an UnitLL instance")
                debug call BJDebugMsg("check also if it's a valid UnitLL head\n\n")
                return
            endif
            if this.trig_deindex == null then
                set this.trig_deindex = CreateTrigger()
            else
                call TriggerClearConditions(this.trig_deindex)
            endif
            call TriggerAddCondition(this.trig_deindex,Filter(toRegister))
        endmethod
        
        static method getDeindexing takes nothing returns thistype
            return thistype.This
        endmethod
        
        private static method onRemove takes nothing returns boolean
            local LL this = LL(LL_main[GetIndexedUnitId()])
            local LL s
            if this == 0 then
                return false
            endif
            set s = this.next
            loop
            exitwhen s == this
                set thistype.This = s.x.head
                call TriggerEvaluate(s.x.head.trig_deindex)
                set thistype.This = 0
                call s.x.removeThis()
            set s = s.next
            endloop
            return false
        endmethod
        
        private static method onInit takes nothing returns nothing
            call RegisterUnitIndexEvent(Filter(function thistype.onRemove),UnitIndexer.DEINDEX)
            set thistype(0).head = 1 // this way "if this.head != this" will be enough even with this == 0
            set thistype(0).end = true
        endmethod
        
    endstruct
    
endlibrary

The last "unofficial" jasshelper version by Cohadar is needed
It can be found here or there.

I use it because it has better loops, fix the madness vJass initializers order, faster to compile, and for future improvements.

JASS:
scope LifeDrain initializer init // uses UnitLL , TimerStack, UnitAPI

    globals
        private constant integer SPELL_ID = 'A000'
        private constant real PERIOD = 2
        private constant real RANGE = 150
        private constant real SPELL_ANIMATION_TIME = 1.5
        private constant real DAMAGE_OVER_WAVE = 100
        private UnitLL array LL
        private timer array Tim
    endglobals
    
    private struct Stack
        lightning light
        thistype next
    endstruct
    
    private function DestroyLights takes nothing returns nothing
        local Stack s = TimerRelease(GetExpiredTimer())
        local Stack temp
        
        call s.destroy()
        loop
        set temp = s
        set s = s.next
        exitwhen s == 0
            call DestroyLightning(s.light)
            set s.light = null
            call s.destroy()
            set temp.next = 0
        endloop
    endfunction
   
    private function Periodic takes nothing returns nothing
        local integer id = TimerGetData(GetExpiredTimer())
        local unit caster = GetUnitById(id)
        local unit u
        local UnitLL ll = LL[id]
        local integer count = ll.count
        local Stack s
        
        set s = Stack.create()
        call TimerNew(0.2,function DestroyLights,s)
        set ll = ll.first
        loop
        exitwhen ll.end // or exitwhen ll == 0, it's the same

            set s.next = Stack.create()
            set s = s.next
            set u = ll.getUnit()
            if IsUnitType(u,UNIT_TYPE_DEAD) then
                call UnitDamageTarget(caster,caster,DAMAGE_OVER_WAVE*3/count,false,false,null,null,null)
                set s.light = AddLightningEx("AFOD",true,GetUnitX(caster),GetUnitY(caster),UnitGetZ(caster)+100,GetUnitX(u),GetUnitY(u),UnitGetZ(u))
            else
                call UnitDamageTarget(caster,u,DAMAGE_OVER_WAVE/count,false,false,null,null,null)
                call UnitDamageTarget(caster,caster,-DAMAGE_OVER_WAVE/count,false,false,null,null,null)
                set s.light = AddLightningEx("DRAB",true,GetUnitX(caster),GetUnitY(caster),UnitGetZ(caster)+100,GetUnitX(u),GetUnitY(u),UnitGetZ(u))
            endif
        set ll = ll.next
        endloop
        set caster = null
        set u = null
    endfunction
    
    private function StopSpell takes nothing returns boolean
        local unit caster
        local integer id
        
        if GetSpellAbilityId() != SPELL_ID then
            return false
        endif
        
        set caster = GetSpellAbilityUnit()
        set id = GetUnitId(caster)
        call LL[id].destroy()

        set LL[id] = 0
        call SetUnitTimeScale(caster,1)
        if Tim[id] != null then
            call TimerRelease(Tim[id])
            set Tim[id] = null
        endif
        set caster = null
        return true
    endfunction
    
    private function StartSpell takes nothing returns boolean
        local unit caster = GetSpellAbilityUnit()
        local integer id = GetUnitId(caster)
        local integer count
        local UnitLL ll
        local Stack s
        local unit u
        local player p = GetTriggerPlayer()
        
        if GetSpellAbilityId() != SPELL_ID then
            return false // yeah yeah, possible handle id leaks i don't care, it's just a quick demo
        endif
        
        // so first we enum unit as usually
        call GroupEnumUnitsInRange(GROUP,GetSpellTargetX(),GetSpellTargetY(),RANGE,null)
        call GroupRemoveUnit(GROUP,caster) // we don't want the caster in the group
        set ll = UnitLL.create() // creates the UnitLL head
        set LL[id] = ll // link the UnitLL and the caster
        for u in GROUP // classic FirstOfGroup/GroupRemoveUnit loop
            if IsUnitEnemy(u,p) then
                call ll.addUnit(u) // addUnit is a method that must be used with the head UnitLL
            endif
        endfor
        set count = ll.count // as the method addUnit, count is a member of the head only
        if count == 0 then // no units were added
            call IssueImmediateOrderById(caster,851973) // "stun" order
            call SimError(p,"Spell aborted, no targeted units found")
            set p = null
            set caster = null
            return true // at this point there is still an empty UnitLL which is created, but il will be destroyed when the endcast event will fire
            // one thing is good with this event : it always fire, no matter how the spell end.
        endif
        call SetUnitTimeScale(caster,SPELL_ANIMATION_TIME/PERIOD)
        set s = Stack.create()
        call TimerNew(0.2,function DestroyLights,s) // start the timer and link it with the stack
        set ll = ll.first // we start with the first unit added
        
        loop // here we go, loop through the UnitLL
        exitwhen ll.end // since UnitLL is a double not circular linked list, or exitwhen ll == 0, it's the same
        
            set s.next = Stack.create()
            set s = s.next
            set u = ll.getUnit()
            if IsUnitType(u,UNIT_TYPE_DEAD) then
                call UnitDamageTarget(caster,caster,DAMAGE_OVER_WAVE*3/count,false,false,null,null,null)
                set s.light = AddLightningEx("AFOD",true,GetUnitX(caster),GetUnitY(caster),UnitGetZ(caster)+100,GetUnitX(u),GetUnitY(u),UnitGetZ(u))
            else
                call UnitDamageTarget(caster,u,DAMAGE_OVER_WAVE/count,false,false,null,null,null)
                call UnitDamageTarget(caster,caster,-DAMAGE_OVER_WAVE/count,false,false,null,null,null)
                set s.light = AddLightningEx("DRAB",true,GetUnitX(caster),GetUnitY(caster),UnitGetZ(caster)+100,GetUnitX(u),GetUnitY(u),UnitGetZ(u))
            endif
            
        set ll = ll.next // here we go forward, but we can go backward with prev, but note that is not a circular linked list
        endloop
        
        set Tim[id] = TimerNew(PERIOD,function Periodic,id) // to do waves
        set caster = null
        set p = null
        set u = null
        return true
    endfunction
    
    private function init takes nothing returns nothing
        local trigger trig = CreateTrigger()
        call TriggerRegisterAnyUnitEventBJ(trig,EVENT_PLAYER_UNIT_SPELL_EFFECT)
        call TriggerAddCondition(trig,function StartSpell)
        set trig = CreateTrigger()
        call TriggerAddCondition(trig,function StopSpell)
        call TriggerRegisterAnyUnitEventBJ(trig,EVENT_PLAYER_UNIT_SPELL_ENDCAST)
        set trig = null
    endfunction
    
endscope

JASS:
scope Ondeindex initializer init

/*
This is just to show how to use the UnitLL API, in case you need to do some stuff on unit deindexing.
Honestly, i can't find a concrete case where you would need that, i mean killing the unit first would be better in lot of cases,
and then you could catch the death event. But there is the possibility to do it.
*/

    globals
        private UnitLL LL1
        private UnitLL LL2
        private UnitLL LL3
    endglobals
    
    private function F1 takes nothing returns nothing
        // hopefully we can use the UnitIndexer API related to the unit deindex event
        local integer id = GetHandleId(GetIndexedUnit())
        local UnitLL ll = UnitLL.getDeindexing()
        
        call RemoveUnit(LL1.last.getUnit()) // yes recursive unit remove is allowed
        
        call BJDebugMsg("unit handle id : " + I2S(id) + " is being removed of the UnitLL : " +I2S(ll) )
        call BJDebugMsg("do some stuff related to the UnitLL LL1 \n\n")
    endfunction
    
    private function F2 takes nothing returns nothing
        local integer id = GetIndexedUnitId()
        
        call BJDebugMsg("unit handle id : " + I2S(id) + " is being removed of the UnitLL : " +I2S(LL2) )
        call BJDebugMsg("do some stuff related to the UnitLL LL2 \n\n")
        /* You can remove the unit from the UnitLL inside this code, but most of time it's pointless,
        simply because it will be removed automatically anyway, right after all onDeindex codes registred for all UnitLL(s)
        (without any delay)
        */
        call LL2.removeUnit(GetIndexedUnit())
    endfunction
    
    private function F3 takes nothing returns nothing
        local integer id = GetHandleId(GetIndexedUnit())
        local UnitLL ll = UnitLL.getDeindexing()
        
        call BJDebugMsg("unit handle id : " + I2S(id) + " is being removed of the UnitLL : " +I2S(ll) )
        call BJDebugMsg("do some stuff related to the UnitLL LL3 \n\n")
    endfunction
    
    private function init takes nothing returns nothing
        local unit u
        // i'm just adding "random" units to the UnitLL(s), the revelant code is below the next comment
        call TriggerSleepAction(0)
        set LL1 = UnitLL.create()
        set LL2 = UnitLL.create()
        set LL3 = UnitLL.create()
        call GroupEnumUnitsOfPlayer(GROUP,Player(1),null)
        set u = FirstOfGroup(GROUP)
        call GroupRemoveUnit(GROUP,u)
        call LL1.addUnit(u)
        set u = FirstOfGroup(GROUP)
        call GroupRemoveUnit(GROUP,u)
        call LL1.addUnit(u)
        set u = FirstOfGroup(GROUP)
        call GroupRemoveUnit(GROUP,u)
        call LL2.addUnit(u)
        set u = FirstOfGroup(GROUP)
        call GroupRemoveUnit(GROUP,u)
        call LL3.addUnit(u)
        // here we go
        call LL1.onDeindex(function F1)
        call LL2.onDeindex(function F2)
        call LL3.onDeindex(function F3)
        call TriggerSleepAction(1)
        // let's remove units
        call RemoveUnit(LL1.first.enum)
        call RemoveUnit(LL2.first.enum)
        call RemoveUnit(LL3.first.enum)
        /*
        I don't have used the method removeUnit() inside the functions F1,F2,F3.
        Once the registered code had fired, the unit will be removed of the UnitLL.
        But nothing forbids you to use it in the registred code.
        Technically you can register only one code to an UnitLL, if there was already a previous one it will be erased.
        I could allow several codes but i don't see the usefulness of it
        */
    endfunction
    
endscope

JASS:
scope UnitLLBoolean initializer init

    globals
        private UnitLL LL_first
        private UnitLL LL_second
    endglobals
    
   private function Enter2 takes nothing returns nothing
        local unit u = GetEnteringUnit()
        if LL_first.isUnitInside(u) then // the unit is inside LL_first
            call BJDebugMsg("the unit with the id : "+I2S(GetUnitId(u))+" will got his reward right now\n\n")
            call KillUnit(u)
            // in case the unit ressurect and you still want this "feature" we remove the unit from the 2 UnitLL
            call LL_first.removeUnit(u)
            call LL_second.removeUnit(u)
        elseif  LL_second.addUnit(u) then // the unit was not inside LL_second
            call BJDebugMsg("the unit with the id : "+I2S(GetUnitId(u))+" needs to enter in region 1 before\n\n")
        endif
        set u = null
   endfunction
   
   private function Enter1 takes nothing returns nothing
        local unit u = GetEnteringUnit()
        if LL_first.addUnit(u) then // the unit was not already inside LL_first
            call BJDebugMsg("the unit with the id : "+I2S(GetUnitId(u))+" enters in region1 for the first time")
            call BJDebugMsg("If you want to get a special reward, go to region2\n\n")
        else // the unit was already inside LL_first
        endif
        set u = null
   endfunction

    private function init takes nothing returns nothing
        local rect zone
        local region reg = CreateRegion()
        local trigger trig = CreateTrigger()
        set LL_first = UnitLL.create()
        set LL_second = UnitLL.create()
        set zone = Rect(500,-1300,1000,-900)
        call SetBlightRect(Player(0),zone,true)
        call RegionAddRect(reg,zone)
        call RemoveRect(zone)
        call TriggerRegisterEnterRegion(trig,reg,null)
        call TriggerAddAction(trig,function Enter1)
        set trig = CreateTrigger()
        set reg = CreateRegion()
        set zone = Rect(500,-700,1000,-300)
        call SetBlightRect(Player(0),zone,true)
        call RegionAddRect(reg,zone)
        call RemoveRect(zone)
        call TriggerRegisterEnterRegion(trig,reg,null)
        call TriggerAddAction(trig,function Enter2)
        set zone = null
        set trig = null
        set reg = null
    endfunction
    
endscope

JASS:
scope UnitLLTest initializer init

    private function init takes nothing returns nothing
        local unit u
        local integer i = 0
        local UnitLL ll_1 = UnitLL.create()
        local UnitLL ll_2 = UnitLL.create()
        
        call TriggerSleepAction(0)
        call GroupEnumUnitsOfPlayer(GROUP,Player(1),null)
        call BJDebugMsg("number of Player(1)'s units == " + I2S( ll_1.addGroup(GROUP,false) ) )
        call GroupEnumUnitsOfPlayer(GROUP,Player(0),null)
        call BJDebugMsg("you should see 6 times the same number")
        call BJDebugMsg("number of Player(0)'s units == " + I2S( ll_2.addGroup(GROUP,true) ) )
        call BJDebugMsg("number of units in GROUP == " + I2S( CountUnitsInGroup(GROUP) ) )
        call BJDebugMsg("number of units added to ll_1 == " + I2S( ll_1.addUnitLL(ll_2) ) ) 
        call BJDebugMsg("number of units removed to ll_1 == " + I2S( ll_1.removeUnitLL(ll_2) ) ) 
        call ll_1.addUnitLL(ll_2)
        call GroupEnumUnitsOfPlayer(GROUP,Player(0),null)
        call BJDebugMsg("number of units removed to ll_1 == " + I2S( ll_1.removeGroup(GROUP,false) ) ) 
        call BJDebugMsg("number of units in GROUP == " + I2S( CountUnitsInGroup(GROUP) ) )
        
        loop
        exitwhen i == 10
        set i = i+1
            call BJDebugMsg("random unit : " + I2S(GetUnitId( ll_1.getRandomUnit() )))
        endloop
        call BJDebugMsg(" ")
        
    endfunction
    
endscope

Changelog :

v 1.1
v 2.0
v 2.1
v 2.2
v 2.25
 

Attachments

  • UnitLL_v1.1.w3x
    41.5 KB · Views: 87
  • UnitLL_v2.0.w3x
    44.4 KB · Views: 85
  • UnitLL_v2.1.w3x
    45.9 KB · Views: 95
  • UnitLL_v2.2.w3x
    45.3 KB · Views: 76
  • UnitLL_v2.25.w3x
    46.2 KB · Views: 92
Last edited:
Level 6
Joined
Jun 20, 2011
Messages
249
Troll-Brain i thought you were some kind of benchmark freak
I dare you to benchmark this on normal groups. This would crash when they would have still 40 fps.
Plus, because you're not using my linked list module your system is already worse.
JASS:
method clear takes nothing returns nothing
            set this = this.first
            loop //looping through the instances is a BAD deallocation method to flush an entire list. Read my LinkedListModule and use it.
            exitwhen this == 0
                call this.removeThis()
            set this = this.next
            endloop
        endmethod
 
Level 17
Joined
Apr 27, 2008
Messages
2,455
Subjectivity is so fun.

I'm not going to use a module with lot of uneeded code, plus your linked list works as a stack, where here it makes more sense to work like a queue in my code.
And it makes more sense to use a not circular linked list here because groups which are kept during time, are most likely being loop trough more times than unit adding/removing on it.
But as said with valid arguments i could change my mind about a circular linked list.

JASS:
        method insertNode takes thistype toInsert returns nothing            
            set this.next.prev=toInsert
            set toInsert.next=this.next
            set this.next=toInsert
            set toInsert.prev=this
        endmethod

->

JASS:
        method insertNode takes thistype toInsert returns nothing            
            set this.prev.next=toInsert
            set toInsert.prev=this.prev
            set this.prev=toInsert
            set toInsert.next=this
        endmethod

Since ForForce opens a new thread i can't imagine that a basic loop is going to lose against it.
Now, as stated the others methods are obviously way slower than the native group function.
But speed is not the main feature here.

Now plz read what i've already said.
Also read my code and try to not use a loop for the clear method.

Finally, i'm not a benchmark freak, i just hate random facts without any proof "like local variables are faster than globals".
 
Level 6
Joined
Jun 20, 2011
Messages
249
Using my LinkedList module you can reach circular linked lists as well as non-circular lists. It mostly handles dynamic lists
Not a single method in my resource would go wasted here
-createNode() -> the head of the group (head==true)
-insertNode() -> adds a new node to the head of the group
-clearNode() -> clears the group, leaves it empty
-flushNode() -> deletes the group, as well as all the units inside of it
And it does that better than how you're doing it in here.

I'm sorry if I didn't speak fully in the terms where I said this was slower...
I've tested this over FirstOfGroup iterations so many times I got bored.
Bottom line, FirstOfGroup was faster in every single possible situation
But you don't care for that as you're trying to handle groups that aren't destroyed when the iteration ends.
And yes, the ForGroup is the slowest method among all the group enumeration natives, but here's the issue:
CreateGroup() is faster than array group allocation
GroupAddUnit() is faster than array group allocation
ForGroup() is slower than array group iteration
This would only be faster if you loop through the group constantly.
I also agree that "count" is a precious variable

"local variables are faster than globals" Depends on the situation, if you're going to declare a local just to read a global once, then no, otherwise yes (as globals are usually also array reads and it makes them even slower) and yes, I have tested this and I have proof.

Also... (it has no remove method, but that can be arranged using your hashtable lookup or maybe even loop lookup)
JASS:
struct AdvUnitGroup

    implement LinkedList
    
    readonly unit unit
    readonly integer count
    
    static method create takes nothing returns AdvUnitGroup
        return createNode()
    endmethod
    
    method add takes unit u returns nothing
        local thistype node = allocate()
        set node.unit = u
        set this.count = this.count+1
        call this.insertNode(node)
    endmethod
    
    method destroy takes nothing returns nothing
        call this.flushNode()
        set this.count = 0
    endmethod
    
    method clear takes nothing returns nothing
        call this.clearNode()
        set this.count = 0
    endmethod
    
endstruct
 
Level 17
Joined
Apr 27, 2008
Messages
2,455
Using my LinkedList module you can reach circular linked lists as well as non-circular lists. It mostly handles dynamic lists
Not a single method in my resource would go wasted here
-createNode() -> the head of the group (head==true)
-insertNode() -> adds a new node to the head of the group
-clearNode() -> clears the group, leaves it empty
-flushNode() -> deletes the group, as well as all the units inside of it
And it does that better than how you're doing it in here.

You keep to ignore ghost units, i have to make a loop.
Plus i never leave handles (unit enum) not nulled, because it can make an handle id leak, you never know how your resource will be used.
For example if it reachs at max 100 instances, and then no more than 10, and handles stored are removed later, you will have handle id leaks.
Yes it doesn't matter that much by itself, but this+this+this, ...

Now, you can argue that this is too much overhead just for this feature, and you could manually check with an if during the loop, but the main point of this is to make easy to use.

And again even if i would your resource you would still have to edit to what i've suggested, else i will definetely not use it.
But to be honest i don't think i will use it anyway i like to inline linked list.

I'm sorry if I didn't speak fully in the terms where I said this was slower...
I've tested this over FirstOfGroup iterations so many times I got bored.
Bottom line, FirstOfGroup was faster in every single possible situation

I was never suggested to use this instead of a fog loop, as stated in the first post.
It is ONLY to remplace groups which are kept during time.

But you don't care for that as you're trying to handle groups that aren't destroyed when the iteration ends.

What ?

And yes, the ForGroup is the slowest method among all the group enumeration natives, but here's the issue:
CreateGroup() is faster than array group allocation
GroupAddUnit() is faster than array group allocation
ForGroup() is slower than array group iteration
This would only be faster if you loop through the group constantly.
I also agree that "count" is a precious variable

Again, as stated in the first post it's obvious that the native group functions are way faster than my custom functions.
I've made it for an easy and efficient looping.
Hence the reason of a not circular linked list (exitwhen array integer == 0 VS exitwhen array integer == integer).
Now, maybe the overhead on add/remove doesn't worth it. But actually i like it like that.

"local variables are faster than globals" Depends on the situation, if you're going to declare a local just to read a global once, then no, otherwise yes (as globals are usually also array reads and it makes them even slower) and yes, I have tested this and I have proof.

I have proof than globals are faster than locals in an heavy usage, but to be fair that was with the same name length (1 letter), so yeah with a good optimizer locals should be faster, since it's easy to make local variable with only one letter, and globals would be 2 ou 3 characters.
Anyway i wasn't targeted you, i just answered to your sentence.

Really i've the feeling that you've just not read my first post, or even read carefully my code.
 
Level 17
Joined
Apr 27, 2008
Messages
2,455
But i can't make it O(1) without breaking the actuals features, and i don't want leave any of them.

I knew since the begin that wasn't for speedfreaks.
Really O(N) is not the worst algorithm complexity, sometimes it's just fine.
I was pretty sure you would not like it (or more).

But oh well, i will add the complexity in the main thread for each method in the documentation.
And also write the practicals limits (number of max instances)
 
Level 17
Joined
Apr 27, 2008
Messages
2,455
I got rid of the .evaluate, added a isUnitInside method, improved a lil the code, and finally added one more example, using UnitLL as a way to link a boolean with an unit.
Still no documentation, but that will be the very next thing.
 
Level 17
Joined
Apr 27, 2008
Messages
2,455
The "new" JassHelper breaks backwards compat with the added keywords throwing syntax errors :/

Well, that doesn't make sense to use these keywords anyway.
And the fix is easy. (people : plz also stop the vJass abuse like spell a member of struct "unit", it's really ugly and bad practice, just because Tesh or whatever highlights it, doesn't make it better)
I really appreciate the effort made by Cohadar, many people (include me) is crying about "how this thing is done in vJass", but actually he is the only one which is working one.
I don't know if he will add/fix more stuff, but the added loops are already an improvement by itself.

Imo, just stick with the Vexorian JassHelper and maybe hold out for Wurst if Frotty incorporates it.

Sadly, it seems that i've quite the opposite opinions of yours in scripting.
I don't like verbose languages, but i hate much more indent based languages.
 
Level 17
Joined
Apr 27, 2008
Messages
2,455
1- Smaller map script size
2- Less garbage
3- 2 Less function calls
4- You can chose what structure you want to allocate/deallocate with.

These are the things you gain when making structs extend arrays.

Here what you lose :

1 code abstraction
2 debug stuff
3 a nice and not fugly api

I don't get your 4) btw.
EDIT : ok, i got it, but why the hell would you need that ?! At least it's not needed here.

These simple things do matter in any real cases.
Not to mention that script size and less function calls don't matter.

Now, i think you're just too young to understand this.
I won't try to convince you again, but plz stop random comments about it, flood is ok but not about these silly "optimizations".
 
Last edited:
Level 17
Joined
Apr 27, 2008
Messages
2,455
Just in case someone is currently using it (who knows, the universe is so big)

I'm going to break backward compatibility with the method addGroup :

It will return an integer instead of a boolean, ofc if you didn't used the returned value, you won't have to edit your code.
Anyway this boolean is half-implemented, if the group is valid and not empty but all units inside it were already inside the UnitLL, it returns true.

It makes more sense to return an integer : the number of units added.
It allows more fancy stuff that i'm going to show in a new sample.

So if someone care enough about this resource and find something wrong it's the time to say something now or never, since i won't break backward compatibility when it will be (eventually) approved.

I will also add a new method addUnitLL, debug stuff, and finally this boring documentation.
 

Bribe

Code Moderator
Level 50
Joined
Sep 26, 2009
Messages
9,464
When I first saw the thread title, I expected to see this:

JASS:
library UnitGroup //v0.1.1.0 by Bribe
    globals
        private hashtable ht = InitHashtable()
        private integer si = 0
        private integer array sr
    endglobals
    
    struct UnitGroup extends array
        readonly unit first
        readonly unit last
        
        method next takes unit u returns unit
            return LoadUnitHandle(ht, this, GetHandleId(u))
        endmethod
        
        method prev takes unit u returns unit
            return LoadUnitHandle(ht, this, -GetHandleId(u))
        endmethod
        
        method contains takes unit u returns boolean
            return LoadBoolean(ht, -this, GetHandleId(u))
        endmethod
        
        method clear takes nothing returns nothing
            call FlushChildHashtable(ht, this)
            call FlushChildHashtable(ht, -this)
            set this.first = null
            set this.last = null
        endmethod
        
        static method create takes nothing returns UnitGroup
            local UnitGroup g = sr[0]
            if g == 0 then
                set g = si + 1
                set si = g
            else
                set sr[0] = sr[g]
            endif
            set sr[g] = -1
            return g
        endmethod
        
        method destroy takes nothing returns nothing
            if sr[this] == -1 then
                call this.clear()
                set sr[this] = sr[0]
                set sr[0] = this
            debug else
                debug call BJDebugMsg("<UnitGroup> Error: Tried to destroy an invalid UnitGroup: " + I2S(this))
            endif
        endmethod
        
        method add takes unit u returns nothing
            local integer id = GetHandleId(u)
            if this.first == null then
                set this.first = u
            else
                call SaveUnitHandle(ht, this, -id, this.last) //this.prev = last
                call SaveUnitHandle(ht, this, GetHandleId(this.last), u) //last.next = this
            endif
            set this.last = u
            call SaveBoolean(ht, -this, id, true)
        endmethod
        
        readonly static unit nextReplaced = null
        readonly static unit prevReplaced = null
        
        method remove takes unit u returns nothing
            local integer id = GetHandleId(u)
            if LoadBoolean(ht, -this, id) then
                set .prevReplaced = LoadUnitHandle(ht, this, -id)
                set .nextReplaced = LoadUnitHandle(ht, this, id)
                if .nextReplaced == null then
                    set this.last = .prevReplaced
                endif
                if .prevReplaced == null then
                    set this.first = .nextReplaced
                    if .nextReplaced == null then
                        call FlushChildHashtable(ht, this)
                        call FlushChildHashtable(ht, -this)
                        return
                    endif
                endif
                call SaveUnitHandle(ht, this, GetHandleId(.prevReplaced), .nextReplaced)
                call SaveUnitHandle(ht, this, -GetHandleId(.nextReplaced), .prevReplaced)
                call RemoveSavedHandle(ht, this, id)
                call RemoveSavedHandle(ht, this, -id)
                call RemoveSavedBoolean(ht, -this, id)
            debug else
                debug call BJDebugMsg("<UnitGroup> Error: Tried to remove an invalid unit (" + I2S(i) + ") from UnitGroup: " + I2S(this))
            endif
        endmethod
        
        method removeEx takes unit u, boolean returnNext returns unit
            call this.remove(u)
            if returnNext then
                return .nextReplaced
            endif
            return .prevReplaced
        endmethod
        
    endstruct
endlibrary

The only issue with the above is that it requires someone to make sure to remove units from the group in enough time to ensure they do not get null handles. Though perhaps that may not be too severe because the hashtable may still point to the handle ID?
 
Last edited:
Level 17
Joined
Apr 27, 2008
Messages
2,455
I wanted to be as fast as possible when you loop through the UnitLL, even if it means more overhead for the others operations.

I thought that was fine actually, but maybe it's too much weird that the list members are only reference, not directly units, and your way could be better.
I'm also sorry if i don't post samples of usage here, but only in the map, it's because i use personal stuff.

If you choice to handle ghost units (as i do) you sadly can't make clear O(1).
Also i know that is a quick write, but actually you don't check if the unit was inside the UnitGroup before add it.

method next takes unit u returns unit

What ?!

I'm not going to use custom struct allocators, so no struct arrays.

To keep iteration through the UnitLL simple, i can't set the current instance pointer .next and .prev to null when you remove an unit inside the UnitLL (i have to do it when the instance got recycled, else it will obvioulsy break the iteration).
With the actual version, that's not a problem since next and prev are integers, but if it becomes units it can make handle id leaks, and i'm not really a fan of this behavior :/

Meh, nevermind, it can't make handle id leaks, as i handle ghost units.
Ok, i'm going to make last,first,prev,next members units instead of integers.

Thx for suggestions.
 
Last edited:

Bribe

Code Moderator
Level 50
Joined
Sep 26, 2009
Messages
9,464
method next takes unit u returns unit, what is bad about it?

If you want to remove a unit while iterating through the list, I could make a "popUnit" method which takes unit u, boolean next returns unit, which is a wrapper for "remove" but returns the previous or next unit (whichever the user is more interested in).

Edit: update to 0.1.1.0
 
Level 17
Joined
Apr 27, 2008
Messages
2,455
Using an argument for a next and prev makes the loop even weirder that it is actually with my actual code, it is also less efficient.
For the pop method, then you will also need a push method for backward looping, i just want to keep it simple.

On a second thought, there is no way that i can keep an efficient loop using your method, and there is also probably no sane way to handle ghost units (yeah i really want to get rid of them).

I really appreciate your effort, but i don't like your way.
So here is the deal, i will post a version with debug stuff and documentation, you will be free to judge of its usefulness or not, but i won't change it in this way.
 

Bribe

Code Moderator
Level 50
Joined
Sep 26, 2009
Messages
9,464
I want to run a test to see if the ghost units will break my approach at all. unit == null will return false even if the unit was removed, for example, because the variable "unit" still points to the handle ID (meaning GetHandleId will still work). However, I don't know if LoadUnitHandle will return the handle ID or just return null, or if that handle ID is even "smart" and won't get recycled by the handle stack.

The thing is the "pop" method (named .removeEx in the latest update) which I added can return the previous unit OR the next unit. And you don't even need it. You can use the readonly variables ".nextReplaced" or ".prevReplaced" with the normal "remove" method.
 
Level 17
Joined
Apr 27, 2008
Messages
2,455
Well, what i meant is that the user should only use prev and next on loop, no more, keep it simple.
And i still find next and prev with an argument weird enough.
Also i want to keep the loop iterating as fast as possible.
You have no .count member, which is quite useful, ofc you can get it with a loop, but that is lame since the overheads for getting it in O(1) and easily are enough negligible.
Now, you can argue about the usefulness of a such member, but i really think that is useful, at least in spell making.

I don't have really thought about your code, but i don't see how you could properly update in real time, and reasonably, all UnitGroups when an unit is removed, without UnitIndexer (or whatever else).
 
Level 17
Joined
Apr 27, 2008
Messages
2,455
Backward compatibility is 100 % broken, i didn't hesitate because it's still a pending submission, nothing official, and some things were wrong.

* static method create takes nothing returns thistype :

Now, it doesn't take anymore a group argument.
Because let's face it, most of time after using a GroupEnum... function, you still need to filter the group.
If you want to add a whole group, there is still the method addGroup.

* method addGroup takes group whichGroup, boolean safety returns integer

Now, there is an additional boolean argument, if it's false then the group is added with a FOG loop, if true then a ForForce is used.
You shouldn't add a group which is kept during time, but this additional boolean woudn't hurt.

* new methods added

* debug stuff added

* you can't add anymore a ghost unit (it was possible with the method addUnit and a ghost unit variable)

* got rid of one hashtable member usage when i just could use a simple array instead

* documentation added
 

Bribe

Code Moderator
Level 50
Joined
Sep 26, 2009
Messages
9,464
Well, I could have written "nextOf" or "prevOf", but I wanted less verbosity.

As far as it goes with loading from hashtables, it does not handle ID memory. So even when I had a variable pointing to a unit, removed it and did a wait, the variable remembered its ID, but the hashtable did not.

Well the plan is still for me to use some kind of indexer if the user wants auto-cleanup, it would not be too hard it just would add a bit of O(# of groups the unit is in) overhead for the unit when it gets removed from the game.
 

Bribe

Code Moderator
Level 50
Joined
Sep 26, 2009
Messages
9,464
I think the concept of auto-removing is not too good, because sometimes if there is a spell being cast the user may want to do some final cleanup of their own to each unit, before they can be removed from the group, and if you have auto-cleanup then the user loses track of the unit completely. I think if the user doesn't want ghost units, he should have to have an "onDeindex" method himself, per struct. Which makes me think, maybe there is a module which can be added to one or both of these libraries, which includes support for an optional "onRemove" method so if a unit is found inside a group when it is removed, that method can be called and not cause problems.
 
Level 17
Joined
Apr 27, 2008
Messages
2,455
I don't think so, i can't think about a concrete case, where you want a ghost unit inside an UnitLL.
I mean, if the user needs it, the much better way is to handle the onRemove event with UnitIndexer, since it won't be delayed.

And anyway this is not possible with groups (there will be, but you can't enum them), and i've never heard complains about it so far.
 

Bribe

Code Moderator
Level 50
Joined
Sep 26, 2009
Messages
9,464
Well I can't imagine needing to store a group of units in the first place, to be able to iterate them, where storing integers in a linked list wouldn't be a better solution. And with UnitIndexer you can lock the index until you are done with it. So I think a combination of Dirac's LinkedListModule and UnitIndexer locks + GetUnitById will do the trick of both of our systems.

For this series in particular, maybe your phrase "Cool != Useful" should apply? I mean, UnitIndexer is already a requirement of this, and LinkedListModule would do the rest, so it could be this + UnitIndexer or UnitIndexer + LinkedListModule... and LinkedListModule is a bit more versatile.
 
Level 17
Joined
Apr 27, 2008
Messages
2,455
Well I can't imagine needing to store a group of units in the first place, to be able to iterate them, where storing integers in a linked list wouldn't be a better solution. And with UnitIndexer you can lock the index until you are done with it. So I think a combination of Dirac's LinkedListModule and UnitIndexer locks + GetUnitById will do the trick of both of our systems.

So graveyard this if the usefulness is not obvious for you, i don't want to argue more about this.
I've writed samples and documentation, i can't do more.

EDIT :

For this series in particular, maybe your phrase "Cool != Useful" should apply? I mean, UnitIndexer is already a requirement of this, and LinkedListModule would do the rest, so it could be this + UnitIndexer or UnitIndexer + LinkedListModule... and LinkedListModule is a bit more versatile.

This is not so easy, at least with implementing the same features.
Also, i have no use of LinkedListModule, coz linked lists are easy to inline and really you can't have only one module for all possible linked lists. For instance here, i needed both double circular linked list and not circular linked list, and the head being a pointer to the head, not just a boolean.
And there is no way that i would allow the user to edit .prev /next/last/first here, since it can't be easily edited without breaking the features.

I would be very disapointed if you reject it, but i would not be surprised, as we are very different in how we make and use scripts.
 
Last edited:

Bribe

Code Moderator
Level 50
Joined
Sep 26, 2009
Messages
9,464
Well you do provide compelling logic, because we have different approaches does not mean I should treat this unfairly and not let it get approved.

The only reason why I won't approve it yet is because I feel Cohadar's JassHelper is still "unnofficial". Like, you can use it personally but for public snippets it's not modular. I even required Dr Gester to upload a vJass version of his resource because it was originally written only in cJass. If you update your script to work with JassHelper 0.A.2.B (it is not hard to do), I will approve this.
 
Level 17
Joined
Apr 27, 2008
Messages
2,455
I really don't care that the new jasshelper break cJass, really cJass is already enough broken and pretty dead since a while.
I'm not bitching their work, but that's a fact, i've myself reported cJass bugs.

I like to support Cohadar, or any other one which is willing to update jasshelper.
I agree that new loops are not that needed, but the compile time is also faster, and the init order madness will be fixed in a near future if we believe Cohadar.

So here is the deal, i will remove the FOG loop, and once the init order madness will be fixed i will use it again, is it ok ?
 
Level 17
Joined
Apr 27, 2008
Messages
2,455
I think Dirac's problem is that :

I am better than you

Really, come on, i have already explained and even written in the main post and the documentation that obviously all methods are way slower than the natives groups alternatives.
Except on iterating (as you can't safely use a FOG loop, with a group which is kept during time, and btw unless you don't need the group anymore you will have to keep the track of units and add them later, kinda lame).
It also add more features, and again this is definetely not for instant enumeration (even if you can use the group spelled GROUP in this library for that).
Again, to be clear this is not for speedfreaks, it just makes the code easier to write for groups which need to be kept during time.
 
Last edited:
Level 17
Joined
Apr 27, 2008
Messages
2,455
Removed the Cohadar's fog loop, so it is compatible with the last official jasshelper.
I will use fog loop again, once the vJass initializer order madness will be fixed, as it will be a real improvement, but i recommend to already use Cohadar's jasshelper.
 
Level 17
Joined
Apr 27, 2008
Messages
2,455
http://www.wc3c.net/showpost.php?p=1133147&postcount=16

>>Ignitedstar

I'm afraid of updating to this JassHelper simply because I don't want to see my map malfunction... but as you say, it should be completely backwards compatible.

>> Cohadar

Don't trust me, trust the tests, I do automatic BINARY comparison of .j files generated by me and old jasshelper.
source\consolejasser\output\CompileAllTests.bat
source\consolejasser\output\RunAllTests.bat

---------

But new vJass keywords are reserved (in,for, to,while,endfor,endwhile), so you can't use them if it's not for the new loops.

There was a bug reported with new loops and functions interfaces, but it's fixed.
Now, nothing forbid you to compare yourself the code generated by the last official jasshelper and Cohadar's one.
And in case you find a bug, report it, but just not use it because you're afraid is kinda silly.
 

Bribe

Code Moderator
Level 50
Joined
Sep 26, 2009
Messages
9,464
Once there are more useful features, like Initialization fixes and the fix with ".name", I can request Ralle to update the PHP tags code.

Until then, it kinda has to be called as it is: unnofficial. Currently it pretty much offers some prettier compiling and prettier loops, but no extra utility. With the init/ExecuteFunc fix, all that will change.
 
Level 17
Joined
Apr 27, 2008
Messages
2,455
I still recommend it, but that's not a must.
I mean i suppose it's fine in the current state to don't use new loops in public resources, as it's not compatible with the last "official" jasshelper, but when the init order will be fixed, there will be no real reason to not support it in public resources. (users lazyness is not a good reason)
Now, even in the current state it's good to use it.
Also nothing forbids you to use it even without the new stuff, the compilation is faster and neater.
 
Top