Added softban unban feature by MikeDX · Pull Request #1522 · PokemonGoF/PokemonGo-Bot

MikeDX · 2016-07-29T01:02:49Z

Short Description:

Upon detection of a softban, bot will stop wasting balls and berries on pokemon and travel to a pokestop to try to unban itself.

Fixes:

Softban from travelling too far / quickly
Some small grammar changes (seem -> seems)

douglascamata · 2016-07-29T01:12:17Z

@MikeDX what about extracting softban unban feature into a worker that runs every tick, checks self.bot.softbanned and run if it is true? Let's make things more modular!

MikeDX · 2016-07-29T01:16:47Z

@douglascamata So, to do this:

if bot banned:
walk to pokestop
spin spin spin
set banned = false?

resume normal operation

douglascamata · 2016-07-29T01:37:34Z

SoftBanFixWorker:

if bot banned:
    walk to pokestop (using MoveToFortWorker)
    spin once (using SeenFortWorker)
    if still banned:
        return WorkerState.RUNNING (so tick is immediately finished and next one starts)

Add this detail to SeenFortWorker

if got loot:
  set banned = false

And this SoftBanFixWorker should be added as first element of the worker list, to run first.

MikeDX · 2016-07-29T01:38:37Z

Ok, makes sense. should the unban worker be in the root, or as a cell worker? (guessing root)

ok it should be a cell. I'll get a new push done soon

MikeDX · 2016-07-29T02:12:48Z

@douglascamata How would you suggest we keep the nearest fort between ticks, since currently it is doing its job but moving between forts to try to fix each tick which isn't what we want.

MikeDX · 2016-07-29T03:02:09Z

New worker created, removed lots of output when iterating through the spins. Works well, although takes about a minute to complete.

Also checks for lures after the ban is lifted

MikeDX · 2016-07-29T03:11:46Z

Resolved conflicts / rebased to dev

wilsonyqm · 2016-07-29T03:44:02Z

Can't wait to see this PR merged

MatthewARoy · 2016-07-29T07:42:58Z

Ah excellent, I really hope this works as expected. It will be a very nice feature to auto-unban.
Thanks guys!

douglascamata · 2016-07-29T08:09:03Z

@MikeDX actually we want it to recalculate the nearest fort at every tick. It should work without self.bot.nearest_fort in SpinNearestFortWorker. You don't even use that worker.

MikeDX · 2016-07-29T08:19:40Z

@douglascamata Using nearest every tick moves to the next one instead of spinning the same stop - I don't think we do want that!

Actually if you do want to do it that way, then just scrap the unban worker completely and just don't catch pokemon if banned, and let the bot wander around pokestops until it unbans itself

MikeDX · 2016-07-29T08:23:18Z

rebased to dev + squashed again.

douglascamata · 2016-07-29T08:45:55Z

@MikeDX but SpinNearestFort worker won't execute at all while you return WorkerResult.RUNNING, because you are the first worker. And SeenFortWorker always tries to search the fort is is given, no matter what.

MikeDX · 2016-07-29T08:50:03Z

@douglascamata I'm not calling that method or using SpinNearestFort anymore - it doesnt need it. The first time the bot runs, banned = false

then it goes through the list, eventually calling SpinNearestFortWorker which does set the nearest_fort value. Even if CatchVisiblePokemonWorker sets the ban to true before SpinNearest runs, SpinNearestFortWorker will still execute and find a stop regardless, so any time the bot is banned and SoftBanWorker is called, it will always have a stop to go to.

douglascamata · 2016-07-29T08:58:23Z

@MikeDX hum, I see now. I think of another option other than the attribute self.nearest_fort: do your own fort search.

As simple as seen in SpinNearestFortWorker (but ignoring cooldown and circle filters):

def nearest_pokestop(self):
    if 'forts' in self.bot.cell:
        forts = [fort
            for fort in self.bot.cell['forts']
             if 'latitude' in fort and 'type' in fort]
        forts.sort(key=lambda x: distance(self.position[
                       0], self.position[1], x['latitude'], x['longitude']))
        return forts[0]
    return None

douglascamata · 2016-07-29T09:00:09Z

#1578 makes it even better! Fort search is not in self.bot and it doesn't include any filter. Perfect for you!

MikeDX · 2016-07-29T09:01:06Z

@douglascamata Would have been handy to have that answer, when I asked the question 7 hours ago...

how does #1578 make it better? That method still excludes the forts we have spun recently, which again is not what we want.

douglascamata · 2016-07-29T09:04:13Z

#1578 self.bot.get_forts includes ALL nearby forts, regardless of any filter. That's almost all you need. You just need to sort the array by distance using:

forts.sort(key=lambda x: distance(self.position[
                    0], self.position[1], x['latitude'], x['longitude']))

elicwhite · 2016-07-29T09:10:55Z

That sort by distance logic should probably be pulled out as well, we use it in a bunch of places.

Side note, @douglascamata I think we need a new place for those types of functions that aren't the bot class.

douglascamata · 2016-07-29T09:15:19Z

@TheSavior yes, we need refactor like #1253. But who wants to test/merge? :P

Limited output when trying to unban

MikeDX · 2016-07-29T09:20:43Z

@douglascamata I have updated the bot to get its own fort position, but since that function is not yet merged, I have replaced it with my own for now.

douglascamata · 2016-07-29T09:23:00Z

@MikeDX ok... I do a quick update of your code in dev after we merge #1578

douglascamata · 2016-07-29T09:23:27Z

👍

douglascamata · 2016-07-29T09:23:50Z

@TheSavior give a thumbs up here... I update the SoftBanWorker to use your utility functions after we merge our PR.

elicwhite · 2016-07-29T09:30:06Z

pokemongo_bot/__init__.py

        self.check_session(self.position[0:2])

        workers = [
+            SoftBanWorker,


This should probably be down the list, right before catching pokemon. Since the bot can still transfer and evolve while banned.

@TheSavior but which pokemon would you transfer or evolve that you didn't the last tick if you cannot catch?

That's a good point, at least with the current behavior of the workers we have.

elicwhite · 2016-07-29T09:30:46Z

👍

elicwhite · 2016-07-29T09:32:29Z

pokemongo_bot/cell_workers/catch_visible_pokemon_worker.py


    def work(self):
-        if not self.config.catch_pokemon:
+        if not self.config.catch_pokemon or self.bot.softbanned:


we shouldn't need to change this

it will waste pokeballs if we don't do this

It will never get to this worker since the softbanned worker returns RUNNING

If there are more than one pokemon within range, yes it will

elicwhite · 2016-07-29T09:32:47Z

👎

elicwhite · 2016-07-29T09:33:13Z

pokemongo_bot/cell_workers/move_to_fort_worker.py


        if dist > 10:
-            logger.log('Moving towards fort {}, {} left'.format(fortID, format_dist(dist, unit)))
+            if self.bot.softbanned == False:


We shouldn't be changing this either.

this one I agree

This is about the only merge comment I agree with!

MikeDX · 2016-07-29T09:58:32Z

@TheSavior @douglascamata Can I suggest one of you take what I've done and do all the testing and re-implementation of what I have done since you don't agree with any of this pull request. Some of your comments seem to suggest you don't even understand what your own code is doing!

MatthewARoy · 2016-07-29T16:46:53Z

Thanks for working on it @MikeDX
Have we done any testing yet? I've seen it on a few websites but wanted to make sure with more thorough testing. If you get a branch in working order I'll check it out, make a new account and softban it a few times to test

MikeDX · 2016-07-29T18:08:59Z

I did a lot of testing yesterday and spent many hours on it getting it to work as it should and bent it to fit "the rules"

Since closing this pull request, dev has changed multiple times and a lot has changed meaning most if not all would have to be re-done. I made a lot of changes to rebase with dev, and it still isn't working properly. One worker returns "11" or "0" - whatever that means, certainly not a SUCCESS or RUNNING value.

As with my previous pull request, I'm not really willing to waste any more time on it for it to be glossed over and nitpicked. See above comments on the code - almost every single one was based upon a misunderstanding of what the problem was and how to fix it.

I have participated in multiple open source projects in the past and I have never seen such a crazy set of rules for merging code before - it is totally hit and miss here. I see garbage merged into the code that breaks the whole system, and even with syntax errors with tabulation, yet when I submit working, tested code, it is pulled apart for not being done in a certain way - and then nitpicked again by someone who did not even understand the problem in the first place. Frustrating and absolutely crazy.

elicwhite · 2016-07-29T21:07:36Z

@MikeDX, I hear you and I definitely agree, I did not understand the problem and your solution as a whole while I was reviewing it. Our goal on this project is to be as inclusive as possible and to help contributors feel as good as possible contributing. We failed on this one, and that is entirely my fault.

We definitely want to get this feature in, it is valuable to many people using this bot. I understand the lack of interest you have iterating on this more without a clear path to success. Recognizing that, we will probably need to spend our time doing it. Because you deleted your branch, we will probably have to pull out the functionality from the diff itself.

We are overwhelmed with pull requests and tend to do a first pass on reviews just pattern matching on the common issues we've seen. Typically that includes things like: implementing a feature that requires changes to lots of workers, workers calling other workers, and PRs that say they do one thing and then actually change a bunch of other things as well. Unfortunately, I pattern matched this PR to the first case, which was not fair to this PR and the work you've done. When spending more time looking at it and making sure I understand the things actually being done, it makes more sense.

Again, this is my mistake and thus I need to remedy that by getting this functionality added. I hope this experience won't completely put you off from this project, we always want to value quality contributions and quality contributors. Thank you for contributing this PR.

elicwhite · 2016-07-29T22:12:12Z

@MikeDX Here is a follow up PR. Please take a look, your code influenced the implementation. #1724

MikeDX · 2016-07-31T15:14:28Z

@TheSavior I appreciate what you are saying, but pull request 1724 is shockingly bad and @douglascamata didn't even follow his own advice that he told me to do after my original commit!

I really hope you get your act(s) together on this project. It showed so much promise but arrogance and a certain level of ignorance has definitely put me off almost entirely now which for me is a real shame as there is absolutely no motivation to work on a feature or bug fix since submissions are so poorly vetted.

MikeDX · 2016-07-31T15:17:14Z

See issue #2049.

As I said before, I have contributed to many projects both open source and within professional environments and this is easily the worst managed one I have ever come across.

douglascamata mentioned this pull request Jul 29, 2016

[FEATURE] Releasing account from soft ban #1513

Closed

douglascamata added the Waiting on Changes label Jul 29, 2016

MikeDX force-pushed the features/softban-unban branch 2 times, most recently from 9c43129 to a1f3aeb Compare July 29, 2016 03:21

MikeDX mentioned this pull request Jul 29, 2016

During softban it errors, rather than removing ban by spinning pokestop many times #1540

Closed

MikeDX force-pushed the features/softban-unban branch from a1f3aeb to fe36d88 Compare July 29, 2016 08:18

MikeDX force-pushed the features/softban-unban branch from fe36d88 to 936b09e Compare July 29, 2016 08:22

Added softban unban feature using a worker

6885cca

Limited output when trying to unban

MikeDX force-pushed the features/softban-unban branch from 936b09e to 6885cca Compare July 29, 2016 09:20

elicwhite reviewed Jul 29, 2016
View reviewed changes

MikeDX closed this Jul 29, 2016

MikeDX deleted the features/softban-unban branch July 29, 2016 09:58

SMWARREN mentioned this pull request Aug 1, 2016

fix for #2104 #2136

Closed

Comments

Conversation

MikeDX commented Jul 29, 2016

Uh oh!

douglascamata commented Jul 29, 2016

Uh oh!

MikeDX commented Jul 29, 2016

Uh oh!

douglascamata commented Jul 29, 2016

Uh oh!

MikeDX commented Jul 29, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MikeDX commented Jul 29, 2016

Uh oh!

MikeDX commented Jul 29, 2016

Uh oh!

MikeDX commented Jul 29, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wilsonyqm commented Jul 29, 2016

Uh oh!

MatthewARoy commented Jul 29, 2016

Uh oh!

douglascamata commented Jul 29, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MikeDX commented Jul 29, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MikeDX commented Jul 29, 2016

Uh oh!

douglascamata commented Jul 29, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MikeDX commented Jul 29, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

douglascamata commented Jul 29, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

douglascamata commented Jul 29, 2016

Uh oh!

MikeDX commented Jul 29, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

douglascamata commented Jul 29, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elicwhite commented Jul 29, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

douglascamata commented Jul 29, 2016

Uh oh!

MikeDX commented Jul 29, 2016

Uh oh!

douglascamata commented Jul 29, 2016

Uh oh!

douglascamata commented Jul 29, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

douglascamata commented Jul 29, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

douglascamata Jul 29, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elicwhite commented Jul 29, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MikeDX commented Jul 29, 2016 •

edited

Loading

MikeDX commented Jul 29, 2016 •

edited

Loading

douglascamata commented Jul 29, 2016 •

edited

Loading

MikeDX commented Jul 29, 2016 •

edited

Loading

douglascamata commented Jul 29, 2016 •

edited

Loading

MikeDX commented Jul 29, 2016 •

edited

Loading

douglascamata commented Jul 29, 2016 •

edited

Loading

MikeDX commented Jul 29, 2016 •

edited

Loading

douglascamata commented Jul 29, 2016 •

edited

Loading

elicwhite commented Jul 29, 2016 •

edited

Loading

douglascamata commented Jul 29, 2016 •

edited

Loading

douglascamata Jul 29, 2016 •

edited

Loading

elicwhite commented Jul 29, 2016 •

edited

Loading

elicwhite commented Jul 29, 2016 •

edited

Loading

MikeDX commented Jul 29, 2016 •

edited

Loading

elicwhite commented Jul 29, 2016 •

edited

Loading