step_walker variance #3943
Conversation
|
@mjmadsen, thanks for your PR! By analyzing the annotation information on this pull request, we identified @TheSavior, @MFizz and @douglascamata to be potential reviewers |
|
@mjmadsen Maybe add a comment that the config is in meters/second? People having speed of 20+ is just a guaranteed ban. |
|
From what I can tell, the previous code uses walk as km per hour. distance_unit is used purely for output (from what I can tell). @DiegoRBaquero Added a brief note in the base config. We need a total overhaul on documentation anyway, but I'd prefer we reduce our bans in the meantime. |
|
I just checked and this is with 10 as the walk setting: and: and Which gives us 13.68 km/h (3.8 m/s), 14.65 km/h (4.07 m/s) and 16 km/ (4.44m/s) respectively. We should try and match km/h by diving the given speed in ~1.45 in that case, thoughts? |
|
@DiegoRBaquero That's interesting. Certainly could explain why my walk = 14 accounts got softbans. I'm not sure why it would be such a difference. The long_lat random is supposed to add a variance of -/+ 9.1 ft, so I don't think that would account for an extra 3-6km/hr. EDIT: Was that using a variance? |
|
@mjmadsen No, that was using 10, no variance at all. The random delta in average shouldn't add extra km/h at all. |
|
Wonder if we have a km to long/lat error somewhere. `====================================================================== FAIL: test_normalized_distance (tests.step_walker_test.TestStepWalker)Traceback (most recent call last): AssertionError: 0 not greater than 0FAIL: test_normalized_distance_times_2 (tests.step_walker_test.TestStepWalker)Traceback (most recent call last): Error from last test. I'm going to sleep. It appears that step_walker_test feeds different speeds values into the stepper. So the whole effort of reducing parameters is lost. Are these functions worth anything? If so, I can revert all parameter changes. Let me know. |
|
@douglascamata The test file hack is less than ideal. But the core code is ultimately more readable because of the changes to parameters. Let me know what you think. |
|
@douglascamata When the ONLY action that runs is walking, as in my comment: #3943 (comment), we can get really near to km/h dividing by 1.45 I can get a more exact average if we will implement the simple division. |
|
The unit is distance, but not velocity. The unit is how far we will move on each step, but we don't make any guarantees as to how many steps per time we make. |
|
Maybe this is done better with setting a boolean in the config and varying the existing speed attribute periodically by a random percentage if the bool was set to true. Doesn't break existing configs. |
|
With regards to speed would it be possible to measure the time between steps and alter the step distance based on that? Then if we've spent ages recycling items or w/e it won't matter we'll just make a bigger step next time? |
|
@TheSavior Then we should add in the doc that the distance is in meters and that a value of 4.16 (default) turns, approximately into 6 km/h. |
|
@Quantra I don't think the app updates position when in menus, stops or catching pokemon. So we may not want to push our distance change if we've been caught up in those actions for a while. @TheSavior I get that we can't guarantee total distance for an hour, but it would be nice if walking speed were directly related to some form of measure or as @DiegoRBaquero mentioned, we indicate an approx. relationship between walk values and km/hr. Basically so we're dictating the speed at which we walk when we ARE walking. However, these (imo) are not in the scope of this PR. I'm at work right now, so I can't test much. However, I'll see about adding a comment on startup about walk in the config. I don't think anyone's config would break with the present code - we still pull default values when the config is loaded. |
pokemongo_bot/step_walker.py
Outdated
|
|
||
| self.speed = speed | ||
| if self.bot.config.walk == 0: | ||
| self.speed = self.bot.config.walk_min - random() * (self.bot.config.walk_max - self.bot.config.walk_min) |
There was a problem hiding this comment.
I think this should be
self.speed = self.bot.config.walk_max - random() * (self.bot.config.walk_max - self.bot.config.walk_min)
?
Adding walk back in made the test files mess up again. Default values for walk_max, walk_min should be fine until folks update their configs.
Configs should work fine with default walk_max and walk_min values.
|
I removed the switch to use walk instead of walk_min/walk_max and removed walk from the config. It will generate a warning on startup and will still have the default values for walk_max/walk_min. |
|
|
||
| old_flags = ['mode', 'catch_pokemon', 'spin_forts', 'forts_spin', 'hatch_eggs', 'release_pokemon', 'softban_fix', | ||
| 'longer_eggs_first', 'evolve_speed', 'use_lucky_egg', 'item_filter', 'evolve_all', 'evolve_cp_min', 'max_steps'] | ||
| 'longer_eggs_first', 'evolve_speed', 'use_lucky_egg', 'item_filter', 'evolve_all', 'evolve_cp_min', |
There was a problem hiding this comment.
We don't want to add it here because this will exit the bot saying that it doesn't exist anymore. You should just check for walk being set, print a message that walk_min and walk_max are used now instead, and then the bot should be able to keep running
There was a problem hiding this comment.
Switch it to a warning below.
|
👍 |
|
I got this error [01:09] Walking from (-7.982077060831172, 112.63066117494552, 0) to (-7.982088600000001, 112.6310973, 0) (48.0422623786 m) |
8 participants
Switched walk in config to walk_max and walk_min to add variance to stepper.
Also added variance when close to stop/pokemon.
Removed redundant parameters.
Updated configs to reflect change.
Updated for conflicts (#3857).