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

Fixes some errors with HeadGetter #1569

Merged
merged 2 commits into from
Nov 8, 2020

Conversation

BONNe
Copy link
Member

@BONNe BONNe commented Nov 6, 2020

There were multiple issues in HearGetter.

  1. If someone 2 GUI's requested player heads at the same time, then one of them could easily throw ConcurrentModificationException, as it used Map structure, where it is not necessary.
  2. If the creator wanted 2 different icons with the same player head, it was not possible, as implementation allowed only one player head with the same name, due to using a hashmap.
  3. There were unnecessary code that overwrites all PanelItems with the same name if they were requested by Head Getter. This also prevented to use the same name for PanelItem but with different player heads.

Also, I added Pair#getKey and Pair#getValue as I like to use getters more than variables directly.

BONNe added 2 commits November 6, 2020 22:35
Fixes an issue when elements with the same name were overwritten by HeadGetter.
@BONNe
Copy link
Member Author

BONNe commented Nov 8, 2020

Level addon definitely will work. You are creating different PanelItems for each button.

The only way how it can fail to update GUI is if someone creates a single instance of PanelItem, and uses it multiple times in the same GUI.

I am not sure how useful to support that feature, but it is not hard. Just need to find all usages of that PanelItem instance in the Items entrySet, and update each item in Inventory.

But I do not think this is a useful feature to support, as I cannot find any good use case for it.

@tastybento tastybento merged commit 3581537 into BentoBoxWorld:develop Nov 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants