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

Inventory Holder #195

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Inventory Holder #195

wants to merge 6 commits into from

Conversation

TrueDarkLord
Copy link
Member

Implement inventory holder:

  • More secure than name checks.
  • Faster than name chacks.

Use the title stored within the holder to determine the menu type.

  • More performant.

Move the event cancellation to the top of the listener that the event is still cancelled, even if the GUI type checks fail.

Use the title that is stored inside of the inventory holder instead of having to constantly convert between component and text.
This should be a bit more performant.
Switch to partial match as the title contains page numbers.
Cancel the click even at the start reducing the chance that errors allow players to get free items.
@ryderbelserion
Copy link
Member

ryderbelserion commented Jan 19, 2025

Don't use the legacy ampersand methods or go from string->component etc. You can just use the deprecated method to create inventories with a string option.

It serves no purpose to convert from string->component. We still only support legacy color codes (and it'll be that way for a while), and the deprecated methods aren't going anywhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants