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

Museum HUD #1064

Open
wants to merge 24 commits into
base: master
Choose a base branch
from
Open

Museum HUD #1064

wants to merge 24 commits into from

Conversation

7azeemm
Copy link
Contributor

@7azeemm 7azeemm commented Nov 24, 2024

2024-11-24_19 38 09
2024-11-24_19 37 36
2024-11-24_19 38 07

Ideas for future:

  • coins to reach next museum milestone
  • on button click open recipe/forgeRecipe/sellerLocation(npc) or open lbin item in auction house
  • support npc price
  • ability to bookmark donation (adds star icon in button? or change bg color to green?)
  • compact donations related to each other in one button maybe like garden hoes and necron blades
  • more complex XpPerCoin algorithm (as example: no one with fully functionnal brain is going to buy all necron's blade, so if he gets one handle the cost of remaining blades is nearly nothing if he gonna convert them)

@LifeIsAParadox LifeIsAParadox added the reviews needed This PR needs reviews label Nov 24, 2024
@AzureAaron AzureAaron added the new feature This issue or PR is a new feature label Nov 25, 2024
@LifeIsAParadox LifeIsAParadox added changes requested This PR need changes and removed reviews needed This PR needs reviews labels Dec 6, 2024
@LifeIsAParadox LifeIsAParadox added reviews needed This PR needs reviews and removed changes requested This PR need changes labels Dec 6, 2024
@7azeemm
Copy link
Contributor Author

7azeemm commented Dec 6, 2024

2024-12-06_17 59 35
Optimized tooltip and 'Coins per XP' sorting and more

Copy link
Collaborator

@AzureAaron AzureAaron left a comment

Choose a reason for hiding this comment

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

You should also rebase this on the latest commit to ensure it works on 1.21.4, though I don't think you will need to change anything regarding the new version.

@LifeIsAParadox LifeIsAParadox added changes requested This PR need changes and removed reviews needed This PR needs reviews labels Dec 12, 2024
@LifeIsAParadox LifeIsAParadox added reviews needed This PR needs reviews and removed changes requested This PR need changes labels Dec 13, 2024
@7azeemm
Copy link
Contributor Author

7azeemm commented Dec 13, 2024

is mouseClicked method changed in 1.21.4??
Inventory clicks are not registering although widget clicks are
i spent 2 hours looking at the code and found that if i change the last return value to false return superClicked => return false all works fine. but this doesn't make sense
image

@Emirlol
Copy link
Collaborator

Emirlol commented Dec 13, 2024

is mouseClicked method changed in 1.21.4??

Screen inherits its mouseClicked method from ParentElement, which now checks the hovered element before calling mouseClicked. Perhaps it has something to do with that?

1.21.4

image

1.21.3

image

@7azeemm
Copy link
Contributor Author

7azeemm commented Dec 13, 2024

Couldn't find any other clean fix than this.
it should be fine i guess

@7azeemm
Copy link
Contributor Author

7azeemm commented Dec 14, 2024

Overlay's dimensions were overlapping with the inventory part, which made the mouse click event being called only for the widget.

@7azeemm
Copy link
Contributor Author

7azeemm commented Dec 14, 2024

Btw, The Hypixel API only registers items that a player has donated, excluding any downgraded variants. For example, if a player donates Fermento Armor, its downgraded versions (e.g., Cropie Armor) will not be registered in the API. So i made few adjustments to it. However, these changes require an update to the local data cache. if museum_item_cache.json file exists the player should run /skyblocker museum resync or delete the json file to work properly without any missing donations.

Copy link
Collaborator

@AzureAaron AzureAaron left a comment

Choose a reason for hiding this comment

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

Just the static block thing otherwise the code seems good

Copy link
Collaborator

@viciscat viciscat left a comment

Choose a reason for hiding this comment

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

naming nitpicks YAY

(previous comments came back, sorry about that)

@LifeIsAParadox LifeIsAParadox added merge conflicts This PR has merge conflicts that need solving. and removed reviews needed This PR needs reviews labels Jan 7, 2025
@LifeIsAParadox LifeIsAParadox added reviews needed This PR needs reviews and removed merge conflicts This PR has merge conflicts that need solving. labels Jan 10, 2025
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 reviews needed This PR needs reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants