(DOT) Damage Over Interval Time. v3

[Moderator]

Damage Over Interval Time. v3 | Reviewed by Maker | 28th May 2013

APPROVED

The system works and it is MUI Fix the indentation Your doitDamageTypeIndex should be named doitAttackTypeIndex You could make the attack and damage types configurable Dead units could still get the credit from kills. Maybe check if the unit exists instead if it is dead

[tr]

 

[edo494]

I think saying to JH how big array should be has no effect whatsoever, maybe it initializes them but I doubt that
why arent those globals private?
I cant see more then several globals because on pyone you cant scroll over the jass tag so thats all from me right now

 

[Magtheridon96]

    unit array doit_Damager [8000]
    unit array doit_Target [8000]
    real array doit_Damage[8000]
    real array doit_Duration[8000]  
    integer array doit_Instance [8000]                                          //id of the current instance per player
    real array doit_Interval [8000]
    integer array doit_Counting   [8000]                                     //per player instance of number of running dots
    integer array doit_InstancePerInstance [8000]                    //HOV MANY INSTANCES EACH DOT HOLDS
    integer array doit_FreeIdValue [8000]                                 //  [instance id (doit_FreeidCount)]
    integer array doit_FreeIdCount [10]

You don't need to size these arrays.

The globals and functions should be private with the exception of the API.
(Everything in the library should be private except for the function that the user has to call)

FunctionNamesShouldBeWrittenLikeThis.

doit_One is not an appropriate name for a function.
Use something similar to Blizzard's natives like ApplyDamageOverTime

Don't use an InitTrig initialization function, use a library initializer:

library Lib initializer Init

    private function Init takes nothing returns nothing
        // This code runs on map init.
    endfunction
endlibrary

This (if GetWidgetLife(doit_Target[instanceID]) <= 0.000 then) is not a valid death check.
Do this instead: if IsUnitType(u, UNIT_TYPE_DEAD) then

Why are you limiting the number of instances?
The system should rely on dynamic indexing to manage instances. You would also use some sort of list/stack.

This is how you would generate a unique id for array storage:

local integer id = recycler[0] // recycler is an integer array.
if id == 0 then
    set instanceCount = instanceCount + 1 // instanceCount is an integer
    set id = instanceCount
else
    set recycler[0] = recycler[id]
endif
// id will be unique as of now.

After that, you'd want to insert it into your list.
You would manage a list with 2 arrays (next and prev, both integer arrays)

// This is how you insert an instance into the list.
// After you generate id from above,
set next[id] = 0
set prev[id] = prev[0]
set next[prev[0]] = id
set prev[0] = id

// This is how you would remove the instance from the list when you're done with it
set next[prev[id]] = next[id]
set prev[next[id]] = prev[id]

After removing an instance from the list, you're going to want to deallocate that instance to your recycler.
This is how you'd do it:

set recycler[id] = recycler[0]
set recycler[0] = id

Now as for the loop, this is how you would loop over all instances in a list:

local integer instance = next[0] // next[0] is the first instance in the list

loop
    exitwhen instance == 0 // exit when the list is empty

    // use instance as an index for your arrays here

    set instance = next[instance] // go to the next instance on the list
endloop

This is what you would have to do basically:
- Generate unique id
- Add id to the list
- Set the data using id as an array index

When the instance is done,
- Remove id from the list
- Deallocate id
- Null data while using id as array index

One other thing I noticed is your reliance on one timer per instance.
Instead of having one timer per instance, you can have one timer for the system entirely.
It would work like this:

You would keep an integer called count to keep track of the number of instances running.
When a new instance is created, increase count by 1. If count is then 1, start the global timer to iterate over instances in the list.
When an instance is destroyed, decrease count by 1. If count is then 0, pause the global timer to stop iterating over the instances in the list.

This way, your system would be pretty efficient for large numbers of DamageOverTime instances. (Hundreds)

 

[mckill2009]

- Lacks documentation
- Make your DoitLoop private
- Do not use TriggerRegisterTimerEvent, use timers so that if no more is using the system, pause the timer
- Use Timer systems such as TimerUtils or TimerTools
- Fix your indention

also, something like this and alike...
if IsUnitType(doitTarget[l], UNIT_TYPE_DEAD) ==false then
should be ==>
if not IsUnitType(doitTarget[l], UNIT_TYPE_DEAD) then

 

[Almia]

Indention brother

0.03125 must be 0.31250000 because reals are not accurate enough.

Just like what mckill2009, putting == false makes it slower. just use not.But if you want a faster,use else(me and bowser found it was faster than "not")
You can also use GetWidgetLife(myWidget) > 0.405

You should use TriggerAddCondition over action because actions are slower due to that they create new thread.

You should also add support for the users to use their own Attack types and Damage Types. Simply using a damagetype and attacktype array will support their needs.

edit
Indention. Never use The_Witcher's Aligner because it uses keywords to indent.
This is how The_Witcher's indention works:

//Example we have a word function here
                             Its indent like this

IsUnitType(doitTarget[l], UNIT_TYPE_DEAD) ==false
Also this. Just like what mckill said.

Also didn't read my suggestion,your system is not damage and attack type friendly.

You can also add "return" after the PauseTimer line,but that's not necessary.

You can also use Struct over functions.

 

[tadekza20]

Can i add Damagetype by my self ?
when i do it DOT system does not deals any damage??

17/9/2559