Fix AceComm for communicating cross realm

[InfusOnWoW]

When sending mulitpart messsages across realms, the messages can be received out of order. Fix this by introducing a new header that contains the message number and message total.

[InfusOnWoW]

There's a bit of debugging code left in there, because I assume that this isn't the final version.

The context of this change is WeakAuras/WeakAuras2#5342

That could obviously also be solved by the game not reordering messages.

[funkydude]

I'd rather you get this fixed in the game than patching in a bandaid only to remove it later.

If it's about "we can do this until it's patched" well we've lived with cross realm comms being unusable previously, so we can live with it being out of order a little longer.

[InfusOnWoW]

I have zero ability to get this fixed in WoW.

[funkydude]

I'll poke around

[Meorawr]

As-is I believe this would still spuriously fail when dealing with out-of-order receipt of streams of multipart chunks where the size is consistent.

Consider a sender that sends a multipart message chunked into two pieces ("1" and "2"), and they're sending them constantly to a receiver such that their message output on the stream looks like "121212".

Now imagine a receiver observes a stream and sees "121212". This might look correct, however there's no metadata in your chunking to prove that the "2"'s have been received in the same order that they were sent - it's entirely possible that the second received "2" is part of any of the three chunk pairs. This invalid ordering would pass through your message count/total checks on receipt and end up stitching together two unrelated messages for addon code to then explode on.

Another bug you'll have is that the receipt code unilaterally trusts that the incoming chunk is actually correctly ordered. Assume a sender transmits a chunk stream like "12312", and a receiver reads "12231". The receipt code as-is will fall over on this badly; you'll effectively double-count the "2" chunk and increment the received counter causing things to get out-of-sync. Further, as before it's possible the second "2" chunk is from an unrelated message and so the assignment of the rest data might actually result in you stitching chunks from unrelated messages again.

Chomp mitigates this issue by including a wrapping "session" ID (0-4095) in the chunk metadata, with the initial value chosen randomly on startup and incremented once when a multipart message is enqueued for transmission. Incoming messages are bucketed into separate spools keyed upon (sender, id), and only when the spool has fully recieved all of its pending chunks do we consider the message received and execute callbacks. There is an edge case where a receiver might have stale data from (sender, id) sitting around and then receives unrelated data two hours later from the same person with the same session ID, but in practice we've not had a report of that in the six years this has been a thing. You could heuristically dump ancient data after ~some time or invalidate all contents in a spool if the message total changes to make it even more unlikely.

As for getting Blizzard to fix it - I'd like SendAddonMessage to not suffer from this to begin with but I'd also note that Battle.net comms have been unordered forever.

[InfusOnWoW]

I added a sequence number, though I didn't add clearing of old sequences yet. I have no preference in what way that is done, or done at all.

This is my test code:
Receiver side:

LibStub:GetLibrary("AceComm-3.0"):RegisterComm("Test", function(prefix, message) print(message:sub(1, 1), message == string.rep(message:sub(1,1), 512)) end)

Sender side:

local L = LibStub:GetLibrary("AceComm-3.0") local function send(char) L:SendCommMessage("Test", string.rep(char, 512), "WHISPER", "Atest-Kil'jaeden", "BULK") end send("A") send("B") send("C") send("D")

And a example output, where I forced the sequence number to wrap around:

The numbers are sequence number, part number, total

In that screenshot the ordering of the messages being wrong is clearly visible in that C is even finished before D. I think that's something that needs documentation but is otherwise not a problem.

[Meorawr]

One other concern I had is the forced use of headered messages on transmission. There'll be a time between which a sender with a new AceComm version and a receiver on an old version will result in all traffic between the two players being dropped, as the old version won't know what to do with the incoming unrecognized message type.

AceComm has wide enough usage across big addons that people are likely to eventually get the update to support the new message type, but I do remain concerned about the risk of it taking longer than is desirable and causing a lot of "broken comms" bug reports in addons using AceComm because the receiving half insists on running their updater clients once every half a year.

Is there any mangling of the API that we could use to perhaps limit the use of the new headered messages only if the transmission actually requires them? Comms between players on the same realm could use the old transmission for example, and hopefully between players of the same connected realm set (see: GetAutoCompleteRealms).

[InfusOnWoW]

Yes, that's surely a problem. One solution for that is to add the receiving support first, and add the sending change at a later date.

I haven't tested, but assume that PARTY/RAID/GUILD are also affected by out of order messaging for the cross-realm receipcients. For these channels checking what method to use is possible but somewhat annoying.

[InfusOnWoW]

So any decison on how to proceed?

[InfusOnWoW]

So if you aren't interested in AceComm working cross-realm, you should say so.

[funkydude]

So if you aren't intrested in AceComm working cross-realm, you should say so.

Waiting to hear back from the server team.

[Justw8]

Has there been an update on this @funkydude ?

[funkydude]

Nope, issue is still open and awaiting investigation from the server team