[System] StructuredDD (Structured Damage Detection)

Level 17
Joined
Feb 11, 2011
Messages
1,860
Not that I know of :D Sorry I'm sure that wasn't the answer you were hoping for.

:O nooooo!

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.

Yes, I used your test scope and only the test scope. I will try it with another spell and see what happens.
 
Level 2
Joined
Apr 8, 2009
Messages
21
Alright, this is just beautiful! I've got it to work on my map without any issues (I've haven't got any spells to test but I'll let you know how those turn out). I'm just wondering, will these events still fire if a UnitDamageTarget or similar event? and just out of curiosity, is there anyway to get the damage type of the damage dealt?
 
Hi wiredbomb0, I'm glad you've found use for this!

I don't have a wc3 developing environment on my active machine so I can't test/recall these answers for you, however:

  1. UnitDamageTarget should be detected
  2. Damage Dealt type is not possible to detect with wc3's natives, but you might consider looking for a system that attempts to do it!

Good luck
 
Level 22
Joined
Nov 14, 2008
Messages
3,256
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

Cause holy light does damage to undeads...
 
Level 17
Joined
Jul 17, 2011
Messages
1,863
... and just out of curiosity, is there anyway to get the damage type of the damage dealt?

it is more efficient to have every unit's damage type preset into a variable and to trigger the rest of the damage, if you think about it most of the time there are several different groups of units. for example you can have 20 footmen or 10 archers there is no point getting every unit's damage type and even if you get the damage type of a group of units there is still no need to make a system which detects something as constant as damage type as for triggered damage, that can easily be detected what type it is by using integer comparisons so since there is no native to get the damage type you will have to set it manually, if there was a system it will only save you about 4-5 minutes of work
 
I've updated this system with a big improvement to the code including:

  • No more support for useless things like DEAD_UNITS_ARE_NULL. If you need that kind of optimization, you can remove dead units yourself in a separate trigger.
  • Improved and shortened the API substantially.
  • Improved code commenting with document style explanations for each function.
  • Fixed a massive bug in the add function.
  • Paved the way for a coming DamageType extension.

I am curious if anyone is interested in extended support for things like:

  • Deallocating handlers
  • Registering special buckets for units that are never removed (eg heroes)
  • Included "safe" damage target by code functions (will be included regardless in the DamageType extension)
  • Introducing priority handlers that will occur sooner than others

Cheers,
 
Last edited:
Level 23
Joined
Apr 16, 2012
Messages
4,041
deallocating handlers sounds good, sometimes I want to detect something only for certain time but I still can lock the function with boolean(it will not prevent it from running tho)
priority handler hm...I dont care really but there are situations where you want something to happen before something else
the rest I dont really care, I wouldnt find use to those but maybe there are people that would

by the way, I have one question, since you mentioned it detects UnitDamageTarget, will this cause infinite loop?
JASS:
scope a initializer init
    private function handler takes nothing returns nothing
        call UnitDamageTarget(GetTriggerUnit(), GetEventDamageSource(), ...)
    endfunction

    private function init takes nothing returns nothing
        call StructuredDD_ddbucket.addHandler(function handler) //or how :D(too lazy to chcek correct syntax)
    endfunction
endscope
?
 
deallocating handlers sounds good, sometimes I want to detect something only for certain time but I still can lock the function with boolean(it will not prevent it from running tho)
priority handler hm...I dont care really but there are situations where you want something to happen before something else
the rest I dont really care, I wouldnt find use to those but maybe there are people that would

by the way, I have one question, since you mentioned it detects UnitDamageTarget, will this cause infinite loop?
JASS:
scope a initializer init
    private function handler takes nothing returns nothing
        call UnitDamageTarget(GetTriggerUnit(), GetEventDamageSource(), ...)
    endfunction

    private function init takes nothing returns nothing
        call StructuredDD_ddbucket.addHandler(function handler) //or how :D(too lazy to chcek correct syntax)
    endfunction
endscope
?

It could, but I'm guessing one of the two units will die before it does (you've called 'damaged' unit to deal damage to 'damage source', so the two would programmatically take some real number r of damage until one of them is dead. I'm guessing though that if r is 0, it would infinite loop.

Thanks for your thoughts on deallocation and priority handlers, I'll wait for some more responses for/against it.
 
Level 23
Joined
Apr 16, 2012
Messages
4,041
Im not going to argue if you should add it or not(its up to you tho), but one real use of deallocation is when I have optional quests where I need to use stuff which uses Damage Detections(lets say the damage itself, or my GetKiller library), if I want to decide if the killer of quest-relevant unit was player x or player y and after the quest is done/failed, I no longer need to run that function
 
Im not going to argue if you should add it or not(its up to you tho), but one real use of deallocation is when I have optional quests where I need to use stuff which uses Damage Detections(lets say the damage itself, or my GetKiller library), if I want to decide if the killer of quest-relevant unit was player x or player y and after the quest is done/failed, I no longer need to run that function

Hmm you shouldn't need damage detection for that - there's a generic unit event for "EVENT_PLAYER_UNIT_DEATH" and then use GetKillingUnit().
 
Level 23
Joined
Apr 16, 2012
Messages
4,041
I explained flaws of GetKillingUnit in my GetKiller library, GetKillingUnit returns unit doing killing blow(last hit), if we want to be more realistic the killer is technically the one that dealt most damage

By the way the system runs 0. damage events(dont know if its supposed to so Im letting you know even throught you didnt mention anything in documentation):
JASS:
call UnitDamageTarget(GetTriggerUnit(), GetEventDamageSource(), any number, true, false, ATTACK_TYPE_HERO, DAMAGE_TYPE_NORMAL, null)

crashes the game without error, but it crashes the game with DamageEvent too(not with 0. damage event) so its general problem with all systems :D
 
I explained flaws of GetKillingUnit in my GetKiller library, GetKillingUnit returns unit doing killing blow(last hit), if we want to be more realistic the killer is technically the one that dealt most damage

I think that's quite subject to opinion but I won't question you on that.

By the way the system runs 0. damage events(dont know if its supposed to so Im letting you know even throught you didnt mention anything in documentation):
JASS:
call UnitDamageTarget(GetTriggerUnit(), GetEventDamageSource(), any number, true, false, ATTACK_TYPE_HERO, DAMAGE_TYPE_NORMAL, null)

crashes the game without error, but it crashes the game with DamageEvent too(not with 0. damage event) so its general problem with all systems :D

That's right. If you'd like to detect 0. damage events, use http://www.hiveworkshop.com/forums/submissions-414/system-damagetype-structureddd-extension-228883/
 
Getting prepared for some hate for a triple post, but I have some new interesting data!

http://i.imgur.com/EwEjdmq.png

According to this, UnitAlive() is significantly less computationally expensive than IsUnitType(u,UNIT_TYPE_DEAD)

Edit: The baseline code looks like this:

JASS:
scope baseline initializer i
    globals
        unit u
        integer count=0
    endglobals
    
    private function p takes nothing returns nothing
        local integer j=0
        loop
            exitwhen j>=count
            set j=j+1
        endloop
    endfunction
    
    private function c takes nothing returns boolean
        set count=count+10
        call DisplayTimedTextToPlayer(Player(0),0.,0.,1.,I2S(count))
        return false
    endfunction
    
    private function i takes nothing returns nothing
        local trigger t=CreateTrigger()
        set u=CreateUnit(Player(1),'hfoo',0.,0.,0.)
        call PanCameraToTimed(3960.,3960.,0.)
        call SetTimeOfDayScale(0.)
        call TriggerRegisterPlayerEvent(t,Player(0),EVENT_PLAYER_END_CINEMATIC)
        call TriggerAddCondition(t,Condition(function c))
        call TimerStart(CreateTimer(),1./10000.,true,function p)
        set t=null
    endfunction
endscope

See attached map for all tests.
 

Attachments

  • unitAliveTest001.w3m
    16.3 KB · Views: 42
Well, yeah, I mean, the Jass VM has more chars to take in to a buffer and interpret when you're using IsUnitType I guess.

I don't think that could make such a significant difference. Try drawing a best fit line to the data.

You could test it more scientifically by testing more pairs of functions I guess.
 
Level 14
Joined
Dec 12, 2012
Messages
1,007
Hm, interessting. I did some tests too, to check how the length of a functions name affects performance...

I took three different function names, here is the test trigger:

JASS:
globals
    constant boolean LONG_FUNCTION_NAME = true
    constant integer ITERATIONS_PER_PERIOD = 15000
    constant real TIME_PERIOD = 0.01
endglobals

scope test initializer Init
    private function aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa takes nothing returns nothing
    endfunction

    private function aaaaaaaaa takes nothing returns nothing
    endfunction
    
    private function callback takes nothing returns nothing
        local integer i = 0
        
        loop
            exitwhen i > ITERATIONS_PER_PERIOD
            static if LONG_FUNCTION_NAME then
                call aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa()
            else
                call aaaaaaaaa()
            endif
            set i = i + 1
        endloop
    endfunction
    
    private function Init takes nothing returns nothing
        call TimerStart(CreateTimer(), TIME_PERIOD, true, function callback)
    endfunction
endscope

fpsfuncnames.jpg


There is a quite big difference, but you need an extrem amount of function calls to make that difference visible (I needed more than 1 million function calls per second).

So it shouldn't be important I think.
 
Level 10
Joined
Sep 19, 2011
Messages
527
Are you talking about the damage handlers?

Yes

The client can use only one if they please, but it limits functionality for marginal performance gain.

I don't follow you :/

I don't know if you get the idea. The dummy trigger (where you will put all the handlers) will be global, just one. The triggers that are in the buckets, will only serve to listen to "unit takes damage" event (not to store code). When one of those (triggers in buckets) gets triggered you just simple execute all the conditions (or actions, whatever you have used to store code in the dummy trigger) of the global dummy trigger.
 
Yes



I don't follow you :/

I don't know if you get the idea. The dummy trigger (where you will put all the handlers) will be global, just one. The triggers that are in the buckets, will only serve to listen to "unit takes damage" event (not to store code). When one of those (triggers in buckets) gets triggered you just simple execute all the conditions (or actions, whatever you have used to store code in the dummy trigger) of the global dummy trigger.

You can't dynamically add lines to a code, whereas handlers can be added dynamically to the bucket array.

Regardless, even if you were hoping this would result in performance gain, it's kind of a far-fetched idea in the first place.
 
Level 10
Joined
Sep 19, 2011
Messages
527
JASS:
// global
trigger dummyTrigger = CreateTrigger()

// all triggers in the buckets must have this function
// to execute when their event is fired
function onDamage takes nothing returns nothing
    call TriggerExecute(dummyTrigger)
endfunction

function addHandler takes code c returns nothing
    call TriggerAddCondition(dummyTrigger, Condition(c))
endfunction

So, when you have to rebuild the trigger, only loop the events.

Regardless, even if you were hoping this would result in performance gain, it's kind of a far-fetched idea in the first place.

Are you sure?, well if that's so, then nvm :/.
 
I see what you're saying now. But this will require each trigger in the buckets to call another trigger, which will in turn call the handlers. Currently the triggers in the buckets call the handlers directly.

I think this would perform worse, but the only way to know would be to test it.

You could do some tests and let me know what you find out if you like.
 
Level 10
Joined
Sep 19, 2011
Messages
527
I see what you're saying now. But this will require each trigger in the buckets to call another trigger, which will in turn call the handlers. Currently the triggers in the buckets call the handlers directly.

I think this would perform worse, but the only way to know would be to test it.

You could do some tests and let me know what you find out if you like.

Aff, you're right xD.

Well, I can write how I was suggesting, I will let the tests to you (I really don't know much about testing).

edit:

Sorry for the agressive modification

JASS:
//*
//* API:
//*
//* static method addHandler takes code func returns nothing
//*     Registers a callback function to the generic
//*     unit damage event. Example: call StructuredDD.addHandler(function h)
//*
//* static method add takes unit member returns nothing
//*     Adds a unit to a bucket. If ADD_ALL_UNITS is enabled,
//*     this method need not be used.
//*
library StructuredDD
    globals
        //* Set this to true if you want all units in your map to be 
        //* automatically added to StructuredDD. Otherwise you will have to 
        //* manually add them with StructuredDD.add(u).
        private constant boolean ADD_ALL_UNITS=true
        
        //* This is the amount of units that exist in each trigger bucket. 
        //* This number should be something between 5 and 30. A good starting 
        //* value will be an estimate of your map's average count of units, 
        //* divided by 10. When in doubt, just use 20.
        private constant integer BUCKET_SIZE=1
        
        //* This is how often StructuredDD will search for empty buckets. If 
        //* your map has units being created and dying often, a lower value 
        //* is better. Anything between 10 and 180 is good. When in doubt, 
        //* just use 60.
        private constant real PER_CLEANUP_TIMEOUT=1.
    endglobals

    private struct Bucket
        unit array members[BUCKET_SIZE]
        integer size
        
        /*
         *  Will be used to listen to
         *  the "a unit takes damage" event
         */
        trigger trig
        
        /*
         *  Check if the bucket is empty
         */
        method isEmpty takes nothing returns boolean
            local integer i = this.size
            
            loop
                /*
                 *  Loop throw all the members
                 *  of the bucket
                 */
                exitwhen i == 0
                
                /*
                 *  UniTypeId wil be 0 if
                 *  the unit has been removed
                 */
                if (GetUnitTypeId(this.members[i]) != 0) then
                    return false
                endif
                
                set i = i - 1
            endloop
            
            return true
        endmethod
        
        /*
         *  Add a unit to the bucket
         */
        method addMember takes unit member returns nothing
            set this.size = this.size + 1
            set this.members[this.size] = member
            
            call TriggerRegisterUnitEvent(this.trig, member, EVENT_UNIT_DAMAGED)
        endmethod
        
        method destroy takes nothing returns nothing
            set this.size = 0
            call DestroyTrigger(this.trig)
            
            call this.deallocate()
        endmethod
        
        static method create takes nothing returns thistype
            local thistype this = thistype.allocate()
            
            set this.size = 0
            
            set this.trig = CreateTrigger()
            call TriggerAddCondition(this.trig, StructuredDD.ON_DAMAGE_CODE)
            
            return this
        endmethod
    endstruct
    
    /*
     *  Do not instanciate it, we just
     *  use this for a pretty, Java-like API :3
     */
    struct StructuredDD extends array
        /* 
         *  Cache
         */
        readonly static unit source = null
        readonly static unit target = null
        readonly static real amount = 0.
        
        readonly static boolexpr ON_DAMAGE_CODE
        
        /*
         *  This trigger will save
         *  all the codes to run when
         *  the event is fired
         */
        private static trigger dummyTrigger = CreateTrigger()
        
        private static Bucket array buckets
        private static Bucket activeBucket = 0
        private static integer bucketsCounter = -1
        
        /*
         *  Get a bucket that is not full or a new one
         */
        private static method getBucket takes nothing returns Bucket
            /*
             *  If there is no activeBucket or
             *  the size of it is >= BUCKET_SIZE
             */
            if (activeBucket == 0 or activeBucket.size >= BUCKET_SIZE) then
                set bucketsCounter = bucketsCounter + 1
                set activeBucket = Bucket.create()
                
                set buckets[bucketsCounter] = activeBucket
            endif
            
            return activeBucket
        endmethod
        
        /*
         *  Add a handler to the system. It will be triggered
         *  when the "a unit takes damage" event is fired
         */
        public static method addHandler takes code func returns nothing
            /*
             *  No need to have "return (bool)"
             *  from func thanks to this
             */
            local boolexpr condition = Condition(func)
            call TriggerAddCondition(dummyTrigger, condition)
        endmethod
        
        private static method onDamage takes nothing returns boolean
            /*
             *  Previus values
             */
            local unit prevSource = source
            local unit prevTarget = target
            local real prevAmount = amount
            
            /*
             *  Updating cache
             */
            set source = GetEventDamageSource()
            set target = GetTriggerUnit()
            set amount = GetEventDamage()
            
            /*
             *  Run all the handles (codes)
             */
            call TriggerEvaluate(dummyTrigger)
            
            /*
             *  Prevent recursion problems
             */
            set source = prevSource
            set target = prevTarget
            set amount = prevAmount
            
            set prevSource = null
            set prevTarget = null
            
            return false
        endmethod
        
        /*
         *  Add a unit to the system
         *  If ADD_ALL_UNITS is true, there is no need to use this
         */
        public static method add takes unit member returns nothing
            call getBucket().addMember(member)
        endmethod
        
        /*
         *  Cleans up any empty bucket
         */
        private static method cleanUpPeriodic takes nothing returns nothing
            local integer i = bucketsCounter
            local Bucket bucket
            
            loop
                /*
                 *  Loop throw all buckets
                 */
                exitwhen i == 0
                
                set bucket = buckets[i]
                
                if (bucket != activeBucket and bucket.isEmpty()) then
                    call bucket.destroy()
                    
                    /*
                     *  Move the last bucket of
                     *  buckets to the position
                     *  of the destroyed one
                     */
                    set buckets[i] = buckets[bucketsCounter]
                    set bucketsCounter = bucketsCounter - 1
                    
                    /*
                     *  ¿?
                     */
                    if (buckets[bucketsCounter] == activeBucket) then
                        set activeBucket = buckets[i]
                    endif
                    
                    /*
                     *  We have moved the last bucket
                     *  of buckets to a new position,
                     *  so don't forgget to iterate it
                     */
                    set i = i + 1
                endif
                
                set i = i - 1
            endloop
        endmethod
        
        static if (ADD_ALL_UNITS) then
            /*
             *  Auxiliary function
             */
            private static method enteringUnitFilter takes nothing returns boolean
                call add(GetFilterUnit())
                return false
            endmethod
            
            /*
             *  Automatically will register all
             *  unit in the damage system
             */
            private static method registerAllUnits takes nothing returns nothing
                local region reg = CreateRegion()
                local unit enumUnit
                
                /*
                 *  Add starting units
                 */
                call GroupEnumUnitsInRect(bj_lastCreatedGroup, bj_mapInitialPlayableArea, null)
                
                loop
                    set enumUnit = FirstOfGroup(bj_lastCreatedGroup)
                    exitwhen enumUnit == null
                    call GroupRemoveUnit(bj_lastCreatedGroup, enumUnit)
                    
                    call thistype.add(enumUnit)
                endloop
                
                /*
                 *  Add entering units
                 */
                call RegionAddRect(reg, bj_mapInitialPlayableArea)
                call TriggerRegisterEnterRegion(CreateTrigger(), reg, Filter(function thistype.enteringUnitFilter))
                
                set reg = null
            endmethod
        endif
        
        /*
         *  Initializer
         */
        private static method onInit takes nothing returns nothing
            set ON_DAMAGE_CODE = Condition(function thistype.onDamage)
        
            /*
             *  Periodic clenup
             */
            call TimerStart(CreateTimer(), PER_CLEANUP_TIMEOUT, true, function thistype.cleanUpPeriodic)
        
            static if (ADD_ALL_UNITS) then
                call registerAllUnits()
            endif
        endmethod
    endstruct
endlibrary

A test (it's the same, just to show you that you can use cache/native):

JASS:
library Test initializer Init requires StructuredDD
    private function onDamage takes nothing returns nothing
        /*
         *  We can use the cache or
         *  simple the native functions
         */
         
        //* Native
        call BJDebugMsg("Source: " + GetUnitName(GetEventDamageSource()))
        
        //* Cache
        call BJDebugMsg("Target: " + GetUnitName(StructuredDD.target))
        call BJDebugMsg("Amount: " + R2S(StructuredDD.amount))
    endfunction

    //===========================================================================
    private function Init takes nothing returns nothing
        /*
         *  Adding a handler
         */
        call StructuredDD.addHandler(function onDamage)
    endfunction
endlibrary

The only that I don't understand is:

JASS:
                    /*
                     *  ¿?
                     */
                    if (buckets[bucketsCounter] == activeBucket) then
                        set activeBucket = buckets[i]
                    endif

Can you explain me?

edit:

Uhmm, seems like this version is not better.

JASS:
scope stressTest initializer i
    globals
        private unit u
        private integer kIPS=0
    endglobals
    
    private function onDamage takes nothing returns nothing
    
    endfunction
    
    private function p takes nothing returns nothing
        local integer j=0
        loop
            exitwhen j>=kIPS
            call UnitDamageTarget(u,u,0.,true,true,ATTACK_TYPE_NORMAL,DAMAGE_TYPE_NORMAL,WEAPON_TYPE_WHOKNOWS)
            set j=j+1
        endloop
    endfunction
    
    private function c takes nothing returns boolean
        set kIPS=kIPS+1
        call DisplayTimedTextToPlayer(Player(0),0.,0.,1.,I2S(kIPS))
        call StructuredDD.addHandler(function onDamage)
        return false
    endfunction
    
    private function i takes nothing returns nothing
        local trigger t=CreateTrigger()
        set u=CreateUnit(Player(0),'hfoo',0.,0.,0.)
        call TimerStart(CreateTimer(),1./1000.,true,function p)
        call TriggerRegisterPlayerEvent(t,Player(0),EVENT_PLAYER_END_CINEMATIC)
        call TriggerAddCondition(t,Condition(function c))
        set t=null
    endfunction
endscope

Tested that and yours gets better results.
 
Last edited:
Hello,

Sorry for the lack of attention I've been giving this thread. I'm quite busy to spend time on these things but it's always fun :)

The only that I don't understand is:

JASS:
                    /*
                     *  ¿?
                     */
                    if (buckets[bucketsCounter] == activeBucket) then
                        set activeBucket = buckets[i]
                    endif

Can you explain me?

The system works by filling up one bucket at a time. If the current bucket which is *not* yet filled up was just moved to the middle of the stack, we have to update the value of the pointer which references the current bucket which is not yet filled.

That's a bit of a mouthful, but I hope it makes sense.

edit:

Uhmm, seems like this version is not better.

JASS:
scope stressTest initializer i
    globals
        private unit u
        private integer kIPS=0
    endglobals
    
    private function onDamage takes nothing returns nothing
    
    endfunction
    
    private function p takes nothing returns nothing
        local integer j=0
        loop
            exitwhen j>=kIPS
            call UnitDamageTarget(u,u,0.,true,true,ATTACK_TYPE_NORMAL,DAMAGE_TYPE_NORMAL,WEAPON_TYPE_WHOKNOWS)
            set j=j+1
        endloop
    endfunction
    
    private function c takes nothing returns boolean
        set kIPS=kIPS+1
        call DisplayTimedTextToPlayer(Player(0),0.,0.,1.,I2S(kIPS))
        call StructuredDD.addHandler(function onDamage)
        return false
    endfunction
    
    private function i takes nothing returns nothing
        local trigger t=CreateTrigger()
        set u=CreateUnit(Player(0),'hfoo',0.,0.,0.)
        call TimerStart(CreateTimer(),1./1000.,true,function p)
        call TriggerRegisterPlayerEvent(t,Player(0),EVENT_PLAYER_END_CINEMATIC)
        call TriggerAddCondition(t,Condition(function c))
        set t=null
    endfunction
endscope

Tested that and yours gets better results.

Glad you were able to test it and figure it out :)

Edit: I've updated StructuredDD to use "extends array" for the StructuredDD struct shim. This is a minor edit.
 
Last edited:
Level 10
Joined
Sep 19, 2011
Messages
527
The system works by filling up one bucket at a time. If the current bucket which is *not* yet filled up was just moved to the middle of the stack, we have to update the value of the pointer which references the current bucket which is not yet filled.

That's a bit of a mouthful, but I hope it makes sense.

Oh I see, thanks.

native UnitAlive takes unit u returns boolean you're not using it xD.

Put this:

private static method autoAddC takes nothing returns boolean

In static if.
 
native UnitAlive takes unit u returns boolean you're not using it xD.

Old version used to use it. Thanks - updated.

Put this:

private static method autoAddC takes nothing returns boolean

In static if.

I could swear that I had tried this and JassHelper gave me an error. Nevertheless it works now. Updated.

Thanks again :)
 

Deleted member 219079

D

Deleted member 219079

It says StructureDD is not of a type that allows . syntax

nvm i misread the name..
 

Deleted member 219079

D

Deleted member 219079

Edited the post, i misread it as Structure-DD instead of Strctured

great system btw
 
Instead of "showing you lines" I prefered to just write some alternative for you, in case I had some time to spare.

When writing descriptions, especially here, you don't need to be verbose as much. Your current lib state reminds me more of a doc file instead of lib with inserted comments. In c++ I usually do:
JASS:
/**
    < 1-3 lines description here > bla bla bla

    @param:
    @return:
*/
If you want to give us a tip in regard what something is here for, add a single line instead of block. This is nice:
JASS:
    //* Our bucket struct which contains a trigger and its associated contents.
    private struct bucket
And this is not:
JASS:
        //* This method is for adding a handler to the system. Whenever a
        //* handler is added, damage detection will immediately trigger that
        //* handler. There is no way to deallocate a handler, so don't try to
        //* do this dynamically (!) Support for handler deallocation is
        //* feasible (please contact me)
        public static method addHandler takes code func returns nothing
Whatmore, instead of:
JASS:
call myfunction1
// comment here related to myfunction2 or just algorithm step
call myfunction2
It's better to do:
JASS:
call myfunction1

//comment here related to myfunction2 or just algorithm step
call myfunction2
When explaining API, it's nice to give some breaks between configurables/methods especially if you want to write more that a couple of words. Blocks such as:
JASS:
//* boolean ADD_ALL_UNITS: If enabled, a trigger turns on which automatically
//*     registers all units in the map.
//* integer BUCKET_SIZE: How many units to add to each 'bucket' - a larger
Could as well be splited to:
JASS:
*          boolean ADD_ALL_UNITS
*          - If enabled, a trigger turns on which automatically registers all units in the map.
*
*          integer BUCKET_SIZE
Consider that this is part most users look at when reading your stuff. If those are unexperienced, scrolling down is even less possible. However, to not ruin what I've said previously: readability of system is still nice, even if just for reviewers.
Also, you have explained the meaning of each global at the top, only to repeat it down in the code - if description is good, it only needs to be written once.

In your script, I've noticed that you don't like spaces. Meaby it's just your writting-style. Spaces are good tho, especially when there are multiple operators such as ++, --, ==, =, >, < around. There were a lot of "thistypes". Since every field member in your struct is static, and considering that when working with "normal" scripting/programming language you usually don't prefix your members with anything its good to drop it.

You use bucket structure, and when "thistype" spam was present, multiple times I've noticed something as: field.field2.field3.invoke(). When such strings appear in groups, you should reduce them as much as possible -> we don't actually need to see whole road of invocation.

Remember it's my and just my opinion. Below, system code without 1-line comments above important parts (those are welcome) and with applied suggestions:
JASS:
/*****************************************************************************
*
*    API:
*
*       Configurables:
*       --------------
*
*          boolean ADD_ALL_UNITS
*          - If enabled, a trigger turns on which automatically registers all units in the map.
*
*          integer BUCKET_SIZE
*          - How many units to add to each 'bucket' - a larger bucket will have their trigger refresh
*            less frequently but will be more computationally expensive. A good starting value is about 20.
*
*          real PER_CLEANUP_TIMEOUT
*          - How many seconds to wait in between each scan for empty buckets. This value should be
*            lower if units die often in your map. A good starting value is about 60.
*
*       Methods:
*       --------
*
*          static method addHandler
*          - Registers a callback function to the generic unit damage event.
*               Example: call StructuredDD.addHandler(function h)
*
*          static method add
*          - Adds a unit to a bucket. If ADD_ALL_UNITS is enabled, this method need not be used.
*
*****************************************************************************/
library StructuredDD

//<< BEGIN SETTINGS SECTION

	globals
        private constant boolean ADD_ALL_UNITS      = true
        private constant integer BUCKET_SIZE        = 20
        private constant real PER_CLEANUP_TIMEOUT   = 60.
    endglobals

//>> END SETTINGS SECTION

    private struct bucket
        integer bucketIndex = 0
        trigger trig = CreateTrigger()
        unit array members[BUCKET_SIZE]
    endstruct

    struct StructuredDD extends array
        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

        private static method getBucket takes nothing returns integer
            local integer index = 0
            local integer returner = -1
            local bucket tempDat

            if ( dbIndex != -1 and bucketDB[dbIndex].bucketIndex < BUCKET_SIZE ) then
                return dbIndex
            else
                set maxDBIndex = maxDBIndex+1
                set dbIndex = maxDBIndex
                set tempDat = bucket.create()
                set bucketDB[maxDBIndex] = tempDat

                loop
                    exitwhen index > conditionsIndex
                    call TriggerAddCondition(tempDat.trig, conditions[index])
                    set index = index+1
                endloop
                return dbIndex
            endif

            return -1
        endmethod

        public static method addHandler takes code func returns nothing
            local bucket tempDat
            local integer index = 0

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

            loop
                exitwhen index > maxDBIndex
                set tempDat = bucketDB[index]
                call TriggerAddCondition(tempDat.trig, conditions[conditionsIndex])
                set index = index+1
            endloop
        endmethod

        public static method add takes unit member returns nothing
            local bucket tempDat
            local integer whichBucket = getBucket()

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

            call TriggerRegisterUnitEvent(tempDat.trig, member, EVENT_UNIT_DAMAGED)
        endmethod

        static if ADD_ALL_UNITS then
            private static method autoAddC takes nothing returns boolean
                call thistype.add(GetTriggerUnit())
                return false
            endmethod
        endif

        private static method bucketIsEmpty takes integer which returns boolean
            local bucket tempDat = bucketDB[which]
            local integer index = 0

            loop
                exitwhen index == BUCKET_SIZE
                if GetUnitTypeId(tempDat.members[index]) != 0 then
                    return false
                endif
                set index = index+1
            endloop

            return true
        endmethod

        private static method perCleanup takes nothing returns nothing
            local integer index = 0

            loop
                exitwhen index > maxDBIndex

                if ( index != dbIndex and bucketIsEmpty(index) ) then
                    call DestroyTrigger(bucketDB[index].trig)
                    call bucketDB[index].destroy()
                    set bucketDB[index] = bucketDB[maxDBIndex]
                    set maxDBIndex = maxDBIndex-1

                    if ( maxDBIndex == dbIndex ) then
                        set dbIndex = index
                    endif

                    set index = index-1
                endif

                set index = index+1
            endloop
        endmethod

        private static method onInit takes nothing returns nothing
            local group grp
            local region reg
            local trigger autoAddUnits
            local timer perCleanup
            local unit FoG

            static if ADD_ALL_UNITS then
                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

                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

            set perCleanup = CreateTimer()
            call TimerStart(perCleanup, PER_CLEANUP_TIMEOUT, true, function thistype.perCleanup)
            set perCleanup = null
        endmethod
    endstruct

endlibrary
 
When writing descriptions, especially here, you don't need to be verbose as much. Your current lib state reminds me more of a doc file instead of lib with inserted comments. In c++ I usually do:
JASS:
/**
    < 1-3 lines description here > bla bla bla

    @param:
    @return:
*/
If you want to give us a tip in regard what something is here for, add a single line instead of block. This is nice:
JASS:
    //* Our bucket struct which contains a trigger and its associated contents.
    private struct bucket
And this is not:
JASS:
        //* This method is for adding a handler to the system. Whenever a
        //* handler is added, damage detection will immediately trigger that
        //* handler. There is no way to deallocate a handler, so don't try to
        //* do this dynamically (!) Support for handler deallocation is
        //* feasible (please contact me)
        public static method addHandler takes code func returns nothing
Whatmore, instead of:
JASS:
call myfunction1
// comment here related to myfunction2 or just algorithm step
call myfunction2
It's better to do:
JASS:
call myfunction1

//comment here related to myfunction2 or just algorithm step
call myfunction2
When explaining API, it's nice to give some breaks between configurables/methods especially if you want to write more that a couple of words. Blocks such as:
JASS:
//* boolean ADD_ALL_UNITS: If enabled, a trigger turns on which automatically
//*     registers all units in the map.
//* integer BUCKET_SIZE: How many units to add to each 'bucket' - a larger
Could as well be splited to:
JASS:
*          boolean ADD_ALL_UNITS
*          - If enabled, a trigger turns on which automatically registers all units in the map.
*
*          integer BUCKET_SIZE
Consider that this is part most users look at when reading your stuff. If those are unexperienced, scrolling down is even less possible. However, to not ruin what I've said previously: readability of system is still nice, even if just for reviewers.
Also, you have explained the meaning of each global at the top, only to repeat it down in the code - if description is good, it only needs to be written once.

In your script, I've noticed that you don't like spaces. Meaby it's just your writting-style. Spaces are good tho, especially when there are multiple operators such as ++, --, ==, =, >, < around.

There were a lot of "thistypes". Since every field member in your struct is static, and considering that when working with "normal" scripting/programming language you usually don't prefix your members with anything its good to drop it.

You use bucket structure, and when "thistype" spam was present, multiple times I've noticed something as: field.field2.field3.invoke(). When such strings appear in groups, you should reduce them as much as possible -> we don't actually need to see whole road of invocation.

All good points, thanks. I assumed there were algorithm/data-structure related issues when you said that you can't read the script.

Regardless, I would like to improve the points you mentioned.

Thanks
 
For some reasons, it causes bugs, I tried to count how many hits for the damaged unit, to picture this, the situation is like this;

I have 4 units, so the count is suppose to be 4 to be damaged, but it returned 8, which is wrong...

On the other hand I tried looking_for_help's DamageEvent, coz maybe I did something wrong, but it returned perfect 4...

I want to use this system, but it has bugs?...if you want the map I can give it to you...
 
Hello,

Structured DD provides an interface to the EventUnitDamaged native; it does not try to correct the behavior of the native. I suggest printing GetEventDamage() , my intuition tells me you are registering some damage events with value 0.0 due to on hit effects like bash, or otherwise. Let me know how that goes.

Best regards,
 
Level 23
Joined
Apr 16, 2012
Messages
4,041
well maybe it would be good to support it locally, but I think notice in the documetation(I went over it and there seems to be no hint to this) that it doesnt filter.

It is additional stuff, so I understand why it isnt there(implicit thought of not being there == not implemented).
 
Top