[Milestone] RCLootCouncil3

[SafeteeWoW]

Although it would be a milestone for the next expansion, I just want to list the ideas for future reference.

Goal for RCLootCouncil3:

1. RCLootCouncil currently has too much redundant code. For example, many code copy&paste for relic/tier buttons can be avoided. Furthermore, inheritance can be used on many widgets for better code reusability. If I want to add a button type, I need to copy/paste/modify lots of code. It's not a good idea.
2. We should minimize the data transmitted in the lootTable. First, smaller=faster=better. Second, communication bug is always harder to find, test and solve and easier bugged. Third, more communication = harder backward compatibility. In my opinion, what really needed to be transmitted in the lootTable in "link" and "bagged". Although some data needs to be cached, there are so many ways to overcome caching. For example, while the item is being cached, voting frame shows the item name as "Retrieving item info"
3. The code should be structured to be allow feature to be added easily.
4. Always try to avoid to write localization related code.
5. I would like to have better button category customization. i.e., allowing user to define category in his own way, not limited to tiers and relics.

[evil-morfar]

1. While I agree some things could be optimized (especially the loot frame stuff, but the whole system wasn't really built for that to begin with) some dublication is still fine imo, if it allows for better readability. Several functions (especially award related stuff, and to an extend, rightclickmenu) has gotten so complex that a. it's hard to understand, and b. making changes will affect a lot of things, i.e. more potential to break stuff.

2. Agreed. One of my goals is to first and foremost eliminate the variable names, as they currently makes up a lot of the data. A function could easily build and reconstruct it whilst keeping track of data added by modules. I even considered writing a library for handle that stuff.

   I don't necessarily agree with more comunication = more bugs. In fact in v1 item data wasn't sent, and people had to gather that data themselves. It caused a lot of bugs getting everything to sync properly not to mention the slowdown involved in it (just think about how long it sometimes takes when doing /test). Also there's only backwards issues when stuff is changed.

   I still think there's more pros than cons for not having clients rely on gathering the data for themselves.

   While some stuff like awarded and quality isn't needed (and could probably be removed now even) it still only takes up a few bytes (the table name aside), and stuff like equiploc enables you to send a candidates gear right away, and not having to wait ~3 seconds for that.

3. That's really easier said than done. There's two ways to do it;

   1. Try to imagine what might chance/what modules might add and implement stuff accordingly (what I've been doing). The problem with that is if you forget/don't think about something, it's probably a hassle to change it.
   2. Make everything really dynamic/changeable. You still have the same problem as above, albeit to a lesser extend, but in turn you're most likely complicating alot of things, and unnecessary decreasing performance.

4. Not sure exactly what you mean by that. But in terms of only using globally defined stuff, or guaranteed english names, then that's always a natural goal.

5. Definitely doable.

---

A few more to the list:

6. Module system. The current (core) module system was intended to enable modules (as in other addons - bad naming) to replace stuff easily. That's simply not used and wasn't intended to e.g. allow new colums to be added to the votingframe (hence the lack of support for it). Focus should shift to easily allow for visual changes.
7. Structuring. I'd like to have a more clearly defined workspace with a more object oriented mindset. This allows for easier inheritance and makes the code easier to read. Something like addon.UI and addon.Util can really cleanup some confusion. Sync and to an extend lootframe are examples of this.

[SafeteeWoW]

1. I looked at v1 code, it's not designed to fetch item info in an organized way. Meanwhile, it's not very likely the item looted is uncached for some raid members.
2. Although you won't wait after data is sent if everything is sent with lootTable, you'll spend more time to send the data, same total time spend ultimately. Meanwhile, the larger the data is, it's more likely people fails to receive it.
3. On the other hand, it's easier to test stuffs when there are less stuffs to transmitted. Slow caching can be simulated by a raw hook to GetItemInfo. Third, handling those item info costs lots of code in every file of the addon.
4. Candidate data shouldn't include role and class, because they can be generated by API.

[SafeteeWoW]

I may slowly write some code for RCLootCouncil3.

[SafeteeWoW]

I want to remove autoAward feature. I don't understand why any guild would use it. I never see any non-Epic items that can be master looted in the raid. Does those kind of items exist in the earlier expansion? And I dont think anyone autoAward epic items

[SafeteeWoW]

See this for performance optimization: https://www.curseforge.com/wow/addons/findglobals

[SafeteeWoW]

Recent bug reports about disconnect suggest more that we shouldn't send unneeded data. Usually server only disconnects you when you send too much data. It should not disconnect when addon has LUA error. If stuck in infinite loop, the client stop responding, but no disconnection

[evil-morfar]

Yeah, but when are we sending/asking for too much (potentially repeating) data? This can't possibly be about the data sent between clients, it has to be about processing data locally - the excact thing I've been trying to avoid. This has never been an issue before!

You'll crash if you break lua rules (most likely infinite loop), you'll get disconnected if you break Blizzard stuff. I'm still not seeing any reasons from the current issues that should cause disconects (except for the 30 people count).

[evil-morfar]

I haven't really seen your edited comment, and not considered your autoAward comment (just for clarification).

For the find globals stuff - I have begun that, but that has been down prioritied during your recent additions. But yeah, it's probably one of the biggest performance enchancements that can currently be done (LibDebug stfu much?).

[SafeteeWoW]

Did people report that the game freezes? If not, disconnection is not caused by infinite loop. Long ago, EPGP module has a bug causes infinite loop. It freezes the game, but no disconnection.

[SafeteeWoW]

1. I have never seen uncached items outside of test mode inside the raid instance(Except when I just reloaded the game), (Mouseover an item and then see retrieving item info), no matter ML or non-ML. Even if it happens, it should happen rarely.
2. Too much redundant code involving the copy and paste lootTable. Processing the item info locally makes the maintenance much easier, as long as we write a small library.
3. What are the problems if the iteminfo is not sent by the ML? Although sometimes in test mode caching takes a while, that's only for large amount of items. A session usually contains 6 items for 30man raid, which shouldn't take much time for caching.
4. Considering an extreme case that 30man raid distributes all items from 11 bosses at the end, 66 items. While caching may take a while, if we send iteminfo with the loottable, sending the extra data may take more time, not considering larger data has higher risk to be lost. I do personally prefer to have shorter time to send the lootTable, and at least raid members can start to response cached items earlier.
5. Does processing the item info locally downgrade the performance? In current version ML is caching the items, and I don't think there is performance issues in ML module. So the performance is at the least to be the addon performance of ML. However, I can make performance better, because the code is more organized by processing item info locally, and I can optimize the code more carefully.
6. I agree that if the item is uncached and data is sent normally, send the item info helps to make everyone on the same page. But considering another scenario that item is cached but lootTable is lost in transmission, send the item info does the opposite.

[evil-morfar]

Auto award was a very requested feature back in the day for giving out random greens/blues to disenchanters.

1. It happens all the time. Items are usually displayed with either icons or links (both doesn't require it to be cached to be displayed) so you only see it when mousing over an item (through the tooltip, which shows "Retrieving iteminfo"). Looting a boss is a great example - the items aren't cached for people that haven't seen the item this session. Inspecting someone is another great example. Normally it isn't a problem because they'll usually be cached before you mouse over them, but when when handled in code, it requires additional delays if it isn't ready.
2. What copy/pasting? The argument goes both ways - sending all the required info makes sure you know when you have all the required data.
3. The problem is we can't be sure how long it takes for any given client to get the info. And while the code is run in parrallel between raiders, you'd still overall need 6 x 30 calls to GetItemInfo() instead of 6. It would require you to potentially make a lot of delays to ensure you've received that data 30 times. Also when considering the current DC problems, this is a way to lower output.
4. This would again require 66 x 30 calls to GetItemInfo() instead of just 66. Even if it would take extra time, we're still talking milliseconds, and then you haven't counted the extra server requests needed. Don't start with data losses - they might aswell occur in our server requests. Besides, in this case number of requests is generally more important than the size of them due to fixed sized packets.
5. Let's talk numbers then. Picking a random item from my log [Vest of Waning Life]:

• Currently that item takes up 289 bytes to send.
• Stripping the already planned stuff (name, equipLoc, etc) plus those that's not really needed (quality, lootSlot, awarded) makes it 155 bytes.
• Further stripping the table names (essentially my vision of it) makes it 142 bytes. At this point we're only sending link, boe, ilvl, relic, token and classes - basically all that's required for clients not to cache the item.
• Stripping it completely down to only itemstring makes it 65 bytes.
• In total the difference is (worst case) 142 x 66 = 9,372 kB vs 65 x 66 = 4,290 kB, a difference of 5,082 kB (54%) over an entire raid.
  But then you still need the GetItemInfo() call from clients. Now, I don't know exactly how that's sent, but assuming it only sends data not available in GetItemInfoInstant and it's somewhat clever with not sending the name twice (name included in link), with AceSerialize that would still amount to 143 bytes. Let's assume Blizzard uses something way more efficient than that, so lets say it's only 1/4 of that (~36 bytes). That's another 66 * 36 = 2,376 kB, meaning the gain is only 2,706 kB overall (29%) (This difference even almost goes away if the efficiency is only 50%). This nicely fits within the 4 kB burst ChatThrottleLib allows.
  Of course this number would go up and down depending on the item, but I think it's safe to say there's benefits in only doing the caching once.

6. GetItemInfo might also be lost. We shouldn't really think about that, as mentioned above.

[evil-morfar]

Now, you could argue all clients will do a GetItemInfo anyway to actually see the item (which is true), but it's still not that much data needed to ensure everything works instantly.

[SafeteeWoW]

TLDR, GetItemInfo does not query the server. It fetches information on the disk to the memory. Since GetItemInfo does not involves network traffic, it should both faster and more reliable than transmitting item info over addon channel.

1. It seems raid no longer drops green/blue stuffs. I assume AutoAward feature can be removed later.

2. Dont equalize 6 calls on 30 machines each with 6 x 30 calls on single machine. If things run in parallel, we should consider the performance as 6 calls instead of 180.

3. Item link can be compressed even further. At least Color code, item name can be removed from the link. |Hitem:151982::::::::110:264:::::||h. This is only 39 bytes. Colons in the item link can also be compressed in some way. Btw, if the item is already cached, calling it with item name and color code removed does not cause it to recache. If I tranmit the item link with some ways to compress the repeated colons, I can reduce the size further. Ultimately, it's may be possible to let 255 characters to contain 6 items.

4. GetItemInfo doesn't always send network request if it is not cached in memory. It first looks at the disk cache. The example is this code https://gist.github.com/SafeteeWoW/ccfa4e6f3fef81d36f9e69b9ed5fb7bb In one experiment, I got GET_ITEM_INFO_RECEIVED on the next frame(after 0.01s) of my GetItemInfo(). If this is sent by network request, this cant be this fast because I have 60 ms latency. This is why GetItemInfo is fast most of the time. Some item id even gets GET_ITEM_INFO_RECEIVED on the same frame.

• I did another test. This time I logged into a CN server, where I had 180ms delay. I tested with all antorus items. All Antorus item info are received with 40ms. Furthermore, even in the 100% of speed of light, it takes 66ms to travel back and forth between my location and that server (10000km away). Therefore I can confirm the item is on disk cache.
• I logged onto PTR, which I haven't logged in for half a year. A lots of Antorus item get cached in the same frame after GetItemInfo. Some other get cached after 100ms. Delay is 140ms.
• I use this item to test item one by one: https://gist.github.com/SafeteeWoW/dc023fb68582df5a0275b2b6f69d71e8 on PTR. Only two cases: 1. The item info is not received after long time. (Maybe because GetItemInfoInstant returns, but that item actually does not exists. It happens). 2. The item info is received in the next frame.
• Now I highly doubt if GetItemInfo query server at all. Maybe all item fetches data from disk. I personally only encountered long Retrieving item info on login. It makes sense, because on login your disk is busy at that moment.
• Wowprogramming documentation of GetItemInfo is out-of-date. I don't trust it.
• I will delete and reinstall PTR to test again. (I should install WoW on a computer that has never installed WoW before to avoid any potential additional caching. I dont have one right now).

5. You are assuming the raiders always need to fetch the item info when the lootTable is received, however, it is not true. If the item data is already cached. Then ML sends the item info is completely not necessary.
   Here are my observations.

• Quit the game clears the item cache. Relog sometimes clear it. but reloading don't or at least very likely don't.
• Item info is id based. GetItemInfo on the link on normal difficulty also caches the link on mythic difficulty and the arg of event GET_ITEM_INFO_RECEIVED is item id.
• Open the Dungeon Journal for Antorus cache all its items.
• Loot the item should always cache it.
• Items in the inventory are always cached.

6. It never takes more than 0.2s for me to cache all Antorus items. No matter if I delete the WoW cache folder, restart the game client, etc. We see items in '/rc fulltest' need more time to be cached because we only refresh it every 1s. It's a performance lost if you transmit all item info. The additional information takes at least an extra second to transmit instead of 0.2s.
7. Suppose GetItemInfo actually does not query the server, I think fetch information on the disk is always more reliable than sending anything on the network. Then GetItemInfo(data from disk) should be always both faster and more reliable than transmit data over addon channel. Besides, it also makes coding easier.
8. Again tested on PTR, but I have reinstalled the PTR game client. The only addon enabled is WoWLua. The code is https://gist.github.com/SafeteeWoW/7a4a63770870af06717830d871619242 The the majority of GetItemInfo, the info is fetch in the next frame (9-10ms for me@100FPS). This should be enough to confirm that GetItemInfo does not query the server. There are some occasionally several hundreds ms of GetItemInfo, but that's the natural of disk. Although somehow my latency drops to 30ms somehow, the network still won't allow me to receive server message within 10ms.
9. Then I did an ultimate test. While running the above code, I unplugged my computer's network cable. But the above code still runs. After 5s, WoW finally decides to tell me I'm disconnected.
10. Another issue is that why I cannot cache all items using for i=1,200000 do GetItemInfo(i) end. I think there is an upper limit (~32k) how many items can be cached inside the memory. Thus the above code never cache all items.
11. Why are there some long retrieving item infos on login? There are usually lots of item needs to be cached on login, especially if you login to a major city. Caching hundreds of items together takes some time. Beside, the disk is busying reading some other stuffs at that moment. This can also explain why retrieving item info is rarely seen during the raid.
12. Let's talk about math. Currently an item takes 289 bytes, so 6 items -=1734 bytes ~= 7 messages (6.8*255). Compressed item links 43 bytes, 6 items = 2 message, considering some additional data. Overal (7-2)/7 = 70% less data.
13. In the current RCLootCouncil version, it seems 1s to too much for recaching the item. I suggest to change it to 0.2s

[evil-morfar]

It seems you're right.

1. Yes, but let's see what happens in the next expansion.
2. If GetItemInfo did receive data from the server, then it would be fair to consider the total implications.
3. I serialized {link = "item:151982::::::::110:265::5:3:3611:1487:3528:::"}. I should have removed "link", although there's only a 3 byte difference as it'll get number indexed. Also "|H" and "|h" is not part of the itemstring, only the itemlink.
   Yes, GetItemInfo takes itemID or "itemString" or "itemName" or "itemLink" as arguments. You can also remove trailing colons, as it's still a valid itemstring. Removing colons from the middle of the string is probably not worth it.
4. I did some tests prior to my last post (quite similar to yours), which confirmed my statement. However, I didn't consider the "lag" after logging in, and consistantly saw 5-7 seconds for some items. Increasing the run time made me realise my error though.

• I found that once every item equipped/in bags shows a tooltip (was in an instance, as I read somewhere it also caches items from people around you), every single call to GetItemInfo would be received the next frame.
• Using your random itemid code, I never once saw any noticeable time increase, and I find it highly unlikely I should have cached all those items.
• I also modifed it to only print if the time was more than 35 ms (my background fps is capped at 30, i.e. 33 ms per frame), bumped the timer to 0.1s, and ran it for ~20mins and didn't see a single print (depending on how fast math.random found a valid id, that should be around 12.000 items checked).
• So in conclusion, you're right in GetItemInfo does not query the server.

---

Furthermore I found that:

• What I thought to be an uncached item (GetItemInfo returns nil) is really a "not in memory" item and not "not on our disk" (I know that's basically the definition of cache, but you get the point).
• Each uncached item will only get 1 "GET_ITEM_INFO_RECEIVED" event per session. The data will always be returned immediately for subsequent GetItemInfo calls. This explains why you had some items that never showed up.
• There's probably a cap on how many items that can be cached at once, but I didn't see it - granted neither did I really look for it - but it's at least bigger than every raid in Legion.
• A /reload will clear most of the cached items, but not all.

Also, Wowprogramming really isn't reliable for anything that's been changed in the last two expansions (quite good for old UI/event stuff though). The wikis are generally much more reliable.

5. Mostly covered above. A few mentions though:

• Items were changes recently (WoD or Legion) so that any "additions" are part of the itemstring, and the itemID is shared between all the different versions of the item. Before that, a heroic version had a completely different itemID the normal version.
• Items gets cached because the corresponding Blizzard addons requests it. Try doing an /etrace and open the encounter journal.

6. Yep.
7. While I don't agree it makes coding easier (you have to handle a potential "wait a frame" before doing stuff), it has certainly proved to be more efficient. It's indeed also faster, but not really more reliable.
8. Weird if it was consistently the same delay, but I suppose that depends on what the computer is doing. I never experienced that, although I didn't try PTR, and have WoW on a seperate disk.
9. Yep - I heard of people using that "trick" to exceed game boundaries in vanilla, as it can restore connection if only unplugged quickly :)
10. Makes sense.
11. Yep.
12. It's not fair to compare with the current version (not that it matters now). I did a few comm delay tests, and it would seem a 1 byte message (well, as small as it gets) takes just as long to send as a 255 byte message. I can't fully confirm it yet though, but this really is to be expected.
13. Indeed.

[SafeteeWoW]

local libS = LibStub:GetLibrary("AceSerializer-3.0")
local libC = LibStub:GetLibrary("LibCompress")
local libCE = libC:GetAddonEncodeTable()

local t = {}

for k, v in pairs(RCLootCouncilML.lootTable) do
    local link = string.match(v.link, ".*|Hitem:(.-):*|h.*")
    t[k] = {['\128'] = link, ['\129'] = true}
end
local s = libS:Serialize(t)
print(s:len())
local c = libC:Compress(s)
local f = libCE:Encode(c)
print(f:len())

So I run '/rc fulltest 6" then run this code. It shows the lootTable now only takes 163 bytes to transmit. I have an idea to compress data. We can have a dictionary to replace commonly used word, if they are used as the key to the table. For example, we can reserve all ascii value >= 128 and replace "link" by "\128", replace "bagged" by "\129", etc. Those stuffs do takes lots of spaces. Because the addon is communication heavy, I do want to make the messages in most cases be within 255 character limit.

[SafeteeWoW]

Further more, duplicate items will be shown in the same session. Just need to adjust the UI a little bit.

[evil-morfar]

I don't think it's worth it to do string replacements. If we know lootTable will always only contain link and bagged, then a number indexed table will do just fine.

---

I can't imagine a good way to display several equal items in the same session - it will be confusing, and imo showing two tabs is better. It might be worth combining equal items when sending out the lootTable though - that's easily done both when sending and recreating at the receiving end.

[evil-morfar]

Also, what's the result print(s:len()) in your example?

[SafeteeWoW]

In another "/rc fulltest 6".
Before compress = 286 -> 2 messages.
After compress = 175 -> 1 message.
This is big improvement

[SafeteeWoW]

Example:
item:151982::::::::110:265::5:3:3611:1487:3528 -- trailing colon not needed
item:151982:::::::::265::5:3:3611:1487:3528 -- player level does nothing unless the item is heirloom.
item:\015\019\082:::::::::\002\065::\005:\003:\036\011:\014\087:\035\028 -- We can change the base of number to compress it further. For example, use base 100 and use one ASCII character to store on digit. (\015 is one ASCII character with the value 15) (This is just an example. The real one will avoid to use ASCII characters that are escaped by AceSerializer)
item:\015\019\082&9\002\065::\005:\003:\036\011:\014\087:\035\028 -- Repeated colons can be reduced by some encoding.
it:\015\019\082&9\002\065::\005:\003:\036\011:\014\087:\035\028 -- Change the prefix "item:" to sth shorter but unique.

Before compress: 48 bytes
After compress: 24 bytes

[evil-morfar]

I only asked because I've always found encoding to increase the size.

There's a huge difference in doing replacements in table names and actual data. The latter is fine, provided it isn't slow.

I did a quick test of your encoding:
Data example (not the string used in the first example)
Method 1 : "item:137487:5890::::::::::35:3:3418:1597:3337"
Method 2: "it:\013\074\087:\058\090\082&9\035:\003:\034\018:\015\097:\033\037"

Single itemstring

.	Raw	Serialized	CompressedLZW	Serialized + Compressed
Method1:	43	49	44	50
Method2:	24	38	25	39

Table of 6 items (last 2 is duplicates)

Edit: 4 items doesn't really make sense.

.	Serialized	CompressedLZW	Huffman	Huffman + Encode
Method1:	326	327	192	205
Method2:	243	244	233	236

I only used LZW compression, as it's the only one I exported to run out of the game.

Also, you don't need to transmit "item" - it can always be added on receive as we know it's needed.

[evil-morfar]

So is it worth it? Maybe, not sure.

In the previous example, once any other data is added, it would be two messages any way, and so far my testing shows a message takes equal time no matter how big that message is (as it really should).

I expect most transmission to be shorted down from 2 to 1 with this change, but it really depends on what needs to be sent.

[SafeteeWoW]

Goal of communcation handling in RCv3

1. Small commands sent together should be combined into one big command. Send 2 messages with 250 characters should be much faster than sending 10 messages with 50 characters.

2. When session contains 6 session in 30man raid, any command from non-ML should not exceed 255 characters.

3. It's very possible that MLdb is within 255 characters if key are number indexed (I don't like this) or there is some string replacement.

4. candidates improvement.

   • Dont think name, class and role should be transmitted through addon comm. There is no need. The game client always fetch these information anyway and these information can be fetched by API. It just redundant to transmit these information again when the game client has done it. While you can say it's possible that the candidate list may be different with ML if it is not transmitted, but it will only happen on logged in when GetRaidRosterInfo is not ready. We can simply delay any other operations until GetRaidRosterInfo is ready.
   • What about the information of disenchanting information? This should be transmitted and stored in the candidate table. but to save space, the key of the candidate table will not be the candidate name. It will be the candidate GUID with "PLAYER-" prefix removed.
     • Alternative, if we assume people in the raid are from the same guild, we can use GetGuildTradeSkillInfo() to get these information.

5. council improvement.

   • I know this may not need be improved because there is usually no more than 5 councils. But I want to mention this any way. Do consider the extreme case.
   • council table should not contain candidate name to save space, but use UnitGUID without "PLAYER-" prefix to save space. For example, 57-0A0BF579, this is 11 characters per candidate. If we also consider the serializer cost ^S57-0A0BF579, that is 13 characters per candidate.
     • Note that it is much smaller to serialize as Serialize(unpack(council)) instead of Serialize(council), because the table key won't be serialized.
   • Send council within MLdb command will be an even better improvement

6. lootTable improvement.

   • As what I said above, array should be serialized by Serialize(unpack(lootTable))

7. General improvment

   • Same idea. In SendCommand, local toSend = self:Serialize(command, {...}) should be changed to local toSend = self:Serialize(command, ...)

8. AceSerializer.

   • AceSerializer is not a serializer with the minimum size. It offers a readable format, but give larger size. I actually want to create another serializer to give smaller size. For your debugging purpose, you can use AceSerializer to print stuff into log.
   • ^N, ^S, ^B stuffs can be reduced to one byte. Replace ^N with \1, ^S with \2 etc. (I do need to consider how to properly escape those, but those character in rare in transmission, so I don't consider that in this example. UTF-8 encoding is the same as ASCII for character 0-127)
   • It saves ~10%-20% space after compression according to my testing.
   • If the table is an array, key is not needed to be serialized. This saves lots of space.
   • While testing with my RCLootCouncil.mldb compressed by LibCompress Before: 1197. After 1072. I haven't removed key for array. Should be less than 1000 if I do that.

[SafeteeWoW]

I only asked because I've always found encoding to increase the size.

The main reason is that it is an example and the encoding didn't avoid the characters escaped by the serializer. non-print characters such as \003 is prefixed by an additional escape character. And the real one should be base 220 instead of base 100.

Can you test again, but replaced all ASCII characters by adding 32? i.e. replace \003 by \035, \002 by \034
I'll do a test myself.

---

Yep, I can confirm it's not worth it. after tested myself.

[evil-morfar]

Yes, Huffman compression is more effective than the version of your encoding I used. I'll update my result for clarity.

[evil-morfar]

As for your communication goals:

1. Agreed, however it probably doesn't happen that often.

2. Also agreed, but I'm not sure it can be guaranteed. Take the current lootAck as an example.

3. Space wise, it would be the same (presuming you'd use 1 byte keys). Depending on the serialization method there could be some (minor) gains to both solutions. I agree mldb is a good example of where to use string replacements instead numbers, as guaranteeing the order could cost extra data if it doesn't need to be sent. Elsewhere (e.g. lootTable) there's no real benefit in using strings.

4. I agree. And since GUIDs are permanent it's easy to store a GUID to playername/class table in SV.

5. Always assume worst case.
   I agree GUIDs should be used here as well (actually anywhere where realname is currently used).
   Doing Serialize(unpack(council)) is not worth it. In general cases (say 6 councilmen) you'll only save 22 bytes. With many more (18 in my case) it grows to 67 bytes. Contructing the council like [name] = true makes the previous mentioned 18 strong council go from 461 bytes to 369, 25 better than unpacking first. It has the added benefit of making looking up values faster.

6. and 7.
   I'm not really sure it's worth it. For lootTable you'd at most save 22 bytes (which presumeably shrinks to almost nothing once compressed). Not to mention you'd also have to handle the unknown number of arguments on receiving (although it's easy to make a function for that).

7. For debugging it doesn't really matter how it's serialized/encoded, as long as it's consistant. It's easy to write a decode out of game.
   I'm not sure if it's really worth it in the end, but feel free to look into it.

One thing to remember when talking encoding/serializing/compression - it can't have too much overhead. If each and every command sent needs a different encoding just to squeeze out a few more bytes, then it's probably not worth it.

[SafeteeWoW]

Next I want to talk about the code structure. I think a very good example is the addon TradeskillMaster. That is a large project with well designed structure.

1. In general, I don't like a big file. It is much frequent to get and harder to resolve merge conflicts for large file. The code should be structured so other people can easily contribute to it.
2. We should split UI stuffs into separate folder/file. Each widget should have a separate file. See TradeSkillMaster\GUI\ for an example
3. Modules should have separate file for its graphic frame and other code. GetFrame() in each module should be in another file.
4. Separate file will be used to enhance scrollTable and rightclick menu.
5. I want to change the key of localization entries to the style of FrameXMl/GlobalStrings.lua Key is all caps English. I don't want to have localized text in the code and this is more convenient if I want to adjust the English text later.

[evil-morfar]

I generally agree, and already mentioned that earlier.
I don't think it's inherently necessary to separate every widget into their own file - TSM does it as it is creating AceGUI widgets, which generally are bigger than those used in RCLootCouncil.

5. It doesn't matter - I just prefer non caps as it's generally easier to read. Where is localized code being used (except for debugs of course)?

[SafeteeWoW]

Can you include test script in the repository? We can exclude it in .pkgmeta
And I found many good test script examples in Ace3 repo; https://repos.wowace.com/wow/ace3/trunk

Started to use this for testing: https://github.com/IUdalov/u-test

[SafeteeWoW]

I have done a new serializer (Some extra tests need to be added later), which roughly 20% smaller after Compressed by LibCompress compared to the compressed result of AceSerializer. (Tested on RCLootCouncil.db.profile and RCLootCouncil.mldb("/rc fulltest 6"). Feel free to give suggestion how to optimize it. (For smaller data that can be contained within 255 characters, roughly 5% smaller)

Optimizations of this Serializer:

1. If the table is an array, key is no longer serialized.
2. All value separator is one byte instead of two bytes in AceSerializer.
3. reuse any string that has been already seen once in the data to compress the data further.
   Example: When we first see the string "Response" then we give it an id, for example 1. Then the next time when we see "Response" during the serialization, we serialize it as 1 instead of "Response". I believe this will give better result for large table. (Further optimization will be applied later).

Cons:

1. The serialized result is no longer readable
2. More CPU usage of serialization

Code to compare RCSerializer and AceSerializer: https://gist.github.com/SafeteeWoW/73fcc52a141376c200acab0606555c75
My Saved Variable for reference: https://gist.github.com/SafeteeWoW/3f48ea63035246353fccc39467a9ea92

RCSerializer-3.0: https://github.com/SafeteeWoW/RCLootCouncil3/blob/master/RCLibs/Serializer.lua
Test Suite: https://github.com/SafeteeWoW/RCLootCouncil3/blob/master/test/RCLibs/Serializer.lua

---

I want to release this serializer as a standalone version, but I have to get rid of all Ace3 code in order to follow its license.

---

30% smaller than AceSerializer after LibCompressed for this data: https://gist.github.com/evil-morfar/b2706d1954c623b292b890d9d04fc19c Correctness has been verified.

[evil-morfar]

Folder for tests/scripts along with those I've been using so far: 0f91642

Keep in mind I did not intend anyone but me to see those, hence the mess. It was originally created using extracts of code (like Serializer) as I wasn't aware someone had made a wow_api.lua.

Great job with the serializer. Are you certain the reserved bits can't be in the text? /009 is tab, which might be included. Of course it could be a requirement not to use those, but I'm more curious about the others.

Also IntToCompressedInt and the like; does it really belong in the serializer?

Do you have an idea of how much more cpu time this costs?

[SafeteeWoW]

The serializer intends to work with any data except function/userdata/thread/NaN(Not a number).
\009 is supported by the serilaizer. It is escaped in the serializer as \001\056
The escape character \001 is escaped as \001\048

If the string was seen more than once during the serialization, it is encoded as a number index instead. IntToCompressedInt is used so I can encode two digits numbers in one byte. This operation does save space after Huffman compared by encode the index in decimal.

[SafeteeWoW]

For the CPU part, I tested with your extreme data in game, https://gist.github.com/evil-morfar/b2706d1954c623b292b890d9d04fc19c, timed by debugprofilestop()

AceSer: 4.24ms (67537 bytes)
RCSer : 22.82ms (34502 bytes)

---

However, because RCSer generates smaller data than AceSer, less CPU used when compressed by LibCompress.

Compress AceSer: 78.27ms
Compress RCSer: 42.69ms

---

Overall, RCSer uses less CPU than AceSer if compressed by LibCompress

AceSer+Compress: 93.51ms (33918 bytes)
RCSer+Compress: 72.9ms (22522 bytes)

---

---

Then I tested with my MLdb(RCLootCouncil.mldb):

AceSer: 0.38ms (2042 bytes)
RCSer: 1.69ms (1116 bytes)
Compress AceSer: 3.72ms
Compress RCSer: 2.53ms
AceSer+Compress: 3.98ms (1519 bytes)
RCSer+Compress: 4.17ms (1058 bytes)

RCSer has nearly the same performance as AceSer

---

In conclusion, although RCSerializer is slower than AceSerializer, however, since it generates smaller data, its performance impact is canceled by the less CPU usage when compressed by Huffman encoding. And I haven't considered the fact slightly more CPU needed to transmit larger data. Thus, no significant performance difference compared to AceSerializer.

[evil-morfar]

Due to my limited time, the ML changes, BfA release, and presumably patch 8.0 a month before, here's my current plans for future updates:

2.7.10
Practically ready. Contains the current master branch + a few smaller fixes.
Release: some time next week.

2.8
This will contain a fully working PL mode using the current framework, just to get it working and tested before BfA. This should also include a new trade UI for raiders, along with any finished improvements in the backlog (I probably won't be able to spend too much time testing/fixing stuff that's not ready).
Release: ASAP.

2.9 / 3.0 (patch 8.0 - probably july 3/4)
This would be the optimal time to have a new comms system running (which includes a potential new serializer). It'll be 3.0 if the comms are ready, otherwise 2.9 with 8.0 API/toc updates, along with anything else finished. I'll start work on comms as soon as 2.8 is done.

3.0 / 3.1 (BfA launch, august 14)
Last chance to do comms update (the absolute latest will be when the raid opens, presumably a week later). If comms are finished before this, then I'll work on any unfinished, planned features for this update.

3.1+
Here's when code structural changes can happen, i.e. most of the things discussed in this thread + #165. Somethings might make sense to change earlier (depending on where it makes sense with other changes - stuff like lootTable, candidates and council needs to be updated with comms, while UI can probably wait). I'll make sure to create anything new (e.g. trade UI) with the discussed changes in mind.

[SafeteeWoW]

That's a good plan. I would help if I can. But my current priority is the new Compressor (already released, hope you can help to test it) and the new serialize (50% ready. working on it this weekend). Getting communication quicker will resolve lots of issue (Also, we don't need to transmit item info. I dont have time to update my PR to the item info module though, but it should mostly works). As I tested out, using the new compressor and my WIP serializer, transmitting all item equipped on a player (removing "item" and trailing :::: needs no more than 2 WoW messages (< 500 bytes), so consider if you would do that.

[evil-morfar]

I'll make sure to test it during development. Also just fyi, I have beta access (and alpha) if you need something tested. I'll probably develop 3.0 on the beta.

[evil-morfar]

As usual things are delayed (this time mostly due to the reduced raiding in my guild meaning me being unable to test stuff when I had a lot of time for it). This means PL isn't quite ready, but will be released soon with patch 8.0 updates as v2.8.

This will need a few updates, which have first priority, but after that I'll focus on v3.0 (skipping 2.9) as per my post above. Any updates on the compressor and/or serializer?

[SafeteeWoW]

Hi, I was afk this month, spending the nights on FIFA world cups. So there is no progress this month.
However, the Compressor is 100% complete already. The new serializer currently makes it 20% more smaller, but I am not satisfied yet. I have restarted the searializer project today and hopfully give out a version Before August.

[evil-morfar]

Yeah, world cups did take a lot of time :)
I'm just finishing up the PL stuff, but right now it doesn't seem realistic with v3.0 for BfA release, as there's simply so much to do. I'll see how much I can get done during the next week or so now that I believe 2.8 is done.

Please keep me updated on the progress on the serializer.

[evil-morfar]

Just an update on v3.0. There's simply too many things that requires refactoring for it to make sense with the comms update. That combined with the PL issues (and the addition of Azerite Armor becoming tradeable) means I won't have v3.0 ready for BFA raids release.

For now I'm going to postpone it to the next major patch/raid release. This also makes it a lot easier to test changes, and allows me to focus on the current new features.