Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix update_live_stats not showing correct experience #3890

Closed
wants to merge 2 commits into from
Closed

Fix update_live_stats not showing correct experience #3890

wants to merge 2 commits into from

Conversation

vicch
Copy link

@vicch vicch commented Aug 14, 2016

Short Description:

update_live_stats not showing correct level xp and completion figures by using prev_level_xp which should instead by current level's xp.

Fixes (provide links to github issues if you can):

@mention-bot
Copy link

@vicch, thanks for your PR! By analyzing the annotation information on this pull request, we identified @Moonlight-Angel, @sergiopalacio and @TheSavior to be potential reviewers

@johannlejeune
Copy link
Contributor

johannlejeune commented Aug 14, 2016

Yep, that's correct. I noticed it already.
The problem is, the values returned by the server contains the values for the previous level and the next level. I couldn't find any values indicating experience threshold for the current level.
Also, UpdateLiveStats doesn't calculate this value itself. It takes it from the Metrics class. So the wrong XP calculation also appears in the summary when you stop the bot.
Thus the fix should be implemented in the Metrics class.

@vicch
Copy link
Author

vicch commented Aug 14, 2016

@Moonlight-Angel Metric has xp_earned() and xp_per_hour(), not actually related to levels. And summary only shows Total XP Earned: 85630 Average: 60801.72/h, so it doesn't seem necessary to modify other logics except UpdateLiveStats.

@elicwhite
Copy link
Contributor

@Moonlight-Angel, I defer to you on this PR

@johannlejeune
Copy link
Contributor

johannlejeune commented Aug 15, 2016

@vicch My bad, it's not shown in the summary when the process exits.
Take a look at the stats shown right after login, the same issue should be present there.
In this case, the same issue is present in the get_player_info method in pokemongobot.__init__.py.
And yes, these values are also computed in update_live_stats.py. Maybe it would be a better idea to create another method somewhere to calculate these values and call them in both __init__.py and update_live_stats.py.

Sorry about the delay and the false path. My accounts got banned with my IP, I can't login anymore to check right now. 😂

@vicch
Copy link
Author

vicch commented Aug 15, 2016

@Moonlight-Angel I see. Ideally there should be a helper that retrieves player stat/level stuff. Searching player_stats you can see workers and other classes doing this by themselves. I'm not confident enough to refactor these. I will have a look.

@smfbrooks
Copy link

While we're at it, can #2596 be fixed?

@douglascamata
Copy link
Member

Is this PR dead or alive?

@vicch
Copy link
Author

vicch commented Aug 16, 2016

Waiting improvements, closed for now.

@vicch vicch closed this Aug 16, 2016
@Gurzeh
Copy link
Contributor

Gurzeh commented Aug 23, 2016

@vicch check #4589 there are some clean still pending (putting list in data rep), but it solves the issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants