Snipping V2 #5443
Snipping V2 #5443
Conversation
Added a global function to retrieve a pokemon's ID by its name (inventory.py). The old snipping system was removed/deprecated and a new one will replace it. This was due to the fact that snipping was first introduced to use with the PokemonGo-Map project, but things have changed and it now offers a lot more flexibility to use third-party sources. All the documentation have been updated, including the example config file.
Updated the snipping's default ordering and some minor refactoring
…st (yea, lazy bitch).
Fixed the snipping bug that would not respect the catch list when under social mode. Fixed the snipping IV filtering to work only when under url mode. Fixed the snipping logic to verify the targets information. Minor log messages refactorings.
Removed jschwerberg as a contributor from the README file, since he has a PR pending as of now and might have added his name himself. He asked me to add it on slack.
|
Please also keep original MoveToMapPokemon task. |
|
Updated first post. |
|
In my opinion removing MoveToMapPokemon is the equivalent of removing one of the most human-like functions of the bot. To me, as the bot has progressed it has always been in the direction to be more human-like. I disagree with your statement in terms of walking-to-pokemon. For example, I play in an area that has a total radius of about 2.5km. Generally, the bot ends up in one of 3 'zones'. The reason MoveToMapPokemon works well for me is that I can set the walk range to a real-life scenario (human-like) and if a desired Pokemon comes within range (generally 350-700meters) then the bot walks to it and encounters there. I do this to avoid making a capture that has a spawn_id nowhere near where I'm playing. I understand using bots is "dangerous" overall, but the idea is to try maintain human-like play. I am able to easily achieve this using the recommend speeds that come in default config. Maintaining PokemonGo-Map (itself) is quite easy. If the user already has PokemonGo-Bot then they already have the dependencies. All they have to do after that is get a working API key (that they may already have from utilizing PolyLineWalker), config pogom's config, and then run the map scanner. In my experience I have seen more users enter chat due to conflicts with map-chat than I have seen users having issues connecting with pogom. I believe both pogom and map-chat have their advantages and both do lead to confusing events. Also, using spawnpoint scanner with pogom keeps it simple and the user no longer needs to update the map. Anyone trying to update the scanner from the bot would probably not have enough accounts scanning and/or not using spawnpoint scanner and they wouldn't get good pokemon data to begin with and probably would not be using pogom at that point anyway. If the new Sniper intention is to be as flexible as possible, why would something that already works be removed with an unforeseen future? Reduction here would make using the bot more restrictive. In conclusion, there has been a lot of work put into MoveToMapPokemon, and even though I have not contributed any coding to the project, I have a great appreciation for the human-like characteristics of MoveToMapPokemon. Hopefully others think similarly. |
|
I just noticed that @TheSavior was planning to do this a month ago and here it is. Lots of related issues were found as well, and I listed some of them on the first post (updated). @crvfts Thank you for your feedback. The only bot feature that goes out of the human-like scope is the sniper and it has always been like that. I have yet to see someone else who have been using walking sniper. To be honest, I dont even know why it was ever implemented. Someone has already explained to you how this exploit works but I'll remind you:
I strongly believe that you have never been lucky enough to catch something like a Snorlax or a Dragonite with your sniping settings, because its clearly very unlikely to happen exclusively to sniping feature, but a matter of luck. With teleport sniping you would of have much more Snorlaxes and Dragonites by now. Also, there's this new PokemonHunter task that might fit your walking requirements. Discussing bot/sniping safeness is like discussing religion and/or politics and I'm not going to do that, however I can tell you that I highly believe that people aren't being banned for sniping, but botting. I bet @joelgreen, one of the API contributors, would love to discuss that with you. 😋 Maintaining PokemonGo-Map can be easy for you, but wasn't for me when I started with all this python thing world. There were times that both the map and the bot used different requirements and you would need to use some third-party containers for this (python sucks). Why wouldn't you want to use someone else's source (no hosting costs, no setup work, no tons of dependency installs, nothing) instead of all this? Either way, you will still be able to use PokemonGo-Map with this new sniper. I'm offering a much bigger flexibility than only sticking with PokemonGo-Map and it's not unforeseen, its how things grows. |
|
Again, I'll reiterate- I UNDERSTAND HOW SNIPING WORKS. Again, my concern- that you catch from a spawn_id that is nowhere near it's location, ie. bot is in NY, encounters pokemon in Japan, catches a spawn_id originating in Japan in NY. I understand it's a glitch but it also something ridiculously simple that Niantic could implement (if they don't already). I'm arguing that the assumption that it is "not confirmed" is not erring on the side of caution. Yes, I understand using a bot whatsoever is the MAIN way to get banned. What I am arguing is a level between utility and fun. Your sniper has fantastic utility, I will not deny that, but for me personally it disengages fun. I am only one person and I completely understand that the direction the bot will take will not hinge on a single member that admittedly has not contributed in the coding of the bot. That's fair.
It DOES make sense to me that people are being banned for botting, not sniping. At the same time, if spawn_ids are run against the catch location they simply will never be within a range that makes sense and I find that to be a simple solution for Niantic to detect botting. This change would force reliance upon a 'glitch'. Currently, walking-to-snipe doesn't rely on a glitch in the same sense. Why rely on a glitch when it is not necessary? MoveToMapPokemon is currently working perfectly. Perhaps removing it should be saved for the time when it requires such overhaul to remain properly implemented no longer makes sense. Ease of use should be a factor but from my experience in Slack people that have a hard time utilizing pogom with the bot have a horrendous time getting bot to work to begin with simply due to not being familiar with any coding or command line whatsoever. This bot can be quite difficult to use for sure but it's pretty awesome! I also prefer to use my own computer because i have a lot of money in it haha. (on a side-note I would HAPPILY contribute my scanner data to something like maps.pikabot.org.) It's nice that I would still be able to use pogom BUT I would no longer be able to use it in the fashion I have come to love. Again, I think the removal of MoveToMapPokemon will not increase flexibility but rather limit it, the opposite of your intention. |
|
|
||
| # Build up the pokemon. Pops are used to destroy random attribute names and keep the known ones! | ||
| for pokemon_dictonary in pokemon_dictionary_list: | ||
| pokemon_dictonary['iv'] = pokemon_dictonary.pop(self.mappings.get(ResponseMapper.IV), 0) |
There was a problem hiding this comment.
Possible keyerror exceptions not handled.
There was a problem hiding this comment.
Not sure again what you mean. This line will assign a value to the 'iv' key. This value will be the corrresponding to the IV mapping name that you have specified. Even if for some reason this key does not exist, it will return 0 and pop fill fail (do nothing). The pops are used to keep a clean dictionary with single keys. By doing that, we will avoid having something like that: { 'id': 143, 'pokemon_id': 143 ... }.
There was a problem hiding this comment.
Oh, i was commenting from the phone. Its about line 258.
if not pokemon_dictonary['name']Now i see that pokemon_dictionary should be filled with None or some positive value
| targets = self._get_pokemons_from_url() | ||
|
|
||
| # Order the targets (descending) | ||
| for attr in self.order_by: |
There was a problem hiding this comment.
Are you sure that this code apply sorting correctly?
There was a problem hiding this comment.
It does sort by the attributes the user has specified, if thats what youre asking. So yes, it sorts correctly (tested).
| VALUES = [URL, SOCIAL] | ||
| DEFAULT = SOCIAL | ||
|
|
||
| class ResponseMapper(): |
There was a problem hiding this comment.
Thats strange idea to make two classes with const vars. And use this const as dict's keys...
There was a problem hiding this comment.
Why is it strange? From what I've heard by other python fans, it's a good practice. Python is all based on dictionaries, so keeping a constant key name should make things less confusing for maintainances.
| task_config = task.get('config', {}) | ||
|
|
||
| # Validate task name refactors | ||
| if type is None: |
There was a problem hiding this comment.
Havent actually changed this, except reordering the lines. Feel free to change. Anyways, whats wrong with it? Yes, this could be "if not type:" instead, but no big deal.
| # Validate task name refactors | ||
| if type is None: | ||
| raise ConfigException('No type found for given task {}'.format(type)) | ||
| elif type == 'EvolveAll': |
There was a problem hiding this comment.
After posting about the "if type is None" line, I checked the changes and I mightve probably confused about task_type and type vars, although I dont get any errors, but I'll look at it.
| from pokemongo_bot import inventory | ||
| from pokemongo_bot.inventory import Pokemon, Pokemons | ||
| from pokemongo_bot.base_task import BaseTask | ||
| from pokemongo_bot.base_dir import _base_dir |
There was a problem hiding this comment.
Havent changed this. Just kept the old imports. I might take it off, if its not really used, though.
| from pokemongo_bot.inventory import Pokemon, Pokemons | ||
| from pokemongo_bot.base_task import BaseTask | ||
| from pokemongo_bot.base_dir import _base_dir | ||
| from pokemongo_bot.constants import Constants |
There was a problem hiding this comment.
Not sure. Havent changed this. Just kept the old imports. I might take it off, if its not really used, though.
| from pokemongo_bot.constants import Constants | ||
| from pokemongo_bot.worker_result import WorkerResult | ||
| from pokemongo_bot.cell_workers.utils import format_dist, format_time, fort_details | ||
| from pokemongo_bot.walkers.walker_factory import walker_factory |
|
|
||
| import os | ||
| import time | ||
| import json |
There was a problem hiding this comment.
os is probably not used, indeed (no files). I might take it off.
| GREATBALL_ID = 2 | ||
| ULTRABALL_ID = 3 | ||
|
|
||
| class OrderMode(): |
There was a problem hiding this comment.
remove parentheses or add (object)
There was a problem hiding this comment.
Yea, its already fixed. There's a new commit comming soon. See below.
|
|
||
| from random import uniform | ||
| from pokemongo_bot import inventory | ||
| from pokemongo_bot.inventory import Pokemon, Pokemons |
There was a problem hiding this comment.
Correct. I might take it off.
|
So, I've still been working after the lastest commit for a new feature, on which you'll be able to specify multiple sources (urls). Lot's of fixes have been done. This PR can still be merged and will even fix issue #5378 @sohje Please dont use this review feature, only if really needed. I've got ~15 emails from it. 😆 |
|
I'm closing this PR because I'm opening another soon, which will be the Sniper V2.1, on which will keep the old sniper and add the "multiple sources" feature to the new one (2.1). |
|
@YvesHenri Please consider the longer sleep time after teleporting to encounter the Pokemon. Currently, it sleeps only 3 second which is too short. See my test below with 2 bots running in parallel, trying to catch the same Pokemon, one with longer wait time at the destination successfully caught it. I reported in the issue and pull request but it was closed prematurely. My apology if you happened to read my comment again but longer sleep is important so people don't miss the rare Pokemons. Let me know if you need a reliable source of confirmed rare Pokemon to test with. Thank you for an awesome work on sniping. I tested with 2 bots: This one sleeps 3 secs (by default) and missed the Pokemon[2016-09-26 20:19:52] [MoveToFort] [INFO] Moving towards pokestop USPS Sign - 0.05km This one sleeps a little bit more and caught the same exact Pokemon[2016-09-26 20:20:06] [ Sniper] [INFO] Teleporting to meet Growlithe (441.7646115485; -72.9637643552)... |
Snipping was first introduced, back in the days when there wasn't any snipping sources, to integrate the PokemonGo-Map project into the bot and this is brilliant. However, things have changed ever since. There are lots of sources nowadays that provides these informations, eg.: pokesnipers, pokespawns, pokecrew and the list goes on. Mostly of them offer APIs (urls) to get these results in a JSON formated fashion way and that's what this update has come for! Lots of efforts were put into it and everything were meticulously thought, including this text.
Below are some considerations that have been taken and why they have been added/removed:
These are the most important subjects but I could keep going forever, otherwise I wouldn't have spent all this time.
The new sniper idea is to make it as flexible as possible, by allowing the user to specify the attribute names of a JSON response, since they are almost always different, depending on the source. This also offers a much more "complex" way to prioritize your targets, instead of VIPs and threshold only. Surely there are lots of stuff to do to cover all these possibilities.
There is no point to keep the old sniper, since theyre basically the same, except that the old one allows you to reach the target by movements but only works strictly with PokemonGo-Map.
Related issues: #3672 #3656 #3666 #3355 #3278 #3226.