Skip to content

Use synchronizedSet#1072

Merged
mastercoms merged 1 commit intoGlowstoneMC:devfrom
PayDevD:synchronizedSet
Dec 23, 2019
Merged

Use synchronizedSet#1072
mastercoms merged 1 commit intoGlowstoneMC:devfrom
PayDevD:synchronizedSet

Conversation

@PayDevD
Copy link
Contributor

@PayDevD PayDevD commented Dec 19, 2019

For issue #1071

GlowDoubleChest extends GlowSuperInventory and its field, parents' GlowInventory viewers are unmodifiable.

protected void initialize(List<GlowInventory> parents, InventoryHolder owner,
        InventoryType type, String title) {
        SuperList<GlowInventorySlot> slots = new SuperList<>();
        SuperSet<HumanEntity> viewers = new SuperSet<>();

        for (GlowInventory parent : parents) {
            slots.getParents().add(parent.getSlots());
            viewers.getParents().add(parent.getViewersSet());
        }

        initialize(slots, viewers, owner, type, title);
        this.parents = parents;
    }

However, viewers should be modifiable because openInventory logic modifies its viewers.
UnmodifiableSet could occur UnsupportedOperationException.
We can use synchronizedSet instead of unmodifiableSet.

@claassistantio
Copy link

claassistantio commented Dec 19, 2019

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@mastercoms mastercoms left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution to Glowstone! LGTM!

@PayDevD
Copy link
Contributor Author

PayDevD commented Dec 23, 2019

@mastercoms
I would appreciate if you could merge this branch.
Thanks for your review!

@mastercoms mastercoms merged commit cf11388 into GlowstoneMC:dev Dec 23, 2019
@PayDevD PayDevD deleted the synchronizedSet branch December 26, 2019 04:07
gdude2002 pushed a commit that referenced this pull request Jan 23, 2020
mastercoms pushed a commit that referenced this pull request Jan 24, 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.

3 participants