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
Improve humanize walking variance #5147
Conversation
mjmadsen
commented
Sep 3, 2016
•
edited
edited
- Account for bearing when adding in walking "sway"
- We now only sway "left" and "right" as we head towards our destination
@mjmadsen, thanks for your PR! By analyzing the annotation information on this pull request, we identified @douglascamata, @sinap and @alexyaoyang to be potential reviewers |
@@ -24,6 +24,9 @@ def random_lat_long_delta(): | |||
# Return random value from [-.000025, .000025]. Since 364,000 feet is equivalent to one degree of latitude, this | |||
# should be 364,000 * .000025 = 9.1. So it returns between [-9.1, 9.1] | |||
return ((random() * 0.00001) - 0.000005) * 5 | |||
|
|||
def random_lat_long_delta2(bearing): | |||
return ((random() * 0.00001) - 0.000005) * 5, ((random() * 0.00001) - 0.000005) * 5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do you plan to use bearing in the calculation...?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're heading at 0/360, we only add variance to latitude. Do some more math between there to scale each respective to that. I'm not sure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait I got that backwards. If we're headed north (0/360), we'd change our longitude.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Delta applies to the destination. So as long as the Delta is smaller than the (destination - origin), you will never walk backward.
And as it is now, it is small enough for that. So not sure you need to change anything.
@@ -106,3 +110,43 @@ def step(self): | |||
|
|||
def _pythagorean(self, lat, lng): | |||
return sqrt((lat ** 2) + (lng ** 2)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you import math
then this should become
math.sqrt
@@ -109,7 +109,7 @@ def step(self): | |||
# alt, False) | |||
|
|||
def _pythagorean(self, lat, lng): | |||
return sqrt((lat ** 2) + (lng ** 2)) | |||
return math.sqrt((lat ** 2) + (lng ** 2)) | |||
|
|||
def calc_bearing(start_lat, start_lng, dest_lat, dest_lng): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it be static method? calc_bearing(self...) ? Or you forget @static decorator?
upd: i'm about calc_bearing()
@static # ??
def calc_bearing(start_lat, start_lng, dest_lat, dest_lng)
@mjmadsen I am afraid that the travis build is failing due to non-ASCII carracters
|
|
||
def step(self): | ||
walk_sway = random_lat_long_delta2(self.bearing) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
random_lat_long_delta2 import missed?
I don't think that adding a random value to only latitude or only longitude based on bearing adds much extra... |
return sqrt((lat ** 2) + (lng ** 2)) | ||
return math.sqrt((lat ** 2) + (lng ** 2)) | ||
|
||
def _calc_bearing(start_lat, start_lng, dest_lat, dest_lng): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make
def _calc_bearing(self, start_lat, start_lng, dest_lat, dest_lng)
or make it static
@staticmethod
def calc_bearing(start_lat, start_lng, dest_lat, dest_lng)
self.calc_bearing(self.initLat, self.initLng, self.dLat, self.dLng)
#self.calc_bearing(self.initLat, self.initLng, self.dLat, self.dLng)
@@ -4,7 +4,7 @@ | |||
|
|||
from random import uniform | |||
from pokemongo_bot.cell_workers.utils import distance | |||
from pokemongo_bot.human_behaviour import random_lat_long_delta2, sleep, random_alt_delta | |||
from pokemongo_bot.human_behaviour import random_lat_long_delta, random_lat_long_delta2, sleep, random_alt_delta |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any usage of random_lat_long_delta?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test files use it still. I'll update those too if I can get this working :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually the CI still fails because of how the test files are setup.
@th3w4y Yeah, I still need to figure out the important part :) |
looks good 💃 might have to edit pokemongo_bot/test/step_walker_test.py to pass CI |
My proposal: We only specify an effect in degrees from the bearing.. where we would want to be at
|
This above obviously can replace the whole pythagorean.. magnitude.. the big convoluted part in StepWalker |
@sohje one explanation would be: pokestops are too close... google maps does not have walking directions in between them but your picture shows that you were automatically fallback to classic behaviour... waking through buildings to confirm that 💯 % do you have the coordinates of those pokestops from your fort files? |
I'm running through all the walkers. Might not finish before I go to bed. @anakin5 does this seem to give your poly the results you're looking for? If we seal this portion down, we don't have to mess with it for a long ass time! |
@th3w4y according to google maps there are 1-3 routes between pokestops on that coords. |
handle special values for speed
@mjmadsen one more commit: https://github.com/mjmadsen/PokemonGo-Bot/pull/2 |
@mjmadsen i know that we'll not see 0.0 or infinite but it make the function complete and protected from ZeroDivizion |
@mjmadsen I found a bug... bearing should be towards the final destination to towards something calculated... |
This is incorrect
|
still moving straight ;] [2016-09-03 20:31:12] [MoveToFort] Moving towards pokestop XX - 0.13km
[2016-09-03 20:31:17] [MoveToFort] Moving towards pokestop XX - 0.11km
[2016-09-03 20:31:23] [MoveToFort] Moving towards pokestop XX - 0.10km
[2016-09-03 20:31:29] [MoveToFort] Moving towards pokestop XX - 0.10km
[2016-09-03 20:31:35] [MoveToFort] Moving towards pokestop XX - 0.11km
[2016-09-03 20:31:42] [MoveToFort] towards pokestop XX - 0.12km |
Should be:
|
yeap, with destLat and destLng works fine. |
@mjmadsen bearing calculation should use final destination https://github.com/mjmadsen/PokemonGo-Bot/pull/3 |
bearing calculation should use final destination
@sohje This should be a very unnoticeable change to look like strafing while walking. |
@sohje so is working now? screenshot please :) |
@th3w4y yeap, with bearing fix works fine. Wait a sec, i'll make pull - edited manually. |
hmm, i dont see on the map any strafing ;[ |
@sohje me too https://s15.postimg.org/4extwtzjf/test4.jpg
Turned catching pokemon off for testing |
Seems we have to implement some sort of speed acceleration and deceleration. |
@sohje for the polyline... that would be hard... |
@th3w4y yeap... |
@sohje i take the challenge then ! but i think i can do that if i also refactor the Polyline to use headings and bearings calculation... will take few days... |
but this does not need to wait for that refactoring... |
Tried, testing and working on my end - can confirm speed variance issue mentioned by @sohje I'm approving this for now, will check in a moment before merging |