nano-shino / genshinhelper

A Discord bot to help with daily Genshin activities
MIT License
33 stars 11 forks source link

Expedition Reminder #3

Closed EffortlessFury closed 2 years ago

EffortlessFury commented 2 years ago

Any interest in incorporating a reminder for expeditions as well? I was thinking about doing it myself, am happy to submit a PR if you'd like it in the project. Also, as an aside (and especially if you would like this feature), ResinCapReminder is also handling teapot reminders. Perhaps it should be renamed and/or the various reminder functionalities broken out?

nano-shino commented 2 years ago

I can rename it to something else. The reason they are together is because I want to limit the number of API calls to Hoyolab server, since resin and teapot are both based on the notes API. Calling too much may trigger some sort of alarming.

I'm open to receiving PRs.

nano-shino commented 2 years ago

Actually I'm gonna implement this because I think that module is due for a cleanup.

EffortlessFury commented 2 years ago

Limiting calls is definitely the right call, absolutely makes sense to have a central module use one call for everything that might need it. If you're sure you want to implement it, thanks! I may have one or two other things I want to improve myself, I'll file them as issues before I get started on them in case you wanted to have any input and have me push it upstream.

nano-shino commented 2 years ago

All done. I've also added caching to make sure we don't call it more than once per minute. And be aware that expedition notification is not enabled by default so you have to update it in /user settings.