-
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
When daily spin limit is reached, let it continue working instead of … #5550
Conversation
@kolinkorr839, thanks for your PR! By analyzing the annotation information on this pull request, we identified @askovpen, @douglascamata and @BriceSD to be potential reviewers |
It's better to add an option to allow user decide if quit when hit daily limit. |
sys.exit(2) | ||
if datetime.now() >= self.next_update: | ||
self.emit_event('spin_limit', formatted='WARNING! You have reached your daily spin limit') | ||
self._compute_next_update() |
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.
Should stop running work here. Maybe add return WorkerResult.ERROR (or SUCCESS depending on your interpretation of this result)
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.
Running is done by MoveToFort or FollowSpiral etc., not by SpinFort. I am also not sure whether returning ERROR here would count towards softban detection... So returning SUCCESS is probably the most nonintrusive way.
Plus - sometimes, you want actually to continue running and apturing pokemons and sniping even with a reached daily limit.
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.
I added a new variable called 'bypass_daily_limit'. This is "false" by default and if the daily_spin_limit is reached, the code will exit. This is the current behavior.
But if the 'bypass_daily_limit' is set to true and the daily_spin_limit is reached, the bot will continue its business but it will give a warning message that the daily spin limit has been reached every 120 seconds (through the min_interval variable which can be changed).
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.
Only one minor issue. Please add something to break out of work method if daily limit reached (eg. return WorkerResult.ERROR/SUCCESS)
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.
New parameter, needs at the very least to be documented in "config.json.sample" and in "configuration_files.md" - also I suggest renaming it to be more descriptive.
sys.exit(2) | ||
if datetime.now() >= self.next_update: | ||
self.emit_event('spin_limit', formatted='WARNING! You have reached your daily spin limit') | ||
self._compute_next_update() |
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.
Running is done by MoveToFort or FollowSpiral etc., not by SpinFort. I am also not sure whether returning ERROR here would count towards softban detection... So returning SUCCESS is probably the most nonintrusive way.
Plus - sometimes, you want actually to continue running and apturing pokemons and sniping even with a reached daily limit.
self.ignore_item_count = self.config.get("ignore_item_count", False) | ||
self.spin_wait_min = self.config.get("spin_wait_min", 2) | ||
self.spin_wait_max = self.config.get("spin_wait_max", 3) | ||
self.min_interval = int(self.config.get('min_interval', 120)) |
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.
I suggest renaming "min_interval" to something more descriptive. Maybe "pause_on_daily_limit" or similar.
Also, it needs documentation.
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.
Throughout the code, the 'min_interval' is used to show some logging/stats updates. This is the reason why I decided to keep this name since its purpose is to show a warning the the spin limit has been reached.
I have added some documentation for this variable as well.
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.
@DBa2016 min_interval is just about the warning message, doesn't pause the bot etc.
Here's 2 tests that I did. If bypass_daily_limit is not changed (default to false), this is the output when daily_spin_limit is reached (it stops the script just like it does right now)
If bypass_daily_limit is set to true, this is the output when daily_spin_limit is reached which allows the bot to continue doing its business. But it gives a warning from time to time that the daily_spin_limit has been reached
|
You'll need to rebase this, it's showing a bunch of commits that aren't related to this PR |
if datetime.now() >= self.next_update: | ||
self.emit_event('spin_limit', formatted='WARNING! You have reached your daily spin limit') | ||
self._compute_next_update() | ||
|
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.
Still needs to stop and return from this method. No point running the rest of the work code if daily_spin_limit is reached.
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.
That is true.. I will make the change.
255a3de
to
980fb5a
Compare
@@ -207,6 +207,9 @@ The behaviors of the bot are configured via the `tasks` key in the `config.json` | |||
* `enable`: Disable or enable this task. | |||
* `spin_wait_min`: Default 3 | Minimum wait time after fort spin | |||
* `spin_wait_max`: Default 5 | Maximum wait time after fort spin | |||
* `daily_spin_limit`: Default 2000 | Daily spin limit | |||
* `min_interval`: Default 120 | When daily spin limit is reached, how often should the warning message be shown | |||
* `bypass_daily_limit`: Default `False` | Disable the spin daily limit protection |
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.
Not sure about "bypass_daily_limit" as a name, since that's not really what it does. If this is true, it lets the bot continue to catch/evolve/transfer etc without spinning forts. The limit is still observed, just the behavior when limit is reached is different.
Maybe: 'exit_on_limit_reached': true/false (default False) and reverse the way the condition works.
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.
Thanks. 'exit_on_limit_reached' sounds better. I will make the change.
a5ab825
to
7c1e1f8
Compare
The only outstanding issue I can see is from @DBa2016 Plus - sometimes, you want actually to continue running and apturing pokemons and sniping even with a reached daily limit. Maybe this could have a "warning only" mode AS WELL, which generates log messages, but still functions anyway. |
OK so dba2016's concern has been addressed. Not sure if we REALLY need that third option then (bypass_daily_limit). Just exit or continue, as it is now. |
…se, then the code will exit. When daily_spin_limit is reached and bypass_daily_limit is set to true, then the bot will continue its business. It will send a message every 2 minutes (default but can be changed) that the daily spin limit has been reached.
703f016
to
cebe496
Compare
OK, looks good. Merging. |
Short Description:
When the daily spin limit is reached, the script exits. This change allow the script to continue but gives a warning that the spin limit is reached every 120 seconds (default which can be changed).
Sample output: