Skip to content
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

Clear finished quests #1030

Open
wants to merge 19 commits into
base: legacy
Choose a base branch
from

Conversation

crhbetz
Copy link
Contributor

@crhbetz crhbetz commented Oct 14, 2020

Review and testing required !!!

Related issue: #869

The recent event bringing a "spin 3 pokestops" quest highlighted the problem of handling finished quests during quest scan. While it's not usually a problem on pure scanner accounts, events and changes can severely impact scan performance by introducing quests a quest scanner unintentionally finishes before it's able to delete them.

The most stable way to handle this issue is usually to set cleanup_every_spin in area settings to True (or using quest enhancer addon). However, this negatively impacts scan performance in dense areas, because the time and resource heavy ocr routines have to run more frequently.

However, this PR attempts to handle finished quests as far as a scanning software is able to. It works as follows:

  1. If during quest deletion scan less than the expected amount of trashcans is found the quest log is unexpectedly full, a check for finished quests will follow.
  2. Using OCR, finished quests are identified by keyword
  3. Finished quests are categorized according to screen position and color tint (stack, retrieveable, blocked have slightly different tints of orange)
  4. Quests are handled accordingly - finished quests will be retrieved; when quests are blocked, it will be attempted to claim the weekly breakthrough and run. This will enable another 7 days of claiming quests
  5. If quests remain blocked, an error will be logged saying the breakthrough has to be caught

In the end, this PR results in - ideally - requiring manual intervention once every 14 days to retrieve weekly breakthroughs, instead of possibly every day to clear up individual finished quests. It'll also explicitly identify the issue in the logs instead of the workers failing without an explicit cause.

Notes:

  • the change in mapadroid/ocr/matching_trash.py was necessary because quests blocked by the breakthrough were identified as having a trashcan - raising the threshold to 0.65 reliably fixed this without causing any false negatives this has been reverted
  • successfully tested on a setup of >30 x96mini devices scanning > 2k quests daily. I achieved close to normal scanning performance despite the ongoing event with cleanup_every_spin set to False
  • ...with default settings regarding screenshot filetype, quality
  • I don't think it is necessary to make this feature optional as I can't see any negative impact - I'm open to other opinions on that, though
  • I consider further testing across different devices necessary

@TechG3n
Copy link
Contributor

TechG3n commented Oct 15, 2020

Im working fine with cleanup_every_spin set to True - But sometimes the quest got solved anyway.
Will your changes work as backup in this case and clean the solved quest?

@JimBim89
Copy link
Contributor

Im working fine with cleanup_every_spin set to True - But sometimes the quest got solved anyway.
Will your changes work as backup in this case and clean the solved quest?

No this wont clean solved quests, but reduce the cases where quests are solved.

@crhbetz
Copy link
Contributor Author

crhbetz commented Oct 15, 2020

It will eventually. To run efficiently, the check for finding finished quests only happens if less trash cans than expected were found - that's less than one in the case of running cleanup every spin.

This means that it'll only clean quests for you if you have three (or less if a trash can can't be found for another reason).
It should then clean them all in one go.

@crhbetz
Copy link
Contributor Author

crhbetz commented Oct 15, 2020

Im working fine with cleanup_every_spin set to True - But sometimes the quest got solved anyway.
Will your changes work as backup in this case and clean the solved quest?

No this wont clean solved quests, but reduce the cases where quests are solved.

This is all about clearing finished (=solved) quests (to the stack).

@JimBim89
Copy link
Contributor

Yeah I thought this was related to PR1029, will also test this ;)

@sn0opy sn0opy changed the title Feature: Clear finished quests Clear finished quests Oct 16, 2020
@sn0opy sn0opy added hacktoberfest-accepted this PR is an acceptable Hacktoberfest entry PR Type: Enhancement This is an enhancement or new feature labels Oct 16, 2020
@crhbetz
Copy link
Contributor Author

crhbetz commented Nov 1, 2020

This is now obviously conflicting with the new AR quest mess, specifically the changes coming with #1051.

I will have to reconsider this whole feature and check if the upcoming PogoDroid will even allow to do this. So put on hold for now.

@Expl0dingBanana Expl0dingBanana added the Meta: Don't merge Don't merge this PR yet label Nov 7, 2020
@crhbetz
Copy link
Contributor Author

crhbetz commented Nov 23, 2020

Reworked to check for finished quests on the initial startup, and when the quest log is unexpectedly full (Failed getting quest but got items).

Working on recent master with updated PD keeping an AR quest in inventory. Testing would now be welcome :-)

@Grennith
Copy link
Collaborator

Grennith commented Jun 8, 2021

Uhm this was left for ages I guess.... What's the state of the PR here?

There would be an exception thrown if the OCR part of quest checking
failed. This commit fixes the problem.
@crhbetz
Copy link
Contributor Author

crhbetz commented Aug 1, 2021

@Grennith been running this myself ever since I created the PR. I guess nobody cares anymore because we didn't get another "spin 3 pokestops" quest since then. It would still be relevant if this quest ever came back.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest-accepted this PR is an acceptable Hacktoberfest entry PR Meta: Don't merge Don't merge this PR yet Type: Enhancement This is an enhancement or new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants