Again, Jass experts needed.

[Macadamia]

Hi everyone.

Still working on my aMazing Balls TD, trying to optimize it again.

The Handle Counter shows there are leaks, but i null all my handles, and remove / destroy all unused units, destructables and timers. And no-one seems to have found a leak in the trigger I posted recently.

So I decided to take another approach to gain performance (and maybe removing leaks indirectly in the process) :

Until recently, I was using a trigger to autocast all my non hardcoded autocast tower abilities.

But the event I used was the Player Event "A Unit is Attacked" for the player owning the creeps (Balls).
As I explained in other posts, I already have an intensive EVENT_UNIT_DAMAGED added to a trigger for each Ball, so this other "attacked" event does not help.

The abilities the towers need to autocast have no target (one targets the position of the casting tower to emulate an AOE stun with no terrain deformation, so it does not need to have a target).

So I decided to use a HashTable that will save (for each tower that has one of these autocasts) a trigger for the tower, and the order for the autocast ability specific to that tower.
The idea is to avoid generic event to improve performance, and also being able to destroy the trigger attached to the tower when this tower is sold / upgraded or removed from the game.


Before i completely rework these triggers, I need information only JASS EXPERTS can give :

1) From this old 2017 thread : ConditionalTriggerExecute vs TriggerEvaluate

You can read that Trigger Condition "Will be destroyed if trigger is destroyed". The link for the proof is unfortunately dead.

So can any expert confirm this ? This would prevent a lot of hassle saving the conditions for these triggers (and all other triggers using conditions only) in order to remove them before the triggers are destroyed.

2) The native I use is
native TriggerRegisterUnitInRange takes trigger whichTrigger, unit whichUnit, real range, boolexpr filter returns event

Of course, I would only need the triggers to be evaluated when a Ball comes in range of the tower, not any other unit.

- so is it better to use the filter from the event registration, or just add another test in the trigger condition ? (performance wise)

If the filter is better, how do I use it ? Filter( function UnitIsABall) ?

Thats it. If anyone has enough knowledge in Jass to answer these two simple questions with no doubt, I would be really grateful.

Thank you,
Take care.

 

[DracoL1ch]

there are multiple object in play with TriggerCondition
the first one is the converted boolexp with Condition(function X)
this one is cached and re-used further, so no new handles here, unlike with actions
and the TriggerAddCondition() actually returns TriggerCondition object which has unknown effects and MAY (only may) stay for quite some time
even if it's not destroyed, it's definitely next to nothing, as the whole dota is built on TriggerConditions and dynamic triggers with no faults regarding memory leaks whatsoever, so I believe it's safe
TriggerRegisterUnitInRange example

function FilterSentinelCreepsOnly takes nothing returns boolean
    return IsUnitOwnedByPlayer(GetTriggerUnit(),Sentinels[0]) and LoadInteger(HY,GetHandleId(GetTriggerUnit()),WaypointAbilID)>0
endfunction

...
call TriggerRegisterUnitInRange(t,u,350.,Condition(function FilterSentinelCreepsOnly))
...

no matter where do you place the check, game still has to run whole thread once trigger evaluates, but perfomance-wise when condition is fulfilled trigger must be launched, so there will be 2 threads now. Using trigger's condition instead will reduce threads count to 1, but the code will be little bit messier.

 

[Macadamia]

Thank you this is very informative

So I can't use the native Filter() in this case but Condition(). This is worth knowing.

Actually, if I understand what I learnt recently, my triggers never execute as I put all my actions in the conditions, returning false. They are just evaluated.

But I suppose adding a filterfunc is like evaluating the trigger twice. Still it is worth knowing how to use these event registration filters.

Also I seem to realize, reading your example, that when I only need to use them once, I should not store the values returned by functions calls in a local variable.
It helps a lot when you have to use these values many times (saving function calls then). But if I save the returned values everytime i call a function, it's a useless local created. I will read through my code and fix this, I suspect I did this mistake a horrendous amount of times.

Again, thank you for this precious information,
Take care !


EDIT: forgot to say this about the conditions :

For safety reasons, with these new triggers attached to my autocast towers, I also saved the conditions in the dedicated hashtable, and I remove the condition from the trigger before destroying it and this also allows me to null the value returned by the TriggerAddCondition call.

The remaining triggers that I kept unattached are mostly based on EVENT_PLAYER_UNIT_SPELL_EFFECT so I suppose it's far less intensive than EVENT_UNIT_ATTACKED was.

 

[DracoL1ch]

filter is an alias for condition
well IDK if conditions are destroyed properly when it's trigger utilized, but I dont, and my maps are fine with that, so I believe its not required
jass is a very very fast thing, dont do extra job if thats unneded

 

[Macadamia]

Yes I only did it on these triggers because I had to save their handle in the hashtable anyways.
But I won't bother removing conditions from the other triggers, that sounds a bit overkill indeed.

---

EDIT :

Well it seems using EVENT_UNIT_TARGET_IN_RANGE is a very bad idea for custom autocast abilities.
It only fires when the other unit comes in range, but if the unit stays in range the autocast won't work.

Rather than increasing the triggers complexity with adding timers and checks for leaving range, I will give a try with EVENT_UNIT_ACQUIRED_TARGET hoping a unit can acquire the same target several times.

EDIT 2:

EVENT_UNIT_ACQUIRED_TARGET did not work very well in the end.
For some reasons the towers were behaving in a strange way, sometimes casting the abilities I wanted them to autocast, sometimes not.

So I consider this path as a failure.

What I can do though is this :

- as my previous way of autocasting abilities was based on EVENT_PLAYER_UNIT_ATTACKED, and worked pretty well, and as I already have triggers attached to each creep (Ball), I will use the existing intensive event i use, EVENT_UNIT_DAMAGED, as Balls always get damaged by the towers attacks.

I will just add unit Id checks to the GetEventDamageSource(), and if it is one of the towers that I want to autocast their abilities, I will give them an order with a delay, using a 0.20s local timer and saving the tower in a hashtable : ISSUING AN ORDER WITH NO DELAY creates big casting attempt bugs.

I wonder if I could make this even more efficient by checking their ability cooldown too, using the native BlzGetUnitAbilityCooldownRemaining() ...

 

[pr114]

I have a question on this topic, and hopefully this will be appropriate in this thread.

I believe it is mentioned that it is better for performance to put your actions into your conditions function which then returns false, as opposed to use conditions and actions seperately.

But how does this compare to using nil for conditions, and then putting your conditions in the action function. To explain exactly what I mean I will put three different methods of scripting a simple Learn Skill trigger that modifies a variable when a Hero learns Scout;

1. Conditions and Actions (not recommended)

trig_Learn = CreateTrigger()
TriggerRegisterPlayerUnitEvent(trig_Learn, p, EVENT_PLAYER_HERO_SKILL, Condition(CheckLearn))
TriggerAddAction(trig_Learn, ExecLearn)

function CheckLearn()
    return GetLearnedSkill() == 1093677875
end

function ExecLearn()
    variable_ScoutLevel = GetLearnedSkillLevel()
end

2. Only Conditions (recommended in this topic)

trig_Learn = CreateTrigger()
TriggerRegisterPlayerUnitEvent(trig_Learn, p, EVENT_PLAYER_HERO_SKILL, Condition(CheckLearn))

function CheckLearn()
    if GetLearnedSkill() == 1093677875 then
        variable_ScoutLevel = GetLearnedSkillLevel()
    end
    return false
end

3. Only Actions (is this method just as good as above?)

trig_Learn = CreateTrigger()
TriggerRegisterPlayerUnitEvent(trig_Learn, p, EVENT_PLAYER_HERO_SKILL, nil)
TriggerAddAction(trig_Learn, ExecLearn)

function ExecLearn()
    if GetLearnedSkill() == 1093677875 then
        variable_ScoutLevel = GetLearnedSkillLevel()
    end
end

Is method 2 recommended above 3?

 

[Drake53]

Is method 2 recommended above 3?

Yes, for some reason trigger condition code runs faster than trigger action code, so it's most efficient to put everything in trigger conditions. (also, try using [code=lua])

 

[Macadamia]

Hello there!

I have absolutely 0 knowledge about lua, so what you wrote is a bit confusing for me, although I understand what it relates to.

For more info, read the thread whose link I posted further up :

ConditionalTriggerExecute vs TriggerEvaluate

It will give you more concrete pointers.

 

[pr114]

Another silly question (I couldn't find this mentioned in this or the other topic):

As I'm in the process of converting my map to run code from Trigger Conditions instead of Trigger Actions, I've tried putting a function in the boolexpr argument of TriggerRegisterPlayerUnitEvent;

TriggerRegisterPlayerUnitEvent(trig_Learn, p, EVENT_PLAYER_HERO_SKILL, Condition(function() print(1) end))

It seems like this code just doesn't run. Is this suppose to?

Strange enough even, if I put something like this:

TriggerRegisterPlayerUnitEvent(trig_Learn, p, EVENT_PLAYER_HERO_SKILL, Condition(function() return false end))

The Trigger Action that I've added will STILL run. It's like the boolexpr is completely ignored, even for filtering purposes.


On the other hand, TriggerAddCondition works well.

TriggerAddConditon(trig_Learn, Condition(function() print(1) end))

 

[Macadamia]

Hello there !

I think I faced the same issue a while back on my first attempts to use filter functions in event registration calls.

It is probably related to the way TriggerEvaluate is coded internally. But this is just a guessing attempt.

But chances are even higher that the filter refuses to work because of the print call because print takes an argument...

Last possibility I can think of is the return false inside the filter. Maybe the parser takes a shortcut and then will filter everything out by default.

 

[DracoL1ch]

Yes, for some reason trigger condition code runs faster than trigger action code, so it's most efficient to put everything in trigger conditions. (also, try using [code=lua])

how so? both action and condition requires starting JASS virtual instance and thats the most heavy thing about it, no matter how many ms you can win by choosing one over another, its less than 1% of total perfomance

Is method 2 recommended above 3?

because I wrote that actions are leaky unlike conditions which are cachable, plus conditions dont have to return anything at all, if you wonder

 

[pr114]

Hello there !

I think I faced the same issue a while back on my first attempts to use filter functions in event registration calls.

It is probably related to the way TriggerEvaluate is coded internally. But this is just a guessing attempt.

But chances are even higher that the filter refuses to work because of the print call because print takes an argument...

Last possibility I can think of is the return false inside the filter. Maybe the parser takes a shortcut and then will filter everything out by default.

I would expect no action calls if it contains a boolexpr that returns false.

In most Blizzard J functions, the way its used in GUI the boolexpr argument for TriggerRegisterPlayerUnitEvent will be left null (like
TriggerRegisterAnyUnitEventBJ), and any filtering is done using TriggerAddCondition.

Though in the Blizzard J function MeleeGrantHeroItems, they do pass a boolexpr: filterMeleeTrainedUnitIsHeroBJ, and I reckon this function works correctly.

how so? both action and condition requires starting JASS virtual instance and thats the most heavy thing about it, no matter how many ms you can win by choosing one over another, its less than 1% of total perfomance


because I wrote that actions are leaky unlike conditions which are cachable, plus conditions dont have to return anything at all, if you wonder

So what you are saying, there is not much of a difference between running a Condition and an Action, as long as you use place all your code in either and only start 1 thread?
In that case, I reckon using a TriggerAction would be preferable, because it will save the extra Condition/Filter function call that is necessary with a trigger condition.

 

[DracoL1ch]

but why? use conditions and put all actions inside those condition function, I have 0 Actions in my codes

 

[pr114]

I don't really know, but it seems to me:
>TriggerAddAction(trigger, actionfunction)
>TriggerAddCondition(trigger,Condition(actionfunction))

They can both run the exact same actionfunction, but the TriggerAddCondition needs to be coupled with an extra Condition native function call.
Will probably not matter much though.

--

Oh that is silly, the Condition() is only called once at trigger creation.

 

[Macadamia]

@DracoL1ch :
In all fairness, the tiny performance gain was also a factor for my switch.
My map is very demanding performance wise, and after weeks of optimizations, I still get a bit of lag in the last levels.
- As I have failed to find anything wrong in the object editor,
- As the only fast periodic timers I use do very simple things (change some destractable animations)
- As my map handle count raises over the time in spite of properly nulling all handles and destroying unused agents
- As reducing the attack speed in favor of higher damage only partially improved the situation,

I had to look everywhere for optimization potential. So this change had a double impact. It reduced my map's handle count over the time and the lag now requires making the game last longer before showing instead.

It really seemed to me every little inch of performance I could reclaim was a victory.

Now maybe I had it wrong in the end, the increasing handle count might not be an issue as the game executable never saturates my computer memory.
Maybe the very intensive fight in a TD with long mazes and a non negligible degree of complexity is just too much for my laptop : it is just a 5 years old i7 with a poor gtx960m graphics card, and maybe reforged graphics and engine "enhancements" is a bit too much.

Anyways in the end of the day, I think that whatever the reasons are, optimizing code can never hurt.

Oh there is something you said I did not really understand properly :

, plus conditions dont have to return anything at all, if you wonder

What do you mean with this ?

I thought the condition had to return false to prevent a TriggerExecute call ?

 

[Drake53]

how so? both action and condition requires starting JASS virtual instance and thats the most heavy thing about it, no matter how many ms you can win by choosing one over another, its less than 1% of total perfomance

JASS Benchmarking Results
Don't ask me why conditions are faster, they just are.
Also, I don't think it has anything to do with JASS, though I haven't benchmarked this in lua so can't say for sure.

 

[DracoL1ch]

there will be no TriggerExecute call if you have no Action added to the trigger

JASS Benchmarking Results
Don't ask me why conditions are faster, they just are.
Also, I don't think it has anything to do with JASS, though I haven't benchmarked this in lua so can't say for sure.

1.32+ is totally different thing, no measurements from previous version are useful there.

 

[Macadamia]

there will be no TriggerExecute call if you have no Action added to the trigger

1.32+ is totally different thing, no measurements from previous version are useful there.

Well thanks, it is worth knowing. None of the sources I read about this mentioned it. They were all insisting on the return of a false boolean.

As for Reforged, you have a valid point there, we don't know if these tests have any validity anymore.
It would probably be interesting to do new benchmarks to know if good practices need to change or not.