Honorspy is sometime mixing up character data
Slivo-fr opened this issue · 35 comments
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.
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
Sounds like something is compressing the data for transmission then never decompressing it.
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.
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.
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
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.
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?
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.
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 ?
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.
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
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?
You have code that disables HonorSpy while the player is manually inspecting another player.
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
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
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.
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.
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
See what I had in mind on #201
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.
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 ?
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
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 !