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

Add chocolate factory helper #683

Merged
merged 52 commits into from
May 24, 2024

Conversation

Emirlol
Copy link
Collaborator

@Emirlol Emirlol commented May 4, 2024

Highlights the best pick based on the price and CPS gain.

Currently, it tries to recalculate every second (when the screen updates due to chocolate gain). I tried to make it update only on click inside of the mixin but doesn't seem to work and I didn't bother to debug as I couldn't really understand the solver implementation. The performance loss is very negligible, and you wouldn't really care about performance in the inventory, but it still bugs me. So lmk if you have a solution.

Also sorry about the optionals, I recently started learning rust and they feel more natural than returning -1. As for the sorted map, I first tried to add the CPS increase factor to the item tooltip but that was a whole another thing I had to read and figure out, so I just left it as is.

@LifeIsAParadox LifeIsAParadox added the wip This PR is a work in progress label May 4, 2024
@AzureAaron
Copy link
Collaborator

Make sure to rebase this, it won't work on 1.20.6 sadly

@Emirlol
Copy link
Collaborator Author

Emirlol commented May 6, 2024

Decided to finalize this draft and make the egg helper separately later on, as it's just a game of waiting until next spring and then finding all egg locations.

@Emirlol Emirlol marked this pull request as ready for review May 6, 2024 13:55
@LifeIsAParadox LifeIsAParadox added reviews needed This PR needs reviews and removed wip This PR is a work in progress labels May 6, 2024
@viciscat
Copy link
Collaborator

viciscat commented May 7, 2024

Looks like it works pretty well, you just need to do your todos!

@Emirlol
Copy link
Collaborator Author

Emirlol commented May 8, 2024

Looks like it works pretty well, you just need to do your todos!

I forgot about them, ehe
Will do

@AzureAaron AzureAaron added the new feature This issue or PR is a new feature label May 10, 2024
@LifeIsAParadox LifeIsAParadox added changes requested This PR need changes reviews needed This PR needs reviews and removed reviews needed This PR needs reviews changes requested This PR need changes labels May 10, 2024
@kevinthegreat1 kevinthegreat1 added the bleeding edge This PR has been accepted into bleeding edge label May 11, 2024
Copy link
Contributor

@Julienraptor01 Julienraptor01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed the logic and enhanced few things

also nitpicking, but the proper way to say "getConcattedLore" is "getConcatenatedLore"
(yes i know engrish eeeh)
Here's a patch version to make all changes at once that you can apply by just doing

git apply 0001-Fix-few-things.patch

0001-Fix-few-things.patch

@LifeIsAParadox LifeIsAParadox added changes requested This PR need changes and removed reviews needed This PR needs reviews labels May 14, 2024
@LifeIsAParadox LifeIsAParadox added reviews needed This PR needs reviews and removed changes requested This PR need changes labels May 14, 2024
@LifeIsAParadox LifeIsAParadox added changes requested This PR need changes and removed reviews needed This PR needs reviews labels May 15, 2024
kevinthegreat1
kevinthegreat1 previously approved these changes May 23, 2024
Copy link
Collaborator

@kevinthegreat1 kevinthegreat1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm

import java.util.concurrent.atomic.AtomicReference;

public class SkyblockTime {
private static final long SKYBLOCK_POCH = 1560275700000L;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't think "poch" is a word, should be "epoch".

@LifeIsAParadox LifeIsAParadox added merge me please Pull requests that are ready to merge and removed reviews needed This PR needs reviews labels May 23, 2024
@LifeIsAParadox LifeIsAParadox added reviews needed This PR needs reviews and removed merge me please Pull requests that are ready to merge labels May 24, 2024
@LifeIsAParadox LifeIsAParadox added merge me please Pull requests that are ready to merge and removed reviews needed This PR needs reviews labels May 24, 2024
@kevinthegreat1 kevinthegreat1 merged commit b6da8de into SkyblockerMod:master May 24, 2024
1 check passed
@LifeIsAParadox LifeIsAParadox removed the merge me please Pull requests that are ready to merge label May 24, 2024
@Emirlol Emirlol deleted the chocolate-factory-helper branch May 24, 2024 04:12
@kevinthegreat1 kevinthegreat1 removed the bleeding edge This PR has been accepted into bleeding edge label May 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature This issue or PR is a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants