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

Schedule mayor cache ticking to the next year rather than every 20 mins #751

Merged

Conversation

Emirlol
Copy link
Collaborator

@Emirlol Emirlol commented Jun 7, 2024

This should reduce the API calls for the mayor endpoint by quite a lot, as a year lasts 5 days and 4 hours and ticking once per year compared to every 20 mins is a big difference.

This PR also fixes the SkyblockTime update scheduling. It was multiplied by 24 instead of 20 as a typo, so it was updating late by 4 minutes each passing day.

One thing I'm not sure about is whether the API updates the current mayor instantly. This is most likely false, but then idk how much extra time should be waited before the API call.

@LifeIsAParadox LifeIsAParadox added the reviews needed This PR needs reviews label Jun 7, 2024
@Emirlol Emirlol changed the title Schedule mayor cache ticking to the next year rather than every 24 mins Schedule mayor cache ticking to the next year rather than every 20 mins Jun 7, 2024
@Fluboxer
Copy link
Contributor

Fluboxer commented Jun 8, 2024

remind me, how do we currently handle jerry perks and won't it deepfry it?

@Emirlol
Copy link
Collaborator Author

Emirlol commented Jun 8, 2024

remind me, how do we currently handle jerry perks and won't it deepfry it?

We don't.

@AzureAaron AzureAaron added the bug Something isn't working label Jun 10, 2024
@kevinthegreat1 kevinthegreat1 added the merge conflicts This PR has merge conflicts that need solving. label Jun 10, 2024
@kevinthegreat1
Copy link
Collaborator

You can do Edit -> Convert Indents -> To Spaces in IntelliJ to convert Utils back into spaces.

@Emirlol
Copy link
Collaborator Author

Emirlol commented Jun 10, 2024

You can do Edit -> Convert Indents -> To Spaces in IntelliJ to convert Utils back into spaces.

Is there a need to, though?

@LifeIsAParadox LifeIsAParadox removed the merge conflicts This PR has merge conflicts that need solving. label Jun 11, 2024
@Emirlol Emirlol force-pushed the more-efficient-mayor-check branch from 9684b31 to 12cfb6c Compare June 19, 2024 08:30
@kevinthegreat1
Copy link
Collaborator

You can do Edit -> Convert Indents -> To Spaces in IntelliJ to convert Utils back into spaces.

Is there a need to, though?

I would appreciate it, this is causing a lot of merge conflicts and is difficult to review.

@kevinthegreat1 kevinthegreat1 added the merge conflicts This PR has merge conflicts that need solving. label Jun 24, 2024
@Emirlol Emirlol force-pushed the more-efficient-mayor-check branch from 12cfb6c to 411a62d Compare July 1, 2024 10:47
@LifeIsAParadox LifeIsAParadox removed the merge conflicts This PR has merge conflicts that need solving. label Jul 1, 2024
@kevinthegreat1
Copy link
Collaborator

Utils should be converted back into spaces. We really can't afford the merge conflicts rn. Edit -> Convert Indents.

@kevinthegreat1 kevinthegreat1 added changes requested This PR need changes bleeding edge This PR has been accepted into bleeding edge labels Jul 5, 2024
@LifeIsAParadox LifeIsAParadox removed the changes requested This PR need changes label Jul 6, 2024
@LifeIsAParadox LifeIsAParadox added merge me please Pull requests that are ready to merge and removed reviews needed This PR needs reviews labels Jul 6, 2024
@kevinthegreat1 kevinthegreat1 merged commit 438ff23 into SkyblockerMod:master Jul 8, 2024
1 check passed
@LifeIsAParadox LifeIsAParadox removed the merge me please Pull requests that are ready to merge label Jul 8, 2024
@Emirlol Emirlol deleted the more-efficient-mayor-check branch July 8, 2024 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bleeding edge This PR has been accepted into bleeding edge bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants