fix: prevent out of bounds crashes in grid

Prefer using GridResource instances instead of an integer value that can change after checking.
refinedmods · Oct 30, 2023 · 3ec0a09 · 3ec0a09
1 parent c4cefbc
commit 3ec0a09
Show file tree

Hide file tree

Showing 5 changed files with 34 additions and 21 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -18,6 +18,10 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 -   You can now recharge the Controller in item form.
 
+### Fixed
+
+-   Fixed a random Grid crash.
+
 ### Removed
 
 -   The `useEnergy` config option for the Wireless Grid. If you do not wish to use energy, use the

diff --git a/...ds/refinedstorage2/platform/common/integration/recipemod/jei/GridGuiContainerHandler.java b/...ds/refinedstorage2/platform/common/integration/recipemod/jei/GridGuiContainerHandler.java
@@ -27,7 +27,7 @@ public Optional<IClickableIngredient<?>> getClickableIngredientUnderMouse(
         final double mouseX,
         final double mouseY
     ) {
-        final GridResource resource = screen.getHoveredGridResource();
+        final GridResource resource = screen.getCurrentGridResource();
         if (resource == null) {
             return Optional.empty();
         }

diff --git a/.../java/com/refinedmods/refinedstorage2/platform/common/screen/grid/AbstractGridScreen.java b/.../java/com/refinedmods/refinedstorage2/platform/common/screen/grid/AbstractGridScreen.java
@@ -67,7 +67,7 @@ public abstract class AbstractGridScreen<T extends AbstractGridContainerMenu> ex
     private ScrollbarWidget scrollbar;
     private int totalRows;
     private int visibleRows;
-    private int gridSlotNumber;
+    private int currentGridSlotIndex;
 
     protected AbstractGridScreen(final T menu,
                                  final Inventory playerInventory,
@@ -207,7 +207,7 @@ protected void renderBg(final GuiGraphics graphics, final float delta, final int
 
         graphics.blit(getTexture(), x, y + TOP_HEIGHT + (18 * visibleRows), 0, 73, imageWidth - 34, bottomHeight);
 
-        gridSlotNumber = -1;
+        currentGridSlotIndex = -1;
 
         graphics.enableScissor(x + 7, y + TOP_HEIGHT, x + 7 + (18 * COLUMNS), y + TOP_HEIGHT + (visibleRows * 18));
         for (int row = 0; row < Math.max(totalRows, visibleRows); ++row) {
@@ -284,7 +284,7 @@ private void renderSlot(final GuiGraphics graphics,
         if (inBounds && isOverStorageArea(mouseX, mouseY)) {
             renderSelection(graphics, slotX, slotY);
             if (resource != null) {
-                gridSlotNumber = idx;
+                currentGridSlotIndex = idx;
             }
         }
     }
@@ -343,8 +343,9 @@ protected void renderTooltip(final GuiGraphics graphics, final int x, final int
     }
 
     private void renderOverStorageAreaTooltip(final GuiGraphics graphics, final int x, final int y) {
-        if (gridSlotNumber != -1) {
-            renderHoveredResourceTooltip(graphics, x, y);
+        final GridResource resource = getCurrentGridResource();
+        if (resource != null) {
+            renderHoveredResourceTooltip(graphics, x, y, resource);
             return;
         }
         final ItemStack carried = getMenu().getCarried();
@@ -355,13 +356,14 @@ private void renderOverStorageAreaTooltip(final GuiGraphics graphics, final int
         Platform.INSTANCE.renderTooltip(graphics, hints, x, y);
     }
 
-    private void renderHoveredResourceTooltip(final GuiGraphics graphics, final int mouseX, final int mouseY) {
-        final GridView view = getMenu().getView();
-        final GridResource resource = view.getViewList().get(gridSlotNumber);
+    private void renderHoveredResourceTooltip(final GuiGraphics graphics,
+                                              final int mouseX,
+                                              final int mouseY,
+                                              final GridResource resource) {
         if (!(resource instanceof PlatformGridResource platformResource)) {
             return;
         }
-        renderHoveredResourceTooltip(graphics, mouseX, mouseY, view, platformResource);
+        renderHoveredResourceTooltip(graphics, mouseX, mouseY, getMenu().getView(), platformResource);
     }
 
     private void renderHoveredResourceTooltip(final GuiGraphics graphics,
@@ -428,11 +430,15 @@ private boolean isModifiedJustNow(final LastModified lastModified) {
 
 
     @Nullable
-    public GridResource getHoveredGridResource() {
-        if (this.gridSlotNumber == -1) {
+    public GridResource getCurrentGridResource() {
+        if (currentGridSlotIndex < 0) {
             return null;
         }
-        return menu.getView().getViewList().get(this.gridSlotNumber);
+        final List<GridResource> viewList = menu.getView().getViewList();
+        if (currentGridSlotIndex >= viewList.size()) {
+            return null;
+        }
+        return viewList.get(currentGridSlotIndex);
     }
 
     @Override
@@ -453,9 +459,10 @@ public boolean mouseClicked(final double mouseX, final double mouseY, final int
         }
 
         final ItemStack carriedStack = getMenu().getCarried();
+        final GridResource resource = getCurrentGridResource();
 
-        if (!getMenu().getView().getViewList().isEmpty() && gridSlotNumber >= 0 && carriedStack.isEmpty()) {
-            mouseClickedInGrid(clickedButton, getMenu().getView().getViewList().get(gridSlotNumber));
+        if (resource != null && carriedStack.isEmpty()) {
+            mouseClickedInGrid(clickedButton, resource);
             return true;
         }
 
@@ -514,8 +521,11 @@ public boolean mouseReleased(final double mx, final double my, final int button)
     public boolean mouseScrolled(final double x, final double y, final double delta) {
         final boolean up = delta > 0;
 
-        if (isOverStorageArea((int) x, (int) y) && gridSlotNumber >= 0) {
-            mouseScrolledInGrid(up);
+        if (isOverStorageArea((int) x, (int) y)) {
+            final GridResource resource = getCurrentGridResource();
+            if (resource != null) {
+                mouseScrolledInGrid(up, resource);
+            }
         } else if (hoveredSlot != null && hoveredSlot.hasItem() && !(hoveredSlot instanceof DisabledSlot)) {
             mouseScrolledInInventory(up, hoveredSlot);
         }
@@ -544,13 +554,12 @@ private void mouseScrolledInInventory(final boolean up, final ItemStack stack, f
         );
     }
 
-    private void mouseScrolledInGrid(final boolean up) {
+    private void mouseScrolledInGrid(final boolean up, final GridResource resource) {
         getMenu().getView().setPreventSorting(true);
         final GridScrollMode scrollMode = getScrollModeWhenScrollingOnGridArea(up);
         if (scrollMode == null) {
             return;
         }
-        final GridResource resource = getMenu().getView().getViewList().get(gridSlotNumber);
         if (!(resource instanceof PlatformGridResource platformGridResource)) {
             return;
         }

diff --git a/...s/refinedstorage2/platform/fabric/integration/recipemod/rei/GridFocusedStackProvider.java b/...s/refinedstorage2/platform/fabric/integration/recipemod/rei/GridFocusedStackProvider.java
@@ -22,7 +22,7 @@ public CompoundEventResult<EntryStack<?>> provide(final Screen screen, final Poi
         if (!(screen instanceof AbstractGridScreen<?> gridScreen)) {
             return CompoundEventResult.pass();
         }
-        final GridResource resource = gridScreen.getHoveredGridResource();
+        final GridResource resource = gridScreen.getCurrentGridResource();
         if (resource == null) {
             return CompoundEventResult.pass();
         }

diff --git a/...ds/refinedstorage2/platform/forge/integration/recipemod/rei/GridFocusedStackProvider.java b/...ds/refinedstorage2/platform/forge/integration/recipemod/rei/GridFocusedStackProvider.java
@@ -22,7 +22,7 @@ public CompoundEventResult<EntryStack<?>> provide(final Screen screen, final Poi
         if (!(screen instanceof AbstractGridScreen<?> gridScreen)) {
             return CompoundEventResult.pass();
         }
-        final GridResource resource = gridScreen.getHoveredGridResource();
+        final GridResource resource = gridScreen.getCurrentGridResource();
         if (resource == null) {
             return CompoundEventResult.pass();
         }