[System] Heal

[Troll-Brain]

Honestly and without a joke i think it would be better to have a custom IsUnitDead function which takes an additional argument, a boolean would be enough (or maybe to be even more accurate an integer flag based on constants integers).

Something like that :

function IsUnitDead takes unit whichUnit , boolean trulyDead returns boolean
    // trulyDead = true > IsUnitDead returns true only if the unit is not under resurection process or whatever
    // trulyDead = false > IsUnitDead returns true only if the unit is under resurection process or not
endfunction

The name "trulyDead" is maybe lame but hopefully you get the idea.

 

[Bribe]

UnitEvent also has the IsUnitReincarnating check.

 

[Troll-Brain]

I personnaly think it fits better with one function and an additional argument, but that's me.
Now that i remember i'm talking to a speedfreak Mag, i suppose he won't like it, since imagine you declare a new argument each time you use the function and there is at least one if/then, it will cause your game to lag !

Seriously i talked more about a general standalone custom IsUnitDead function, not only in this library.

 

[Magtheridon96]

I don't like it for 2 reasons:
- It won't inline
- You could use IsUnitReincarnating as Bribe said

Man, you know me so well

edit

Actually:

function IsUnitDeadEx takes unit whichUnit, boolean trulyDead returns boolean
    return IsUnitDead(whichUnit) == trulyDead or IsUnitReincarnating(whichUnit)
endfunction

Clever?
If you think about it, this function works perfectly

 

[jim7777]

yeah there working now D:
you forgot to add UnitEvent in the documentation

 

[Troll-Brain]

Mag, this won't work.

Imagine the unit is just alive, and you have trulyDead to false, then IsUnitDeadEx will returns true while it would be false since the unit is alive.

 

[Magtheridon96]

Oh ok
Forget it then ^_^

 

[Nestharus]

It might be better to use 2 timers rather than 1.

One timer for destroying the effect
One timer for applying the effect

This would be a faster solution. You would no longer have to do the +.03125 thing and compare it to the duration of the effect ; ).

When I do over time effects like this, I always do 2 timers, one for application and one for expiration.

 

[Magtheridon96]

I used to do the same thing too
I stopped doing it because I somehow got the idea that: More Timers = Slower Map
I might consider your solution though

 

[Nestharus]

More timers != slower map. More timers expiring at the same time = slower map ; P. However, if you make it so that the expiration one expires first, then the application one won't run =). Also, if you make them expire at slightly different periods, then they won't expire at same time : ).


If they both expire at the same time, I'm not sure which would be faster =o.

 

[Bribe]

I'd like to see you expand the documenation with an argument on what makes this resource so great.

I'd like the emphasis to be on its usefulness, not on its speed. A timer system or a timer-dependant system can brag about speed, and I admire that you have part of this resource dedicated to that, but I could program that myself. What I'd like to see is why this event detection is important, how it fits into warcraft 3 and what kinds of things would benefit from it where they couldn't benefit from it before.

 

[Magtheridon96]

sure

i'll get to work on that as soon as i can fix my keyboard (i'm using a slow on-screen keyboard and it's taking me forever to type

 

[Bribe]

On screen keyboard oh my god. Don't get me started how horrible that is. I do my best work on my charming little Asus laptop at home and the keyboard is extraordinarily ergonomic. I can't not have an ergonomic keyboard and Asus has spoiled every other keyboard for me. This Apple keyboard at work is one of the worst I've ever used.

 

[Magtheridon96]

Updated.

I listed a few uses for the events in the documentation and because I'm so evil, I broke backwards compatibility by changing the names of the events:
- onInstant -> INSTANT
- onTimed -> TIMED

This made sense to me because of how I named ANY.

Sorry guys.

edit
Oh, and I made a slight optimization.

 

[watermelon_1234]

You should include documentation of the struct API with the function API since they are accessible.

The only thing I might suggest would be allowing the user to stop the healing over time prematurely, but that might be too complicated.

 

[Bannar]

Forcing user to pass boolexprs sucks.

    function Register<freaking event> takes boolexpr b returns nothing
        call <freaking event>.register(b)
    endfunction
    
->>>>>
    function Register<freaking event> takes code c returns nothing
        call <freaking event>.register(Filter(c))
    endfunction

 

[Bribe]

Yeah code should always take the place of "boolexpr", this shortens the user's required verbosity and also allows the passed code to "return nothing".

 

[Magtheridon96]

You should include documentation of the struct API with the function API since they are accessible.

Done.

The only thing I might suggest would be allowing the user to stop the healing over time prematurely, but that might be too complicated.

I'm not going to add that ;D
It's still possible while taking advantage of the TIMED event though.
That may be inefficient, but you aren't going to be doing something like that a lot of the time
I might consider adding something like this in the future, but that could make 'Heal' stacking impossible without countless 'pointer' Tables

Forcing user to pass boolexprs sucks.

Yeah code should always take the place of "boolexpr", this shortens the user's required verbosity and also allows the passed code to "return nothing".

Agreed and Fixed.

 

[Bribe]

You should link to required resources. It would also benefit if you had some example script that showed off testing this out.

 

[Magtheridon96]

Done with the links, time to write a demo script.

edit

Added Demo Code. (Guess what?! It compiles )

 

[Bribe]

Approved.

You can merge the two structs btw.

 

[Magtheridon96]

True, but the difference would be 1 global integer ;P

 

[Troll-Brain]

True, but the difference would be 1 global integer ;P

Strange, you even don't care about it ?!!!!

What happens to you ?

 

[Bribe]

Well the optimizer is "supposed to" kill that constant integer.

 

[Magtheridon96]

What happens to you ?

I'm just tired now ;p
I'll see if I can combine those structs later

edit
Updated about an hour ago ;P

 

[Magtheridon96]

Ok, NOW this compiles.

 

[Whaleboobs]

I get the "UNIT INDEXER ERROR: ATTEMPT TO LOCK NULL INDEX" error. But it seems to work. Could i just comment this debus msg out? or even better, whats the cause?

 

[Nestharus]

It happens when you are attempting to heal a unit that isn't indexed (null unit or w/e).

-> call sourceId.lock()

that could also be the cause. If the source unit is null, it'll throw that error.


Blame mag, lol

 

[Magtheridon96]

Ok, I know the cause.
When you want to heal a unit, most of the time, you're going to pass 'null' as the source.

I'll fix this.

edit
Updated.

 

[jim7777]

Go ahead and graveyard Constant Timer Loop 32. It is totally deprecated by the new CTM Module ;o.

http://www.hiveworkshop.com/forums/members/bribe/#vmessage279187

are you gonna update this one with Timer Tools??

 

[Magtheridon96]

Yes.

 

[Nestharus]

Ok, here comes the nasty nestharus review on your resource because I felt like making you mad at me too ;p

static boolean enabled
Useful for things like prevent units from being healed for a period of time. Good job.

*       - Heal.INSTANT
*           - Fires on instant heal events.
*           - This would be used for systems like:
*               - Texttag Displays
*
*       - Heal.TIMED
*           - Fires 32x a second during a timed heal.
*           - This was only added for a sense of 
*             completeness. It may be useful.

These are useless. The only thing you need is ANY. This is useful for things like modifying a heal as well as displaying texttags. However, if you are going to do healing modifications, you should support healing types, that way the user know how to modify the heal. This would also mean that you could have 4 healing effects on a unit, meaning that you'd fire ANY for all 4 of them. You'd also want to show the texttag for the combined healing.

Look at my DamageEvent stuff in order to see how to properly implement this to support cool things.

The only way this library will ever be useful is if you code spells as a variety of effects. For example, one spell could have a healing effect + an armor boosting effect for a period of time. It'd create the healing effect using your heal system and the armor boosting effect using perhaps an incoming damage modifier.

Really, I'd almost implement a healing system by dealing damage of type HEAL or w/e that way it could run with the rest of the damage stuff. This would allow you to easily support healing types, like undead healing, which does in fact damage living targets.

I actually think that healing should only be done as damage now >.>. For example, if a unit has 33% spell resistance, they'd also resist healing spells. Having your thing not run on damage requires all of the same events to be registered for both damage and non damage types, which makes everything harder and messier to do.


So, in the end, my verdict is that this resource is not flexible enough to be a general system. If it was made to be flexible enough, then it'd be a copy of damage modifiers that just inverts the damage (-damage instead of +damage) ;\. Simpler maps would just use SetWidgetLife and advanced maps would use damage modifiers. The Heal system doesn't really fit in anywhere >.>.

 

[khamarr3524]

Does this allow negative values, to deplete hp over time?

 

[Magtheridon96]

Yeah.

 

[gorillabull]

bah this thing requires 6 other systems why dont you make it a bit more compatible

 

[Nestharus]

Many required resources = modularity. Few required resources = lots of identical repeated code in your own resource that could otherwise be abstracted to another resource.

Modularity = less code, less work, smaller map sizes, same performance.

A resource should only do a specific task =). Anything beyond that task can be abstracted out to another resource ^_-.

 

[gorillabull]

i dont think i can run the lua scripts for this