Route all messages through ChatThrottleLib #50
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Meorawr
force-pushed
the
patch/atone-for-our-sins
branch
from
6b0b959
to
5d52027
Compare
This removes all internal management of internal transmission queues, leaving Chomp as a wrapper that really just provides "smart" routing with the worst implementation of message serialization known to mankind. The upstream CTL library has at this point merged in all the support required for sending logged messages. Battle.net message support is still pending but queued up. One thing that we still provide for now is an increase to CTL's ancient, outdated, and totally crappy default transmission rates. Hopefully we can just get newer happy values upstreamed at some point, but that might be a bit of an ask in the face of all the other major changes made to appease Blizzard's new per-prefix throttles.
For reasons of traffic fairness mostly; with CTL's default rates a 4 KB message takes 5 seconds to send and blocks everything in the same priority bracket.
As the relevant PR adding BNSendGameData support is still outstanding on the merge front, let's for now just directly poke at its innards and enqueue such messages directly. This is the lesser of two evils compared to what Chomp used to do, and means we can actually ship this new version of Chomp _today_ without comms being nerfed by 40% on retail.
Meorawr
force-pushed
the
patch/atone-for-our-sins
branch
from
152e347
to
0b9ab65
Compare
Nerfing our burst figure and message overhead adjustments slightly. The burst figure this has been lowered to is one that I'm proposing for upstream CTL usage, and is the midpoint between its own current figure and ours. Burst is the one that's usually the most likely to make the server angry and disconnect you, so having this be a bit lower makes sense from a safety perspective. The CPS value remains the same, as this is the one that we honestly really care about - however this may get lowered to 1536 in due course. For message overhead, our 27 byte figure isn't sane. The client packet used for comms messages includes in the worst case a 128-bit UUID for a channel identifier, plus about 8 bytes of additional overhead for the rest of the packet _and_ we also need to factor in the target name and prefix. Really the overhead should be higher but it amortizes out over time - but either way, 27 bytes assumes most messages are broadcasts rather than whispers - so let's bump it just a tiny bit.
|
Merging on the basis that I've tested this with blasting TRP requests to an entire server over the past ~3 days and it's worked fine - and because we need this for 10.2.7 when people start upgrading CTL to versions that we can't patch functions in. If I break everything, it's Ghost's fault. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This removes all internal management of internal transmission queues, leaving Chomp as a wrapper that really just provides "smart" routing with the worst implementation of message serialization known to mankind.
The upstream CTL library has at this point merged in support required for logged messages. Battle.net messages are still outstanding (WoWUIDev/Ace3#14), but we just directly reach in and enqueue messages to work around that limitation.
One thing that we still provide for now is an increase to CTL's ancient default transmission rates. Hopefully we can just get newer happy values upstreamed at some point, but that might be a bit of an ask in the face of all the other major changes made to appease Blizzard's new per-prefix throttles.