Attachment System Challenge - KillCounter

[Flare]

Winers: (in chronological order)
Flare - Ugly solution with group and a timer, but at least it works
TheDamien - Solid solution with a timer but still not usable for serious mapping.
TheDamien-CSData - first usable solution that uses one standard system, CSData + group trick.
Group trick can be used with almost all attaching systems.
Grim001 - an inlined 0x100000 with a group trick.

I left out Cohadar from that since he used UserData ^^

 

[chobibo]

oh cool you're the grand prize winner LOL

 

[Cohadar]

Just look at first post and see who has most <3 and why.

EDIT:
I updated my PUI Tutorial using experience gained from this thread.

It is worth reading no matter what system you use.

 

[Strilanc]

Well at first I didn't think this could be done with ABC, HAIL, or HSAS because they store the handles and prevent them from expiring. The solutions so far rely on expiration and the unit group trick to avoid leaks. I don't really like that, since anything else using those systems needs to deal with garbage data coming out expired units in the kill counter (unit group trick doesn't work too well on triggers!).

In any case, it turns out I was wrong. It is possible to use systems that block expirations by using the GetUnitTypeId trick and sweeping over the units. Memory doesn't leak because any unit that expires will be cleaned when the sweep hits them.

library KillCounter initializer InitKillCounter uses HAIL
    globals
        private unit array sweepUnits
        private integer sweep = 0
        private integer numSweepUnits = 0
        private constant integer SWEEP_RATE = 2
    endglobals
    
    //! runtextmacro HAIL_CreateProperty("UnitKillCount", "integer", "")

    private function catchDeath takes nothing returns boolean
        local unit u = GetKillingUnit()
        local integer i
        if u == null then
            return false
        endif
        
        //increase kill count
        set i = GetUnitKillCount(u)
        call SetUnitKillCount(u, i+1)
        
        //insert first time units in sweep list
        if i == 0 then
            set sweepUnits[numSweepUnits] = u
            set numSweepUnits = numSweepUnits + 1
        endif

        //check some units in the sweep list for expiry
        set i = 1
        loop
            set u = sweepUnits[sweep]
            debug call BJDebugMsg("Checking " + I2S(sweep))
            if GetUnitTypeId(u) == 0 then
                debug call BJDebugMsg("Swept " + I2S(HAIL_H2I(u)))
                call ResetUnitKillCount(u)
                set numSweepUnits = numSweepUnits - 1
                set sweepUnits[sweep] = sweepUnits[numSweepUnits]
                set sweepUnits[numSweepUnits] = null
            else
                set sweep = sweep + 1
            endif
            if sweep >= numSweepUnits then
                set sweep = 0
            endif
            
            exitwhen i >= SWEEP_RATE
            set i = i + 1
        endloop

        set u = null
        return false
    endfunction
    
    private function InitKillCounter takes nothing returns nothing
        local trigger t = CreateTrigger()
        call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_DEATH)
        call TriggerAddCondition(t, Condition(function catchDeath))
    endfunction
endlibrary

*edit* Fixed it to sweep two units instead of one, making it work instead of following exactly one unit behind the last death. I actually discovered a little quirk in HAIL thanks to this. The low handles expire, but the high ones don't. A problem if you forget to reset. Also found out that you can make a collision train.

 

[Cohadar]

Using sweeps in every place you need unit attaching is not a very good idea.
And using some global sweep comes down to what PUI is doing internally.

So the final conclusion is that all attachments systems suck when it comes down to unit attaching. PUI is indexing system

 

[Strilanc]

Using sweeps in every place you need unit attaching is not a very good idea.
And using some global sweep comes down to what PUI is doing internally.

So the final conclusion is that all attachments systems suck when it comes down to unit attaching. PUI is indexing system

You think sweeping is expensive but let IsUnitInGroup off the hook? IsUnitInGroup is not a super-cheap call, and you still need to do it every time you read or write a kill count. Saying "sweeping is bad" then saying "PUI is best" is also all kinds of ironic when you explicitly asked for no custom data usage.

Actually, sweeping and the unit group trick are only necessary here because of a special circumstance: it's hard to detect when units are really gone. If you have a map with resurrect, raise dead, etc, even the maker probably can't do a good job telling your system when units are really gone. Essentially, for any other type people are expected to tell your system to clean up before they destroy the handle.

*heavy edit*

*edit* here's one that does less sweeping and still works

library KillCounter initializer InitKillCounter uses HAIL
    globals
        private unit array sweepUnits
        private integer sweep = 0
        private integer numSweepUnits = 0
    endglobals
    
    //! runtextmacro HAIL_CreateProperty("UnitKillCount", "integer", "")

    private function catchDeath takes nothing returns boolean
        local unit u = GetKillingUnit()
        local integer i
        if u == null then
            return false
        endif
        
        //increase kill count
        set i = GetUnitKillCount(u)
        call SetUnitKillCount(u, i+1)
        
        //insert first time units in sweep list
        if i == 0 then
            set sweepUnits[numSweepUnits] = u
            set numSweepUnits = numSweepUnits + 1
        endif

        //check some units in the sweep list for expiry
        loop
            //next
            set sweep = sweep - 1
            set u = sweepUnits[sweep]
            exitwhen GetUnitTypeId(u) != 0 or u == null
            //sweep
            debug call BJDebugMsg("Swept " + I2S(HAIL_H2I(u)))
            call ResetUnitKillCount(u)
            set numSweepUnits = numSweepUnits - 1
            set sweepUnits[sweep] = sweepUnits[numSweepUnits]
            set sweepUnits[numSweepUnits] = null
        endloop
        if sweep <= 0 then
            set sweep = numSweepUnits
        endif

        set u = null
        return false
    endfunction
    
    private function InitKillCounter takes nothing returns nothing
        local trigger t = CreateTrigger()
        call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_DEATH)
        call TriggerAddCondition(t, Condition(function catchDeath))
    endfunction
endlibrary

 

[Cohadar]

You think sweeping is expensive but let IsUnitInGroup off the hook? IsUnitInGroup is not a super-cheap call, and you still need to do it every time you read or write a kill count. Saying "sweeping is bad" then saying "PUI is best" is also all kinds of ironic when you explicitly asked for no custom data usage.

IsUnitInGroup has a log(n) complexity. What that means is that if there is 1024 units in group it will take 10 operations to execute it.
512 units = 9 operations
256 units = 8 operation
128 units = 7 operations
...
It is not that bad because it is done directly in game instead in jass but it still sux.

Your loops sucks for a completely different reason: It is too complicated.
You don't really expect people to write that monstrosity every time they want to attach something to unit.
And besides it is not without operation overhead either.

PUI needs only one operation for that.

Actually, sweeping and the unit group trick are only necessary here because of a special circumstance...

So I guess you now understand why ABC does not support unit attaching and why I made special system for that.

And PUI really is the best when it comes to units.
It is both simpler to use and faster that any other system.

I did start this thread to see if someone can come up with something better,
and so far no one did, not even close

 

[Strilanc]

You can't possibly be serious. You enforce much stronger restrictions and are surprised when what people come up with isn't more efficient? :nuts:

Do you expect people to write PUI every time they want to attach something? No, of course not. That's why PUI exists in the first place, to abstract that work away. Of course I don't expect people to write that loop every time they want to attach something to units, that loop belongs inside a system where people don't even have to know about it.

 

[Trollvottel]

so you DID start this thread to see if someone comes up with something better than PUI, but at the same time you wanted us to use systems like HAIL and HSAS to show nothing can be better than PUI? But that contradicts itself.:nuts: Anyway, i like PUI <3

 

[Magentix]

My attempt:

Works at first sight.
Only function you'll need is GetUnitKills(unit)

Combination of user-defined extended arrays and 0x100000

Spoiler

library CUK initializer Init

    // COUNT UNIT KILLS (CUK)
    // By: Magentix
    //
    
    // CONFIG:
    //
    // All you must do is set how many units this system
    // may monitor.
    //
    // IT MUST BE A POWER OF 2, larger than 2^3 (=8)!
    // If you write anything less than 8, you're a cunt.
    //
    // Note: the lower MAX_UNITS, the faster the system is.    

    globals
        // 2^14 --> Array size of 2048
        private constant integer MAX_UNITS = 16384 
    endglobals

    // USAGE:
    //
    // Use GetUnitKills(unit)-->integer
    //
    // P.S.: This resets a dead unit's kills to zero.
    // Can be changed in later versions
    
    //=======================================================
    // Do not touch anything beyond this point
    //=======================================================
    
    globals
        private constant integer ARRAYS_AMT = 8
        private constant integer ARRAY_SIZE = MAX_UNITS / ARRAYS_AMT
        
        private constant integer TRANSFER_1 = ARRAY_SIZE * 2
        private constant integer TRANSFER_2 = ARRAY_SIZE * 3
        private constant integer TRANSFER_3 = ARRAY_SIZE * 4
        private constant integer TRANSFER_4 = ARRAY_SIZE * 5
        private constant integer TRANSFER_5 = ARRAY_SIZE * 6
        private constant integer TRANSFER_6 = ARRAY_SIZE * 7
        
        private integer array KILLS_1[ARRAY_SIZE]
        private integer array KILLS_2[ARRAY_SIZE]
        private integer array KILLS_3[ARRAY_SIZE]
        private integer array KILLS_4[ARRAY_SIZE]
        
        private integer array KILLS_5[ARRAY_SIZE]
        private integer array KILLS_6[ARRAY_SIZE]
        private integer array KILLS_7[ARRAY_SIZE]
        private integer array KILLS_8[ARRAY_SIZE]
    endglobals
    
    // STFU Cohadar
    private function H2I takes handle h returns integer
        return h
        return 0
    endfunction
    
    // Assigns an array based on a modulo of 8.
    //
    // This way you don't cram the first -let's say- 80 values
    // into the first array, but divide it over all 8 arrays.
    //
    // This should increase performance on large arrays.
    private function GetArrayIndex takes unit u returns integer
        local integer i = H2I(u) - 0x100000
        return (i-(i/ARRAYS_AMT)*ARRAYS_AMT) * ARRAY_SIZE + (i/ARRAYS_AMT)
    endfunction

    private function GetKills takes integer i returns integer
        if i < TRANSFER_3 then
            if i < TRANSFER_1 then
                if i < ARRAY_SIZE then
                    return KILLS_1<i>
                else
                    return KILLS_2[i-ARRAY_SIZE]
                endif
            else
                if i &lt; TRANSFER_2 then
                    return KILLS_3[i-TRANSFER_1]
                else
                    return KILLS_4[i-TRANSFER_2]
                endif            
            endif
        else
            if i &lt; TRANSFER_5 then
                if i &lt; TRANSFER_4 then
                    return KILLS_5[i-TRANSFER_3]
                else
                    return KILLS_6[i-TRANSFER_4]
                endif
            else
                if i &lt; TRANSFER_6 then
                    return KILLS_7[i-TRANSFER_5]
                else
                    return KILLS_8[i-TRANSFER_6]
                endif            
            endif        
        endif
        
        return 0
    endfunction
    
    private function IncreaseKills takes integer i returns nothing
        if i &lt; TRANSFER_3 then
            if i &lt; TRANSFER_1 then
                if i &lt; ARRAY_SIZE then
                    set KILLS_1<i> = KILLS_1<i> + 1
                    return
                else
                    set KILLS_2[i-ARRAY_SIZE] = KILLS_2[i-ARRAY_SIZE] + 1
                    return
                endif
            else
                if i &lt; TRANSFER_2 then
                    set KILLS_3[i-TRANSFER_1] = KILLS_3[i-TRANSFER_1] + 1
                    return
                else
                    set KILLS_4[i-TRANSFER_2] = KILLS_4[i-TRANSFER_2] + 1
                    return
                endif            
            endif
        else
            if i &lt; TRANSFER_5 then
                if i &lt; TRANSFER_4 then
                    set KILLS_5[i-TRANSFER_3] = KILLS_5[i-TRANSFER_3] + 1
                    return
                else
                    set KILLS_6[i-TRANSFER_4] = KILLS_6[i-TRANSFER_4] + 1
                    return
                endif
            else
                if i &lt; TRANSFER_6 then
                    set KILLS_7[i-TRANSFER_5] = KILLS_7[i-TRANSFER_5] + 1
                    return
                else
                    set KILLS_8[i-TRANSFER_6] = KILLS_8[i-TRANSFER_6] + 1
                    return
                endif            
            endif        
        endif
    endfunction
    
    private function ResetKills takes integer i returns nothing
        if i &lt; TRANSFER_3 then
            if i &lt; TRANSFER_1 then
                if i &lt; ARRAY_SIZE then
                    set KILLS_1<i> = 0
                    return
                else
                    set KILLS_2[i-ARRAY_SIZE] = 0
                    return
                endif
            else
                if i &lt; TRANSFER_2 then
                    set KILLS_3[i-TRANSFER_1] = 0
                    return
                else
                    set KILLS_4[i-TRANSFER_2] = 0
                    return
                endif            
            endif
        else
            if i &lt; TRANSFER_5 then
                if i &lt; TRANSFER_4 then
                    set KILLS_5[i-TRANSFER_3] = 0
                    return
                else
                    set KILLS_6[i-TRANSFER_4] = 0
                    return
                endif
            else
                if i &lt; TRANSFER_6 then
                    set KILLS_7[i-TRANSFER_5] = 0
                    return
                else
                    set KILLS_8[i-TRANSFER_6] = 0
                    return
                endif            
            endif        
        endif
    endfunction
    
    private function Actions takes nothing returns nothing
        local unit KILLER  = GetKillingUnit()
        local unit DIER    = GetTriggerUnit()
        
        if KILLER != DIER then
            call IncreaseKills(GetArrayIndex(KILLER))
        endif
        
        call ResetKills(GetArrayIndex(DIER))
    endfunction
    
    
    //=======================================================
    // This is the only function you can use
    //=======================================================
    
    function GetUnitKills takes unit u returns integer
        return GetKills(GetArrayIndex(u))
    endfunction

    
    //=======================================================
    // Initializer, do not touch... Unless you&#039;re an epic cunt
    //=======================================================
    private function Init takes nothing returns nothing
        local trigger T = CreateTrigger()
        
        if MAX_UNITS &lt; 8 then
            call DestroyTrigger(T)
            set T = null
            return
        endif
        
        call TriggerRegisterAnyUnitEventBJ(T,EVENT_PLAYER_UNIT_DEATH)
        call TriggerAddAction(T,function Actions)
    endfunction
    
endlibrary</i></i></i></i>

 

[Strilanc]

You missed the invisible condition where you can't reset kills for the dying unit, since it might be revived or something. Also don't forget the condition where you can't leak. Those two are what make the problem hard.

 

[Strilanc]

How's this? The implementation:

library KillCounter initializer InitKillCounter uses DUI
    globals
        private integer array kills
    endglobals
    
    private function cleanKills takes nothing returns nothing
        set kills[DUI_index] = 0
    endfunction
    
    private function catchDeath takes nothing returns nothing
        local unit k = GetKillingUnit()
        local integer i
        if k != null then
            set i = DUI_GetUnitIndex(GetKillingUnit())
            set kills<i> = kills<i> + 1
            call BJDebugMsg(&quot;Kills: &quot; + I2S(kills<i>))
            set k = null
        endif
    endfunction
    
    private function InitKillCounter takes nothing returns nothing
        local trigger t = CreateTrigger()
        call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_DEATH)
        call TriggerAddAction(t, function catchDeath)
        call DUI_RegisterForCleanup(function cleanKills)
    endfunction
endlibrary</i></i></i>

and the library:

library DUI initializer initDUI
    globals
        private constant boolean SHOW_DEBUG_MESSAGES = true
        private constant boolean SHOW_ERROR_MESSAGES = true
    endglobals

    globals
        integer DUI_index //global argument for cleanup callback
    
        private integer num = 0 //number of active indexes
        private integer max = 0 //highest index given out so far
        private unit array units //list of indexed units
        private integer array indexes //list of given indexes
        
        private constant real SWEEP_PERIOD = 1.0 //how often swish is called
        private integer sweep = 0 //last swept index
        private trigger cleanTrigger = CreateTrigger() //storage for cleanup callbacks
    endglobals
    
    ///Checks one index for expiration
    private function swish takes nothing returns nothing
        local integer t
        if num > 0 then
            //next!
            set sweep = sweep - 1
            if sweep <= 0 then
                set sweep = num
            endif

            //check for expired unit
            if GetUnitTypeId(units[sweep]) == 0 then
                set DUI_index = indexes[sweep]
                //remove index
                debug if SHOW_DEBUG_MESSAGES then
                debug     call BJDebugMsg("DUI: Cleaned Index @" + I2S(indexes[sweep]) + " [" + I2S(num) + "]")
                debug endif
                call SetUnitUserData(units[num], sweep)
                set t = indexes[sweep]
                set indexes[sweep] = indexes[num]
                set indexes[num] = t
                set units[sweep] = units[num]
                set units[num] = null
                set num = num - 1
                //inform the outside
                call TriggerExecute(cleanTrigger)
            endif            
        endif
    endfunction
    
    ///Adds a callback for when indexes are swept
    function DUI_RegisterForCleanup takes code c returns nothing
        call TriggerAddAction(cleanTrigger, c)
    endfunction

    ///Gets a unique index for the given unit
    function DUI_GetUnitIndex takes unit u returns integer
        //get index
        local integer i = GetUnitUserData(u)
        //create index
        if units<i> != u then
            if units<i> != null then //this can&#039;t happen unless something else is changing unit data
                if SHOW_ERROR_MESSAGES then
                    call BJDebugMsg(&quot;DUI Error: Unit Custom Data was modified outside of DUI. (DUI_GetUnitIndex)&quot;)
                    return 0
                endif
            endif
            //check one index for cleanup
            call swish() //this stops the periodic sweeper from being overwhelemed by adding tons of units
            //get index
            set num = num + 1
            set i = num
            if num &gt; max then
                set max = num
                set indexes<i> = max
            endif
            //assign index
            set units<i> = u
            call SetUnitUserData(u, i)
            debug if SHOW_DEBUG_MESSAGES then
            debug     call BJDebugMsg(&quot;DUI: Created Index @&quot; + I2S(indexes<i>) + &quot; [&quot; + I2S(num) + &quot;]&quot;)
            debug endif
        endif
        //return index
        return indexes<i>
    endfunction
    
    ///Initializes the library
    private function initDUI takes nothing returns nothing
        local trigger t = CreateTrigger()
        call TriggerAddAction(t, function swish)
        call TriggerRegisterTimerEvent(t, SWEEP_PERIOD, true)
        
        debug if SHOW_DEBUG_MESSAGES then
        debug     call BJDebugMsg(&quot;Initialized DUI with SHOW_DEBUG_MESSAGES&quot;)
        debug endif
    endfunction
endlibrary
</i></i></i></i></i></i>

and a different implementation for a different library that works in a similar way but doesn't need custom data:

library KillCounter initializer InitKillCounter uses HAUL
    globals
        private integer array kills
    endglobals
    
    private function cleanDeath takes nothing returns nothing
        set kills[HAUL_index] = 0
    endfunction
    
    private function catchDeath takes nothing returns nothing
        local unit k = GetKillingUnit()
        local integer i
        if k != null then
            set i = HAUL_GetUnitIndex(GetKillingUnit())
            set kills<i> = kills<i> + 1
            call BJDebugMsg(&quot;Kills@&quot; + I2S(i) + &quot;: &quot; + I2S(kills<i>))
            set k = null
        endif
    endfunction
    
    private function InitKillCounter takes nothing returns nothing
        local trigger t = CreateTrigger()
        call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_DEATH)
        call TriggerAddAction(t, function catchDeath)
        call HAUL_RegisterForCleanup(function cleanDeath)
    endfunction
endlibrary
</i></i></i>

and THAT library:

library HAUL initializer initHAUL
    globals
        constant boolean HAUL_SHOW_DEBUG_MESSAGES = true
        constant boolean HAUL_SHOW_ERROR_MESSAGES = true
    endglobals

//===================================================================================================
//===================================================================================================
//===================================================================================================

    globals
        private constant integer TABLE_SIZE = 8191
        private constant integer MAX_UNITS = 8190
        
        integer HAUL_index = 0
        
        private integer numUnits = 0
        private unit array units
        private boolean array probed
        private trigger cleanTrigger = CreateTrigger()
    endglobals
    
    ///Returns the address of a handle
    private function H2I takes handle h returns integer
        return h
        return 0
    endfunction

    //! textmacro inc takes v
        if $v$ <= 0 then
            set $v$ = $v$ + TABLE_SIZE
        endif
        set $v$ = $v$ - 7
    //! endtextmacro
    //! textmacro dec takes v
        set $v$ = $v$ + 7
        if $v$ >= TABLE_SIZE then
            set $v$ = $v$ - TABLE_SIZE
        endif
    //! endtextmacro
    //! textmacro hash takes i
        set $i$ = $i$ - ($i$ / TABLE_SIZE) * TABLE_SIZE
    //! endtextmacro
    
    function check takes integer i returns nothing
        local integer j
        //=== REMOVE
        if GetUnitTypeId(units<i>) == 0 and units<i> != null then
            //clear
            set numUnits = numUnits - 1
            set units<i> = null
            //can only remove probes if the next index isn&#039;t probed
            set j = i
            //! runtextmacro inc(&quot;j&quot;)
            if not probed[j] then
                //remove the whole probe tail ending at i
                set j = i
                loop
                    exitwhen units[j] != null or not probed[j]
                    set probed[j] = false
                    //! runtextmacro dec(&quot;j&quot;)
                endloop
            endif
            //notify the outside to clean data attached to the index
            set HAUL_index = i
            call TriggerExecute(cleanTrigger)
            debug if HAUL_SHOW_DEBUG_MESSAGES then
            debug     call BJDebugMsg(&quot;HAUL: Destroyed Index @&quot; + I2S(i) + &quot; [&quot; + I2S(numUnits) + &quot;]&quot;)
            debug endif
        endif
    endfunction
    
    function HAUL_RegisterForCleanup takes code c returns nothing
        call TriggerAddAction(cleanTrigger, c)
    endfunction
    
    function HAUL_GetUnitIndex takes unit u returns integer
        local integer i = H2I(u)
        local integer f
        //! runtextmacro hash(&quot;i&quot;)
        //=== FIND (matching index, otherwise first unused index)
        //find the first index that matches or is empty or has an expired unit
        loop
            exitwhen units<i> == u or not probed<i> or GetUnitTypeId(units<i>) == 0
            //! runtextmacro inc(&quot;i&quot;)
        endloop
        //keep looking for a match
        if units<i> != u and probed<i> then
            //remember the first empty slot we met
            set f = i
            //find the first index that matches or is not part of a probe chain
            loop
                //check this slot for an expired unit
                call check(i)
                //next slot
                //! runtextmacro inc(&quot;i&quot;)
                //check for match
                exitwhen (units<i> == u) == probed<i>
            endloop
            if units<i> != u then
                //no match, go back to first empty slot we met
                set i = f
            endif
        endif
        
        //=== INSERT
        if units<i> != u then
            if numUnits &gt;= MAX_UNITS then
                if HAUL_SHOW_ERROR_MESSAGES then
                    call BJDebugMsg(&quot;HAUL ERROR: MAX UNITS EXCEEDED&quot;)
                endif
                return 0
            endif
            set numUnits = numUnits + 1
            set units<i> = u
            set probed<i> = true
            debug if HAUL_SHOW_DEBUG_MESSAGES then
            debug     call BJDebugMsg(&quot;HAUL: Created Index @&quot; + I2S(i) + &quot; [&quot; + I2S(numUnits) + &quot;]&quot;)
            debug endif
            //do some cleanup in some other parts of the table (avoid long probe chains)
            set f = i*i
            //! runtextmacro hash(&quot;f&quot;)
            call check(f)
            set f = f*f
            //! runtextmacro hash(&quot;f&quot;)
            call check(f)
        endif
        
        return i
    endfunction

    ///Initializes the library
    private function initHAUL takes nothing returns nothing
        debug if HAUL_SHOW_DEBUG_MESSAGES then
        debug     call BJDebugMsg(&quot;Initialized HAUL with SHOW_DEBUG_MESSAGES&quot;)
        debug endif
    endfunction
endlibrary
</i></i></i></i></i></i></i></i></i></i></i></i></i></i>

 

[Cohadar]

I had am unpublished version of PUI that exported cleanup trigger but when I used it in pyramidal defence it caused me some problems so I forsaken it.
(it only used conditions not actions)

Using Pivot arrays is better in any case.
The amount of code is same and there is no execution overhead.

Besides using "on cleanup" trigger is same as using "unit enters map" trigger
and since most maps already have "unit enters map" trigger it is better to put everything in there.
In fact since most of unit attaching is done by physics and damage detection engines that both have "unit enters map" triggers I put cleanup in there.

So the only case left are spells, and if you have 20 spells that all attach action to the cleanup trigger you are going to create some unneeded overhead there.
It is unneeded because not every unit that get's removed from game has attachments that need cleanup. (most have none in fact)
If you use Pivot arrays cleanup is called only when spell is called (if it is needed at all) which is much better.

You can of course create unit wrapper for that too and so on..

Read about all this techniques and how and when to use them in PUI Tutorial

 

[Magentix]

So, what do you think about my solution, Cohadar?

 

[Cohadar]

I don't even want to look at it until you remove that array mess.
vJass supports big arrays now, use them.

 

[Magentix]

That "mess" is there as an attempt to improve speed a bit.

Instead of cramming 80 units in one array, it divides them over 8 arrays.

After only 3 "if"s, the system knows what array to get the unit from and then only has to take 1/8th of the array-scrolling time to get the unit than it usually would.

This was implemented because large arrays are slowwwww


At least, that's how it theoretically works. Don't know if it actually improves speed or not.

 

[Cohadar]

It doesn't, 3 versions ago Vexorian implemented binary if-ing for arrays so using vJass is actually faster than what you have there.

 

[SerraAvenger]

Winers:

Is that a "whiners" or a "winners", or maybe something in between that just cannot decide?^^

Well anyway I'm currently not using H2I, as it was allready used in some of the other versions.
Might be a "bit" easier ; D I cold simply hash the returned integer and attach integers to it.

But I thought "no system twice", so I'm not using any lazy H2I exploits.

Spoiler

library Lolz initializer InitLolz needs Hash
globals
             unitsoftype array UnitsOfType
             hashtable         UnitTypeHash
endglobals

struct unitsoftype[ 65500 ]
private     unit    array units[ 100 ]
            integer array value[ 100 ]
private     integer numUnits
private     integer unittypeid
    
    static method create takes integer unittypeid returns unitsoftype
        local integer     index = UnitTypeHash.Search( unittypeid )
        local UnitsOfType new 
        if index == - 1 then
            return UnitsOfType[ index ]
        else
            set index = UnitTypeHash.Hash( unittypeid )            
            set new   = index

            set UnitsOfType[ index ] = new

            set new.numUnits   = -1
            set new.unittypeid = unittypeid
        endif
        return new
    endmethod
    
    method add takes unit toadd, integer unittypeid returns integer
        if unittypeid == .unittypeid then
            set .numUnits = .numUnits + 1
            if .numUnits > 100 then
                set .numUnits = .numUnits - 1
            else
                set .units[ .numUnits ] = toadd
                return .numUnits
            endif
        endif    
        return -1
    endmethod
    
    method find takes unit tofind, integer unittypeid returns integer
        local integer index = 0
        if unittypeid == .unittypeid then
            loop
                exitwhen index > .numUnits
                if .units[ index ] == tofind then
                    return index
                endif
                set index = index + 1
            endloop
        endif
        return -1
    endmethod
        
    method drop takes unit todrop, integer unittypeid returns nothing
        local integer index = .find( todrop, unittypeid )
        if index != -1 then
            set .units[ index ]     = .units[ .numUnits ]
            set .units[ .numUnits ] = null
            set .numUnits           = .numUnits - 1
        endif                        
    endmethod
        
endstruct
            
function SetUnitData takes unit whichunit, integer value returns nothing
    local integer     unittypeid = GetUnitTypeId( whichunit )
    local integer     hashindex  = UnitTypeHash.Search( unittypeid )
    local integer     unitindex
    local unitsoftype targetGroup
    
    if hashindex == - 1 then
        set targetGroup = unitsoftype.create( unittypeid )
    else
        set targetGroup = UnitsOfType[ hashindex ]
    endif
    set unitindex = targetGroup.find( whichunit, unittypeid )
    if unitindex == -1 then
        set unitindex = targetGroup.add( whichunit, unittypeid )
    endif
    set targetGroup.value[ unitindex ] = value
endfunction

function GetUnitData takes unit whichunit returns integer
    local integer     unittypeid = GetUnitTypeId( whichunit )
    local integer     hashindex  = UnitTypeHash.Search( unittypeid )
    local integer     unitindex
    local unitsoftype targetGroup
    
    if hashindex == - 1 then
        return 0
    else
        set targetGroup = UnitsOfType[ hashindex ]
    endif
    set unitindex = targetGroup.find( whichunit, unittypeid )
    if unitindex == -1 then
        
    endif
    return targetGroup.value[ unitindex ]
endfunction

private function InitLolz takes nothing returns nothing
    set UnitTypeHash = hashtable.create()
endfunction

endlibrary

Now here the more interesting version with the H2I exploit:

Spoiler

library UnitAttachmentSystem initializer InitUAS needs Hash

globals
    hashtable       H2IHash
    integer   array UnitValue
endglobals

private function H2I takes handle h returns integer
    return h
    return 0
endfunction


function SetUnitData takes unit whichunit, integer value returns nothing
    local integer handleId = H2I( whichunit )
    local integer index    = H2IHash.Search( handleId )
    if index == -1 then
        set index = H2IHash.Hash( handleId )
    endif
    set UnitValue[ index ] = value
endfunction

function GetUnitData takes unit whichunit returns integer
    local integer handleId = H2I( whichunit )
    local integer index    = H2IHash.Search( handleId )
    if index == -1 then
        return 0
    endif
    return UnitValue[ index ]
endfunction

private function InitUAS takes nothing returns nothing
    set H2IHash = hashtable.create()
endfunction

endlibrary

Spoiler

library Hash

// What it does
// The Hash system maps integers to slots in an array. This is extremely useful 
// when you are trying to attach something to integers that are greater than 8192.

// How To Use
// First you need to create a Hash Table. At most 25 HashTables can be created!
// local hashtable Table = hashtable.create()
// Now you can attach integers to that hashtable with the Hash method.
// The Hash function will return an integer. Use that integer as array-index when attaching the things to an integer.
// When you need the integer again, just use the Search method. It will return -1 if the item is not found, or the 
// right index elsewise.
// If the key is no longer needed, you can free it by using hashtable.Remove( key ).

// ---- EXAMPLE ----: This saves the Gold Costs of an Item.

//// library WeaponPrice initializer InitPrices needs Hash
//// globals
////     hashtable       WeaponData 
////     integer   array Price
//// endglobals
////
//// function InitPrices takes nothing returns nothing
////     set WeaponData = hashtable.create()
//// endfunction
////
//// function SetItemPrice takes integer itemtypeid, integer price returns nothing
////     local integer index = WeaponData.Hash( itemtypeid )
////     set   Price[ index ] = price
//// endfunction

//// function GetItemPrice takes integer itemtypeid returns integer 
////     local integer index = WeaponData.Search( itemtypeid )
////     if index == -1 then
////        return Price[ index ]
////     endif
////     return 0                         // Return 0 if the item cannot be found
//// endfunction

//// endlibrary


////  ---- API ----
//  1. create
//  2. Hash / HashEx
//  3. Search        
//  4. Remove
//  5. Refresh
//  6. HashSize
                                                                           
//  1. --- method --- hashtable.create( nothing ) returns hashtable

// Creates a new Hashtable.
// Can be used to hash huge integers to a much smaller Universe.
// Useful for hashing ability codes to a normal JASS array.

//  2. --- method --- hashtable.Hash  ( key )         returns integer
//     --- method --- hashtable.HashEx( key, target ) returns nothing

// Hashes a key to a slot. 
// The standard Hash function will return a unique index for each unique key.
// The returned slot will stay the same during one game, but may differ from
// game to game.
// The Extended Hash function ( HashEx ) will allways map the key to the target slot.
// Thus you can specify the

//  3. --- method --- hashtable.Search( key ) returns integer

// Returns the slot previously mapped to the key. 
// If the key was not hashed yet, -1 will be returned.

// Example:

//// function TestSearch takes nothing returns boolean
////     local integer indexA = Data.Hash  ( 5023 )
////     local integer indexB = Data.Search( 5023 )
////     return indexA == indexB // <---- TRUE
//// endfunction
                         
//// function TestFalseSearch takes nothing returns boolean
////     local integer indexA = Data.Hash  ( 5023 )
////     local integer indexB = Data.Search( 5774 )
////     return indexA == indexB // <---- FALSE, as indexB is -1
//// endfunction
                                                  
                                                                           
//  4. --- method --- hashtable.Remove( key ) returns hashtable

// Recycles a key from the hashtable when no longer needed.
// Used to free slots in the hashtable to allow for more keys to be hashed.
// Recycled slots do not increase the search performance!
// Automatically applies Refresh ( see point 5 )
//  when more than 1/3rd of the Hashtable was recycled.
// Please allways change your hashtable variable to the returned hashtable
//  to prevent data loss.

// Example:

////
//// function Test2 takes nothing returns boolean
////     set Data = Data.Remove( 5023 ) // Needs to be done. If Data would not be set to the new one,
                                        //  the table might be refreshed resulting in a complete data loss!
////     return Data.Search( 5023 ) == - 1  // <--- TRUE
//// endfunction

//  5. --- method --- hashtable.Refresh( nothing ) returns hashtable

// Creates a new Hashtable and hashes all values from the old one to the new one.
// During this process, the old Hashtalble is destroyed.
// Accomplishes the performance loss when there were too many key deletions.
// Automatically applied when more than 1/3rd of the Hashtable was recycled.
// Please allways change your hashtable variable to the returned hashtable
//  to prevent data loss.

// 6. --- constant --- HashSize is integer

// The maximum number of keys that can be saved to a HashTable.
// A high number allows for more slots and makes the Table faster,
// but can also result in weak memory usage.
// Primes work best as Hashsizes.






globals
    // settings
    constant integer HashSize = 113 // Please enter a prime here that is as close to the number of slots you need as possible.
    
    // End of settings. Please do not apply changes to the following variables.                                                
    constant real    Multiplicator = ( SquareRoot( 5 ) - 1 ) / 2
    constant integer HashSlots = 25000
endglobals
             

private function KeyHash takes integer key returns integer
    local  real product = key * Multiplicator
    return R2I( HashSize * ( product - R2I( product ) ) )
endfunction

private function SlotHash takes integer key returns integer
    return ModuloInteger( key * 2, HashSize - 1 ) + 1
endfunction

struct hashtable[ HashSlots ]
    integer array Key [ HashSize ]
    integer array Slot[ HashSize ]
    
    integer array Base  [ HashSize ]
    integer array BaseId[ HashSize ]
    integer       keys
    
    integer       recycled = 0
    
    static method create takes nothing returns hashtable
        local hashtable new = hashtable.allocate()
        local integer index = 0
        set   new.keys = 0
        loop
            exitwhen index > HashSize
            set new.Key[ index ] = -1
            set index = index + 1
        endloop
        return new
    endmethod
                
       
    
    method Hash takes integer key returns integer
        local integer loopId   = 0
        local integer keyhash  = KeyHash ( key )
        local integer slothash = SlotHash( key )
        local integer slot     = keyhash
        loop
            exitwhen .Key[ slot ] == -1
            exitwhen .Key[ slot ] ==  0

            if .Key[ slot ] == key then
                return slot            
            elseif loopId > 10 then
                return -1
            endif
            
        
            set loopId = loopId  + 1
            set slot   = ModuloInteger( keyhash + loopId * slothash, HashSize )
        endloop
        set .Key [ slot ] = key
        set .Slot[ slot ] = slot
        
        set .BaseId[  slot ] = .keys
        set .Base  [ .keys ] =  key
        set .keys = .keys + 1
        return slot
    endmethod
    
    
    
    method HashEx takes integer key, integer target returns nothing
        local integer loopId   = 0
        local integer keyhash  = KeyHash ( key )
        local integer slothash = SlotHash( key )
        local integer slot     = keyhash
        loop
            exitwhen .Key[ slot ] == -1
            exitwhen .Key[ slot ] ==  0
            
            if .Key[ slot ] == key then
                return
            elseif loopId > 10 then
                return
            endif
            
        
            set loopId = loopId  + 1
            set slot   = ModuloInteger( keyhash + loopId * slothash, HashSize )
        endloop
        set .Key [ slot ] = key
        set .Slot[ slot ] = target
        
        set .BaseId[  slot ] = .keys
        set .Base  [ .keys ] =  key
        set .keys = .keys + 1
    endmethod

    method Search takes integer key returns integer
        local integer loopId   = 0
        local integer keyhash  = KeyHash ( key )
        local integer slothash = SlotHash( key )
        local integer slot     = keyhash
        loop
            exitwhen .Key[ slot ] == key            
            if .Key[ slot ] == -1 then
                return -1
            elseif loopId > 10 then
                return -1
            endif
            
            set loopId = loopId  + 1
            set slot   = ModuloInteger( keyhash + loopId * slothash, HashSize )
        endloop
        return .Slot[ slot ]
    endmethod
    
    method Refresh takes nothing returns hashtable
        local hashtable new      = hashtable.create()
        local integer   keyIndex = 0
        local integer   newSlot
        local integer   oldSlot
        local integer   curKey
        
        loop
            exitwhen keyIndex > .keys
            set curKey   =    .Base  [ keyIndex ]
            set newSlot  = new.Hash  ( curKey   )  
            set oldSlot  =    .Search( curKey   )
            set new.Slot[ newSlot ] = oldSlot             
            set keyIndex = keyIndex + 1            
        endloop
        call .destroy()
        return new
    endmethod   
    
    method Remove takes integer key returns hashtable
        local integer slot = .Search( key )

        set .Key [ slot ] =  0
        set .Slot[ slot ] = -1
        
        set .Base  [ .BaseId[ slot ] ] = .Base[ .keys ]
        set .BaseId[ slot ] = .BaseId[ .keys ]

        set .Base  [ .BaseId[ .keys ] ] = -1
        set .BaseId[ .keys ] = -1 
        set .keys  = .keys - 1
        
        set .recycled = .recycled + 1
        if .recycled > HashSize / 3 then
            return .Refresh()
        endif
        return this
    endmethod

endstruct

endlibrary

EDIT: note that the UAS would need a higher HashSize than 113. 7001 might be suitable.

 

[Strilanc]

I had am unpublished version of PUI that exported cleanup trigger but when I used it in pyramidal defence it caused me some problems so I forsaken it.
(it only used conditions not actions)

Any more details about this? The main issue I see is that someone may call the library from the callback in Get and cause reentrancy problems.

Using Pivot arrays is better in any case.
The amount of code is same and there is no execution overhead.

The overhead is simply moved from cleanup to initialization

Besides using "on cleanup" trigger is same as using "unit enters map" trigger
and since most maps already have "unit enters map" trigger it is better to put everything in there.
In fact since most of unit attaching is done by physics and damage detection engines that both have "unit enters map" triggers I put cleanup in there.

We weren't allowed to catch those events. In any case if a non-unit gets the handle id and is permanent, you leak any attached handle data like timers.

So the only case left are spells, and if you have 20 spells that all attach action to the cleanup trigger you are going to create some unneeded overhead there.
It is unneeded because not every unit that get's removed from game has attachments that need cleanup. (most have none in fact)
If you use Pivot arrays cleanup is called only when spell is called (if it is needed at all) which is much better.

If your spell doesn't need cleanup then you don't register for the callback. Once again, we're just moving the '20 calls' overhead from cleaning to when a unit enters the map.

DUI also has other advantages. It does much less processing per second (PUI does up to 512 checks per second! totally unreasonable!), and it can detect when unit custom data is modified. It also doesn't contain testing code that belongs outside the library.

In any case only the second library is really 'admissible' here, since the first one uses unit custom data. But maybe the first one will give you a kick to improve PUI.

 

[Cohadar]

The overhead is simply moved from cleanup to initialization
...
If your spell doesn't need cleanup then you don't register for the callback. Once again, we're just moving the '20 calls' overhead from cleaning to when a unit enters the map.

You are not getting it right, you mixed up spell attachments with those of damage detection system. Damage detection attachments are mandatory for all units on the map so moving overhead around has no impact. (I already explained this as a reason for not exporting trigger from PUI)
But spell unit attachments are not mandatory and treating them as they are creates overhead.

We weren't allowed to catch those events.

That contest is over we talk as developers now...

In any case if a non-unit gets the handle id and is permanent, you leak any attached handle data like timers.

Only if you use HAIL, PUI does not have that problem.
(because PUI depends on it's own custom indexes and not on unit handle)

DUI also has other advantages. It does much less processing per second (PUI does up to 512 checks per second! totally unreasonable!),

There is a thing called configuration constant you know...

and it can detect when unit custom data is modified.

bad solution as I already explained

It also doesn't contain testing code that belongs outside the library.

It is marked properly as debug.

But maybe the first one will give you a kick to improve PUI.

PUI cannot be improved, it is perfect unit indexing.

=================
You seem to be a little hard in the head and like to answer before you think.
(which is also obvious from our PM discussion)
I am tired of always saying same thing three times so you can finally get it right.

=================
To all others:
This "contest" is over, it served it's purpose as educational confrontation.
No more submissions will be processed.