Unit and Trigger conditions are evaluated when they shouldn't be
Closed this issue ยท 5 comments
WoW Version
Retail
TellMeWhen Version
11.2.2
Describe the bug
Unit Conditions and Trigger Conditions on the Icons are constantly evaluated even when the Group Condition fails. This is unexpected behavior and probably bad for performance.
Trigger Conditions are also evaluated even when the Icon/Group is disabled.
Additionally, Trigger Conditions are evaluated using "Global: Performance -> Update Interval" instead of "Trigger Frequency" on the Notification, which I believe is incorrect behavior.
Please see the issue reproduced in the Group and Icon using the export string below.
Export Strings
^1^T^SGUID^STMW:group:1eqwvDAjFfSm ^SPoint^T ^Sy^F7852435020185602 ^f-46^Sx ^F-8776254568267777^f-47 ^t^SLocked^B ^SColumns^N1 ^SName^S!Diagnostic ^SIcons^T ^N1^T ^SType^Sunitcondition ^SConditions^T ^N1^T ^SType^SLUA ^SName^SDEFAULT_CHAT_FRAME:AddMessage("Passing-Condition-1",~`1,~`1,~`1);~J return~`true; ^t^Sn^N1 ^t^SUnitConditions^T ^N1^T ^SType^SLUA ^SName^SDEFAULT_CHAT_FRAME:AddMessage("Passing-Unit-1",~`1,~`1,~`1);~J return~`true; ^t^Sn^N1 ^t^SEvents^T ^N1^T ^SType^SLua ^SLua^S--~`<Untitled~`Lua~`Code>~J ~J local~`icon~`=~`...~J ~J --Your~`code~`goes~`here:~J ~J DEFAULT_CHAT_FRAME:AddMessage("Passing-Notification-1",~`1,~`1,~`1);~J ~J ~J ~J ~J ^SEvent^SWCSP ^SOnConditionConditions^T ^N1^T ^SType^SLUA ^SName^SDEFAULT_CHAT_FRAME:AddMessage("Passing-Trigger-1",~`1,~`1,~`1);~J return~`true; ^t^Sn^N1 ^t^t^Sn^N1 ^t^SCustomTex^S447418 ^SEnabled^B ^t^t^SConditions^T ^N1^T ^SType^SLUA ^SName^Sreturn~`false; ^t^Sn^N1 ^t^t^N11020201^S~`~| ^Sgroup^N138 ^^
Conditions that aren't regular icon or group conditions (so, unit conditions and notification trigger conditions, as you identified) execute using a different system that does indeed evaluate on the global update interval.
These condition instances are shared between all occurrences of the same identical condition set, which is one reason of many why their evaluation is driven by a common engine rather than the system they're configured for. If you make an exact duplicate of your test icon, you'll see that the Passing-Notification-1 messages are duplicated every second, but the Passing-Trigger-1 messages do not become duplicated - the two icons are sharing the same condition set for that notification. This is especially impactful for unit conditions, where its very plausible that the same conditions against the same units are being checked by multiple different icons, e.g. an icon for each buff/debuff being tracked.
The performance implication in the overwhelming majority of use cases is zero, since most common conditions trigger on event, not on interval. Lua conditions are one such condition that forces interval evaluation, so you're going to see that when the debugging is inside the condition itself.
If the condition evaluation respected the trigger frequency, then a low trigger frequency (maybe a sound that plays every 5 seconds, for example) could have its first execution delayed by up to that amount if the underlying condition becomes true just moments after an evaluation.
Trigger Conditions are also evaluated even when the Icon/Group is disabled.
Yep, good catch. Fixed in da95fff
Thanks for clarification.
In my test example there is only a single icon using this specific condition set for Unit Conditions, and the icon is not being shown because Group Condition is not passing. There is no reason for condition engine to evaluate that condition set on non-shown icon every interval. Engine should be able to see how many icons are using the condition set, check if they are shown, and skip evaluation if none of them are. Evaluating Unit Conditions should be done last, after Group Conditions and Icon Conditions pass, since that makes most sense.
But if that's not a bug for you then okay. I just expected all icons in a group to become fully inactive (stop updating on events/intervals) if Group Conditions are not passing. This seems to be the case for Icon Conditions and Meta Icon updates, but not for Unit and Trigger conditions.
Lua conditions are one such condition that forces interval evaluation
The performance implication in the overwhelming majority of use cases is zero, since most common conditions trigger on event
I noticed other than "Lua" conditions, "Icon is Shown" and "Macro" conditions also force interval evaluation. Those are very common, at least in my setup. I guess I should stop using them, because performance is getting significantly worse.
As a side note, I understand that for "Lua" and "Macro" it has to be interval evaluation, but for "Icon is Shown" maybe you should consider implementing some internal event that gets emitted when icon "Shown" status changes, so both "Icon is Shown" condition and Meta Icons could stop using interval and start using that event. I use Meta Icons extensively, so that could probably boost my performance a bit.
Yeah, you're right that its buggy and wrong that those few things don't follow the same rules as everything else. Fixed for unit conditions and event handlers.
Meta and Icon Is Shown don't use event driven updates because they have to eagerly check for pending enqueued (but not yet executed) icon updates on the things they're targeting. If we don't do this, then an icon in a lower-numbered group (that is updated first) that is depending on an icon in a higher-numbered group (that is updated later) would have its update delayed by an update cycle. A long time ago it used to work that way and people didn't like it. I'll see if I can come up with a more intelligent, dynamic way to order icon updates to perhaps prevent that as best as possible so it could become event driven. But no promises.
Ok, event-driven updates for meta icons and icon shown conditions: ef1d864