Arrow Spells

Hey guys, I found sthg. My script gets VERY buggy when the units gets out of mana... Think about it.
So I admit i've sinned, I broke up a dota 6.XX code to see how Impetus works. I copied the script, OPTIMIZED it, and i think you should take a look at it. I think it's better than mine. It uses the "buff-detection" that so many people here advised, and it doesn't get buggy.
Sry, i don't know who the real author is (i bet it's not IceFrog), so i give a credit to Anonymus. It is very tricky with that condition, i like it...

function Trig_ArrowSpells_Conditions takes nothing returns boolean
    return GetLearnedSkill() == 'A00B' and GetLearnedSkillLevel() == 1
endfunction

function ArrowSpells_Damage takes nothing returns nothing
    local unit target = GetTriggerUnit()
    local unit attacker = GetEventDamageSource()
    if GetUnitAbilityLevel(target, 'B001')>0 and GetUnitAbilityLevel(attacker, 'A00B')>0  then
        call DestroyTrigger(GetTriggeringTrigger())
        //
        //Effects come here:
        //
    endif
    set target = null
    set attacker = null
endfunction

function ArrowSpells_Condition takes nothing returns boolean
    if GetTriggerEventId() == EVENT_PLAYER_UNIT_ATTACKED then
        return GetUnitAbilityLevel(GetAttacker(), 'A00B')>0 and GetHandleInt(GetAttacker(),"arrowon")==1 and not IsUnitType(GetTriggerUnit(), UNIT_TYPE_STRUCTURE)
    elseif GetTriggerEventId() == EVENT_UNIT_ISSUED_ORDER then
        if GetIssuedOrderId() == OrderId("poisonarrows") then
            call SetHandleInt(GetTriggerUnit(), "arrowon", 1)
        elseif GetIssuedOrderId() == OrderId("unpoisonarrows") then
            call SetHandleInt(GetTriggerUnit(), "arrowon", 0)
        endif
    elseif GetTriggerEventId() == EVENT_UNIT_SPELL_EFFECT then
        return GetSpellAbilityId()== 'A00B'
    endif
    return false
endfunction

function ArrowSpells_Actions takes nothing returns nothing
    local unit target = null
    local trigger dmgtrig = CreateTrigger()
    if GetTriggerEventId() == EVENT_UNIT_SPELL_EFFECT then
        set target = GetSpellTargetUnit()
    else
        set target = GetTriggerUnit()
    endif
    call TriggerRegisterUnitEvent(dmgtrig, target, EVENT_UNIT_DAMAGED)
    call TriggerAddAction(dmgtrig, function ArrowSpells_Damage)
    call TriggerSleepAction(2)
    call DestroyTrigger(dmgtrig)
    set dmgtrig = null
    set target = null
endfunction

function Trig_ArrowSpells_Actions takes nothing returns nothing
    local trigger t = CreateTrigger()
    local unit attacker = GetTriggerUnit()
    call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_ATTACKED)
    call TriggerRegisterUnitEvent(t, attacker, EVENT_UNIT_SPELL_EFFECT)
    call TriggerRegisterUnitEvent(t, attacker, EVENT_UNIT_ISSUED_ORDER)
    call TriggerAddCondition(t, Condition(function ArrowSpells_Condition))
    call TriggerAddAction(t, function ArrowSpells_Actions)
    set t = null
    set attacker = null
endfunction

//===========================================================================
function InitTrig_ArrowSpells takes nothing returns nothing
    set gg_trg_ArrowSpells = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ(gg_trg_ArrowSpells, EVENT_PLAYER_HERO_SKILL)
    call TriggerAddCondition(gg_trg_ArrowSpells, Condition(function Trig_ArrowSpells_Conditions))
    call TriggerAddAction(gg_trg_ArrowSpells, function Trig_ArrowSpells_Actions)
endfunction

I would just implant a mana check into the conditions, rather than doing UGLY buff detection that will cause more than 1 unit using the same spell to interfere with eachother. Thus, that method of checking is only good for games such as AoS maps where there is only 1 unit that is able to use that ability at any one time.

(, I wonder what would happen if you tried to run that in DotA's Same Hero mode or whatever its called... looks like it would cause multi-hitting)

Or, store the Attacker on the Attacked Unit, and check if it is the Damage Source in a condition. That would make it fully MUI.

the Mana way uses less handles, which is a good thing

paskovichYou're right with removing the 'flamer' in the damagetrig IN THIS CASE. But if you want sthg complex effect (or maybe more damage), you need the attacker.

Storing "DamageSource" and "DamageTrigger" is the key moment of the whole thing! This makes it 99% (i haven't found any) bugless.

I still don't understand why you should store DamageSource and DamageTrigger in GameCache. If you want to use the attacker, you can just use the function GetDamageSource() (like I did in my version of the condition). And what should you need the dmgtrig for? Blocking it? You could also do that using an extended condition (e.g. checking the target's custom value). GameCache is not that fast, as far as I know. Well, in this case this won't play an important role since the spell won't be casted that often.

paskovichIn which condition does the unit leak?

I'm not completely sure since I'm not a leak-specialist, but what's with this:

function Trig_ArrowSpells_Hit_Conditions takes nothing returns boolean
return GetEventDamageSource() == GetHandleUnit(GetTriggeringTrigger(), "DamageSource")
endfunction 

Oh and this Dota-Version really looks like it would cause multi-damage and on the other hand it's very difficult to read because of its more or less sensless multievents. I like yours better!

Aye, the OPTIMIZED DotA [stolen] version needs a better check, as multiple heroes of the same type can cause screwups that way

Ok first of all, "Why storing DamageSource?":
Let's think it over. We have a trigger with EVENT_UNIT_ATTACKED. This fires every time, the unit is attacked by an other one. The trigger creates an other one, that detects the damage caused by any unit. What if the unit is attacked, and while the projectile is still "flying", the unit is damaged by an other (probably melee) unit. Storing the attacker, and checking it in the condition precludes these (exrteme) situations to happen.

"Why storing the DmgTrigger?":
What if the unit misses? The damage trigger is created, and still waiting for the event. The unit attacks again, but this time it does not miss, an other damage trigger is created. Now there are 2 active damage triggers. (Repeat: Storing the trigger, and checking it in the condition precludes these (exrteme) situations to happen.)

And i don't think that condition leaks. But I think PurplePoot can tell this. He's the expert...

And also an other thing about my script: It does not work if the ability has a cooldown... It is a bigger problem than mana.

course it doesnt leak, or not in any way that I can tell.

We can basically break leaks down into 4 types

A) Handle Leak

B) Null Leak

C) Gamecache Leak

D) String Leak

a Handle Leak is when we drop all references to a from-that-point-on-useless handle (thus not a trigger condition, etc), but leave the handle lying around, thus wasting memory

a Null Leak occurs when you have a local variable that is a handle, which you do not (set <name> = null) when you are done with it. The leak itself should not make much difference in performance, but there is no reason not to prevent it, unless the solution is slower than the leak

a Gamecache Leak is when you are using the Handle Vars and omit to FlushHandleLocals (or different names for different systems) at the end.

a String Leak occurs once for every string displayed, and is unpreventable

As you can probably see, that does not fall under any of these

Thanks for the leak help, seems clear to me now, PurplePoot!

Now again to our main discussion (btw, if you don't like to discuss your script any longer, just say so and I'll keep quite!):

paskovichOk first of all, "Why storing DamageSource?":
Let's think it over. We have a trigger with EVENT_UNIT_ATTACKED. This fires every time, the unit is attacked by an other one. The trigger creates an other one, that detects the damage caused by any unit. What if the unit is attacked, and while the projectile is still "flying", the unit is damaged by an other (probably melee) unit. Storing the attacker, and checking it in the condition precludes these (exrteme) situations to happen.

The trigger does not fire everytime actually, at least not its actions, because there is the condition, that the attacker has the arrow ability turned on. To check this, you don't need to store the attacker itself if you do the check like I did in my version of the condition. This way you will also only fire once for each missile, because there can always impact only one missile from one attacker at the same time.

"Why storing the DmgTrigger?":
What if the unit misses? The damage trigger is created, and still waiting for the event. The unit attacks again, but this time it does not miss, an other damage trigger is created. Now there are 2 active damage triggers. (Repeat: Storing the trigger, and checking it in the condition precludes these (exrteme) situations to happen.)

You already kill old created triggers after a short wait in your script, don't you? The only possibility that this does not work is that the caster fires again before the first missile impacts, am I right?
Well, usually not the case but you did well in preventing everything. Slowly but surely
I get behind the quality of your script.
I myself normally try to optimize my spells for my special single case and not as multicompatible as possible - another way of working.

And also an other thing about my script: It does not work if the ability has a cooldown... It is a bigger problem than mana.

Hm, you're again right. Not that easy if you still want to take care of the possibility of more than one missile of the same unit in the air. If you exclude this possibility, it should be enough to check the cooldown in the "unit is attacked" trigger, I think.

I look forward to your examples making my ideas not work anymore!

...there can always impact only one missile from one attacker at the same time

In some extreme situations (and of course when there are more units with the ability) this CAN happen! Each attacker has the ability, each of them "arrowon" set to 1, now what if they attack the same unit in the same second? This "storing-the-attacker" thing is only a safety action that is useful in maps more simple units have the ability (for exaple in a melee map), and the player controls and orders them in a group. But if you have only one unit with the ability (like in hero arena maps), it's not neccessary of course.

You're right with the trigger-storing thing, weird things (double effect) can happen if the unit's attack cooldown is somewhere below 2 (or how much we wait in the trigger to destroy the damage trigger).