Skip to content

Commit f500e53

Browse files
NichtStudioCodeNichtStudioCode
committed
Remove virtual inventory resizing
Resizing causes several problems: - Previously valid SlotElement.InventoryLink could start pointing to an out of bounds index after resizing, which is unexpected - The current implementation of handling resizing in paged- and scroll guis causes a memory leak, as the virtual inventory holds a strong reference to the gui instance through a registered resize handler Since virtual inventory resizing has little benefit and can be easily replaced by copying contents in a new virtual inventory with a different size, removing this feature seemed like the best solution.
1 parent af13c25 commit f500e53

File tree

6 files changed

+11
-261
lines changed

6 files changed

+11
-261
lines changed

‎invui/src/main/java/xyz/xenondevs/invui/gui/PagedInventoriesGuiImpl.java‎

Lines changed: 3 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,15 @@
22

33
import org.jspecify.annotations.Nullable;
44
import xyz.xenondevs.invui.inventory.Inventory;
5-
import xyz.xenondevs.invui.inventory.VirtualInventory;
65
import xyz.xenondevs.invui.item.ItemProvider;
76
import xyz.xenondevs.invui.state.MutableProperty;
87

9-
import java.util.*;
10-
import java.util.function.BiConsumer;
8+
import java.util.ArrayList;
9+
import java.util.List;
10+
import java.util.SequencedSet;
1111

1212
final class PagedInventoriesGuiImpl<C extends Inventory> extends AbstractPagedGui<C> {
1313

14-
private final BiConsumer<Integer, Integer> resizeHandler = (from, to) -> bake();
15-
private final Set<VirtualInventory> lastKnownInventories = new HashSet<>();
16-
1714
public PagedInventoriesGuiImpl(
1815
int width, int height,
1916
List<? extends C> inventories,
@@ -58,28 +55,10 @@ public void bake() {
5855
pages.add(page);
5956
}
6057

61-
applyResizeHandlers(inventories);
6258
setBakedPages(pages);
6359
setPage(getPage()); // corrects page and refreshes content
6460
}
6561

66-
@SuppressWarnings("DuplicatedCode")
67-
private void applyResizeHandlers(List<? extends Inventory> inventories) {
68-
// remove resize handlers from previous inventories
69-
for (var inv : lastKnownInventories) {
70-
inv.removeResizeHandler(resizeHandler);
71-
}
72-
lastKnownInventories.clear();
73-
74-
// add resize handlers to new inventories
75-
for (var inv : inventories) {
76-
if (inv instanceof VirtualInventory vi) {
77-
vi.addResizeHandler(resizeHandler);
78-
lastKnownInventories.add(vi);
79-
}
80-
}
81-
}
82-
8362
public static final class Builder<C extends Inventory> extends AbstractBuilder<C> {
8463

8564
public Builder() {

‎invui/src/main/java/xyz/xenondevs/invui/gui/ScrollInventoryGuiImpl.java‎

Lines changed: 3 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,15 @@
22

33
import org.jspecify.annotations.Nullable;
44
import xyz.xenondevs.invui.inventory.Inventory;
5-
import xyz.xenondevs.invui.inventory.VirtualInventory;
65
import xyz.xenondevs.invui.item.ItemProvider;
76
import xyz.xenondevs.invui.state.MutableProperty;
87

9-
import java.util.*;
10-
import java.util.function.BiConsumer;
8+
import java.util.ArrayList;
9+
import java.util.List;
10+
import java.util.SequencedSet;
1111

1212
final class ScrollInventoryGuiImpl<C extends Inventory> extends AbstractScrollGui<C> {
1313

14-
private final BiConsumer<Integer, Integer> resizeHandler = (from, to) -> bake();
15-
private final Set<VirtualInventory> lastKnownInventories = new HashSet<>();
16-
1714
public ScrollInventoryGuiImpl(
1815
int width, int height,
1916
List<? extends C> inventories,
@@ -46,28 +43,10 @@ public void bake() {
4643
}
4744
}
4845

49-
applyResizeHandlers(inventories);
5046
setElements(elements);
5147
setLine(getLine()); // corrects line and refreshes content
5248
}
5349

54-
@SuppressWarnings("DuplicatedCode")
55-
private void applyResizeHandlers(List<? extends Inventory> inventories) {
56-
// remove resize handlers from previous inventories
57-
for (var inv : lastKnownInventories) {
58-
inv.removeResizeHandler(resizeHandler);
59-
}
60-
lastKnownInventories.clear();
61-
62-
// add resize handlers to new inventories
63-
for (var inv : inventories) {
64-
if (inv instanceof VirtualInventory vi) {
65-
vi.addResizeHandler(resizeHandler);
66-
lastKnownInventories.add(vi);
67-
}
68-
}
69-
}
70-
7150
public static final class Builder<C extends Inventory> extends AbstractBuilder<C> {
7251

7352
public Builder() {

‎invui/src/main/java/xyz/xenondevs/invui/inventory/Inventory.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,10 @@
5050
* </li>
5151
* </ul>
5252
*/
53-
@SuppressWarnings("SynchronizeOnNonFinalField") // VirtualInventory synchronizes on viewers when changing the field
5453
public sealed abstract class Inventory implements Observable permits VirtualInventory, CompositeInventory, ObscuredInventory, ReferencingInventory {
5554

5655
protected int size;
57-
protected @Nullable Set<ObserverAtSlot>[] observers;
56+
protected final @Nullable Set<ObserverAtSlot>[] observers;
5857
private @Nullable List<Consumer<? super InventoryClickEvent>> clickHandlers;
5958
private @Nullable List<Consumer<? super ItemPreUpdateEvent>> preUpdateHandlers;
6059
private @Nullable List<Consumer<? super ItemPostUpdateEvent>> postUpdateHandlers;

‎invui/src/main/java/xyz/xenondevs/invui/inventory/VirtualInventory.java‎

Lines changed: 4 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,9 @@
88
import xyz.xenondevs.invui.util.ItemUtils;
99

1010
import java.io.*;
11-
import java.util.*;
12-
import java.util.function.BiConsumer;
11+
import java.util.Arrays;
12+
import java.util.BitSet;
13+
import java.util.UUID;
1314
import java.util.logging.Level;
1415
import java.util.zip.GZIPInputStream;
1516
import java.util.zip.GZIPOutputStream;
@@ -19,13 +20,11 @@
1920
*
2021
* @see VirtualInventoryManager
2122
*/
22-
@SuppressWarnings("SynchronizeOnNonFinalField")
2323
public final class VirtualInventory extends Inventory {
2424

2525
private final UUID uuid;
26-
private @Nullable ItemStack[] items;
26+
private final @Nullable ItemStack[] items;
2727
private int[] maxStackSizes;
28-
private @Nullable List<BiConsumer<Integer, Integer>> resizeHandlers;
2928

3029
/**
3130
* Constructs a new {@link VirtualInventory}
@@ -166,7 +165,6 @@ public VirtualInventory(VirtualInventory inventory) {
166165
}
167166
setPreUpdateHandlers(inventory.getPreUpdateHandlers());
168167
setPostUpdateHandlers(inventory.getPostUpdateHandlers());
169-
setResizeHandlers(inventory.getResizeHandlers());
170168
}
171169

172170
/**
@@ -281,81 +279,6 @@ public void serialize(OutputStream out) {
281279
}
282280
}
283281

284-
/**
285-
* Sets the handlers that are called every time this {@link VirtualInventory} is resized.
286-
*
287-
* @param resizeHandlers The handlers to set.
288-
*/
289-
public void setResizeHandlers(@Nullable List<BiConsumer<Integer, Integer>> resizeHandlers) {
290-
this.resizeHandlers = resizeHandlers;
291-
}
292-
293-
/**
294-
* Gets the handlers that are called every time this {@link VirtualInventory} is resized.
295-
*
296-
* @return The handlers.
297-
*/
298-
public @Nullable List<BiConsumer<Integer, Integer>> getResizeHandlers() {
299-
return resizeHandlers;
300-
}
301-
302-
/**
303-
* Adds a handler that is called every time this {@link VirtualInventory} is resized.
304-
*
305-
* @param resizeHandler The handler to add.
306-
*/
307-
public void addResizeHandler(BiConsumer<Integer, Integer> resizeHandler) {
308-
if (resizeHandlers == null)
309-
resizeHandlers = new ArrayList<>();
310-
311-
resizeHandlers.add(resizeHandler);
312-
}
313-
314-
/**
315-
* Removes a handler that is called every time this {@link VirtualInventory} is resized.
316-
*
317-
* @param resizeHandler The handler to remove.
318-
*/
319-
public void removeResizeHandler(BiConsumer<Integer, Integer> resizeHandler) {
320-
if (resizeHandlers != null)
321-
resizeHandlers.remove(resizeHandler);
322-
}
323-
324-
/**
325-
* Changes the size of the {@link VirtualInventory}.
326-
* <p>
327-
* {@link ItemStack ItemStacks} in slots which are no longer valid will be removed from the {@link VirtualInventory}.
328-
* This method does not call an event.
329-
*
330-
* @param size The new size of the {@link VirtualInventory}
331-
*/
332-
public void resize(int size) {
333-
if (this.size == size)
334-
return;
335-
336-
int previousSize = this.size;
337-
338-
this.size = size;
339-
items = Arrays.copyOf(items, size);
340-
maxStackSizes = Arrays.copyOf(maxStackSizes, size);
341-
synchronized (observers) {
342-
observers = Arrays.copyOf(observers, size);
343-
}
344-
345-
// fill stackSizes with the last stack size if the array was extended
346-
if (size > previousSize) {
347-
int stackSize = previousSize != 0 ? maxStackSizes[previousSize - 1] : 64;
348-
Arrays.fill(maxStackSizes, previousSize, maxStackSizes.length, stackSize);
349-
}
350-
351-
// call resize handlers if present
352-
if (resizeHandlers != null) {
353-
for (BiConsumer<Integer, Integer> resizeHandler : resizeHandlers) {
354-
resizeHandler.accept(previousSize, size);
355-
}
356-
}
357-
}
358-
359282
/**
360283
* Sets the array of max stack sizes for this {@link Inventory}.
361284
*

‎invui/src/test/java/xyz/xenondevs/invui/gui/PagedInventoriesGuiTest.java‎

Lines changed: 0 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
import xyz.xenondevs.invui.inventory.VirtualInventory;
1111

1212
import java.util.List;
13-
import java.util.Objects;
1413
import java.util.function.Function;
1514

1615
import static org.junit.jupiter.api.Assertions.*;
@@ -72,68 +71,4 @@ private void testPagedInventoriesInit(Function<? super List<Inventory>, ? extend
7271
}
7372
}
7473

75-
@Test
76-
public void testPageContentUpdateOnVirtualInventoryResize() {
77-
var inv1 = new VirtualInventory(3);
78-
var inv2 = new VirtualInventory(3);
79-
80-
var gui = PagedGui.inventoriesBuilder()
81-
.setStructure(
82-
"x x x",
83-
"x x x",
84-
"x x x")
85-
.addIngredient('x', Markers.CONTENT_LIST_SLOT_HORIZONTAL)
86-
.setContent(List.of(inv1, inv2))
87-
.build();
88-
89-
for (int i = 0; i < 6; i++) {
90-
assertInstanceOf(SlotElement.InventoryLink.class, gui.getSlotElement(i));
91-
}
92-
for (int i = 6; i < 9; i++) {
93-
assertNull(gui.getSlotElement(i));
94-
}
95-
96-
// resize to fill entire gui with inventories
97-
inv1.resize(5);
98-
inv2.resize(4);
99-
100-
for (int i = 0; i < 9; i++) {
101-
assertInstanceOf(SlotElement.InventoryLink.class, gui.getSlotElement(i));
102-
}
103-
104-
// resize to original state
105-
inv1.resize(3);
106-
inv2.resize(3);
107-
108-
for (int i = 0; i < 6; i++) {
109-
assertInstanceOf(SlotElement.InventoryLink.class, gui.getSlotElement(i));
110-
}
111-
for (int i = 6; i < 9; i++) {
112-
assertNull(gui.getSlotElement(i));
113-
}
114-
}
115-
116-
@Test
117-
public void testPagedInventoriesRemoveResizeHandlers() {
118-
var inv1 = new VirtualInventory(3);
119-
var inv2 = new VirtualInventory(3);
120-
121-
var gui = PagedGui.inventoriesBuilder()
122-
.setStructure(
123-
"x x x",
124-
"x x x",
125-
"x x x")
126-
.addIngredient('x', Markers.CONTENT_LIST_SLOT_HORIZONTAL)
127-
.setContent(List.of(inv1))
128-
.build();
129-
130-
assertEquals(1, Objects.requireNonNull(inv1.getResizeHandlers()).size());
131-
assertTrue(inv2.getResizeHandlers() == null || inv2.getResizeHandlers().isEmpty());
132-
133-
gui.setContent(List.of(inv2));
134-
135-
assertTrue(inv1.getResizeHandlers() == null || inv1.getResizeHandlers().isEmpty());
136-
assertEquals(1, Objects.requireNonNull(inv2.getResizeHandlers()).size());
137-
}
138-
13974
}

‎invui/src/test/java/xyz/xenondevs/invui/gui/ScrollInventoriesGuiTest.java‎

Lines changed: 0 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
import xyz.xenondevs.invui.inventory.VirtualInventory;
1111

1212
import java.util.List;
13-
import java.util.Objects;
1413
import java.util.function.Function;
1514

1615
import static org.junit.jupiter.api.Assertions.*;
@@ -73,68 +72,4 @@ private void testScrollInventoriesInit(Function<? super List<Inventory>, ? exten
7372
}
7473
}
7574

76-
@Test
77-
public void testScrollContentUpdateOnVirtualInventoryResize() {
78-
var inv1 = new VirtualInventory(3);
79-
var inv2 = new VirtualInventory(3);
80-
81-
var gui = ScrollGui.inventoriesBuilder()
82-
.setStructure(
83-
"x x x",
84-
"x x x",
85-
"x x x")
86-
.addIngredient('x', Markers.CONTENT_LIST_SLOT_HORIZONTAL)
87-
.setContent(List.of(inv1, inv2))
88-
.build();
89-
90-
for (int i = 0; i < 6; i++) {
91-
assertInstanceOf(SlotElement.InventoryLink.class, gui.getSlotElement(i));
92-
}
93-
for (int i = 6; i < 9; i++) {
94-
assertNull(gui.getSlotElement(i));
95-
}
96-
97-
// resize to fill entire gui with inventories
98-
inv1.resize(5);
99-
inv2.resize(4);
100-
101-
for (int i = 0; i < 9; i++) {
102-
assertInstanceOf(SlotElement.InventoryLink.class, gui.getSlotElement(i));
103-
}
104-
105-
// resize to original state
106-
inv1.resize(3);
107-
inv2.resize(3);
108-
109-
for (int i = 0; i < 6; i++) {
110-
assertInstanceOf(SlotElement.InventoryLink.class, gui.getSlotElement(i));
111-
}
112-
for (int i = 6; i < 9; i++) {
113-
assertNull(gui.getSlotElement(i));
114-
}
115-
}
116-
117-
@Test
118-
public void testScrollInventoriesRemoveResizeHandlers() {
119-
var inv1 = new VirtualInventory(3);
120-
var inv2 = new VirtualInventory(3);
121-
122-
var gui = ScrollGui.inventoriesBuilder()
123-
.setStructure(
124-
"x x x",
125-
"x x x",
126-
"x x x")
127-
.addIngredient('x', Markers.CONTENT_LIST_SLOT_HORIZONTAL)
128-
.setContent(List.of(inv1))
129-
.build();
130-
131-
assertEquals(1, Objects.requireNonNull(inv1.getResizeHandlers()).size());
132-
assertTrue(inv2.getResizeHandlers() == null || inv2.getResizeHandlers().isEmpty());
133-
134-
gui.setContent(List.of(inv2));
135-
136-
assertTrue(inv1.getResizeHandlers() == null || inv1.getResizeHandlers().isEmpty());
137-
assertEquals(1, Objects.requireNonNull(inv2.getResizeHandlers()).size());
138-
}
139-
14075
}

0 commit comments

Comments
 (0)