Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prioritize communication from tanks #37

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bjornenalfa
Copy link

This helps mitigate the problem where a tanks threat updates might be delayed because of other addons communicating. There are more improvements that can be done to the addon communication like discarding old updates that are still enqueued in ChatThrottleLib, but that would require doing changes in that library. Another thing to consider that could be done right now is monitor the communication delay using callbacks and notify the user if the delay is significant so that they can call out their threat manually in comms or turn off other addons.

This helps mitigate the problem where a tanks threat updates might be delayed because of other addons communicating. There are more improvements that can be done to the addon communication like discarding old updates that are still enqueued in ChatThrottleLib, but that would require doing changes in that library. Another thing to consider that could be done right now is monitor the communication delay using callbacks and notify the user if the delay is significant so that they can call out their threat manually in comms or turn off other addons.
@dfherr
Copy link
Owner

dfherr commented May 1, 2020

Just wondering, did you read the conversation today in the WoWUiDev discord?

I'm absolutely in favor of dropping old updates. An easier thing we discussed today is using the chatthrottlelib callback to not send updates, when there is still a ltc2 message queued and only allow the next update when the last one was actually sent out. This at least doesn't fill up the queue by itself.

However, I'm not in favor of using the ALERT priority. I do not think this is a good idea, because this is not a real priority. It's just a different queue and all "priorities" get a equal share of the outgoing bandwidth.

So, for one, ltc2 does generate quite some outgoing bandwith for tanks to begin with. Now imagine the other addon that is causing throttle is also using alert. Now we actually made the situation worse for tanks.

Now imagine a situation where we have 300 cps available for 2 queues. LTC2 is sending to the alert queue, another addon X is sending to the normal queue. LTC2 is sending 400 CPS to alert and X is sending 300 CPS to normal. Now X is going through right at the limit, while just LTC2 will be starting to queue up in alert. Actually queueing up nearly twice as much as if both of them would be sending in normal.

I do not think using the ALERT channel for heavy loads of data makes sense nor is it being a good citizen of the community.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants