-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Task EvolvePokemon should give us a status updates so we know it is working #5570
Changes from 1 commit
f76b84a
f45267f
2fa4fda
c8036a4
2b10ee4
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 |
---|---|---|
|
@@ -17,6 +17,8 @@ def __init__(self, bot, config): | |
|
||
def initialize(self): | ||
self.start_time = 0 | ||
self.next_update = None | ||
self.interval = self.config.get('interval', 60) | ||
self.evolve_list = self.config.get('evolve_list', []) | ||
self.donot_evolve_list = self.config.get('donot_evolve_list', []) | ||
self.min_evolve_speed = self.config.get('min_evolve_speed', 25) | ||
|
@@ -41,7 +43,7 @@ def initialize(self): | |
def _validate_config(self): | ||
if isinstance(self.evolve_list, basestring): | ||
self.evolve_list = [str(pokemon_name).lower().strip() for pokemon_name in self.evolve_list.split(',')] | ||
|
||
if isinstance(self.donot_evolve_list, basestring): | ||
self.donot_evolve_list = [str(pokemon_name).lower().strip() for pokemon_name in self.donot_evolve_list.split(',')] | ||
|
||
|
@@ -55,6 +57,9 @@ def work(self): | |
if not self._should_run(): | ||
return | ||
|
||
self._compute_next_update() | ||
|
||
result_message = "" | ||
filtered_list, filtered_dict = self._sort_and_filter() | ||
|
||
pokemon_to_be_evolved = 0 | ||
|
@@ -65,19 +70,38 @@ def work(self): | |
candy = inventory.candies().get(pokemon.pokemon_id) | ||
pokemon_to_be_evolved = pokemon_to_be_evolved + min(candy.quantity / (pokemon.evolution_cost - 1), filtered_dict[pokemon.pokemon_id]) | ||
|
||
if pokemon_to_be_evolved >= self.min_pokemon_to_be_evolved: | ||
self._print_check(pokemon_to_be_evolved) | ||
|
||
has_minimum_to_evolve = pokemon_to_be_evolved >= self.min_pokemon_to_be_evolved | ||
if has_minimum_to_evolve: | ||
if self.use_lucky_egg: | ||
self._use_lucky_egg() | ||
cache = {} | ||
for pokemon in filtered_list: | ||
if pokemon.can_evolve_now(): | ||
self._execute_pokemon_evolve(pokemon, cache) | ||
|
||
def _print_check(self, pokemon_to_be_evolved): | ||
has_minimum_to_evolve = pokemon_to_be_evolved >= self.min_pokemon_to_be_evolved | ||
result_message = ("Gotta catch`em all!", "Gotta evolv`em all!")[has_minimum_to_evolve] | ||
self.emit_event( | ||
'pokemon_evolve_check', | ||
formatted='Checking... Has {has}, needs {needs}. {message}', | ||
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 think it can be even more simplistic: |
||
data={ | ||
'has': pokemon_to_be_evolved, | ||
'needs': self.min_pokemon_to_be_evolved, | ||
'message': result_message | ||
} | ||
) | ||
|
||
def _should_run(self): | ||
if not self.evolve_list or self.evolve_list[0] == 'none': | ||
return False | ||
return True | ||
|
||
return self.next_update is None or datetime.now() >= self.next_update | ||
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 the duration that user set now affects how frequent evolve pokemon task is ran. I think only the logging should be reduced, but not the task itself. For example, if I set While evolving more pokemon than the quota is not a big deal for me, I'm not sure if that applies to everyone else. Also if user accidentally sets 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'm sorry, but I added documentation for this and the way I see this, people should read it so as not to put a huge number there. Also, how many extra pokemon can they catch in 120 seconds? Having said that, I can still tweak this to a 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. Hmmm yes I see what you mean and I agree with you, but I'd rather not change the behavior of the task since the purpose of this PR is for giving a status update, and not to reduce frequency of evolve task. Assuming that not all users read the documentation, we are essentially giving users the power/ability to screw up the evolve task. IMO, if the frequency of evolve task should be reduced, it should be in another PR. What do you think? 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. @alexyaoyang Hey man, chill. It's his first contribution :) :) :) Just kidding, I agree with you. Keeping this PR to just the log message, not the task itself, would be better. 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. @Gobberwart Not sure why but I found that very funny. Now people looking at me kinda weird in Starbucks. Thanks! |
||
|
||
def _compute_next_update(self): | ||
self.next_update = datetime.now() + timedelta(seconds=self.interval) | ||
|
||
def _use_lucky_egg(self): | ||
using_lucky_egg = time.time() - self.start_time < 1800 | ||
if using_lucky_egg: | ||
|
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.
Small issue, but I'm not sure if users will be confused by
interval
andmin_evolve_speed
. Perhaps a better name islog_interval
?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.
this number represents how often - in seconds - the task runs. I just realized I should also add that to the docs.
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.
@alexyaoyang I've added the documentation line. I have been trying this at home for 2 days now and I feel much more comfortable with 120 as the default value because we don't want this message to be spammy 😄