HonorSpy icon indicating copy to clipboard operation
HonorSpy copied to clipboard

Honorspy randomly reset estimated honor

Open Slivo-fr opened this issue 3 years ago • 4 comments
trafficstars

In some undefined edge case, honorspy re-set the estimated honor to the actual blizzard "this week" value This can lead to large difference in real honor if it happens in the middle of a PVP night.

I have a specific case I can describe here that will probably not help solving it but should help understanding what I'm explaining. I had a player aiming to stack his honor that was going to take a plagueland tower : He had 106634 honor and had few HK during the day. When he gained the tower honor, his honor felt back to 106299 (blizz week value + tower honor, HK honor was lost)

Here is a screen of his character sheet after the tower

image

I can't find out how and why yet, still investigating

Slivo-fr avatar Jun 04 '22 09:06 Slivo-fr

I've been able to debug a bit, here is what I found.

At HonorSpy:CheckNeedReset I had this print:

    local _, thisWeekHonor = GetPVPThisWeekStats()
    if (HonorSpy.db.char.original_honor ~= thisWeekHonor) or (HonorSpy.db.char.last_reset ~= must_reset_on) then
		print("HonorSpy:CheckNeedReset", "reset", skipUpdate, HonorSpy.db.char.estimated_honor)
		print(HonorSpy.db.char.original_honor, thisWeekHonor, HonorSpy.db.char.last_reset, must_reset_on)

And when it happened to me the output was (line 2 and 3)

[16:51:56] HonorSpy:CheckNeedReset skipUpdate nil 80759
[16:51:56] HonorSpy:CheckNeedReset reset nil 80759
[16:51:56] 49218 0 1656486000 1656486000
[16:51:56] HonorSpy:CheckNeedReset2 reset nil 0
[16:51:56] HonorSpy:CheckNeedReset skipUpdate nil 0

The player thisWeekHonor was 0 at the moment. Could the GetPVPThisWeekStats return a wrong value in some edge cases ? The point is I also had this debug print :

function HonorSpy:CheckNeedReset(skipUpdate)

	print("HonorSpy:CheckNeedReset", "skipUpdate", skipUpdate, HonorSpy.db.char.estimated_honor)

	if (not skipUpdate) then
		HonorSpy:UpdatePlayerData(function() HonorSpy:CheckNeedReset(true) end)
	end

And as you can see on the output above, HonorSpy:CheckNeedReset is only called once with skipupdate true, without a skipupdate nil first and I can't find any other call to HonorSpy:CheckNeedReset(true) in the code outside HonorSpy:CheckNeedReset function.

image

Slivo-fr avatar Jul 03 '22 15:07 Slivo-fr

If GetPVPThisWeekStats is the culprit here, we can't control the faulty behavior. In this case I'm considering a timed call back (5-10s ?) to checkneedreset with a "doublecheck" boolean when the returned value is 0 so it does confirm a second time the actual value and doesn't reset on the first missreturn

Any opinion about this workaround ?

Slivo-fr avatar Jul 04 '22 12:07 Slivo-fr

We can try that, maybe you can test it for some time locally first and see if GetPVPThisWeekStats is the actual cause?

kakysha avatar Jul 04 '22 19:07 kakysha

I'm definitely debugging more on my side before even putting up a merge request yeah I'll keep you updated

Slivo-fr avatar Jul 04 '22 20:07 Slivo-fr

I unable to reproduce this issue for a long time now. It was quite common when I first opened this thread. My fix should be functionnal and ready but I didn't had the chance to test it properly because of that Maybe something changed on blizzard end or in the context somehow ?

I will keep my debug/test local version. Maybe I can open a draft merge request with my WIP fix and we let it be so it's there if it's ever needed later on ?

Slivo-fr avatar Sep 28 '22 10:09 Slivo-fr

Sure, let's keep it for history

kakysha avatar Sep 28 '22 11:09 kakysha

Happened to a guildmate lately, so it's not fully gone

Slivo-fr avatar Oct 07 '22 07:10 Slivo-fr