TimedAbility v1.1

[ZiBitheWand3r3r]

I'm going to use this lib in UtimateBattle project to simplify many spells, I'm very interested in your opinions / checks
edit 06-07-2017
updated to version 1.1

in function UnitAddTimedAbility I replaced

Spoiler: this

    local integer oldLevel = 0
    if GetUnitAbilityLevel(u, abiId)>0 then
        if Update(u, abiId, abiLevel, duration, eff, attachmentPoint) then //has ability /from this library/ and updated succefully
            return
        else                                                        //has ability but *not* from this library
            set oldLevel = GetUnitAbilityLevel(u, abiId) //remember ability level
        endif
    endif

to

Spoiler: this

    local integer oldLevel = GetUnitAbilityLevel(u, abiId) //remember ability level
    if oldLevel>0 and Update(u, abiId, abiLevel, duration, eff, attachmentPoint) then
        return //has ability [from this library] and ability was updated succefully
    endif

 

[Reventhous]

My quick review
-I think UnitAddTimedAbility should returns integer as user will be able to to store the id
-

            set instTarget[id] = instTarget[maxId]
            set instAbilityId[id] = instAbilityId[maxId]
            set instDuration[id] = instDuration[maxId]
            set instRemoveWhenUnitDies[id] = instRemoveWhenUnitDies[maxId]
            set instBuffId[id] = instBuffId[maxId]
            set instBuffCheckDelay[id] = instBuffCheckDelay[maxId]
            set instCounter[id] = instCounter[maxId]
            set instBuffSet[id] = instBuffSet[maxId]
            set instEffect[id] = instEffect[maxId]
            set externalAbiLevel[id] = externalAbiLevel[maxId]
            //clean:
            set instTarget[maxId]=null
            set instEffect[maxId]=null
            //---
            set maxId=maxId-1
            set id=id-1

I don't think this kind of indexing is suitable for this kind of system.
Imagine, the maxId is 20 then user destroy 15. So,

            set instTarget[15] = instTarget[20]
            set instAbilityId[15] = instAbilityId[20]
            set instDuration[15] = instDuration[20]
            ...
            //clean:
            set instTarget[20]=null
            set instEffect[20]=null
            //---
            set maxId=maxId-1 //==19

 

[ZiBitheWand3r3r]

instance nr 15 expires
data from instance 20 (maxID) has been moved to variables with index 15
there are 19 active instances,
IF new instance will be created it will be with number 20
I cant see anything abnormal here.. or I did not understand what you mean.

original native UnitAddAbility returns boolean. What for user should need integer id? It is ability added for given duration, or when speciic buff dissaperars, instantce is not supposed to be destroy-able by a user.

 

[Reventhous]

if instCounter[id] <= 0 and (GetUnitAbilityLevel(instTarget[id], instBuffId[id])==0) then

I'd miss that part, XP.
So, everything looks fine but like I always say, better with struct

Btw, is there any system like this out there?

 

[ZiBitheWand3r3r]

yeah, by Almia
Timed Ability v1.3

 

[EdgeOfChaos]

You could simply this system immensely by using TimerUtils with a struct that remembers the ability added and the unit, plus whatever other fields you want. For example:

struct TempAbilityData
    unit whichUnit
    integer whichAbility
endstruct

And in place of that big timer loop:

local TempAbilityData data = TempAbilityData.create()
set data.whichUnit = ...
set data.whichAbility= ...
call TimerStart(NewTimerEx(data),duration,false,function removeAbility)

Then in your removeAbility, just grab the data and do whatever you need

local timer t = GetExpiredTimer()
local TempAbilityData data = GetTimerData(t)
call ReleaseTimer(t)
//now use data.whichUnit and data.whichAbility

 

[ZiBitheWand3r3r]

I need this buff-checks so I need periodic checks. In many spell I use this feature.

 

[Almia]

Can you elaborate the advantages of this resource against TimedHandle by TriggerHappy(?)

 

[ZiBitheWand3r3r]

TimedHandles - Wc3C.net

TimedHandles Use this to destroy a handle after X amount of seconds.

I see examples to use TimedHandles for timed effects and lightning, I dont know how to extend this for timed abilities. Sorry Reventhous, I still don't get structs And it uses hashtable.

This lib has no requirements, no hashtable. With each instance it is possible to set boolean <removeOnDeath>, you can (not obligatory) specify a buff, and add special effect (if you wish to). I guess I described function UnitAddTimedAbility parameters well at the top.

I just uploaded this to gather users opinions. I already use this lib in UB project after many tests. If you decide it is useless - no problem for me to remove this library.

 

[Flux]

Some thoughts:
- I think you overdo the number of input arguments. I think the only important ones are unit u, integer abilityId and real duration. Based on experience, functions with lots of input arguments are a pain to work with. You offered an alternative which is "UnitAddTimedAbility_Simple" but I think this one should be the main API and just add "UnitAddTimedAbilityEx" that includes all your other stuffs (buff, effects, attachments) if you really want to include those.
- I haven't thoroughly check the code, but I think applying this sequence of actions will reveal a bug.
> Add ability X for 5 seconds
> Wait 3 seconds
> Add ability X (same ability) for 10 seconds
> Wait 2 seconds
> Ability gets removed when it should not be. (Bug)
> Wait 8 seconds
> This is the part when ability should be removed.

 

[Almia]

TimedHandles - Wc3C.net

I see examples to use TimedHandles for timed effects and lightning, I dont know how to extend this for timed abilities. Sorry Reventhous, I still don't get structs And it uses hashtable.

This lib has no requirements, no hashtable. With each instance it is possible to set boolean <removeOnDeath>, you can (not obligatory) specify a buff, and add special effect (if you wish to). I guess I described function UnitAddTimedAbility parameters well at the top.

I just uploaded this to gather users opinions. I already use this lib in UB project after many tests. If you decide it is useless - no problem for me to remove this library.

I'm not updated to Hive's JASS policy afaik hashtables are of no concern. TimedHandle can perform with or without the hashtable.

Anyways, I can't say if i'm right or wrong, I have been inactive in this community for like 2 years lol