ThreatClassic2

ThreatClassic2

9M Downloads

[BUG] Units appear on threat list late, in this case tank

cdmichaelb opened this issue · 16 comments

commented

What steps will reproduce the problem?

  1. Play in raid
  2. Have Tank
  3. Generate Threat

What version of ThreatClassic2 are you using? (Stating 'latest' is not useful)

2.0.7 / 6

Do you have an error log of what happened?

no, how

Any additional information? (example: WoW language if not English) (Attach screenshots here if available, do not link externally) Have a video but says do not link externally.

Tank shows up late on pull and a dps pulls aggro, eventually tank shows up on list.

I suspect it has to do with ChatThrottleLib, no prio is set so it defaults to NORMAL, most addons also don't set a prio, so all addons using chatthrottlelib are on normal. The way chatthrottlelib seems to work is it gives each channel (normal, bulk, alert) a certain bandwidth to prevent clients from disconnecting. If a channel is full it'll delay it's send and queue. I think if you used alert or bulk instead, it would send sooner since it'll split some of normal's bandwidth into alert or bulk if it has sends waiting.

commented

Hi, yes this is a known unwanted behavior. It's especially bad, if your tank runs LibThreatClassic2 and ThreatClassic-1.0, because it doubles the chat output. It is highly recommended to update any addon with ThreatClassic-1.0 to get rid of it. Afaik no (major) addon except ClassicThreatMeter is still using ThreatClassic-1.0 in its latest version.

I think you tracked it down correctly to ChatThrottleLib, but I somewhat doubt increasing its prio is acceptable. Threat is an important and realtime message, but it's also one of the most spammy messages currently in use by addons. The higher priority should in my opinion be reserved for rarely send messages with high priority (e.g. a pull timer or an infight raid warning) and ThreatLibs constant noise shouldn't interfere with that.
Reducing the prioto bulk on the hand, could make the situation even worse, especially if players still run ThreatClassic-1.0 alongside.

commented
commented

You can log LibThreatClassic2 through ThreatClassic2 options menu and (kinda ironically) you can install ClassicThreatMeter to log people that still run the old versions ;)

commented

Yea you're right, I forgot that it's not really a prio, but bulk still gets at least 1/3 of the share, so messages at least trickle through. I'm still not sure, if this causes any improvement for the situation and may cause additional delay in certain other situations.

Considering ThreatLib causes a rather high message load, switching the priority bucket doesn't really help much. If you run into throttle, you would run into it regardless of normal or bulk. In case of normal, messages are just sent out in the order they came in. If I switch to bulk, I might even get additional delay in some edge cases, because I'm taking more than 1/2 or 1/3 of the bandwith and other addons get priority with the other half/twothirds of the bandwidth.

commented

Is there perhaps a method you could implement that would send certain events with higher priority? I assume right now all threat comm is sent use the same (NORMAL) priority. What about sending your own threat at ALERT priority if the mob is targeting you?

From a DPS point of view, their own threat is not sent via comm (or shouldn't be) and they will always see themselves on the threat table. If the tank has aggro, the Tank's messages are sent at higher prio and hopefully the DPS can gauge better where he stands in regards to the tank.

However, the Tank wont see the DPS creeping up in threat since the DPS is still sending at NORMAL priority; This change wont be useful for Tanks.

commented

This might help in a few cases and might cause pretty weird (though mostly harmless) race conditions in others, when actively switching targets. The bigger problem is that threat for all targets is actually sent at the same time, so adding this option would require reworking that and alongside actually causing more bandwidth, because not only the mob GUID but also the player GUID has to be sent in each request (thus making the problem worse).

A good and simple fix would be to detect throttling is happening (I'm not sure if and how that's doable) and then cut down on the sync interval, so instead of spamming into the buffer and causing more and more delay, threat updates would just come slower but steadily. Warriors, druids and paladins (the tank classes) currently publish twice as often as everybody else (that's why this issue appears most frequent on the tank)

Also, there are a few changes in the pipeline/already in pull requests of the Lib that might help out in some situations (and might make this worse in exchange for new functionality in some other situations...). Quite heavy changes with a lot of code changes and I'm pretty busy last weeks.

What would really improve the situation is cutting down the bandwith by removing unneeded information from GUID. By my estimations this could cut down bandwith by over 30%. Additional simple compression might cut down another 20-30%. However, these changes take a lot of effort and are backwards incompatible (i.e. players with the newer revision won't be able to sync with the old version). The good news is, as it's the same addon, I can inform other players in chat, that they need to update :).

commented

I don't follow why player GUID would need to be sent. I don't know what the packets look like for ThreatClassic2 but you don't need to split the threat table by which mobs you have aggro on. Just send it all as ALERT if you have aggro on ANY of them.

It's a naive approach for sure but most raid scenarios have limited numbers of enemies so the potential number of unique ALERT senders would be minimal.

commented

An addition to my above comment, you could even restrict this to certain enemies. IE, don't send ALERT comms if I have aggro on an Onyxia Whelp, but DO send ALERT if I have aggro on Onyxia herself.

commented

The throttle is happening locally, for each player individually. If you cause chat throttling sending all messages through a different priority doesn't help, but might even cause more delay, because other addons also use the the comm channel. It's not a real priority, it's just 3 different queues that each get equal bandwidth once throttling hits. If there are messages of other addons in normal, and threat messages in "alert" and threat is more than 50% of the bandwith, this approach causes more delay instead of less, because other addons are bypassing on the other queue (which might be fair, but makes this issue worse).

commented

But I just had another idea. This problem could be completly avoided by dynamically adjusting the publish interval to the number of enemies (nearly proportional to the bandwidth) and avoid running into chrottle in the first place. That's also not too hard to implement. I will see what I can do.

commented

If there are messages of other addons in normal, and threat messages in "alert" and threat is more than 50% of the bandwith

This isn't what I'm saying to do. I'm not saying to publish all threat messages as ALERT. What should be published as ALERT are only messages from individuals that are currently being targeted.

On an example raid boss, you would typically have 39 members publishing their threat as NORMAL (not targeted by the boss) and 1 member publishing threat as ALERT (targeted by the boss).

I understand it's not a real priority and its more like 3 queues. But it doesn't make sense to push Tank threat into the same queue with everyone else because Tank threat is much more important than anyone else's message and thusly should go in the least used queue.

commented

Even in scenarios where all queues are fully utilized, it makes sense to publish Tank threat in a different queue than everyone else's threat.

In such scenario, according to https://wow.gamepedia.com/ChatThrottleLib#/media/File:ChatThrottleLib.gif, each BULK, NORMAL, and ALERT have 333 bytes published every second and everything else throttled afterwards.

ALL NORMAL:
Pushing 40 messages (lets assume each message is also 40bytes) to NORMAL will take 4.8 seconds for every message to get sent.

SPLIT:
Pushing 39 messages to NORMAL will take 4.7 seconds for every message to get sent.
Pushing 1 message to ALERT will take 0.1 seconds to get sent.

commented

I understand it's not a real priority and its more like 3 queues. But it doesn't make sense to push Tank threat into the same queue with everyone else because Tank threat is much more important than anyone else's message and thusly should go in the least used queue.

you are missing the fundamental point of the whole system. Throttling is happening based on one player and one player alone. It's a completely local system. The other 39 players in your raid are irrelevant. It's only about outgoing bandwith of yourself. What everyone else is doing has absolutely no impact whatsoever.

commented
commented

Reading through the documentation on https://wow.gamepedia.com/ChatThrottleLib I'm drawn to the paragraph on that page that says:

If you would like to see rate changes in action during development, load ChatThrottleStats.lua in your addon. It displays a small string in the top left corner of your screen showing current rate through the library, burst availability, and rate of traffic bypassing the library. FPS based throttling being in effect causes the string to go red.

Would it be possible to utilise this somehow so we can enable a "debug mode" of sorts with ThreatClassic2 in order to show how much the outbound chat is being utilised?

commented

this will be no longer an issue once the threat api is available in the next patch.