Honorspy is sometime mixing up character data

[Slivo-fr]

On some undefined edge cases, Honorspy stores data from a first char to another one, including it's class. On the example below, Elerina is a rogue and has been given Edens ranking data (Last week honor, standing, etc)

[Slivo-fr]

This get fixed if the player with the faulty data use the addon and come back online. Maybe even if he gets inspected again while not using the addon?

[kakysha]

Yes, it gets fixed when the player is inspected. The problem arises due to some sort of race condition, when the "inspector" quickly switches their targets so when the data of old player arrives he is already targeting another player so the data got assigned to wrong player. We have checks for this iirc, but probably the check is not 100% guarantee.

[Slivo-fr]

I will try to understand extacly what happens see if I can find and suggest a fix, may take a long while! Was this issue already existing before the SoM and connected ERA realms changes ?

[teelolws]

Yes theres no real fix for this, and I kind of blame Blizzard for this for not giving us a way to get the Character Name or even GUID of the player we have Inspect data about.

[Slivo-fr]

This would certainly be way easier if blizzard would give that information I agree I do think we may be able to figure out what is exactly this race condition is and how to workaround

[Slivo-fr]

Here is a suggestion : Once we started inspecting a player, we stop inspecting players until INSPECT_HONOR_UPDATE fires or a defined safety time has been elapsed.

This should prevent any data mixing as there will ever only be one single player inspected at a time

[teelolws]

The addon already has an internal cooldown on calling for inspects. But that doesn't stop other addons requesting inspect data, and HonorSpy seeing the results with NFI where the request came from.

[Slivo-fr]

The addon already has an internal cooldown on calling for inspects

Yeah but this one only avoid inspecting the same player again

Good point about the possible other addons requesting inpect data however, this one is tricky

[teelolws]

Correction. This is the code I was thinking of:

if (GetServerTime() - player.last_checked < 30) then -- 30 seconds until new inspection request return end

That cooldown is per-player. May not be a bad idea to add another cooldown of, say, three seconds, to inspects called by HonorSpy overall.

[Slivo-fr]

3 seconds seems a lot, didn't you say we could expect the results in .4s approximately ? I mean, we could miss a lot of data in 3 seconds

[teelolws]

I'd be generous for players with lag.

[Slivo-fr]

See what I had in mind on #201

[kakysha]

As per my own observations, the race condition happens when player is inspecting the other player manually by opening Honor tab, and then mouseovering players nearby him. Does the timeout help with this case?

[teelolws]

You have code that disables HonorSpy while the player is manually inspecting another player.

[Slivo-fr]

I think the issue may be elsewhere :

maflateb is a poolboosting level1 toon that has not been online for more than two days and has been given Gamlir honor data today. This excludes an inspecting race condition

[kakysha]

Hmmm, that's interesting. Also we should not forget that anyone can spoof the data manually, but values being the same all the time with another players make me think this is not the case in here.

[Slivo-fr]

I can almost garantee that noone is spoofing on my small french realm, we know every player. Could it be some comm issues ? Or maybe some race condition on friend check ?

[Slivo-fr]

Just found one more similar case, ogrog eleven is also a poolboost char that was not online for at least 2 days

it seems to happen more and more while the dataset grows

[kakysha]

Not all the columns are the same though, surprisingly. Need to think a bit.

[Slivo-fr]

It's because Silents has been playing since the "clone" and it's estimated honor has increased. I'm pretty sure the initial clone had the same values

[Slivo-fr]

Searching for more occurrence I found out there is a lot of mixing among the poolboosting chars that hasn't been online for long

It may be easier to track the moment where the cloning happens once the sorting will be fixed

[Slivo-fr]

I just remembered a similar case I was talking about to Teloo previously :

See the "thi" key ? Could the com message somehow get truncated or altered ?

[Slivo-fr]

And I just found another one :

See the last key ? its concatenated with something else !

[Slivo-fr]

Another exemple from a current clone

There is an additional broken key in the string(s) section

[Slivo-fr]

Could we try updating the ace libs (if outdated) and/or reducing the number of player per com message ?

EDIT : Well the message we send is about 2k characters anyway, not sure shortening it could help

EDIT 2 : few more example of fucked up keys :

Player Mimoi-Amnennar - Key : standiSestHonor - Message saved
Player Valawen-Finkle - Key : rans - Message saved
Player Mumuu-Sulfuron - Key : thisWeor - Message saved
Player Arhianna-Sulfuron - Key : ras - Message saved
Player Maflateun-Finkle - Key : thiSunitID - Message saved
Player Dedeg-Finkle - Key : thisWeekHonoclass - Message saved
Player Maflateb-Amnennar - Key : thisWeekHonrogress - Message saved
Player Djiisept-Sulfuron - Key : thisWeekHitID - Message saved
Player Nezukoh-Finkle - Key : rankProgHonor - Message saved
Player Bbccddeeg-Finkle - Key : raess - Message saved
Player Ogrogeleven-Finkle - Key : rD - Message saved
Player Hrtthtfjree-Finkle - Key : rankPror - Message saved
Player Ogrogbg-Amnennar - Key : rSclass - Message saved
Player Ogrogquinze-Finkle - Key : this - Message saved

I've setup some debug code to try catch a faulty com message, I'll be back if it works !

[Slivo-fr]

I got one !

^1^Sfiltered_players^T^SAbcdeff-Sulfuron^T^SestHonor^N6^SunitID^Splayer^Sclass^SMAGE^Slast_checked^N1648666601^SlastWeekHonor^N6^Sstanding^N68^SRP^N6491^SrankProgress^F5368997419679744^f-54^SthisWeekHonor^N1^Srank^N3^t^SKukkhffhf-Finkle^T^SestHonor^N6^SRP^N6491^Sclass^SPRIEST^Slast_checked^N1648665970^SlastWeekHonor^N6^Sstanding^N76^SrankProgress^F5368997419679744^f-54^SunitID^Splayer^SthisWeekHonor^N1^Srank^N3^t^SLolilool-Amnennar^T^SestHonor^N6^SrankProgress^F5368997419679744^f-54^Sclass^SWARRIOR^Slast_checked^N1648995001^SlastWeekHonor^N6^Sstanding^N219^SunitID^Splayer^SRP^N6491^SthisWeekHonor^N1^Srank^N3^t^SLyrosz-Finkle^T^SestHonor^N6^SrankProgress^N0^Sclass^SWARRIOR^Slast_checked^N1648994949^SlastWeekHonor^N0^Sstanding^N0^SunitID^Splayer^SRP^N0^SthisWeekHonor^N1^Srank^N0^t^SDaddsi-Finkle^T^SestHonor^N13839^SrankProgress^N0^Sclass^SHUNTER^Slast_checked^N1648995416^SlastWeekHonor^N0^Sstanding^N0^SunitID^Smouseover^SRP^N0^SthisWeekHonor^N13839^Srank^N0^t^SZaalanne-Amnennar^T^SestHonor^N2077^SunitID^Smouseover^Sclass^SROGUE^Slast_checked^N1649088552^SlastWeekHonor^N0^Sstanding^N0^SRP^N0^SrankProgress^N0^SthisWeekHonor^N2077^Srank^N0^t^SYopp-Amnennar^T^SestHonor^N6^SRP^N4648^Sclass^SWARLOCK^Slast_checked^N1648994869^SlastWeekHonor^N6^Sstanding^N483^SrankProgress^F7947529354215424^f-53^SunitID^Splayer^SthisWeekHonor^N1^Srank^N2^t^SLyrosi-Finkle^T^SestHonor^N6^SRP^N0^Sclass^SWARRIOR^Slast_checked^N1648995023^SlastWeekHonor^N0^Sstanding^N0^SrankProgress^N0^SunitID^Splayer^SthisWeekHonor^N1^Srank^N0^t^SOgrogbonusde-Amnennar^T^SestHonor^N6^SunitID^Splayer^Sclass^SWARRIOR^Slast_checked^N1648665207^SlastWeekHonor^N6^Sstanding^N382^SRP^N4895^SrankProgress^F8689298660392960^f-53^SthisWeekHonor^N1^Srank^N2^t^SBcdefg-Amnennar^T^SestHonor^N6^SthisWeeestHonor^N6^SunitID^Splayer^Sclass^SWARRIOR^Slast_checked^N1648667022^SlastWeekHonor^N6^Sstanding^N447^SrankProgress^F8300752833347584^f-53^SRP^N0^SthisWeekHonor^N1^Srank^N1^t^t^^j

The key thisWeeestHonor is faulty. I can't tell for now if that's a new one or it's just been sent as it is because it exists in the player's local database, I'll try to find out.

EDIT : Seems like the key exists in the player DB for this one.

[Slivo-fr]

I got a very interesting one just now. It's a message I sent myself, and received from myself. The message DOES contain a faulty key but my database DOES NOT contains it ! (Probably dropped somehow)

^1^Sfiltered_players^T^SPaulettonne-Amnennar^T^SestHonor^N11^SunitID^Splayer^Sclass^SDRUID^Slast_checked^N1648661457^SlastWeekHonor^N6^Sstanding^N581^SRP^N6040^SrankProgress^F7488338831343617^f-55^SthisWeekHonor^N1^Srank^N3^t^SDewulfdix-Sulfuron^T^SunitIekHonor^N6^Sstanding^N503^SrankProgress^F7488338831343617^f-55^SunitID^Splayer^SthisWeekHonor^N1^Srank^N2^t^SMimob-Amnennar^T^SestHonor^N6^SRP^N0^Sclass^SWARRIOR^Slast_checked^N1648662820^SlastWeekHonor^N6^Sstanding^N550^SrankProgress^F8936555095785472^f-53^SunitID^Splayer^SthisWeekHonor^N1^Srank^N1^t^SFinkled-Finkle^T^SestHonor^N6^SrankProgress^F5368997419679744^f-54^Sclass^SWARRIOR^Slast_checked^N1648661830^SlastWeekHonor^N6^Sstanding^N302^SunitID^Splayer^SRP^N6491^SthisWeekHonor^N1^Srank^N3^t^SPaterwulf-Finkle^T^SestHonor^N37249^SunitID^Smouseover^Sclass^SPRIEST^Slast_checked^N1649087828^SlastWeekHonor^N27860^Sstanding^N25^SRP^N38883^SrankProgress^F6993825960558592^f-53^SthisWeekHonor^N37249^Srank^N9^t^SÉkys-Sulfuron^T^SunitID^Smouseover^Sclass^SWA^SlastWeekHonor^N6^Sstanding^N367^SunitID^Splayer^SRP^N6491^SthisWeekHonor^N1^Srank^N3^t^SNerai-Sulfuron^T^SestHonor^N6^SunitID^Splayer^Sclass^SWARRIOR^Slast_checked^N1648490075^SlastWeekHonor^N6^Sstanding^N240^SRP^N6001^SrankProgress^F7205760048037887^f-55^SthisWeekHonor^N1^Srank^N3^t^SWalawg-Sulfuron^T^SestHonor^N6^SRP^N6491^Sclass^SWARRIOR^Slast_checked^N1648662405^SlastWeekHonor^N6^Sstanding^N91^SrankProgress^F5368997419679744^f-54^SunitID^Splayer^SthisWeekHonor^N1^Srank^N3^t^t^^

The key is unitIekHonor

It looks like he issue happens before sending the com message

EDIT : Confirmed, another one, received from myself

^1^Sfiltered_players^T^SBouboubourse-Amnennar^T^SestHonor^N46037^SrankProgress^F7876884658388992^f-53^Sclass^SWARRIOR^Slast_checked^N1648977750^SlastWeekHonor^N41322^Sstanding^N5^SunitID^Splayer^SRP^N54373^SthisWeekHonor^N46410^Srank^N12^t^SVil-Amnennar^T^SestHonor^N28289^SRP^N46726^Sclass^SROGUE^Slast_checked^N1649021452^SlastWeekHonor^N28114^Sstanding^N21^SrankProgress^F6216733769596928^f-54^SunitID^Smouseover^SthisWeekHonor^N8169^Srank^N11^t^SShindoe-Sulfuron^T^SestHonor^N44543^SunitID^Smouseover^ScleekHonor^N6^Sstanding^N579^SRP^N0^SrankProgress^F8724621008306176^f-53^SthisWeekHonor^N1^Srank^N1^t^SLkmokkj-Amnennar^T^SestHonor^N6^SRP^N0^Sclass^SWARRIOR^Slast_checked^N1649000839^SlastWeekHonor^N0^Sstanding^N0^SrankProgress^N0^SunitID^Splayer^SthisWeekHonor^N1^Srank^N0^t^SVilpbtrois-Amnennar^T^SestHonor^N6^SunitID^Splayer^Sclass^SWARRIOR^Slast_checked^N1648998887^SlastWeekHonor^N6^Sstanding^N653^SRP^N0^SrankProgress^F8724621008306176^f-53^SthisWeekHonor^N1^Srank^N1^t^SMimoc-Amnennar^T^SestHonor^N6^SReekHonor^N6^Sstanding^N365^SrankProgress^F5368997419679744^f-54^SunitID^Splayer^SthisWeekHonor^N1^Srank^N3^t^SXdthes-Finkle^T^SestHonor^N6^SRP^N0^Sclass^SWARRIOR^Slast_checked^N1648662897^SlastWeekHonor^N0^Sstanding^N0^SrankProgress^N0^SunitID^Splayer^SthisWeekHonor^N1^Srank^N0^t^SVilpbquatre-Amnennar^T^SestHonor^N6^SunitID^Splayer^Sclass^SWARRIOR^Slast_checked^N1648999224^SlastWeekHonor^N6^Sstanding^N654^SRP^N0^SrankProgress^F8724621008306176^f-53^SthisWeekHonor^N1^Srank^N1^t^t^^ 

Key ReekHonor, nil in my db table

[teelolws]

Sounds like something is compressing the data for transmission then never decompressing it.

[kakysha]

or the bug of message chunking feature of Ace?

[Slivo-fr]

Which bug ? It could somehow be related to acecom I believe

[Slivo-fr]

I've discussed this on wowaddon discord.

This bug is most likely due to some split of the original message not being sent by blizzard Acecomm does not check for data integrity when putting back together the pieces of the original message

I will be adding a compression step on coms to reduce the amount of data sent and eventually limiting the number of messages that are not sent/blocked by server. I will also add a check to verify if the message is complete before processing it, rejecting it if it's corrupted.

[kakysha]

I just suggested that there is a bug in chunking / chunk aggregation process inside AceComm, which is what other guys also pointed out as a possible cause. Compressing with checksum is the way to workaround this I believe, no signs of similar problems on AceComm issues? UPD: Ace didn't even moved to Github yet... the first step we should do is update Ace libs.

[Slivo-fr]

As far as we checked with teelo, they are up to date.

This is not a bug in ace com, it's just not handling message integrity it not all parts are sent

[teelolws]

Its something you'd think AceComm could handle for us, but for whatever reason, they didn't. My suggestion was to add the library LibDeflate, add a Compress and Decompress layer, and bump the Comm Prefix again. Apparently a checksum would also help.

[Slivo-fr]

I'm working on implementing both