Updated docs for MoveToMap

[DBa2016]

As mentioned in PR#4937, udpated configuration_files.md

[mention-bot]

@DBa2016, thanks for your PR! By analyzing the annotation information on this pull request, we identified @TheSavior, @Gurzeh and @supercourgette to be potential reviewers

[Jasperrr91]

@DBa2016 you messed up the logic in the move_to_map_pokemon. Can you please change it back from:
if pokemon['dist'] > self.config['max_distance'] and not self.config['snipe']:

To
if pokemon['dist'] > self.config['max_distance'] or not self.config['snipe']:

Edit: Just saw you also changed #4855
it the previous time. That change was reverted #4857 because it broke the logic:

Then it was broken again in #4860

[solderzzc]

The real confusing comes from the meaning of 'max_distance'.
It should be split into two config fields:
'max_walk_distance' and 'max_snip_distance', otherwise, no mater how to change, all can't be on the same page.

[Jasperrr91]

Indeed, the naming of the field is causing the confusion. It should indeed
be renamed to max_snip_distance. Not sure if we need the max_walk_distance
since after this if statement, there's logic that checks if the distance is
walkable right?

Op woensdag 31 augustus 2016 heeft Simba Zhang [email protected]
het volgende geschreven:

The real confusing if comes from the meaning of 'max_distance'.
It should be split into two config fields:
'max_walk_distance' and 'max_snip_distance', otherwise, no mater how to
change, all can't be on the same page.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#4969 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AKz7qJBLiHCb7-N0NlOScYRLaUUIHlvpks5qlKtMgaJpZM4JxAAh
.

[solderzzc]

@Jasperrr91 Some scary guys want to disable the snipe but only set the bot on the popular location or several bots together as cluster, walk to the location reported by map/others if possible(there's code to calc if have enough time).

[DBa2016]

I agree the config variable should be renamed to "max_snipe_distance", however this particular PR is about doc changes only. Until this one is merged, I cannot work on other changes.

[Jasperrr91]

I'll put in that PR when I get back to my computer. I had to hijack this PR
as you don't respond to the relevant PR's and you keep blatantly changing
the logic back to AND.

Op woensdag 31 augustus 2016 heeft DBa2016 [email protected] het
volgende geschreven:

I agree the field should be renamed to "max_snip_e__distance", however
this particular PR is about doc changes only.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#4969 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AKz7qN4bzIv00DwcXq-C81Flti4TOEquks5qlLvzgaJpZM4JxAAh
.

[Jasperrr91]

Thereby breaking the bot.

Op woensdag 31 augustus 2016 heeft Jasper van der stoop <
[email protected]> het volgende geschreven:

I'll put in that PR when I get back to my computer. I had to hijack this
PR as you don't respond to the relevant PR's and you keep blatantly
changing the logic back to AND.

Op woensdag 31 augustus 2016 heeft DBa2016 <[email protected]
javascript:_e(%7B%7D,'cvml','[email protected]');> het volgende
geschreven:

I agree the field should be renamed to "max_snip_e__distance", however
this particular PR is about doc changes only.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#4969 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AKz7qN4bzIv00DwcXq-C81Flti4TOEquks5qlLvzgaJpZM4JxAAh
.

[DBa2016]

@Jasperrr91 : there is a reason why a PR needs an approval... If you think a PR breaks the logic - don't approve it. Also - please help me understand the "you don't respond to relevant PRs" statement.

[mjmadsen]

@Jasperrr91 Sorry I merged the other and missed the problem!

[Jasperrr91]

@DBa2016 the 'breaking' PR #4860 was approved within 10 mins so I hadn't seen it.

I was referring to #4855 which refers back to #4772. In there I wrote down that the AND NOT statement breaks the bot and should be OR NOT. Although there's a better solution, working on that now.

@mjmadsen no problem! Those things can happen ^^

[DBa2016]

@Jasperrr91 : by all means, I am not following the comments on a PR after it has been closed, I just don't have time for this. So yes, shame on me, I did not realize that "max distance" was in fact meaning "max_sniping_distance", I took it for "max visibility" or something similar.

[Jasperrr91]

@DBa2016 no problem, glad that you realise it now. I'm putting out a PR in a few mins.

[mjmadsen]

As we've lost some of our original devs, I hope we don't have any bad blood here. We need every programmer we can get our hands on. What I think we can take away is that we should try to clear up variable naming conventions to resolve ambiguity. Second, we should be more proactive in commenting our code (it always seems obvious to the person writing it). Third, I'll let PRs sit around a bit longer before merging them - especially for files I do not know thoroughly.

Aside from that, shall we rebase/merge dev and update our md? :)

[DBa2016]

This is a trivial change for the docs. Is there any reason not to approve it? It prevents me from doing more important stuff.

[DBa2016]

Oh, and since we are at it... Any reason #4951 was approved and merged though it is breaking the bot's functionality without anyone giving the contributor hard time?
I cannot even fix it because my repository is being held hostage here.

[Jasperrr91]

Sorry if I offended you @DBa2016, I was just a little bit annoyed that the max_distance logic was broken twice after I fixed it. I have not yet looked into the whole Telegram function but if that PR break it, that's indeed not good.

A tip for the future, you can rebranch and work on new fixes while your old PR's remain active in here. I'll merge this PR right away.

[Jasperrr91]

@mjmadsen sounds good! I'll work on adding comments to the code and enforcing DRY. Leaving PR's open a little longer also sounds like a good idea 👍