HonorSpy

HonorSpy

2M Downloads

Issue with latest release - column values

HolteUnderscore opened this issue ยท 30 comments

commented

I think your formula is a bit awry on the latest update pushed yesterday. The EstTodayHonor column contains a value that is adding what you actually got today to the known honour. Thus when it calculates for EstWeekHonor it looks like you have twice as much as you do have.

commented

If you patched the addon midweek there might be some inconsistencies until the next reset. Let me know if its still wrong after doing some pvp after the reset.

commented

Okay I think it wasn't resetting properly for people in a specific situation: people who hadn't patched the addon before reset today, and patched the addon after reset today before logging in. Unfortunate timing. I've fixed it in this: 03cf4c3 but I can't backdate it to fix for anyone who already patched and logged in today. Also need @kakysha to accept the PR!

commented

I got something really looking like this issue on latest update today

image

As you can see, the estWeekHonor is just the estToday doubled

commented

This guy has a "failed blizzard update" hence the estToday but the value is still doubled

image

commented

That guy is either running an outdated version or he updated the addon halfway through the week. There isn't much I can do about someone updating at weird times.

commented

I had him install it yesterday so it would reset properly, isn't this enough ?

commented

It'll be right next week. OR you have the EU issue. From the front page:

Reset Day Is Wrong: I'm playing on EU server, but HonorSpy resets on Tuesday instead of Wednesday: its because your client thinks you are in US. Check your WoW Classic Folder > WTF > Config file, it should have SET portal "EU", not "US".

commented

I don't think the reset day is wrong, it's just blizz not updating honor/rank as it often happens on Wednesday!
This is why there is still honor registered this week. This is not the issue however.

It'll be right next week

Does it means that new user may have doubled value on their first week of use ?

commented

Does it means that new user may have doubled value on their first week of use ?

Its not a "new user" problem. Your friend has used HonorSpy before. Maybe a long time ago.

commented

Allright !

commented

I still have the issue this week :/

image

EDIT : And my config.wtf is fine

commented

Did you log into that character last week after updating the addon?

commented

Yes! I installed the addon last week, before the previous week reset on wednesday.

Installed on march 1st, reset was on march 2nd, played on the 2nd to 8th of march week, new reset was yesterday on march 9th

commented

This seems to display proper values : Slivo-fr@0d7d243

image

I can pull request if you wish so

commented

The problem exists in the Honor tab. It says This Week Honor 1750 (1750) when it should say This Week Honor 1750 (0).

commented

You've changed GUI.lua (which I hadn't touched). I'll need to check that your implementation is consistent with how original_honor and estimated_honor are intended to be used. What does the Honor tab look like with your changes?

commented

No changes, I only touched the main window table display part. Honor tab shouldn't be affected

commented

I don't think so. As far as I understand the code, it's intended for the week honor to include the blizzard week values and the estimated honor of the day.

commented

No. 1750 is what Blizzard has calculated you have already earned before today, this week. If you haven't done any PVP today then it should be 0 in brackets.

commented

I disagree with that.

Today should be zero, but the week value should equal the blizz value at daily reset.
Then when you start gaining honor, it increase both today and this week value.

Maybe @kakysha could help us there

commented

Er yeah something is definitely wrong here. 218 was the estimated honor yesterday, which has been corrected to 204 by Blizzard.

image

commented

Okay, I agree with your changes in Slivo-fr@0d7d243

estHonor and thisWeekHonor do not function the way I expected them. They're different to orignal_honor and estimated_honor

Go ahead and PR it.

commented

Done ! :)

I think I'll have to handle conflicts after first merge

commented

Should just make them the same PR.

commented

Would avoid the conflicts but that's two separate issues, I prefer each having their own merge request !
I'll be quick with the conflict so we don't waste time :)

commented

This does raise a question however, do we really need the EstTodayHonor column in the GUI ?
Because estWeekHonor will now includes it and should be enough itself imo
You would have known honor for the blizz value and EstWeekHonor for the player estimated value of the week, today estimated just being the week estimation minus blizz value

commented

I does look like there is still something a bit odd somewhere and I get estimated values that are smaller than the blizz value

image

commented

I look like it happens when the blizz value gets updated without the estimation being updated.
I'd say it's not a GUI issue, I could however set it to display 0 if it's negative.

What do you think ?

Edit : did that in the #190

commented

Yeah it'll be when you've inspected that player, but haven't gotten an estimation update from them. Probably because they turned HonorSpy off.

if (tonumber(estHonor) ~= nil) then and if (tonumber(estHonor)) then is the same code? I also might have to fix the sorting function

commented

if (tonumber(estHonor) ~= nil) then and if (tonumber(estHonor)) then is the same code?

Kinda, but the nil check is more clean imo