Mekanism Additions

Mekanism Additions

21M Downloads

Opening the Formulaic assembler crashes the game

quinchs opened this issue ยท 9 comments

commented

Summary

When opening the Formulaic assemblers gui it crashes the game.

Steps to reproduce:

  1. Place formulaic assembler
  2. Attempt to right click

Version (make sure you are on the latest version before reporting):

Forge: 36.1.18
Mekanism: v10.1 (build of the current v10.1 branch)

Crash message

The game crashed whilst rendering screen
Error: java.lang.ArrayIndexOutOfBoundsException: 9
Exit Code: -1

Crash log

https://pastebin.com/AeBUeRLj

commented

This is the culprit function that throws the exception

    @Override
    protected ItemStack checkValidity(int slotIndex) {
        int i = slotIndex - 19;
        if (i >= 0 && tile.formula != null && tile.formula.isValidFormula()) {
            ItemStack stack = tile.formula.input.get(i);
            if (!stack.isEmpty()) {
                Slot slot = menu.slots.get(slotIndex);
                //Only render the "correct" item in the gui slot if we don't already have that item there
                if (slot.getItem().isEmpty() || !tile.formula.isIngredientInPos(tile.getLevel(), slot.getItem(), i)) {
                    return stack;
                }
            }
        }
        return ItemStack.EMPTY;
    }

tile.formula.input.get(i); throws out of bounds

commented

Adding && menu.slots.size() > i && tile.formula.input.size() > i to the if statement in the checkValidity function results in this
image

commented

the formula is encoded to be an energy tablet.

commented

Wrapping everything in a try catch, then logging the exceptions result in this

        if (i >= 0 && tile.formula != null && tile.formula.isValidFormula()) {
            try {
                ItemStack stack = tile.formula.input.get(i);
                if (!stack.isEmpty()) {
                    Slot slot = menu.slots.get(slotIndex);
                    // Only render the "correct" item in the gui slot if we don't already have that
                    // item there
                    if (slot.getItem().isEmpty()
                            || !tile.formula.isIngredientInPos(tile.getLevel(), slot.getItem(), i)) {
                        return stack;
                    }
                }
            } catch (Exception x) {
                System.out.println("Caught " + x.toString() + "\nIndex was " + i + ". input size: "
                        + tile.formula.input.size() + ". Slot size: " + menu.slots.size());

            }
        }    
[25Jun2021 00:35:50.796] [Render thread/INFO] [STDOUT/]: [mekanism.client.gui.machine.GuiFormulaicAssemblicator:checkValidity:121]: Caught java.lang.ArrayIndexOutOfBoundsException: 9
Index was 9. input size: 9. Slot size: 73
[25Jun2021 00:35:50.797] [Render thread/INFO] [STDOUT/]: [mekanism.client.gui.machine.GuiFormulaicAssemblicator:checkValidity:121]: Caught java.lang.ArrayIndexOutOfBoundsException: 10
Index was 10. input size: 9. Slot size: 73
[25Jun2021 00:35:50.814] [Render thread/INFO] [STDOUT/]: [mekanism.client.gui.machine.GuiFormulaicAssemblicator:checkValidity:121]: Caught java.lang.ArrayIndexOutOfBoundsException: 9
Index was 9. input size: 9. Slot size: 73
[25Jun2021 00:35:50.814] [Render thread/INFO] [STDOUT/]: [mekanism.client.gui.machine.GuiFormulaicAssemblicator:checkValidity:121]: Caught java.lang.ArrayIndexOutOfBoundsException: 10
Index was 10. input size: 9. Slot size: 73
commented

My theory is that the GuiMekanism is calling the validate function for the crafting grid slots, the formula slot, and the energy slot. The index's' max value is 11 and the grid only has 9 slots.

The GuiMekanism class calls the checkValidity function for every slot thats marked as ContainerSlotType.VALIDITY, I would assume the enery input slot and the crafting formula slot have their slot type set as VALIDITY therfor the checkValidity function is called for them as well

// https://github.com/mekanism/Mekanism/blob/v10.1/src/main/java/mekanism/client/gui/GuiMekanism.java#L422
 if (slotType == ContainerSlotType.VALIDITY) {
    int index = i;
    guiSlot.validity(() -> checkValidity(index));
}

It looks like the code in checkValidity does not account for the energy slot and the crafting slot, it only checks the crafting input slot

commented

I believe the issue is just int i = slotIndex - 19; is wrong, but I need to look at it further when I get the chance. At one point if memory serves we moved the index of the energy slot so that shift clicking doesn't prioritize it with things like redstone, and my guess is we just forgot to update that line to be like slotIndex - 20 or something

commented

Yeah ive figured out the issues, after logging the i (slotIndex - 19) to the debug file i saw that it was going from 2 - 11, after adding i < 9 to the if statement and setting the stack to ItemStack stack = tile.formula.input.get(i - 2); it seemed to fix it.

image

image

commented

I mean the correct thing is to change how i is calculated to be subtracted by two further so that isIngredientInPos also gets called with the proper value as well

commented

Yep, I've changed i to be int i = slotIndex - 21; and it seems to be fixed.