Damage Detection Woes ><.

02-06-2009, 02:51 AM

Well I decided to wander into the terrible world of Damage Detection. I made a horrible, horrible system which makes me cringe;

JASS:

library DamageDetection initializer init
    globals
        private trigger OnDamage
        private trigger WhenDamage = CreateTrigger()
    endglobals
    
    function OnDamageFunc takes nothing returns nothing
        local real r = GetEventDamage()
        local unit u = GetEventDamageSource()
        
        call BJDebugMsg(GetUnitName(u))
    endfunction
    
    function Wrapper takes nothing returns nothing
        local unit u = GetTriggerUnit()
        
        call TriggerRegisterUnitEvent(WhenDamage, u, EVENT_UNIT_DAMAGED)
    endfunction
    
    private function init takes nothing returns nothing
        set OnDamage = CreateTrigger()
        call TriggerAddAction(OnDamage, function Wrapper)
        call TriggerRegisterAnyUnitEventBJ(OnDamage, EVENT_PLAYER_UNIT_ATTACKED)
        
        call TriggerAddAction(WhenDamage, function OnDamageFunc)
    endfunction
endlibrary

Horrible efficiency issues. Not only does it create a trigger event whenever a unit is damaged, it adds it more then once EVERY attack...

So that won't work well. I'm rather confused as to how to implement a more stable system. It seems the 'cool kids' add a damage event to every unit and use conditions to parse through.

However, I heard somewhere that TriggerEvent leaks are not high-memory leaks, to top that I could add a condition to check if the attacked unit is being attacked by a unit registered to the system. I could also recycle the trigger at some point.

So really I'm looking to know if:

Destroying a trigger cleans TriggerEvents.

It's safe to destroy triggers, probably at a point where I've reached 1024 events or some large ambiguous number. (In other words, destroy them very seldom)

There isin't a better way I can do this (without following the 'cool kids' :P)

That's about it, if I know thoes things I can implement a more useful system that isin't awfulllll.

02-06-2009, 02:58 AM

Zerzax

Well, if you've looked through the submitted scripts you probably noticed that (a the authors did destroy triggers and (b gave people an option to keep the triggers permanently, if people were picky about the elusive bug or the event leaks. Dusk has frequently said that trigger event leaks are incredibly minor, so don't worry about destroying despite the leaks. Destroy them seldomly at intervals of 5 to 10 minutes and you will still be following in the footsteps of the authors. I don't think there's any way around it, though I've never experimented with "good" damage detection so I don't have the final word. Also, don't be so hard on yourself. Coding is just words, and experimenting will only deepen your knowledge.

02-06-2009, 03:10 AM

Rising_Dusk

I'm a firm proponent that the event leaks are a minor inconvenience at worst. They make absolutely no impact on the game's state at all; trust me on that, I've got some pretty hectic maps and they do just fine.

Anyways, I don't think making your own system makes a lot of sense. Standardizing damage detection is pretty easy these days. You've got options, particularly between Anitarf's ADamage and my IDDS. Both are really good, but work differently and can be used in different ways. I recommend checking them out in the code database.

02-06-2009, 05:33 AM

Blackroot

Quote:

Originally Posted by Rising_Dusk

Anyways, I don't think making your own system makes a lot of sense. Standardizing damage detection is pretty easy these days. You've got options, particularly between Anitarf's ADamage and my IDDS. Both are really good, but work differently and can be used in different ways. I recommend checking them out in the code database.

I code more as a hobby, really I'm not working towards a goal. That in mind, I'm more coding for the sake of coding, as opposed to coding to design something useful (although, I certainly like to try making them useful.) Also, I find trying to use other's code a hastle really, implementing it becomes hard and interfacing with it becomes very iffy. If I need certain things to do something different, I end up sifting through the sea to find a single pearl. Polymorphism is not a common design feature in wc3 code ;|.

Quote:

It is an inconvenience to be sure, and I recall destroying a thread while it's running is bad. Which means I'd have to announce a new thread and stop the trigger, throw a boolean to cause all new attempts at initializing the thread to stop (meaning I lose the ability to have correctly working damage detecting at that time) then disable the trigger, wait for some minescel period, and reinitiate the trigger.

I figure at best the odds of malfunction are insanely low. But as always my curiosity gets the best of me; Is it worth (if(x > y) do z) y times (where x is times run, y is the number of times to ignore x, and z is the set of functions to run if true) to have the condition equal true once... In the case of y = 8000 you run the trigger 8000 times and execute once. That in mind, how often do 8000 units exist and add to the stack? That in mind, it seems logical to me that I'd ignore this case and keep the leak.

So that in mind, I've re-designed my code keeping minimalizm in mind (wip):

JASS:

library DamageDetection initializer init uses UnitIdLib
    globals
        private trigger OnDamage
        private trigger WhenDamage = CreateTrigger()
        private integer EventStack = 0
        private integer EventFired
        private Table Units
        private Table RegisteredAttackers
        private Table RegisteredAttackees
    endglobals

    function interface OnDamageTrigger takes nothing returns nothing 
    
    function OnDamageFunc takes nothing returns nothing
        local real r = GetEventDamage()
        local unit u = GetEventDamageSource()
        
        call BJDebugMsg(GetUnitName(u))
        call BJDebugMsg("Damage Dealt " + R2S(r))
        call BJDebugMsg("Stack #... " + I2S(EventFired))
    endfunction
    
    function Wrapper takes nothing returns nothing
        local unit u = GetTriggerUnit()
        local unit n = GetAttacker()
        local integer I = 0
        
        set I = AssignUnitId(u)
        if(RegisteredAttackers[AssignUnitId(n)] == 0 or RegisteredAttackees[i] == 0)then
            call BJDebugMsg("Attacked " + I2S(AssignUnitId(u)))
            call BJDebugMsg("Attacker " + I2S(AssignUnitId(n)))
            return
        endif
        set EventStack = EventStack + 1
        if(Units.exists(I) == false)then
            set Units[i] = EventStack
            set EventFired = EventStack
            call TriggerRegisterUnitEvent(WhenDamage, u, EVENT_UNIT_DAMAGED)
        else
            set EventFired = Units[i]
        endif
    endfunction
    
    function RegisterAttackEvent takes OnDamageTrigger t, unit u returns nothing
        set RegisteredAttackers[AssignUnitId(u)] = 1
    endfunction
    
    function DeletedAttackEvent takes unit u returns nothing
        set RegisteredAttackers[AssignUnitId(u)] = 0
    endfunction
    
    function RegisterAttackedEvent takes OnDamageTrigger t, unit u returns nothing
        set RegisteredAttackees[AssignUnitId(u)] = 1
    endfunction
    
    function DeletedAttackedEvent takes unit u returns nothing
        set RegisteredAttackees[AssignUnitId(u)] = 0
    endfunction
    
    private function init takes nothing returns nothing
        set OnDamage = CreateTrigger()
        call TriggerAddAction(OnDamage, function Wrapper)
        call TriggerRegisterAnyUnitEventBJ(OnDamage, EVENT_PLAYER_UNIT_ATTACKED)
        
        set Units = Table.create()
        set RegisteredAttackers = Table.create()
        set RegisteredAttackees = Table.create()
        call TriggerAddAction(WhenDamage, function OnDamageFunc)
    endfunction
endlibrary

I like it actually, it's very simple. I love the fact I can use function interfaces, it simplifies everything. Vex should get an award for them or something :P.

I almost wonder if it's possible to recycle event indexs? Can anyone confirm/deny if I do this:

Code:

globals
 unit u
endglobals

call TriggerRegisterUnitEvent(sometrigger, u, EVENT_SOMETHING)

Will this consider code for whatever unit u was or whatever unit u is? (In other terms, will the unit registered be whatever u was (registered = uwas) or whatever it is (registered = u) sort of like a pointer?

I'll make a test ;P.

02-06-2009, 07:27 AM	#5
xombie	Rising_Dusk, where do you see this "ADamage" system? I typed exactly that in the search utility and it brought up nothing of relevance.

02-06-2009, 08:55 AM	#6
Captain Griffen	Anitarf submission.

02-06-2009, 11:13 AM

Anitarf

Quote:

Originally Posted by Blackroot

I'm rather confused as to how to implement a more stable system.

Really, try IDDS, ADamage or LLDD.

Quote:

Also, I find trying to use other's code a hastle really, implementing it becomes hard and interfacing with it becomes very iffy.

Maybe I'm biased because I remember a time before vJass but seriously, hard and iffy? As opposed to what, writing your own system which is so easy and non-iffy that you don't need any help at all with it... oh, wait, what's this thread for then?

02-06-2009, 11:47 AM

moyack

Quote:

Originally Posted by Anitarf

Really, try IDDS, ADamage or LLDD.

I wish to see mine included one day...

And for stay on topic, my system has solved the event leak issue by recycling the units.

02-06-2009, 12:12 PM

Rising_Dusk

Quote:

Originally Posted by moyack

my system has solved the event leak issue by recycling the units.

Technically, if used properly, your recycling of units could apply easily to any of the damage detection systems in the database.

Quote:

Originally Posted by moyack

I wish to see mine included one day...

Maybe! :)

Quote:

Originally Posted by Blackroot

I'm with Anitarf on this one. Using others' code is a blessing, because that code was made specifically for being used and required by newer libraries. That modularity is what, I think, makes vJass so great. If you use a damage detection system, it is because you want to detect damage. The ones in the database all do that extremely well.

If you are coding this as a hobby, that's fine, but don't claim it's because the stuff in the database is 'iffy' or 'hard' to implement. We don't even approve script resources if they match either of those qualities.

Quote:

Originally Posted by Blackroot

The one time it causes a problem it causes the map to utterly fall apart. I speak from experience; AotZ used to do this profusely. Back in the day, I was all into dynamic triggers. Bad idea. That one failure is enough to break the game in most cases.

If you must destroy them, wait like 20 seconds before doing so. It's a hellishly inelegant solution, but in most cases it works fine. (Unless a trigger in your map has a thread spaced over more than 20s, in that case make 20 a bigger number) The problem is destroying it while its thread is running (we currently think).

02-06-2009, 09:29 PM

#10

Flame_Phoenix

Quote:

Really, try IDDS, ADamage or LLDD.

If you are a begginer I strongly advise IDDS as it is the best so far. ADamage is also good, but is definitely suited for more advanced users and to make a correct/good use of it you need to import xe and xedamage module while IDDS works fine as a stand alone version. However IDDS is quite unstable, although it works fine, it's creator Dusk is still working on addons and aditions that the users request so using IDDS may give you trouble in the future due some updates. Yet, as I said, I strongly advise IDDS, I find it almost impossible to find something easier to use and with the shield add-on Dusk plans to release, it will soon become unstoppable.
About LLD, think it is outdated... or so I heard ... I think Griffen wanted to release a new version of it but I am not sure and I can not backup this statement, I know however that from the 3, it is the oldest afaik.

Quote:

I wish to see mine included one day...

And for stay on topic, my system has solved the event leak issue by recycling the units.

Moyack also did a great system, however his work is based on recycling units and not on the DD system, just like ADamage, IDDS and LLD are.
Although it does offer you DD control, it is more limited than the others afaik.

Also, you can always make your own system but keep in minds mods will most likely see no use for it, as they have designed their own systems for the same purpose.

Hope it helped, this is the view from a user that tested all the DD systems referred here(except LLD lol).
Talking about old DD systems, grim001 also made one pretty good, however it is outdated as well and it is on grave:
http://www.wc3campaigns.net/showthre...hlight=grim001

Also, i saw some people are having trouble to find ADamage, so:
http://www.wc3campaigns.net/showthre...hlight=ADamage

02-06-2009, 11:13 PM

#11

Blackroot

Maybe you'll see why i designed my own in a moment;

JASS:

library DamageDetection initializer init uses UnitIdLib
    globals
        private trigger OnDamage
        private trigger WhenDamage = CreateTrigger()
        private integer EventStack = 0
        private integer EventFired
        private Table Units
        private Table RegisteredAttackers
        private Table RegisteredAttackees
        private Table PreAttackEvent
        private Table PreAttackedEvent
    endglobals

    function interface OnDamageTrigger takes nothing returns nothing 
    
    function OnDamageFunc takes nothing returns nothing
        local real r = GetEventDamage()
        local unit u = GetEventDamageSource()
        local unit n = GetTriggerUnit()
        local OnDamageTrigger I 
        
        set I = RegisteredAttackers[AssignUnitId(u)]
        if(I > 0)then
            set I = I
            call I.evaluate()
        endif
        
        set I = RegisteredAttackees[AssignUnitId(n)]
        if(I > 0)then
            set I = I
            call I.evaluate()
        endif
    endfunction
    
    function Wrapper takes nothing returns nothing
        local unit u = GetTriggerUnit()
        local unit n = GetAttacker()
        local integer I = 0
        local OnDamageTrigger trig
        
        set I = AssignUnitId(u)
        if(RegisteredAttackers[AssignUnitId(n)] == 0 and RegisteredAttackees[i] == 0)then
            return
        endif
        set EventStack = EventStack + 1
        
        set trig = PreAttackEvent[AssignUnitId(n)]
        if(trig > 0)then
            call trig.execute()
        endif
        
        set trig = PreAttackedEvent[i]
        if(trig > 0)then
            call trig.execute()
        endif
        
        if(Units.exists(I) == false)then
            call BJDebugMsg("New event added to stack... Stack at " + I2S(EventStack))
            set Units[i] = EventStack
            set EventFired = EventStack
            call TriggerRegisterUnitEvent(WhenDamage, u, EVENT_UNIT_DAMAGED)
        else
            set EventFired = Units[i]
        endif
    endfunction
    
    function RegisterAttackEvent takes unit u, OnDamageTrigger t, OnDamageTrigger pre returns nothing
        if(t > 0)then
            set RegisteredAttackers[AssignUnitId(u)] = t
        endif
        if(pre > 0)then
            set PreAttackEvent[AssignUnitId(u)] = pre
        endif
    endfunction
    
    function DeletedAttackEvent takes unit u returns nothing
        set RegisteredAttackers[AssignUnitId(u)] = 0
        set PreAttackEvent[AssignUnitId(u)] = 0
    endfunction
    
    function RegisterAttackedEvent takes unit u, OnDamageTrigger t, OnDamageTrigger pre returns nothing
        if(t > 0)then
            set RegisteredAttackees[AssignUnitId(u)] = t
        endif
        if(pre > 0)then
            set PreAttackedEvent[AssignUnitId(u)] = pre
        endif
    endfunction
    
    function DeletedAttackedEvent takes unit u returns nothing
        set RegisteredAttackees[AssignUnitId(u)] = 0
        set PreAttackedEvent[AssignUnitId(u)] = 0
    endfunction
    
    private function init takes nothing returns nothing
        set OnDamage = CreateTrigger()
        call TriggerAddAction(OnDamage, function Wrapper)
        call TriggerRegisterAnyUnitEventBJ(OnDamage, EVENT_PLAYER_UNIT_ATTACKED)
        
        set Units = Table.create()
        set RegisteredAttackers = Table.create()
        set RegisteredAttackees = Table.create()
        set PreAttackEvent = Table.create()
        set PreAttackedEvent = Table.create()
        call TriggerAddAction(WhenDamage, function OnDamageFunc)
    endfunction
endlibrary

That's all I needed it to do. Just execute a function before and after a unit is hit. I don't need sorting or catagorization of neccesities, they take up too much computing power and snipping them out of a system takes to long, I'd rather write my own minimalist system.

The only problem with this is when a unit attacks but doesn't finish I'll need some sort of safety net.

02-06-2009, 11:15 PM

#12

Captain Griffen

Quote:

About LLD, think it is outdated... or so I heard ... I think Griffen wanted to release a new version of it but I am not sure and I can not backup this statement, I know however that from the 3, it is the oldest afaik.

I don't want to release a new version. Erm, I think there's a couple of additions I maybe should make to allow you to more easily stop feedback loops, but that's it. It's not the oldest; newer than IDDS anyway, not sure about ADamage.

02-06-2009, 11:30 PM

#13

Rising_Dusk

Quote:

Originally Posted by Blackroot

The only problem with this is when a unit attacks but doesn't finish I'll need some sort of safety net.

This is the worst possible way to detect damage.

Quote:

Originally Posted by Blackroot

I'd rather write my own minimalist system.

So you - instead of using a single trigger - are creating dynamic triggers and are running both an ATTACKED event and a series of DAMAGED events. This is not even considering all of the functions and the struct methods you left out from your code. Honestly, if I'd have to guess, I'd say the stuff in the database is doing less 'work' per run.

Quote:

Originally Posted by Blackroot

I don't need sorting or catagorization of neccesities, they take up too much computing power and snipping them out of a system takes to long

The prioritizing doesn't even happen at runtime in the IDDS... Have you even looked at the systems you are claiming take too much 'computer power'? If you want to even talk about how fast a system's response is, you'd better do some benchmarks before making accusations.

02-07-2009, 12:43 AM

#14

Blackroot

I have, as I said, looked over the code. It's not easy to read through.

What I gather from your system;

It sorts the stack when triggers are added and removed. Adds a damage event to all units on the map, ignoring attacked events (which I need.) And uses priority to catagorize which events to fire first.

It works, and probably very well. The problem is it completely ignores what happens before the damage.

Reguarding adamage, the whole thing is a giant struct-blob. I can't make heads or tails of it.

Moyacks system uses SetUnitUserData which I'm currently taking advantage of already.

And again, LLDD doesn't take advantage of when the unit is Attacked.

I'm not trying to bash good code, it all works but it doesn't do what I need. That's why I'm writing this.

Reguarding your criticizm of the code, I have two constant trigger that are never altered, and use simple execution for events stored in tables? I don't see where I'm creating dynamic triggers - I'm using dynamic events. Although, it looks like I'm going to need a timer.

If you'd like to point out my inefficiencies I'd love to know, right now I'd think this would be a fairly quick system but it sounds like you know something I don't about what this is doing.

02-07-2009, 01:15 AM

#15

moyack

Quote:

Originally Posted by Flame

Did you know that in the first post I offer a DD independent of Unit recycling??? And why do you consider it limited?? because I don't offer a function DoShield?? man...

Quote:

Moyacks system uses SetUnitUserData which I'm currently taking advantage of already.

Yes??? I don't remember using that function...

The thing is that we need to know what are your needs in DD in order to give a more accurate critic and suggestion. In my case, I did it in this way because in my map I don't have any problem adding a set of conditions to be evaluated to all the units that can receive damage, but probably in your map you need to categorize the detection and effects in other way.

My suggestion is that you think what you really want. Define if it's necessary to manage one trigger per spell per unit, or one trigger managing all the events in the game, or a trigger per unit storing all the damage events, etc...