Skip to content
This repository has been archived by the owner on Apr 20, 2024. It is now read-only.

Configuration Improvements Tasklist #124

Closed
5 tasks done
akshualy opened this issue Apr 6, 2023 · 5 comments · Fixed by #146
Closed
5 tasks done

Configuration Improvements Tasklist #124

akshualy opened this issue Apr 6, 2023 · 5 comments · Fixed by #146
Labels
enhancement New feature or request help wanted Community PRs would be welcome

Comments

@akshualy
Copy link
Collaborator

akshualy commented Apr 6, 2023

Description

This is a continuation of #101 for things we want to add to the configuration. This issue aims to let the user configure how the bot should work for them without having to fork it and constantly pull changes.

Ideas so far:

  • Wanted traits
  • Timeouts
  • Log level
  • Economy decisions
  • Version detection

Wanted Traits

A string list of traits the bot should roll for. It will be checked against the traits in the constants and fall back to whichever traits we pick for the season.

Timeouts

The various timeouts we use throughout the project. This is useful to configure since it allows people to configure timeouts according to their hardware. Especially people with worse hardware will benefit from this.

Log level

We currently have the option to set verbose, but I think we can expand this to being able to adjust the entire log level. Useful for people who only care about seeing if something goes wrong (aka. level WARN or ERROR).

Economy decisions

Since the gold detection looks reliable now, we could allow people to toggle the bot into economic mode. This mode would save up to 50 gold and only then start leveling and rolling until we're at 50 again.

Version detection

Right now, the config version is an integer. I'd like it to be the same as the version in VERSION and use it to notify the user of more recent releases (not implemented yet) and auto-update the config (already implemented, but would need adjustment to respect semantic versioning).

Notes

If you have more suggestions, please feel free to comment below, and I'll add them.

If you'd like to contribute, please either make a separate issue or mention in a comment below that you're working on feature X. After the PR is merged, I'll check the necessary box for you.

@akshualy akshualy added enhancement New feature or request help wanted Community PRs would be welcome labels Apr 7, 2023
@akshualy
Copy link
Collaborator Author

akshualy commented Apr 17, 2023

@Kyrluckechuck
I'd like your opinion on the economy decisions part. We can take two paths here: OCR and about every gold value from 40 to maybe 80 to the gold folder. I think this is more efficient with OCR. The downside is that including the only viable solution - pytesseract - alongside its local data (that we can also pack into the .exe) will bloat the final release .exe by probably 100-ish MB.
Another downside is that this would "go against" the bots' core idea to image-match instead of recognizing characters. So the project-compliant version would be to add the gold value images. What do you think?

@Kyrluckechuck
Copy link
Owner

@Gadsee totally fair!

So to be completely honest, the "core idea" is definitely not to use image-match instead of OCR 😛

This bot was forked from the original upstream bot, and so I was pretty lazy about the implementation at first, just trying to "make it work" for me in a few ways extra, and then eventually it spiralled and now we're here 😅

I do really dislike the idea of adding 50-80+ images, so.. if you feel up to the task of getting an initial implementation of pytesseract (or another approach), I'll happily entertain it as long as:

  • It's performance is smooth/quick
  • It's reliable
  • It does not use crazy intensive resources

If we start shipping 100mb+ binaries, that's not the end of the world, but definitely will make us re-evaluate how the build pipeline works (though tbh it might be fine), again, as long as it's reliable and performant.

@akshualy
Copy link
Collaborator Author

akshualy commented Apr 18, 2023

Ah okay, thanks for the input :)

I'll do some performance tests for sure and also don't feel too bad about dropping the task from this issue. The bot is honestly good enough from a logic perspective, I just felt that this could be the cherry on top 😄

@akshualy
Copy link
Collaborator Author

akshualy commented Apr 18, 2023

The snippet that's more or less equivalent to what we're doing now:

tesseract_config = '--oem 3 --psm 7 -c tessedit_char_whitelist=0123456789 -c page_separator=""'


def get_gold() -> int:
    with mss.mss() as screenshot_taker:
        screenshot = screenshot_taker.grab((867, 881, 924, 909))

    pixels = numpy.array(screenshot)
    gray_scaled_pixels = cv2.cvtColor(pixels, cv2.COLOR_BGR2GRAY)
    return int(pytesseract.image_to_string(~gray_scaled_pixels, config=tesseract_config) or 0)

Time measured with timeit:

Runs  | Total               |  Average
------+---------------------+----------------------
100   | 16.8238055000038s   | 0.168238055000038s
250   | 41.003226100001484s | 0.16401290440000593s
500   | 84.59437780000007s  | 0.16918875560000016s
1000  | 164.06434689999878s | 0.16406434689999877s
10000 | 1621.764111700002s  | 0.1621764111700002s

Time of the current version by comparing images (Run against just one image, 1.png)

Runs  | Total               |  Average
------+---------------------+-----------------------
100   | 1.6867553000047337s | 0.016867553000047338s
500   | 8.327335099995253s  | 0.016654670199990507s
1000  | 16.66349619999528s  | 0.01666349619999528

This estimates the current version to be faster for our current logic (at the seven images we have atm, it is ~50ms faster), but I'm okay with that performance impact. At first glance, the OCR is reliable as well. What do you think?

@Kyrluckechuck
Copy link
Owner

Kyrluckechuck commented Apr 19, 2023

Hmm, if that's the sorta impact we're looking at, I feel 50ms is a worthwhile hit so long as it doesn't feel slow! 😛

Based on that, I think an exploration PR could be neat, but let's not 100% tie ourselves to the idea until after it's explored 😅

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request help wanted Community PRs would be welcome
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants