Be more resilient against broken patterns (removed items, mods, etc.) to avoid crashing when loading worlds

build.gradle

@@ -84,9 +84,10 @@ dependencies {
     implementation 'com.google.code.findbugs:jsr305:3.0.2'
 
     // unit test dependencies
-    testImplementation("org.junit.jupiter:junit-jupiter-api:5.7.2")
-    testImplementation("org.junit.jupiter:junit-jupiter-params:5.7.2")
-    testRuntimeOnly("org.junit.jupiter:junit-jupiter-engine:5.7.2")
+    testImplementation("org.junit.jupiter:junit-jupiter-api:5.8.1")
+    testImplementation("org.junit.jupiter:junit-jupiter-params:5.8.1")
+    testRuntimeOnly("org.junit.jupiter:junit-jupiter-engine:5.8.1")
+    testImplementation("org.junit.platform:junit-platform-launcher:1.8.1")
     testImplementation("org.assertj:assertj-core:3.19.0")
     testImplementation("com.google.guava:guava-testlib:21.0")
     testImplementation("org.mockito:mockito-junit-jupiter:3.9.0")

src/main/java/appeng/crafting/pattern/AECraftingPattern.java

@@ -76,11 +76,11 @@ public AECraftingPattern(CompoundTag definition, Level level) {
 
         // Find recipe
         var recipeId = AEPatternHelper.getRecipeId(definition);
-        var recipe = level.getRecipeManager().byKey(recipeId).orElse(null);
-        if (recipe == null || recipe.getType() != RecipeType.CRAFTING) {
-            throw new IllegalStateException("recipe id is not a crafting recipe");
+        var recipe = level.getRecipeManager().byType(RecipeType.CRAFTING).get(recipeId);
+        if (!(recipe instanceof CraftingRecipe craftingRecipe)) {
+            throw new IllegalStateException("recipe id '" + recipeId + "' is not a valid crafting recipe");
         }
-        this.recipe = (CraftingRecipe) recipe;
+        this.recipe = craftingRecipe;
 
         // Build frame and find output
         this.testFrame = new CraftingContainer(new NullMenu(), 3, 3);
@@ -89,7 +89,13 @@ public AECraftingPattern(CompoundTag definition, Level level) {
                 testFrame.setItem(i, sparseInputs[i].createItemStack());
             }
         }
+        if (!this.recipe.matches(testFrame, level)) {
+            throw new IllegalStateException("The recipe " + recipe + " no longer matches the encoded input.");
+        }
         this.outputs = new IAEItemStack[] { AEItemStack.fromItemStack(this.recipe.assemble(testFrame)) };
+        if (this.outputs[0] == null) {
+            throw new IllegalStateException("The recipe " + recipeId + " produced an empty item stack result.");
+        }
 
         // Compress inputs
         var condensedInputs = AEPatternHelper.condenseStacks(sparseInputs);

src/main/java/appeng/crafting/pattern/AEPatternHelper.java

@@ -20,7 +20,6 @@
 
 import java.util.Arrays;
 import java.util.Comparator;
-import java.util.List;
 import java.util.Objects;
 import java.util.function.Function;
 import java.util.stream.Collectors;
@@ -97,7 +96,7 @@ public static IAEItemStack[] getCraftingInputs(CompoundTag nbt) {
     }
 
     public static <T extends IAEStack> T[] condenseStacks(T[] collection) {
-        final List<T> merged = Arrays.stream(collection).filter(Objects::nonNull)
+        var merged = Arrays.stream(collection).filter(Objects::nonNull)
                 .collect(Collectors.toMap(Function.identity(), IAEStack::copy,
                         (left, right) -> {
                             left.setStackSize(left.getStackSize() + right.getStackSize());

src/main/java/appeng/crafting/pattern/CraftingPatternItem.java

@@ -47,7 +47,7 @@ public AECraftingPattern decode(CompoundTag tag, Level level, boolean tryRecover
 
         try {
             return new AECraftingPattern(tag.copy(), level);
-        } catch (IllegalStateException e) {
+        } catch (Exception e) {
             AELog.warn("Could not decode an invalid crafting pattern %s: %s", tag, e);
             return null;
         }

src/main/java/appeng/crafting/pattern/ProcessingPatternItem.java

@@ -37,8 +37,8 @@ public AEProcessingPattern decode(ItemStack stack, Level level, boolean tryRecov
     public AEProcessingPattern decode(CompoundTag tag, Level level, boolean tryRecovery) {
         try {
             return new AEProcessingPattern(tag.copy());
-        } catch (IllegalStateException e) {
-            AELog.warn("Could not decode an invalid crafting pattern %s: %s", tag, e);
+        } catch (Exception e) {
+            AELog.warn("Could not decode an invalid processing pattern %s: %s", tag, e);
             return null;
         }
     }

src/test/java/appeng/crafting/pattern/CraftingPatternItemTest.java

@@ -0,0 +1,152 @@
+package appeng.crafting.pattern;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.util.HashMap;
+import java.util.Objects;
+import java.util.concurrent.atomic.AtomicReference;
+
+import org.junit.jupiter.api.Test;
+
+import net.minecraft.data.recipes.RecipeProvider;
+import net.minecraft.data.recipes.ShapedRecipeBuilder;
+import net.minecraft.nbt.CompoundTag;
+import net.minecraft.nbt.Tag;
+import net.minecraft.resources.ResourceLocation;
+import net.minecraft.world.inventory.CraftingContainer;
+import net.minecraft.world.item.ItemStack;
+import net.minecraft.world.item.Items;
+import net.minecraft.world.item.crafting.CraftingRecipe;
+import net.minecraft.world.item.crafting.Recipe;
+import net.minecraft.world.item.crafting.RecipeManager;
+import net.minecraft.world.item.crafting.RecipeSerializer;
+import net.minecraft.world.item.crafting.RecipeType;
+import net.minecraft.world.item.crafting.ShapedRecipe;
+import net.minecraft.world.level.Level;
+
+import appeng.api.crafting.PatternDetailsHelper;
+import appeng.core.AppEng;
+import appeng.core.definitions.AEItems;
+import appeng.util.BootstrapMinecraft;
+
+@BootstrapMinecraft
+class CraftingPatternItemTest {
+
+    private static final ResourceLocation TEST_RECIPE_ID = AppEng.makeId("test_recipe");
+
+    private final ShapedRecipe TEST_RECIPE = buildRecipe(ShapedRecipeBuilder.shaped(Items.STICK)
+            .pattern("xy")
+            .define('x', Items.TORCH)
+            .define('y', Items.DIAMOND));
+
+    private ShapedRecipe buildRecipe(ShapedRecipeBuilder builder) {
+        var result = new AtomicReference<ShapedRecipe>();
+        builder.unlockedBy("xxx", RecipeProvider.has(builder.getResult()));
+        builder.save(finishedRecipe -> {
+            var recipe = RecipeSerializer.SHAPED_RECIPE.fromJson(TEST_RECIPE_ID, finishedRecipe.serializeRecipe());
+            result.set(recipe);
+        }, TEST_RECIPE_ID);
+        return Objects.requireNonNull(result.get());
+    }
+
+    @Test
+    void testDecodeWithEmptyTag() {
+        assertNull(decode(new CompoundTag()));
+    }
+
+    /**
+     * Sanity check that an encoded pattern that contains item-ids that are now invalid (i.e. mod removed, item removed
+     * from mod, etc.) do not crash when being decoded.
+     */
+    @Test
+    void testDecodeWithRemovedIngredientItemIds() {
+        var encoded = createTestPattern();
+        var encodedTag = encoded.getTag();
+
+        var inputTag = encodedTag.getList("in", Tag.TAG_COMPOUND).getCompound(0);
+        assertEquals("minecraft:torch", inputTag.getString("id"));
+        inputTag.putString("id", "minecraft:unknown_item_id");
+
+        var decoded = decode(encodedTag);
+        assertNull(decoded);
+    }
+
+    private ItemStack createTestPattern() {
+        return PatternDetailsHelper.encodeCraftingPattern(
+                TEST_RECIPE,
+                new ItemStack[] {
+                        new ItemStack(Items.TORCH),
+                        new ItemStack(Items.DIAMOND),
+                        ItemStack.EMPTY,
+                        ItemStack.EMPTY,
+                        ItemStack.EMPTY,
+                        ItemStack.EMPTY,
+                        ItemStack.EMPTY,
+                        ItemStack.EMPTY,
+                        ItemStack.EMPTY,
+                },
+                new ItemStack(Items.STICK),
+                true,
+                true);
+    }
+
+    private AECraftingPattern decode(CompoundTag tag) {
+        var level = mock(Level.class);
+        var recipeManager = mock(RecipeManager.class);
+        when(level.getRecipeManager()).thenReturn(recipeManager);
+        var recipeMap = new HashMap<ResourceLocation, Recipe<CraftingContainer>>();
+        recipeMap.put(TEST_RECIPE_ID, TEST_RECIPE);
+        when(recipeManager.byType(RecipeType.CRAFTING)).thenReturn(recipeMap);
+
+        return AEItems.CRAFTING_PATTERN.asItem().decode(
+                tag, level, false);
+    }
+
+    private static class TestRecipe implements CraftingRecipe {
+        public boolean acceptAssemble = true;
+
+        @Override
+        public RecipeType<?> getType() {
+            return RecipeType.CRAFTING;
+        }
+
+        @Override
+        public boolean matches(CraftingContainer container, Level level) {
+            for (int i = 2; i < container.getContainerSize(); i++) {
+                if (!container.getItem(i).isEmpty()) {
+                    return false;
+                }
+            }
+            return container.getItem(0).getItem() == Items.TORCH
+                    && container.getItem(1).getItem() == Items.DIAMOND;
+        }
+
+        @Override
+        public ItemStack assemble(CraftingContainer container) {
+            return getResultItem();
+        }
+
+        @Override
+        public boolean canCraftInDimensions(int width, int height) {
+            return width >= 3 && height >= 3;
+        }
+
+        @Override
+        public ItemStack getResultItem() {
+            return new ItemStack(Items.STICK);
+        }
+
+        @Override
+        public ResourceLocation getId() {
+            return TEST_RECIPE_ID;
+        }
+
+        @Override
+        public RecipeSerializer<?> getSerializer() {
+            throw new UnsupportedOperationException();
+        }
+    }
+}

src/test/java/appeng/crafting/pattern/ProcessingPatternItemTest.java

@@ -0,0 +1,102 @@
+package appeng.crafting.pattern;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
+
+import org.junit.jupiter.api.Test;
+
+import net.minecraft.nbt.CompoundTag;
+import net.minecraft.nbt.Tag;
+import net.minecraft.world.item.ItemStack;
+import net.minecraft.world.item.Items;
+
+import appeng.api.crafting.PatternDetailsHelper;
+import appeng.api.storage.data.IAEItemStack;
+import appeng.api.storage.data.IAEStack;
+import appeng.core.definitions.AEItems;
+import appeng.util.BootstrapMinecraft;
+
+@BootstrapMinecraft
+class ProcessingPatternItemTest {
+    @Test
+    void testDecodeWithEmptyTag() {
+        assertNull(decode(new CompoundTag()));
+    }
+
+    /**
+     * Sanity check that an encoded pattern that contains item-ids that are now invalid (i.e. mod removed, item removed
+     * from mod, etc.) do not crash when being decoded.
+     */
+    @Test
+    void testDecodeWithRemovedIngredientItemIds() {
+        var encoded = PatternDetailsHelper.encodeProcessingPattern(
+                new IAEStack[] {
+                        IAEItemStack.of(new ItemStack(Items.TORCH)),
+                        IAEItemStack.of(new ItemStack(Items.DIAMOND))
+                },
+                new IAEStack[] {
+                        IAEItemStack.of(new ItemStack(Items.STICK))
+                });
+        var encodedTag = encoded.getTag();
+
+        var inputTag = encodedTag.getList("in", Tag.TAG_COMPOUND).getCompound(0).getCompound("is");
+        assertEquals("minecraft:torch", inputTag.getString("id"));
+        inputTag.putString("id", "minecraft:unknown_item_id");
+
+        var reDecoded = decode(encodedTag);
+        assertNotNull(reDecoded);
+        // The missing input should be gone
+        assertEquals(1, reDecoded.getInputs().length);
+    }
+
+    /**
+     * Sanity check that an encoded pattern that contains item-ids that are now invalid (i.e. mod removed, item removed
+     * from mod, etc.) do not crash when being decoded.
+     */
+    @Test
+    void testDecodeWithRemovedResultItemIds() {
+        var encoded = PatternDetailsHelper.encodeProcessingPattern(
+                new IAEStack[] {
+                        IAEItemStack.of(new ItemStack(Items.TORCH)),
+                        IAEItemStack.of(new ItemStack(Items.DIAMOND))
+                },
+                new IAEStack[] {
+                        IAEItemStack.of(new ItemStack(Items.STICK))
+                });
+        var encodedTag = encoded.getTag();
+
+        var inputTag = encodedTag.getList("out", Tag.TAG_COMPOUND).getCompound(0).getCompound("is");
+        assertEquals("minecraft:stick", inputTag.getString("id"));
+        inputTag.putString("id", "minecraft:unknown_item_id");
+
+        assertNull(decode(encodedTag));
+    }
+
+    /**
+     * Sanity check that an encoded pattern that contains stacks that reference missing storage channels.
+     */
+    @Test
+    void testDecodeWithRemovedStorageChannels() {
+        var encoded = PatternDetailsHelper.encodeProcessingPattern(
+                new IAEStack[] {
+                        IAEItemStack.of(new ItemStack(Items.TORCH)),
+                        IAEItemStack.of(new ItemStack(Items.DIAMOND))
+                },
+                new IAEStack[] {
+                        IAEItemStack.of(new ItemStack(Items.STICK))
+                });
+        var encodedTag = encoded.getTag();
+
+        var inputTag = encodedTag.getList("in", Tag.TAG_COMPOUND).getCompound(0);
+        assertEquals("appliedenergistics2:item", inputTag.getString("chan"));
+        inputTag.putString("chan", "some_mod:missing_chan");
+
+        assertNull(decode(encodedTag));
+    }
+
+    private AEProcessingPattern decode(CompoundTag tag) {
+        return AEItems.PROCESSING_PATTERN.asItem().decode(
+                tag, null, false);
+    }
+}

src/test/java/appeng/util/BootstrapMinecraftExtension.java

@@ -1,5 +1,12 @@
 package appeng.util;
 
+import java.nio.file.Files;
+import java.nio.file.Path;
+
+import com.google.common.io.MoreFiles;
+import com.google.common.io.RecursiveDeleteOption;
+
+import org.junit.jupiter.api.extension.AfterAllCallback;
 import org.junit.jupiter.api.extension.BeforeAllCallback;
 import org.junit.jupiter.api.extension.Extension;
 import org.junit.jupiter.api.extension.ExtensionContext;
@@ -8,15 +15,31 @@
 import net.minecraft.SharedConstants;
 import net.minecraft.server.Bootstrap;
 
+import appeng.core.AEConfig;
 import appeng.core.AppEngBootstrap;
 
-public class BootstrapMinecraftExtension implements Extension, BeforeAllCallback {
+public class BootstrapMinecraftExtension implements Extension, BeforeAllCallback, AfterAllCallback {
+
+    Path configDir;
+
     @Override
-    public void beforeAll(ExtensionContext context) {
+    public void beforeAll(ExtensionContext context) throws Exception {
         LauncherAccessor.init();
 
         SharedConstants.tryDetectVersion();
         Bootstrap.bootStrap();
         AppEngBootstrap.runEarlyStartup();
+
+        configDir = Files.createTempDirectory("ae2config");
+        if (AEConfig.instance() == null) {
+            AEConfig.load(configDir.toFile());
+        }
+    }
+
+    @Override
+    public void afterAll(ExtensionContext context) throws Exception {
+        if (configDir != null && Files.exists(configDir)) {
+            MoreFiles.deleteRecursively(configDir, RecursiveDeleteOption.ALLOW_INSECURE);
+        }
     }
 }