[JASS] Freeze when 1 specific part of a trigger executes

[KitsuneTailsPrower]

This is a follow up on this topic (please read it first): [vJASS] - Solved! Looking for the cause of a freeze with this library

I attempted to re-add the "hero dies bonus" trigger with some code tweaks but it still freezes. See comments in the code for more information.

function CriticalTextForPlayer takes unit whichUnit,string text,real duration,integer red,integer green,integer blue,integer alpha,real text_size,player p returns nothing
                local texttag tx= CreateTextTag()
                
                call SetTextTagText(tx, text, text_size)
                call SetTextTagPosUnit(tx, whichUnit, 0)
                call SetTextTagColor(tx, red, green, blue, alpha)
                call SetTextTagLifespan(tx, duration)
                call SetTextTagVelocity(tx, 0.0, 0.0355)
                call SetTextTagPermanent(tx, false)
        if GetLocalPlayer() != p then
            call SetTextTagVisibility(tx, false)
        endif
                set tx=null
endfunction

function Trig_Speedster_Hero_Dies_Bonus_Conditions takes nothing returns boolean
    if not UnitAlive(udg_speedster_unit) then
    return false
    endif
    if not IsUnitType(GetTriggerUnit(), UNIT_TYPE_HERO) then
    return false
    endif
    if ( ( GetKillingUnit() == udg_speedster_unit ) ) then
        return true
    endif
    if DistanceBetweenPointsReal(GetUnitX(udg_speedster_unit) , GetUnitY(udg_speedster_unit) , GetUnitX(GetTriggerUnit()) , GetUnitY(GetTriggerUnit())) <= 2000.0 then
    return true
    endif
    return false
endfunction

function Trig_Speedster_Hero_Dies_Bonus_Timer_Expires takes nothing returns nothing
    call CriticalTextForPlayer(udg_speedster_unit , "Death Bonus!" , 1.5 , 0 , 255 , 0 , 255 , 0.019 , GetOwningPlayer(udg_speedster_unit))
    call UnitAddCooldownReductionFlat(udg_speedster_unit , udg_lesser_cdr_value)
    //call AddUnitBonus(udg_speedster_unit , BONUS_STRENGTH , 5.0)
    //call AddUnitBonus(udg_speedster_unit , BONUS_INTELLIGENCE , 5.0)
    //call AddUnitBonus(udg_speedster_unit , BONUS_AGILITY , 5.0)
    call SetHeroStr(udg_speedster_unit,GetHeroStr(udg_speedster_unit,false)+5,true)
    call SetHeroInt(udg_speedster_unit,GetHeroInt(udg_speedster_unit,false)+5,true)
    call SetHeroAgi(udg_speedster_unit,GetHeroAgi(udg_speedster_unit,false)+5,true)
    call DestroyTimer(GetExpiredTimer()) //[S]Freezes on this line, I see the stats go up just as the recording ends[/S] Actually this is wrong, the hero was leveling up //as it froze. I saw the text from a critical hit from an object editor based Critical Strike skill for the fatal blow but not the "death bonus" text mentioned above.[S][/S]
endfunction

function Trig_Speedster_Hero_Dies_Bonus_Actions takes nothing returns nothing
    call TimerStart(CreateTimer(), 0.03, false, function Trig_Speedster_Hero_Dies_Bonus_Timer_Expires) //Using a timer here because for some reason running the same actions without one also freezes
endfunction

//===========================================================================
function InitTrig_Speedster_Hero_Dies_Bonus takes nothing returns nothing
    set gg_trg_Speedster_Hero_Dies_Bonus=CreateTrigger()
    call DisableTrigger(gg_trg_Speedster_Hero_Dies_Bonus)
    call TriggerRegisterAnyUnitEventBJ(gg_trg_Speedster_Hero_Dies_Bonus, EVENT_PLAYER_UNIT_DEATH)
    call TriggerAddCondition(gg_trg_Speedster_Hero_Dies_Bonus, Condition(function Trig_Speedster_Hero_Dies_Bonus_Conditions))
    call TriggerAddAction(gg_trg_Speedster_Hero_Dies_Bonus, function Trig_Speedster_Hero_Dies_Bonus_Actions)
endfunction

 

[Pyrogasm]

You didn't specify here: what exactly happens if you comment out the DestroyTimer line? No freeze and everything about the code seems to be working as intended? In a scenario like this, the line of reasoning I follow is: how much of this can I comment out and still encounter <the issue>? I would try to understand what needs to be commented to prevent the freeze, and what can be commented out that doesn't change anything at all.

It's likely not the issue, but the set of conditions you've built for this trigger is strange and doesn't work like you might expect. I imagine you want to run the trigger only in this specific instance:

1. Speedster is alive.
2. The dying unit was a hero.
3. The unit was killed by the Speedster.
4. The Speedster is within 2000 units of the killed hero.

But that isn't what will happen. The first return statement that the condition function encounters will be what's returned, and it won't check the rest of it. So in order:

1. If the speedster is dead: FALSE
2. If the killed unit was a hero: FALSE
3. If the unit was killed by the Speedster: TRUE <----- this line trumps the one below because it is encountered first, so the next is never checked
4. If the Speedster is close enough: TRUE

So your distance function is basically irrelevant. Every time any hero is killed by the Speedster (and the Speedster is alive), the trigger will run. Distance will never be checked. Here is how the condition function should be structured:

function Trig_Speedster_Hero_Dies_Bonus_Conditions takes nothing returns boolean
    return UnitAlive(udg_speedster_unit) /*
         */ and not IsUnitType(GetTriggerUnit(), UNIT_TYPE_HERO) /*
         */ and GetKillingUnit() == udg_speedster_unit /*
         */ and DistanceBetweenPointsReal(GetUnitX(udg_speedster_unit) , GetUnitY(udg_speedster_unit) , GetUnitX(GetTriggerUnit()) , GetUnitY(GetTriggerUnit())) <= 2000.0
endfunction

This comment concerns me: //Using a timer here because for some reason running the same actions without one also freezes because I don't see any reason that should be the case. Such a technique is usually used to overcome recusion or when you need a frame-delay for something to update, neither of which are relevant here. Total spitball ideas:

• Is there any other way for this Speedster to gain Str/Agi/Int on-kill? Maybe there's some issue with trying to set the stats in an on-death event that leads to recursion because they're being modified in two different ways. (Of course this should all be sequential but something odd is happening here.)
• You aren't using a timer stack/allocation system that expects you to use something like NewTimer() and ReleaseTimer() instead of creating and destroying them, are you? If you're not, you could try using such a thing to see if it affects the freeze.

 

[Luashine]

//Freezes on this line, I see the stats go up just as the recording ends

If you see the stats increase then the game had enough time to render the next frame. Sometimes the crash happens before the next frame with updated data is rendered.
The wisdom of the ancients says to use PauseTimer first before destroying timers.

I wonder if passing GetExpiredTimer() directly as an argument is an issue? What if it's saved into a local variable and that variable is used as the argument. Remember to null this local at the end of the function.

I'd like to have this crashing timer map for testing please? If you don't want to share the full map publicly, I would message you on Discord.

 

[KitsuneTailsPrower]

You didn't specify here: what exactly happens if you comment out the DestroyTimer line? No freeze and everything about the code seems to be working as intended? In a scenario like this, the line of reasoning I follow is: how much of this can I comment out and still encounter <the issue>? I would try to understand what needs to be commented to prevent the freeze, and what can be commented out that doesn't change anything at all.

It's likely not the issue, but the set of conditions you've built for this trigger is strange and doesn't work like you might expect. I imagine you want to run the trigger only in this specific instance:

1. Speedster is alive.
2. The dying unit was a hero.
3. The unit was killed by the Speedster.
4. The Speedster is within 2000 units of the killed hero.

But that isn't what will happen. The first return statement that the condition function encounters will be what's returned, and it won't check the rest of it. So in order:

1. If the speedster is dead: FALSE
2. If the killed unit was a hero: FALSE
3. If the unit was killed by the Speedster: TRUE <----- this line trumps the one below because it is encountered first, so the next is never checked
4. If the Speedster is close enough: TRUE

So your distance function is basically irrelevant. Every time any hero is killed by the Speedster (and the Speedster is alive), the trigger will run. Distance will never be checked. Here is how the condition function should be structured:

function Trig_Speedster_Hero_Dies_Bonus_Conditions takes nothing returns boolean
    return UnitAlive(udg_speedster_unit) /*
         */ and not IsUnitType(GetTriggerUnit(), UNIT_TYPE_HERO) /*
         */ and GetKillingUnit() == udg_speedster_unit /*
         */ and DistanceBetweenPointsReal(GetUnitX(udg_speedster_unit) , GetUnitY(udg_speedster_unit) , GetUnitX(GetTriggerUnit()) , GetUnitY(GetTriggerUnit())) <= 2000.0
endfunction

This comment concerns me: //Using a timer here because for some reason running the same actions without one also freezes because I don't see any reason that should be the case. Such a technique is usually used to overcome recusion or when you need a frame-delay for something to update, neither of which are relevant here. Total spitball ideas:

• Is there any other way for this Speedster to gain Str/Agi/Int on-kill? Maybe there's some issue with trying to set the stats in an on-death event that leads to recursion because they're being modified in two different ways. (Of course this should all be sequential but something odd is happening here.)
• You aren't using a timer stack/allocation system that expects you to use something like NewTimer() and ReleaseTimer() instead of creating and destroying them, are you? If you're not, you could try using such a thing to see if it affects the freeze.

My desired result is actually that the speeder is alive, the dying unit is a hero, and either the speedster is the killer or he's nearby.

If you see the stats increase then the game had enough time to render the next frame. Sometimes the crash happens before the next frame with updated data is rendered.
The wisdom of the ancients says to use PauseTimer first before destroying timers.

I wonder if passing GetExpiredTimer() directly as an argument is an issue? What if it's saved into a local variable and that variable is used as the argument. Remember to null this local at the end of the function.

I'd like to have this crashing timer map for testing please? If you don't want to share the full map publicly, I would message you on Discord.

I watched the recording again and it turns out he was leveling up from the kill not getting the stat bonus that I wrote in the code, since the increases didn't match what my lines of code said. Its worth noting that I saw object editor based Critical Hit text but not the "death bonus" text that I'd programmed when it happened.

edit; I posted the code for CriticalTextForPlayer and I'm going to try removing that text for my next attempt

 

[KitsuneTailsPrower]

Update: I've isolated the issue to this line: "call UnitAddCooldownReductionFlat(udg_speedster_unit , udg_lesser_cdr_value)". So apparently movespeedX and cooldown reduction don't play nicely with each other.

 

[Pyrogasm]

Cooldown = (Default Cooldown - Cooldown Offset) * [(1 - source1)(1 - source2)...] * (1 - Cooldown Reduction Flat)

If you give a unit more than 100% flat cooldown reduction I'm sure the game won't like setting an ability's cooldown to a negative number. Are you sure that's not happening? I don't see any logic to limit the total flat CDR you give, nor do I see it in the CDR system definition (though I did not look exhaustively).