Proposal: 8.0 day protocol-level upgrades

[ghost]

I'd like to make a proposal that we make a few protocol upgrades on 8.0-day, assuming all the parts can be pulled together in the proper timeframe.

The upside is that none of the changes I'd like to propose are going to cause any application-level incompatibilities -- they would be entirely handled in MSP at the protocol level, all data exposed by LibMSP to addons would remain identical.

The downside however is that they would guarantee that anyone using an outdated addon would not work correctly or see updated profiles. Regardless of other compatibility changes needed for BfA, these changes would kill the protocol level compatibility completely.

If we get consensus between at least @Ellypse, myself, and GnomTEC's author (are they on Github?) on doing so, here are my personal proposals for protocol-level changes:

• Switch version numbers to hexadecimal. This would shrink the size of version numbers down somewhat, and enable future possibilities (like CRC32c checksums replacing them) without backwards compatibility problems. Hexadecimal is handled natively in lua with tonumber(num, 16), so it should not be a performance change in the slightest.

• Logged- and BattleNet-only. If we're doing compatibility-breaking changes, might as well go all the way and break it cleanly off.

• Eliminate the messy four-prefix system. I want to use the message-handling system I'm developing in Chomp to replace it. It will have most the same qualities of multiple prefixes (separation of first/middle/last messages), but only using a single prefix. However, this does have a practical drawback in that the system I'm developing for Chomp relies on using an extra 12 bytes of overhead per-message (I can consider slimming it down to 9 bytes, but I don't think I can go any lower without hitting a remote-but-possible risk of problems). This is about an extra 17/25% overhead (9/12 bytes) on an empty message (WoW protocol overhead is ~40 bytes), but that percent does drop off fast as the message is filled up.

• (Optional, only if any of above are adopted) Pick a new prefix. This would make sure that no legacy clients even attempt to interact with new RP clients. Something like "MSP2" or "NMSP" ("NewMSP"), or similar. This isn't really necessary (most legacy clients should fail gracefully...), but it would make the compatibility break even cleaner.

If we decide to adopt any of these proposals, I can work an 8.0-automatic changeover into the new version of LibMSP, to switch to the new methods of doing so as soon as the interface version of the running client is >= 80000, but still permit legacy operation on 7.3 clients until patch day.

[Ellypse]

From my point of view, the release of an expansion is the best time to break backward compatibility and get everyone to update their add-ons anyway. Keeping backward compatibility while moving to SendAddonMessageLogged was not my objective anyway, we should only accept RP profiles from logged messages (meaning only display profiles we know can be reported for their content) and since supporting those require an update, it is the perfect time to change what needs to be changed.

With that in mind, I do agree that using a different/new prefix would probably be cleaner and clearer.

I trust you for the technical changes in the version numbers and prefixes of the messages. Especially since in the end it would be more secured in cases of out of order messages when sent via Bnet.

[Ellypse]

Oh, one thing I must relay from GnomeTEC that we could add to the LibMSP (and that would help me too in TRP3) is a callback for when the transfer is completely finished. Currently, LibMSP only has callbacks called every time it receives chunk of data.

Being able to just execute a callback when we know that everything has been downloaded would be nice.

[ghost]

For the rewrite I worked on ages ago, I had two callbacks. The received callback (a full sequence of messages had been received and processed), and the updated callback (fired when any individual field's contents had changed).

The way I handled each of those was that received gave callback handlers the standard arguments (just the name), but updated also provided the field name and field contents directly to the handler. It saved a few table lookups for most of the times you'd want to deal with the field contents directly, and possibly allowed for progressive loading (processing messages as they come in, rather than only after fully assembling a sequence).

We can also ditch the XC field from this implementation, going this route -- Chomp will have all that metadata instead, and I can make sure it passes that information along to the handler. I might drop in a third callback that is informational only, providing data on messages received (for updating progress bars and the like).

I'll look at implementing the first three suggestions I made in a test version, and we can decide on the prefix near the end. Keeping just MSP as the prefix would probably work fine, especially if we also restrict to logged and BattleNet only. It might take me a couple days here, but I'll try to have something testable before the end of the week at least.

[ghost]

A brief update on a few of these, as far as my planning/work has gotten so far:

• The standard separator byte is planned to be 0x7f (\127). This byte is NOT permitted in RP profiles, and addons should check for this before setting profile data. LibMSP may emit an error if this byte is found in any fields during msp:Update().

• The separator byte 0x09 (\009, \t) will be used in broadcast MSP (formerly "Group MSP") to permit setting a target of a broadcast request/response. This byte is unrestricted, and may appear in profile data without issue. It will only be used once per series of broadcast messages in a way such that extra appearances of the byte are ignored.

• The prefix "MSPB" ("MSP Broadcast") will be used for broadcast MSP (formerly GMSP/"Group MSP"). "MSP" or "MSPU" ("MSP Unicast") will be used for standard/unicast MSP (depending on if there's agreement to change prefixes or no.

• There will be no support for parsing of partial message sequences. This is a limitation on Chomp's implementation of message buffers, but matches old LibMSP behaviour anyway. It wouldn't parse them either -- only XRP ever attempted to, and almost never actually experienced this happening.

[Ellypse]

Sweet. I'm all in for your suggestions. It's way better to use characters that the user cannot input themselves, and I'm glad you were able to test and find the ones we can use.

I'm okay with keeping the current prefix for peer to peer transfers, but one detail that I think is important to mention is that it is better to not pollute the logs (as I was told by Blizzard, they are okay with some metadata, but the less the better), so it would be better to use non logged messages when probing for updates. Maybe those will cause issue with people using older versions listening to the MSP prefix 🤔

[ghost]

Limiting requests/responses to different message methods is ... Somewhat complicated, but possible.

We'd probably want to make use of a third/fourth prefix (for unicast/broadcast, respectively) that's dedicated to requests. This is a less frivolous use of extra prefixes than four for the base protocol, however, so it might be worth doing. It's possible to do with a single prefix (more processing needed), but you run a higher risk of bugs that mix logged/unlogged messages in dangerous ways.

If we do this, I imagine we should use four prefixes: "MSPU", "MSPUR", "MSPB", "MSPBR" (respectively: unicast, unicast request, broadcast, broadcast request). That would make splitting it fairly clean and easy. (Edit: Maybe change the prefixes around a bit, since we could use the unlogged for both requests (?) and no change (!) statuses for fields. That would make "request" a somewhat inappropriate label for them.)

Edit2: Do we know if Blizzard is actually planning on retaining unlogged addon messages permanently, or are they intending to transition exclusively to logged? If it were me, I'd be pushing for the latter, as there can be legal concerns with unlogged communications, as well as harassment ones. I don't think Blizzard qualifies as a common carrier for legal purposes.

[ghost]

Here's the rewrite.

It'll need extensive testing, but I currently have it loaded in a copy of MRP in beta and it doesn't error out immediately or anything.

[ghost]

I decided to implement hash-based versioning instead of number-based versioning. It increases the bandwidth of simple requests somewhat (?TT123 becomes ?TTF8372ABC), but I'm compensating by dropping the version number from all TT-bound fields instead, and whenever a single field that is part of TT is requested, the entire TT is sent instead. So a TT reponse looks more like VP=2\127NA=Itarater Silverlance[...]\127TTA78CF83E, now. Instead of a dozen individual version numbers, you get one longer version number. This also means (hopefully) much less re-requesting of saved field data.

I'm also adding a function to reload a saved data cache into the msp.char table, so for addons like XRP and TRP3 that already do some caching, if MSP field contents and version numbers are saved, they can be reloaded next session to further reduce bandwidth consumption (at the cost of extra storage/memory overhead -- but these days, those are way cheaper than bandwidth).

The results of all CRC32C calculations are cached for reuse and lookup (rather than storing version numbers in msp.myver, which will be 100% unused by LibMSP), and the overhead on an obscenely long field (a 16-kilobyte string) is ~8.5 milliseconds. The overhead on a 255-byte string is only 0.13 milliseconds. And these will only be calculated the first time a specific string is ever encountered, the cached result is used subsequently.

It's not quite ready for wider testing yet, as I still have some details to go over in the code first, but it seems to work well with some preliminary testing.

EDIT: The rewrite branch has CRC32C support ready for testing now. (/run msp:DebugHashTest(text) does a test of the CRC32C hash speed, spitting out number of iterations per ~1000ms, and approximate time per iteration. It takes longer with longer fields, but it's approximately linear, rather than parabolic or exponential.)