[System] StructuredDD (Structured Damage Detection)

Cokemonkey11

Code Reviewer
Level 28
Joined
May 9, 2006
Messages
3,461
[System] StructuredDD

StructuredDD

Preface:

StructuredDD is a damage detection system which enables users to register a pseudo-generic "unit damaged" event. Many systems exist to accomplish the same result, but the intended design paradigms represented by StructuredDD make it unique:

  • Code Simplicity: StructuredDD is well commented and is self-documented. The design concept is easy to understand for programmers and JASS users with intermediate experience.
  • Fully independent of UnitUserData. Many inexperienced spell programmers use UnitUserData to create their abilities, and I want to ensure that it is always available for that.
  • Cohesion: The system does exactly what it intends to do, and nothing more. There are no required libraries, and any additional functionality is handled in separate components.

Design Explanation:

  1. User defines one or more code to be executed on damage detection
  2. User indexes units they want to be used for damage detection (alternatively StructuredDD can auto-index all units)
  3. StructuredDD handles instances of detection triggers, each tied to a "bucket" of units
  4. StructuredDD automatically checks each trigger periodically if the entire bucket contains null units before destroying the trigger

Limitations:

  • StructuredDD requires vJass
  • There is currently no support for deallocating handlers for dynamic use
  • There is currently no support for periodic cleanup timeouts that self-optimize.
  • There is currently no support for "long-term" unit buckets built to handle units that aren't removed from the game (for example heroes)
  • There is no included "safe" method for damaging a unit - it is trivial for the client to do this in their own handler(s).
  • There is no included method to check for physical/spell/code damage - the computational cost for such systems is a higher order of magnitude than the system itself, thus these systems are built as extensions.
  • Handlers are added in order to an array and are executed in order. If you have handlers that depend on each other, it is your job to avoid race conditions.

The script:

JASS:
// StructuredDD is a damage detection system for handling the generic "unit is
// damaged" event response. The API follows:
//
//
//      StructuredDD.addHandler(function f)
//
//          * Registers function f as a callback function, which will occur
//            whenever a unit, which has been added to the system, is damaged.
//          * In the context of a handler function f, use standard jass natives
//            like:
//                - GetTriggerUnit()       - returns the damaged unit
//                - GetEventDamageSource() - returns the attacking unit
//                - GetEventDamage()       - returns how much damage was dealt
//
//
//      StructuredDD.add(unit u)
//
//          * Manually adds a unit the damage detection system, so that all
//            damage handlers are called when it receives damage.
//          * Note that if `ADD_ALL_UNITS` is enabled, there is no need to
//            manually add a unit.
//
//
library StructuredDD
    globals

        ///////////////////////////////////////////////////////////////////////
        //                            CONFIGURATION
        ///////////////////////////////////////////////////////////////////////

        // When enabled, all units in the map will be added to the damage
        // detection system.
        private constant boolean ADD_ALL_UNITS = true

        // The bucket size determines how many units should be added to each
        // damage detection bucket. Many variables impact the performance
        // for this process. If in doubt, use a number between 10 and 30.
        private constant integer BUCKET_SIZE = 20

        // Controls the period, in seconds, for the vacuum interval. Higher
        // values will have higher success rate, but may become more costly. If
        // in doubt, use a number between 30. and 180.
        private constant real PER_CLEANUP_TIMEOUT = 60.

        ///////////////////////////////////////////////////////////////////////
        //                          END CONFIGURATION
        ///////////////////////////////////////////////////////////////////////

    endglobals


    // A bucket represents a fragment of the units that are managed by the
    // damage detection engine. Essentially, each bucket has a trigger and a
    // set of units. When all the units in the bucket die, the trigger is
    // destroed. This allows more granular recycling of triggers, rather than
    // rebuilding one huge trigger synchronously, as traditional methods do.
    private struct bucket
        integer bucketIndex = 0
        trigger trig = CreateTrigger()
        unit array members[BUCKET_SIZE]
    endstruct


    // The StructuredDD struct is just a wrapper, which provides the API with
    // "dot" syntax.
    struct StructuredDD extends array

        // The conditions array manages *all* conditions in the context of the
        // damage detector. When a new bucket is instantiated, all conditions
        // are added to it. When a condition is added, all existing bucket's
        // triggers receive the condition.
        private static boolexpr array conditions

        private static bucket   array bucketDB

        private static integer conditionsIndex = -1
        private static integer dbIndex         = -1
        private static integer maxDBIndex      = -1

        // This auxilliary method is used for getting the next available
        // bucket. Since buckets get units added to them one at a time, but
        // have capacity for many, this method arbitrates when to use the last
        // bucket and when to build a new one.
        private static method getBucket takes nothing returns integer
            local integer index    =  0
            local integer returner = -1
            local bucket tempDat

            if thistype.dbIndex != -1 and thistype.bucketDB[thistype.dbIndex].bucketIndex < BUCKET_SIZE then

                // A non-full bucket context is already known, so use it.
                return thistype.dbIndex
            else

                // This is either the first bucket requested, or the last
                // bucket used is now full - instantiate a new one.
                set thistype.maxDBIndex = thistype.maxDBIndex + 1
                set thistype.dbIndex = thistype.maxDBIndex
                set tempDat = bucket.create()
                set thistype.bucketDB[.maxDBIndex] = tempDat

                // Add all known handlers to the new bucket.
                loop
                    exitwhen index > thistype.conditionsIndex

                    call TriggerAddCondition(tempDat.trig, thistype.conditions[index])

                    set index = index + 1
                endloop

                return thistype.dbIndex
            endif

            // This line never executes, but some versions of JassHelper will
            // flag an error without it.
            return -1
        endmethod

        // Adds a new "handler" function to the list of functions that are
        // executed when a unit is damaged. Adding a handler will immediately
        // enable it in all buckets. Removing handlers is not supported, so
        // this should not be used dynamically.
        public static method addHandler takes code func returns nothing
            local bucket tempDat
            local integer index = 0

            set thistype.conditionsIndex = thistype.conditionsIndex + 1
            set thistype.conditions[thistype.conditionsIndex] = Condition(func)

            // Immediately add this new handler to all buckets.
            loop
                exitwhen index > thistype.maxDBIndex

                set tempDat = thistype.bucketDB[index]
                call TriggerAddCondition(tempDat.trig, thistype.conditions[thistype.conditionsIndex])

                set index = index + 1
            endloop
        endmethod

        // Adds a unit to the damage detection system using some bucket. When
        // the unit dies or is removed, the bucket will eventually empty and
        // get recycled. If you enable the `ADD_ALL_UNITS` configuration
        // variable, then there is no need to use this method.
        public static method add takes unit member returns nothing
            local bucket tempDat
            local integer whichBucket = thistype.getBucket()

            set tempDat = thistype.bucketDB[whichBucket]
            set tempDat.bucketIndex = tempDat.bucketIndex+1
            set tempDat.members[tempDat.bucketIndex] = member

            // When a unit is added to a bucket, the event for it is
            // immediately enabled.
            call TriggerRegisterUnitEvent(tempDat.trig,member, EVENT_UNIT_DAMAGED)
        endmethod

        // This auxilliary method is part of the implementation for
        // `ADD_ALL_UNITS`. It adds a unit to the damage detection context,
        // when triggered below.
        static if ADD_ALL_UNITS then
            private static method autoAddC takes nothing returns boolean
                call thistype.add(GetTriggerUnit())

                return false
            endmethod
        endif

        // This auxilliary method is used to check if a bucket is empty, and
        // is used to arbitrate when a bucket (and its associated trigger) can
        // be deallocated. This occurs as part of the periodic cleanup system
        // and can be disabled.
        private static method bucketIsEmpty takes integer which returns boolean
            local bucket tempDat = thistype.bucketDB[which]
            local integer index = 0

            loop
                exitwhen index == BUCKET_SIZE

                // If a unit is removed, it's `TypeId` will return 0, at least
                // for some period before its pointer leaves cache or is
                // reused.
                if GetUnitTypeId(tempDat.members[index]) != 0 then
                    return false
                endif

                set index = index + 1
            endloop

            return true
        endmethod

        // This method is called periodically and checks the buckets contents,
        // and recycles them if they are empty. A better implementation would
        // cycle through one bucket a time per iteration, rather than
        // synchronously iterating through all buckets.
        private static method perCleanup takes nothing returns nothing
            local integer index = 0

            loop
                exitwhen index > thistype.maxDBIndex

                if index != thistype.dbIndex and thistype.bucketIsEmpty(index) then

                    // The bucket at this index is empty, so begin
                    // deallocating.
                    call DestroyTrigger(thistype.bucketDB[index].trig)
                    call thistype.bucketDB[index].destroy()

                    set thistype.bucketDB[index] = thistype.bucketDB[thistype.maxDBIndex]
                    set thistype.maxDBIndex = thistype.maxDBIndex - 1

                    if thistype.maxDBIndex == thistype.dbIndex then
                        set thistype.dbIndex = index
                    endif

                    set index = index - 1
                endif

                set index = index + 1
            endloop
        endmethod

        // This struct initialization method is necessary for setting up the
        // damage detection system.
        private static method onInit takes nothing returns nothing
            local group   grp
            local region  reg
            local trigger autoAddUnits
            local timer   perCleanup
            local unit    FoG

            // If the `ADD_ALL_UNITS` configuration is enabled, turns on a
            // trigger which allocates all new units to a bucket. Also checks
            // units that are on the map at initialization.
            static if ADD_ALL_UNITS then

                // Add pre-placed units.
                set grp = CreateGroup()
                call GroupEnumUnitsInRect(grp, bj_mapInitialPlayableArea, null)

                loop
                    set FoG = FirstOfGroup(grp)
                    exitwhen FoG == null

                    call thistype.add(FoG)

                    call GroupRemoveUnit(grp,FoG)
                endloop

                // Add units that enter the map using a trigger.
                set autoAddUnits = CreateTrigger()
                set reg = CreateRegion()

                call RegionAddRect(reg, bj_mapInitialPlayableArea)
                call TriggerRegisterEnterRegion(autoAddUnits, reg, null)
                call TriggerAddCondition(autoAddUnits, Condition(function thistype.autoAddC))

                set autoAddUnits = null
                set reg = null
            endif

            // Enable the periodic cleanup module, which vacuums each bucket
            // periodically.
            set perCleanup = CreateTimer()
            call TimerStart(perCleanup, PER_CLEANUP_TIMEOUT, true, function thistype.perCleanup)

            set perCleanup = null
        endmethod
    endstruct
endlibrary

Example Test Script:

JASS:
// This script requires that StructuredDD.ADD_ALL_UNITS is enabled.
scope Test initializer init
    private function h takes nothing returns nothing
        local string attackerName = GetUnitName(GetEventDamageSource())
        local string damage       = R2S(GetEventDamage())
        local string struckName   = GetUnitName(GetTriggerUnit())

        call DisplayTextToPlayer(GetLocalPlayer(), 0., 0., attackerName + " dealt " + damage + " to " + struckName)
    endfunction

    private function init takes nothing returns nothing
        call FogMaskEnable(false)
        call FogEnable(false)

        call StructuredDD.addHandler(function h)
    endfunction
endscope

Change Log:


2015.09.07 - fixed up white space and improved documentation
2013.05.25 - removed UnitAlive declaration and reduced length of processed code using static if
2013.05.24 - made one small change (StructuredDD extends array) which will reduce the size of the compiled code. This change does not affect the API.
2013.05.13 - updated the API and documentation
2013.01.18 - big update - updated API, removed some components, fixed a massive bug. Please note that I have also stopped maintaining the version pastebin mirrors as they were a waste of time to maintain.
2012.06.03 - Updated the null check to use GetUnitTypeId() which fixes a deindexing bug where DEAD_UNITS_ARE_NULL was false. The script now declares native UnitAlive, thus IsUnitType() has been replaced with UnitAlive(). Will revert this change if it is not recommended.
2012.06.02 - The periodic cleanup now uses a timer instead of a trigger for efficiency reasons.
2012.06.01 - Updating the script to follow proper CamelCase. The API has also been changed to reflect only 1 use of addHandler, which takes code as an argument as suggested by fellow hive users. Also fixed two major bugs in the decrement portion of the stack.
2012.05.30 #2 - Updated submission with additional method and changed constant nomenclature.
2012.05.30 - Initial submission to Hive: Jass Resources: Submissions


Special Thanks:

  • PurplePoot who thought of bucket-based damage detection systems and created it in the first place, but never released his version.
  • Nestharus and Troll-Brain for explaining how to use GetUnitTypeId() for null checks.
  • -Kobas- for creating a map submission template, which turned out useful for system submissions as well.
  • Vexorian for developing JassHelper. Without vJass, I almost certainly would not still be scripting for wc3.
  • The various developers of JNGP including PitzerMike and MindworX.
 
Last edited:

Bribe

Code Moderator
Level 45
Joined
Sep 26, 2009
Messages
9,043
Please remove your signature. Also, APILIKETHISISHARDLYREADABLE, IT_IS_MUCH_MORE_READABLE_LIKE_THIS. Also, boolexpr's as an argument is pretty ugly syntax, I recommend using code arguments and convert them to boolexpr's yourself, saving the user the workload. Also, bj_mapInitialPlayableArea does not cover the whole map where units may be and you also miss out on Locust units this way (not that you need them for a damage detection system, but it's possible). Basing this off an indexer makes a lot more sense.
 

Cokemonkey11

Code Reviewer
Level 28
Joined
May 9, 2006
Messages
3,461
Thanks for the reply.

Also, boolexpr's as an argument is pretty ugly syntax, I recommend using code arguments and convert them to boolexpr's yourself, saving the user the workload.

That's your opinion. Blizzard seems to disagree since they take boolexpr's as arguments, but regardless I've added a new method to the API which takes code as an argument instead.

Also, bj_mapInitialPlayableArea does not cover the whole map where units may be and you also miss out on Locust units this way (not that you need them for a damage detection system, but it's possible).

The ADD_ALL_UNITS functionality is just for users who want to make using my system easy. If they are doing something complex like locust units outside the initial playable map are on initialization, they can write their own "add all units" script. Regardless, I've added this as a note in the API to save users the trouble of reading my script.

Basing this off an indexer makes a lot more sense.

I don't use indexing systems (and neither should you). This script is open source however, if you'd like to change it, you're more than welcome.


---

Again, thanks for the comments. (+reps of course)
 

Bribe

Code Moderator
Level 45
Joined
Sep 26, 2009
Messages
9,043
>> Blizzard seems to disagree since they take boolexpr's as arguments

If you are not using the true or false utility of boolexpr's, accept code as the argument.

>> I don't use indexing systems (and neither should you).

Indexing systems are great. You have provided no reason why you nor I shouldn't use indexing systems. GUI Unit Indexer has been a really great help for lots of users recently, which was unprecedented seeing as how Jesus4Lyf's "GUI AIDS" never really took off.
 

Cokemonkey11

Code Reviewer
Level 28
Joined
May 9, 2006
Messages
3,461
If you are not using the true or false utility of boolexpr's, accept code as the argument.

Yep I've added that to the API as I said. But I have no plans to use TriggerAddAction. If you want to use TSA in your DD handlers, don't use this script.

Indexing systems are great. You have provided no reason why you nor I shouldn't use indexing systems. GUI Unit Indexer has been a really great help for lots of users recently, which was unprecedented seeing as how Jesus4Lyf's "GUI AIDS" never really took off.

Because unit indexing systems use unit custom value and/or hashtables. My system isn't intended for "new" or "casual" developers, but instead for people who want to use efficient vJass. Chances are the users of this system know how to make it themself, but simply don't want to spend the time (I know I've been putting this off for awhile)
 

Bribe

Code Moderator
Level 45
Joined
Sep 26, 2009
Messages
9,043
>> But I have no plans to use TriggerAddAction.

You don't need to. Trigger conditions can return nothing and therefore give the same speed benefit. However, when using vJass which is slaved to pJass, pJass blocks those kind of "returns nothing" conditions from compiling. The way to bypass it is to pass the code argument to a function and that function calls the "Filter/Condition" native to compile it without pJass' obsolete safety. Thus it is less verbose to the user and preserves conditions as replacements to actions.

>> My system is ... for people who want to use efficient vJass.

UnitUserData / hashtables are too slow to be useful? Don't go down this slippery slope of efficiency. Once you crop you can't stop. Seriously, no one cares. Except Nestharus but he really is addicted to it. Your system is not the most efficient anyway, you might as well focus on more important areas.
 

Cokemonkey11

Code Reviewer
Level 28
Joined
May 9, 2006
Messages
3,461
You don't need to. Trigger conditions can return nothing and therefore give the same speed benefit. However, when using vJass which is slaved to pJass, pJass blocks those kind of "returns nothing" conditions from compiling. The way to bypass it is to pass the code argument to a function and that function calls the "Filter/Condition" native to compile it without pJass' obsolete safety. Thus it is less verbose to the user and preserves conditions as replacements to actions.

That's quite nice to know actually, can you show me how in an example?

UnitUserData / hashtables are too slow to be useful? Don't go down this slippery slope of efficiency. Once you crop you can't stop. Seriously, no one cares. Except Nestharus but he really is addicted to it. Your system is not the most efficient anyway, you might as well focus on more important areas.

UnitUserData is too limited because it only supports 1 value per unit.

Hashtables are too slow for unit control and spells. They are useful in other ways, like when O(n) is slower than the hashtable counter-part, but for my own preferences I'm going to stick with my linear stacks.

I understand you don't care about efficiency (Or you wouldn't still be making systems for GUI) but that doesn't mean that "no one" cares. Seriously don't generalize stuff like that.
 

Bribe

Code Moderator
Level 45
Joined
Sep 26, 2009
Messages
9,043
>> Hashtables are too slow for unit control and spells.

I use hashtables pretty frequently in Retro for really comprehensive data, my poor-grade Intel Atom N450 can handle it even on the scale of 100+ units without dropping in FPS.

>> UnitUserData is too limited because it only supports 1 value per unit.

Too limited? It provides an array index for every unit, it is a supplementary utility. You don't need more than one UnitUserData.

>> When O(n) is slower than the hashtable counter-part.

Which is essentially every time "n" exceeds 2 or 3. JASS reading is extremely "slow" thanks to it being an interpreted language. Calling hashtables slow is like saying the FLASH is slower than SUPERMAN. Who cares? Which brings us to...

>> That doesn't mean that "no one" cares.

No one cares about a 0.0000001% margin, which is the case here. If people think they care, they are overanalyzing and need to look at real-world scenarios. It's ridiculous percentiles like this that allows Intel to sell i3-2130's at a price difference 5x the difference between the 2100 and the 2120, despite providing only half the edge in performance (a 100MHz overclock instead of a 200MHz overclock costing $25 more instead of $5 more resulting in a huge, totally worth the money 3% performance gain).
 
Here's an example:

JASS:
function Hi takes nothing returns nothing


endfunction

// ...
local code c = function Hi
call TriggerAddCondition(t, Filter(c))
// ...

The compiler can't stop you from doing this since you're passing a variable ;)
Of course, when the code is a parameter, you would need to make your function 2 lines long (add a return statement after the actual code) so that it wouldn't inline.

If it inlines, pJass says "Woah bro, that function doesn't return a boolean. You're in big trouble Mister."
 

LeP

LeP

Level 11
Joined
Feb 13, 2008
Messages
516
However, when using vJass which is slaved to pJass, pJass blocks those kind of "returns nothing" conditions from compiling.

Someone counted how often i mentioned Zoxcs Jass Parser?
Or how to use it.

But this is new: using my incredible google-fu i ofc found a valid download of the cli-parser here.

And ofc you could always compile it from source.

Or change pJass' source.
 

LeP

LeP

Level 11
Joined
Feb 13, 2008
Messages
516
LeP, it's fine for your own resources, but for public resources people don't like to download a new .exe just to run your script. I know about that resource and yes one can get it to work but no one cannot expect everyone to have it.

But it's okay for a public resource to circumvent the syntax-checker?
Cool idea.
 

LeP

LeP

Level 11
Joined
Feb 13, 2008
Messages
516
Yes, it is.

There's no reason as to why workarounds are bad, hence, they are not bad, in other words, it's fine to use them.

They were technically bad (desync).
They are still semantically bad (TriggerAddCondition, and we all know a conditions return something truthy or falsy.).

______
Also, when your needs change you adopt your tools.

"Hey heard of this new tool called JassHelper?", "Yeah, user don't want to download any programm.", "Okay, let's continue using Jass the way it is."
 

Cokemonkey11

Code Reviewer
Level 28
Joined
May 9, 2006
Messages
3,461
>> Hashtables are too slow for unit control and spells.

I use hashtables pretty frequently in Retro for really comprehensive data, my poor-grade Intel Atom N450 can handle it even on the scale of 100+ units without dropping in FPS.

That's excellent! I'm glad hashtables work for you and I won't try to convince you otherwise.

>> UnitUserData is too limited because it only supports 1 value per unit.

Too limited? It provides an array index for every unit, it is a supplementary utility. You don't need more than one UnitUserData.
And what happens if a user of your system isn't so knowledgeable and wants to use unituserdata to make his or her spells? What happens if there's an equally important system which is designed to be used in GUI and also needs unituserdata?

What I'm saying is that using it in a system is not fundamentally safe unless you declare "this system uses unituserdata!"

>> When O(n) is slower than the hashtable counter-part.

Which is essentially every time "n" exceeds 2 or 3. JASS reading is extremely "slow" thanks to it being an interpreted language. Calling hashtables slow is like saying the FLASH is slower than SUPERMAN. Who cares? Which brings us to...

2 or 3? I'm not going to lie to you and tell you I've tested it myself, but I'm quite sure you're wrong. I'm pretty sure "n" has to be quite a bit higher than 200 (which is a pretty large number of units/spells occuring simultaneously for wc3) before it becomes slower than hashtables.

>> That doesn't mean that "no one" cares.

No one cares about a 0.0000001% margin, which is the case here. If people think they care, they are overanalyzing and need to look at real-world scenarios. It's ridiculous percentiles like this that allows Intel to sell i3-2130's at a price difference 5x the difference between the 2100 and the 2120, despite providing only half the edge in performance (a 100MHz overclock instead of a 200MHz overclock costing $25 more instead of $5 more resulting in a huge, totally worth the money 3% performance gain).

That's just excellent, don't use my system then (I won't feel bad)

Here's an example:

JASS:
function Hi takes nothing returns nothing


endfunction

// ...
local code c = function Hi
call TriggerAddCondition(t, Filter(c))
// ...

The compiler can't stop you from doing this since you're passing a variable ;)
Of course, when the code is a parameter, you would need to make your function 2 lines long (add a return statement after the actual code) so that it wouldn't inline.

If it inlines, pJass says "Woah bro, that function doesn't return a boolean. You're in big trouble Mister."

Excellent! Thanks for the example, I will look into updating the script to do just that. Thanks to you as well as Bribe.

Someone counted how often i mentioned Zoxcs Jass Parser?
Or how to use it.

But this is new: using my incredible google-fu i ofc found a valid download of the cli-parser here.

And ofc you could always compile it from source.

Or change pJass' source.

That looks like quite a bit of work for every JNGP user to do just to use my script.

However, JNGP is not some proprietary work. If you go and make v5e using something better than pJass (And while you're at it, use the newest JassHelper and update TESH to include GetSpellTargetX() and GetSpellTargetY() ) and I'll gladly use it, as well as recommend to others to do the same :)
 

LeP

LeP

Level 11
Joined
Feb 13, 2008
Messages
516
Users wouldn't care.

They would rather just omit the return false line (It's such an evil line.)

It's not about the user but about nice code. And they aren't even exclusive.
It doesn't harm the user in any way.

That's why you use TriggerAddAction or, even better, function-interfaces.
No unneeded return-line. No tricking the syntax-checker. Same speed as TriggerEvaluate. You can even use friggin' arguments.
 

Bribe

Code Moderator
Level 45
Joined
Sep 26, 2009
Messages
9,043
@LeP, function interfaces doubling map size is quite criminal. I had some really big functions which were used in conjunction with Berb's Projectile system, my code size exploded. Although the use of arguments is very useful syntax, it's an absolute mess when you're looking at compiled code.

>> 2 or 3? I'm not going to lie to you and tell you I've tested it myself, but I'm quite sure you're wrong.

Being quite sure is what gets a lot of bad ideas publicity. A hashtable read is only a few times slower than an array read, and that HAS been benchmarked. So yes, a loop iterating through and checking 2-3 different array indices will achieve parity with a hashtable at best.

>> What I'm saying is that using it in a system is not fundamentally safe unless you declare "this system uses unituserdata!"

That's why AutoIndex has a "Use Hashtable" mode. It's a good feature. Unfortunately, I had to omit that kind of modularity in GUI Unit Indexer. I may just as well make a version of vJass Unit Indexer that has a "Use Hashtable" mode, but I'm not passionate if someone beats me to it because I have so little passion for modding WC3 these days.

>> That's just excellent, don't use my system then (I won't feel bad)

That's not what I meant, and I would use your system even though it's not the most efficient and the reason why is because I don't CARE about a 0.00001% margin which is what would be gained if you optimized yours. I seriously don't care about that margin and no one should. You made a great resource.
 

Cokemonkey11

Code Reviewer
Level 28
Joined
May 9, 2006
Messages
3,461
Being quite sure is what gets a lot of bad ideas publicity. A hashtable read is only a few times slower than an array read, and that HAS been benchmarked. So yes, a loop iterating through and checking 2-3 different array indices will achieve parity with a hashtable at best.

If you prove it to me I'll feel like a dipshit, but for now I'm content with being "that guy who thinks he's more efficient".

That's not what I meant, and I would use your system even though it's not the most efficient and the reason why is because I don't CARE about a 0.00001% margin which is what would be gained if you optimized yours. I seriously don't care about that margin and no one should. You made a great resource.

Oh, didn't expect that. Thanks then.

Regarding the use of Condition() with a code, I've decided I'm going to leave it how it is for now. If JNGP is updated with a replacement for pJass I'll update this script to reflect that.

But I'm still willing to take suggestions. Surprised no one has asked for a "removeHandler" method yet.
 

LeP

LeP

Level 11
Joined
Feb 13, 2008
Messages
516
@LeP, function interfaces doubling map size is quite criminal. I had some really big functions which were used in conjunction with Berb's Projectile system, my code size exploded. Although the use of arguments is very useful syntax, it's an absolute mess when you're looking at compiled code.

Why should i care about the compiled code again?

Okay, you've got some problems with function-interfaces. You fixed your problems. Now function-interfaces are criminal?
They are a fantastic feature with no speed but a bit of space overhead.
I'd say: use 'em if you need 'em, and only avoid 'em if they make you trouble.

Also function interfaces don't double your map size. They don't even double the script size. They copy every function you use as a "function interface".
It's a bit sub-optimal, but i rather have nice written code than nice generated code.

And your problem sounds easy to fix: split up your big function into smaller functions and -boom- only the small function you use for your function interface gets copied. (Also, smaller functions are actually something to strive for…)
 
Level 17
Joined
Apr 27, 2008
Messages
2,455
ExecuteTrigger is about the same speed as ExecuteFunc, and like 2-3 times slower than TriggerEvaluate (iirc, at least the scale is about few several times), it was benchmarked and experienced many times.
Also, when you don't need arguments it's just faster (i mean to use, nothing about efficiency), easier, whatever, to use than a function interface argument.

But yes i have to agree that's from the point of view of the user of the code, for the guy who will use a code argument to write his custom function, some extra work is needed, when you don't need that much if you use a function interface.

What i really dislike about function interfaces : you can't use the returned boolean of a .evaluate, i mean the result is not right.
Ofc you rarely want to know it, albeit it can be useful some times, and yes there are workarounds.
But maybe Cohadar will fix that.
 
Level 31
Joined
Jul 10, 2007
Messages
6,306
Well, could you by any chance update it again to support user-accessible unit buckets? o-o


why would I want to do that?

That doesn't make sense, lol... ;p

Every unit gets registered to it

edit
Oh, I see, this does multiple triggers for different types of damage events??

I have shown that you can easily do this in another lib when I did DamageModificationEffect =)

edit
So this is essentially a slower version of DamageEvent where the user needs to plug in all of the systems they want to use rather than the library already using them, icic. Also, you have multiple triggers assigned to different groups of units for the ultimate in flexibility. Furthermore, correct me if I'm wrong, but each group is split up into different triggers to make rebuilding those groups faster? It's this part that this lib would need improvement. Look at DamageEvent to see how to accomplish this, that is, if I'm correct about this last point.
 

Cokemonkey11

Code Reviewer
Level 28
Joined
May 9, 2006
Messages
3,461
Furthermore, correct me if I'm wrong, but each group is split up into different triggers to make rebuilding those groups faster?

Nope, this script never rebuilds triggers for unit groups. It waits for all the units to be null before it destroys the relevant trigger in the stack.

The slowest part of the script occurs after the periodic cleanup duration passes and it has to loop through (nearly) every unit of every trigger in the stack, checking for empty buckets. But this is still faster than enumerating every unit on the map every couple minutes and rebuilding the trigger from scratch, which is what many systems have done in the past.

I can't even read DamageEvent, let alone understand how it's faster. That's usually a pretty good indicator for me of when I should use or avoid something ;)

Edit: I've updated the script. Check Change Log for more information.
 
Last edited:
Level 31
Joined
Jul 10, 2007
Messages
6,306
I can't even read DamageEvent, let alone understand how it's faster. That's usually a pretty good indicator for me of when I should use or avoid something ;)

Er, the code is very simple in DamageEvent.. the methods are very tiny and straight forward, lol.

DamageEvent rebuilds the triggers when it is cost effective to do so (based on a ratio of null units to registered units). It stores the triggers into a binary heap based on how many units are in them. The triggers with the least units registered to them get units registered to them first (priority queue using binary heap).

Yours appears to use a linked list, meaning you do it in O(n) instead of O(Log n).

edit
Also, I don't see where you are nulling

tempDat.members[index]

The units will never be null if you aren't nulling that : |, meaning that your triggers will never be rebuilt >.>.

Variables don't magically null themselves.
 
Last edited:

Cokemonkey11

Code Reviewer
Level 28
Joined
May 9, 2006
Messages
3,461
Er, the code is very simple in DamageEvent.. the methods are very tiny and straight forward, lol.

DamageEvent rebuilds the triggers when it is cost effective to do so (based on a ratio of null units to registered units). It stores the triggers into a binary heap based on how many units are in them. The triggers with the least units registered to them get units registered to them first (priority queue using binary heap).

Yours appears to use a linked list, meaning you do it in O(n) instead of O(Log n).

edit
Also, I don't see where you are nulling

tempDat.members[index]

The units will never be null if you aren't nulling that : |, meaning that your triggers will never be rebuilt >.>.

Variables don't magically null themselves.

Uhh that's kind of important. It was my understanding that after a unit finished decaying, variables linking to it became null. Is that incorrect?

Regardless, DEAD_UNITS_ARE_NULL exists for a reason.
 
Level 17
Joined
Apr 27, 2008
Messages
2,455
Uhh that's kind of important. It was my understanding that after a unit finished decaying, variables linking to it became null. Is that incorrect?

That's exactly the opposite.
There is a reason why we use set ... = null for locals.
When the local variable "dies" (end of the function), its value should be flushed, but it is not, and then when the associated handle is destroyed, its id is not recycled.
Function arguments are not concerned by this bug though (takes ...)
 

Cokemonkey11

Code Reviewer
Level 28
Joined
May 9, 2006
Messages
3,461
That's exactly the opposite.
There is a reason why we use set ... = null for locals.
When the local variable "dies" (end of the function), its value should be flushed, but it is not, and then when the associated handle is destroyed, its id is not recycled.
Function arguments are not concerned by this bug though (takes ...)

So how can I fix the issue? How can I null a unit when it finishes decaying?

Maybe instead I should just remove the "DEAD_UNITS_ARE_NULL" system and simply make the loop check for dead units.

I should also be using the native UnitAlive (right?)
 
Level 17
Joined
Apr 27, 2008
Messages
2,455
"Modern" unit indexers can detect when an unit is removed of the game on event, exploiting a bug.

With a removed unit, or any other invalid one, GetUnitTypeId should return 0.

JASS:
        static if ENABLE_PERIODIC_CLEANUP then
                set perCleanup=CreateTrigger()
                call TriggerRegisterTimerEvent(perCleanup,PER_CLEANUP_TIMEOUT,true)
                call TriggerAddCondition(perCleanup,Condition(function ddBucket.perCleanupC))
            endif

You could directly use a timer instead.
 

Cokemonkey11

Code Reviewer
Level 28
Joined
May 9, 2006
Messages
3,461
"Modern" unit indexers can detect when an unit is removed of the game on event, exploiting a bug.

With a removed unit, or any other invalid one, GetUnitTypeId should return 0.

That seems quite useful. I think with my script, if the client uses RemoveUnit(), the associated bucket and trigger will never be destroyed, which is a big problem.

Can you show me how to use this exploit?

JASS:
        static if ENABLE_PERIODIC_CLEANUP then
                set perCleanup=CreateTrigger()
                call TriggerRegisterTimerEvent(perCleanup,PER_CLEANUP_TIMEOUT,true)
                call TriggerAddCondition(perCleanup,Condition(function ddBucket.perCleanupC))
            endif

You could directly use a timer instead.

That would be more efficient, right? Because the callback function in a timer is faster than checking the conditions of a trigger?
 
Level 17
Joined
Apr 27, 2008
Messages
2,455
Can you show me how to use this exploit?

Unit indexers provide a custom event for that, such as UnitIndexer, AutoIndex, AIDS.
If you want a sample, check my UnitLL resource.

That would be more efficient, right? Because the callback function in a timer is faster than checking the conditions of a trigger?

That should be yes, but anyway you just don't need a trigger.
 

Cokemonkey11

Code Reviewer
Level 28
Joined
May 9, 2006
Messages
3,461
Unit indexers provide a custom event for that, such as UnitIndexer, AutoIndex, AIDS.
If you want a sample, check my UnitLL resource.

I looked through nes' non-lua version of UnitIndexer and couldn't find anything useful. If I can't figure it out and there's no way to explain it to me I'll just make a "safeRemoveUnit" method.

That should be yes, but anyway you just don't need a trigger.

Thanks. I've updated the script to use a timer.
 
Level 31
Joined
Jul 10, 2007
Messages
6,306
rather than comparing to null, compare GetUnitTypeId to 0.

UnitIndexer is in a rather optimized state atm, rendering it nigh unreadable ;p.

Whenever a unit decays or is removed, 'Adef' fires two times. The second time it fires, the ability level of 'Adef' is 0. You register a trigger to fire whenever 'Adef' is used and check to see if the ability level is 0. If the ability level is 0, the unit went out of scope via decay or remove =). That is how UnitIndexer works = p. It also has a bit of other code for event registration, but that is the bug itself ; ). Things like AIDS and wc3c don't take advantage of 'Adef' being 0 on the second fire, so AIDS uses a stupid timer to check to see if the unit is gone and wc3c hooks RemoveUnit and : |.

I do not recommend building an entire unit indexer into the code. The fastest it will ever be is UnitIndexer, so if you are going to do indexing, you might as well just use UnitIndexer >.<.


One thing... I don't think hexed units are ever deindexed when they are removed via RemoveUnit as all abilities on a Hexed unit are disabled. Same with silenced unit ;o.
 

Cokemonkey11

Code Reviewer
Level 28
Joined
May 9, 2006
Messages
3,461
rather than comparing to null, compare GetUnitTypeId to 0

Oh perfect. I've updated the script (And added you and Troll-Brain in my credits :p )

Thanks for explaining this.

UnitIndexer is in a rather optimized state atm, rendering it nigh unreadable ;p.

Whenever a unit decays or is removed, 'Adef' fires two times. The second time it fires, the ability level of 'Adef' is 0. You register a trigger to fire whenever 'Adef' is used and check to see if the ability level is 0. If the ability level is 0, the unit went out of scope via decay or remove =). That is how UnitIndexer works = p. It also has a bit of other code for event registration, but that is the bug itself ; ).

Interesting. Why don't you also use GetUnitTypeId()? Is UnitAddAbility() faster?

Things like AIDS and wc3c don't take advantage of 'Adef' being 0 on the second fire, so AIDS uses a stupid timer to check to see if the unit is gone and wc3c hooks RemoveUnit and : |.

*cringe* what a nightmare.


One thing... I don't think hexed units are ever deindexed when they are removed via RemoveUnit as all abilities on a Hexed unit are disabled. Same with silenced unit ;o.

But I'm sure you've documented that into UnitIndexer. Anyway, GetUnitTypeId() will not have to worry about that so I've used it.

One more thing, I've also updated the script to use the UnitAlive() native. I assume that's better. Anyone recommend reverting it to the way it was for some reason?
 
Level 31
Joined
Jul 10, 2007
Messages
6,306
One more thing, I've also updated the script to use the UnitAlive() native. I assume that's better. Anyone recommend reverting it to the way it was for some reason?

Use that

Interesting. Why don't you also use GetUnitTypeId()? Is UnitAddAbility() faster?

??

The ability is added to all units as the units are created and it is never removed ;p
 

Cokemonkey11

Code Reviewer
Level 28
Joined
May 9, 2006
Messages
3,461
The ability is added to all units as the units are created and it is never removed ;p

Gotcha. Carry on.

I'm pretty happy with this system in its current state. Anyone have any questions or requests?

I'm thinking about writing a tutorial on how to use it for GUI users. Perhaps that's a waste of time.
 
Level 17
Joined
Apr 27, 2008
Messages
2,455
About UnitAlive, i personnaly used IsUnitType(...UNIT_TYPE_DEAD), but i guess that the UnitAlive native have a different behavior about dead units but being under the process of resurrection.
In fact a custom function IsUnitAlive which have an extra boolean argument to consider ressurecting units alives ot not should be the best way.
 

Cokemonkey11

Code Reviewer
Level 28
Joined
May 9, 2006
Messages
3,461
About UnitAlive, i personnaly used IsUnitType(...UNIT_TYPE_DEAD), but i guess that the UnitAlive native have a different behavior about dead units but being under the process of resurrection.
In fact a custom function IsUnitAlive which have an extra boolean argument to consider ressurecting units alives ot not should be the best way.

Under the process of resurrection means they're not alive or dead? Seems like a really specific bug to consider.

Also, since I'm declaring "native UnitAlive ...", will everything be handled correctly if another library declares the same native or will that cause a problem?
 
Level 17
Joined
Apr 27, 2008
Messages
2,455
Jasshelper handle multiple native functions declaration (only consider the first and ignore the other ones).

I'm just saying i'm not sure how IsUnitType(...UNIT_TYPE_DEAD) and IsUnitAlive handle units which are being resurected, you should test it.
 

Bribe

Code Moderator
Level 45
Joined
Sep 26, 2009
Messages
9,043
To get it approved? Someone has to move the thread to the JASS Resources forum.

To get it reviewed is something different. I could approve this but it is still "another damage detection system" to join the ranks of far too many to keep track of.

Also, to respond to the Unitalive/IsUnitType thing, they both return the same when a unit is resurrecting.
 

Cokemonkey11

Code Reviewer
Level 28
Joined
May 9, 2006
Messages
3,461
To get it approved? Someone has to move the thread to the JASS Resources forum.

Thanks for the sarcasm. Let me try again. What constitutes a submission worthy of the JASS Resources forum?

To get it reviewed is something different. I could approve this but it is still "another damage detection system" to join the ranks of far too many to keep track of.

Instead of explaining why this is different/better than other public resources, I'm just going to say that it's not your job to keep track of these systems. The question is "Is the resource useful?"

Also, to respond to the Unitalive/IsUnitType thing, they both return the same when a unit is resurrecting.

Thanks, but I was more curious about which one is less computationally expensive.
 
Level 17
Joined
Feb 11, 2011
Messages
1,860
Hi. I'm not great with damage detection stuff, but is there a way to detect only physical attacks with this system?

EDIT: I tried it with a Paladin casting Holy Light on an Acoltye. The result messages:

Paladin dealt 0.000 damage to: Acolyte
Paladin dealt 100.000 damage to: Acolyte
 

Cokemonkey11

Code Reviewer
Level 28
Joined
May 9, 2006
Messages
3,461
Hi. I'm not great with damage detection stuff, but is there a way to detect only physical attacks with this system?

Not that I know of :D Sorry I'm sure that wasn't the answer you were hoping for.

EDIT: I tried it with a Paladin casting Holy Light on an Acoltye. The result messages:

Paladin dealt 0.000 damage to: Acolyte
Paladin dealt 100.000 damage to: Acolyte

That's pretty weird, you used the test scope with ADD_ALL_UNITS = true and didn't make another trigger for adding units? Regardless, if you didn't it would just display the same line twice. This looks more like something specific to holy light, maybe due to the allied/enemy duality involved with the spell.
 
Top