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
Some XP efficiency improvements #2889
Changes from 6 commits
204ad0e
c9b25d6
081d90d
4f3a744
235f6bf
ec83d2b
1fd60e7
7490bf8
a205de8
8de542a
bb931f4
ef2bb53
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,31 +1,43 @@ | ||
import logging | ||
import time | ||
|
||
|
||
class BaseTask(object): | ||
TASK_API_VERSION = 1 | ||
|
||
def __init__(self, bot, config): | ||
self.bot = bot | ||
self.config = config | ||
self._validate_work_exists() | ||
self.logger = logging.getLogger(type(self).__name__) | ||
self.initialize() | ||
|
||
def _validate_work_exists(self): | ||
method = getattr(self, 'work', None) | ||
if not method or not callable(method): | ||
raise NotImplementedError('Missing "work" method') | ||
|
||
def emit_event(self, event, sender=None, level='info', formatted='', data={}): | ||
if not sender: | ||
sender=self | ||
self.bot.event_manager.emit( | ||
event, | ||
sender=sender, | ||
level=level, | ||
formatted=formatted, | ||
data=data | ||
) | ||
|
||
def initialize(self): | ||
pass | ||
TASK_API_VERSION = 1 | ||
|
||
def __init__(self, bot, config): | ||
self.bot = bot | ||
self.config = config | ||
self._validate_work_exists() | ||
self.logger = logging.getLogger(type(self).__name__) | ||
self.last_ran = time.time() | ||
self.run_interval = config.get('run_interval', 10) | ||
self.initialize() | ||
|
||
def _update_last_ran(self): | ||
self.last_ran = time.time() | ||
|
||
def _time_to_run(self): | ||
interval = time.time() - self.last_ran | ||
if interval > self.run_interval: | ||
return True | ||
return False | ||
|
||
def _validate_work_exists(self): | ||
method = getattr(self, 'work', None) | ||
if not method or not callable(method): | ||
raise NotImplementedError('Missing "work" method') | ||
|
||
def emit_event(self, event, sender=None, level='info', formatted='', data={}): | ||
if not sender: | ||
sender=self | ||
self.bot.event_manager.emit( | ||
event, | ||
sender=sender, | ||
level=level, | ||
formatted=formatted, | ||
data=data | ||
) | ||
|
||
def initialize(self): | ||
pass |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,10 @@ | ||
# -*- coding: utf-8 -*- | ||
from __future__ import unicode_literals | ||
|
||
from pokemongo_bot.cell_workers.utils import fort_details | ||
from pokemongo_bot.cell_workers.pokemon_catch_worker import PokemonCatchWorker | ||
from pokemongo_bot.base_task import BaseTask | ||
from pokemongo_bot.constants import Constants | ||
from pokemongo_bot.cell_workers.utils import fort_details, distance | ||
from pokemongo_bot.cell_workers.pokemon_catch_worker import PokemonCatchWorker | ||
|
||
|
||
class CatchLuredPokemon(BaseTask): | ||
|
@@ -12,39 +13,52 @@ class CatchLuredPokemon(BaseTask): | |
def work(self): | ||
lured_pokemon = self.get_lured_pokemon() | ||
if lured_pokemon: | ||
self.catch_pokemon(lured_pokemon) | ||
for pokemon in lured_pokemon: | ||
self.catch_pokemon(pokemon) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happens if your pokebag gets full while catching pokemon? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it just skips the catch, like it does already |
||
|
||
def get_lured_pokemon(self): | ||
forts_in_range = [] | ||
pokemon_to_catch = [] | ||
forts = self.bot.get_forts(order_by_distance=True) | ||
|
||
if len(forts) == 0: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems like a big win is limiting this API request to only run on near enough forts. Another win would be caching these lookups so we don't make these requests every tick anyways. I wonder if doing that (here and in more places) would be enough that we wouldn't need to do this internal loop that breaks the model we have had about the tick. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What we need is an ORM-like lib to avoid doing any manual call using the raw api object. Otherwise we code lots of duplicated cache, like the one we already have for pokestops timers. Also caching all seen pokestops may have a very big impact in memory usage for long-running bots. A simple cache for all pokestops is not the way, we need something smarter. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree. I'd rather us refactor a dumb cache to make it smarter, than break our model of what a tick does while we wait for a smart cache to come. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @TheSavior yes... we will need a size-limited dict for forts with more data and not only the timer. Using a LRU policy should be good to keep the size limit. |
||
return False | ||
|
||
fort = forts[0] | ||
details = fort_details(self.bot, fort_id=fort['id'], | ||
latitude=fort['latitude'], | ||
longitude=fort['longitude']) | ||
fort_name = details.get('name', 'Unknown') | ||
for fort in forts: | ||
distance_to_fort = distance( | ||
self.bot.position[0], | ||
self.bot.position[1], | ||
fort['latitude'], | ||
fort['longitude'] | ||
) | ||
|
||
encounter_id = fort.get('lure_info', {}).get('encounter_id', None) | ||
if distance_to_fort < Constants.MAX_DISTANCE_FORT_IS_REACHABLE and encounter_id: | ||
forts_in_range.append(fort) | ||
|
||
|
||
encounter_id = fort.get('lure_info', {}).get('encounter_id', None) | ||
for fort in forts_in_range: | ||
details = fort_details(self.bot, fort_id=fort['id'], | ||
latitude=fort['latitude'], | ||
longitude=fort['longitude']) | ||
fort_name = details.get('name', 'Unknown') | ||
encounter_id = fort['lure_info']['encounter_id'] | ||
|
||
if encounter_id: | ||
result = { | ||
'encounter_id': encounter_id, | ||
'fort_id': fort['id'], | ||
'fort_name': u"{}".format(fort_name), | ||
'latitude': fort['latitude'], | ||
'longitude': fort['longitude'] | ||
} | ||
pokemon_to_catch.append(result) | ||
|
||
self.emit_event( | ||
'lured_pokemon_found', | ||
formatted='Lured pokemon at fort {fort_name} ({fort_id})', | ||
data=result | ||
) | ||
return result | ||
|
||
return False | ||
return pokemon_to_catch | ||
|
||
def catch_pokemon(self, pokemon): | ||
worker = PokemonCatchWorker(pokemon, self.bot) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,10 @@ def initialize(self): | |
self.previous_level = 0 | ||
|
||
def work(self): | ||
if not self._time_to_run(): | ||
return False | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should return SUCCESS |
||
self._update_last_ran() | ||
|
||
self.current_level = self._get_current_level() | ||
|
||
# let's check level reward on bot initialization | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,6 +25,8 @@ def work(self): | |
if not self._should_run(): | ||
return | ||
|
||
self._update_last_ran() | ||
|
||
response_dict = self.api.get_inventory() | ||
inventory_items = response_dict.get('responses', {}).get('GET_INVENTORY', {}).get('inventory_delta', {}).get( | ||
'inventory_items', {}) | ||
|
@@ -42,6 +44,9 @@ def work(self): | |
self._execute_pokemon_evolve(pokemon, candy_list, cache) | ||
|
||
def _should_run(self): | ||
if not self._time_to_run(): | ||
return False | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here |
||
|
||
if not self.evolve_all or self.evolve_all[0] == 'none': | ||
return False | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,6 +21,10 @@ def _process_config(self): | |
self.longer_eggs_first = self.config.get("longer_eggs_first", True) | ||
|
||
def work(self): | ||
if not self._time_to_run(): | ||
return False | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. and here |
||
self._update_last_ran() | ||
|
||
try: | ||
self._check_inventory() | ||
except: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
from pokemongo_bot.human_behaviour import sleep | ||
from pokemongo_bot.base_task import BaseTask | ||
|
||
|
||
class NicknamePokemon(BaseTask): | ||
SUPPORTED_TASK_API_VERSION = 1 | ||
|
||
|
@@ -10,6 +11,10 @@ def initialize(self): | |
self.template = "" | ||
|
||
def work(self): | ||
if not self._time_to_run(): | ||
return False | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. here too |
||
self._update_last_ran() | ||
|
||
try: | ||
inventory = reduce(dict.__getitem__, ["responses", "GET_INVENTORY", "inventory_delta", "inventory_items"], self.bot.get_inventory()) | ||
except KeyError: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -380,7 +380,7 @@ def work(self, response_dict=None): | |
data={'pokemon': pokemon_name} | ||
) | ||
break | ||
time.sleep(5) | ||
time.sleep(2) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I also wonder the impact of this. It was originally 5 because that is how long the app takes. Changing it to two would make it faster, but would also not match what is possible. Perhaps making this configurable would also have a large enough impact on the tick loop that we wouldn't need to have internal loops in the tasks that break our model of how |
||
|
||
def count_pokemon_inventory(self): | ||
# don't use cached bot.get_inventory() here | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,10 @@ def _validate_item_filter(self): | |
raise ConfigException("item {} does not exist, spelling mistake? (check for valid item names in data/items.json)".format(config_item_name)) | ||
|
||
def work(self): | ||
if not self._time_to_run(): | ||
return False | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. another one |
||
self._update_last_ran() | ||
|
||
self.bot.latest_inventory = None | ||
item_count_dict = self.bot.item_inventory_count('all') | ||
|
||
|
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.
It seems like this should default to
None
. It will always run every tick unless both the task utilizes_time_to_run
, and the user configures how frequently..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.
@TheSavior the default value is 10 to make it work if the task does run in a time-based way. Otherwise if task is time-based and user don't provide
run_interval
every time he will see an error.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.
Unless the task was written such that it would delay only if it was configured with an interval. Otherwise it would run every tick