Fix crash when joining a dedicated server

We can't use FriendlyByte.readCollection to read to a
pre-allocated/array-backed NonNullList, as that doesn't implement
List.add. Instead, we just need to do a normal loop.

We add a couple of tests to round-trip our recipe specs. Unfortunately
we can't test the recipes themselves as our own registries aren't set
up, so this'll have to do for now.

projects/common/build.gradle.kts

@@ -39,4 +39,6 @@ dependencies {
     testModImplementation(testFixtures(project(":core")))
     testModImplementation(testFixtures(project(":common")))
     testModImplementation(libs.bundles.kotlin)
+
+    testFixturesImplementation(testFixtures(project(":core")))
 }

projects/common/src/main/java/dan200/computercraft/shared/recipe/RecipeUtil.java

@@ -21,7 +21,10 @@ public final class RecipeUtil {
     }
 
     public static NonNullList<Ingredient> readIngredients(FriendlyByteBuf buffer) {
-        return buffer.readCollection(x -> NonNullList.withSize(x, Ingredient.EMPTY), Ingredient::fromNetwork);
+        var count = buffer.readVarInt();
+        var ingredients = NonNullList.withSize(count, Ingredient.EMPTY);
+        for (var i = 0; i < ingredients.size(); i++) ingredients.set(i, Ingredient.fromNetwork(buffer));
+        return ingredients;
     }
 
     public static void writeIngredients(FriendlyByteBuf buffer, NonNullList<Ingredient> ingredients) {

projects/common/src/test/java/dan200/computercraft/shared/recipe/RecipeArbitraries.java

@@ -0,0 +1,50 @@
+// SPDX-FileCopyrightText: 2023 The CC: Tweaked Developers
+//
+// SPDX-License-Identifier: MPL-2.0
+
+package dan200.computercraft.shared.recipe;
+
+import dan200.computercraft.test.shared.MinecraftArbitraries;
+import it.unimi.dsi.fastutil.ints.IntIntImmutablePair;
+import net.jqwik.api.Arbitraries;
+import net.jqwik.api.Arbitrary;
+import net.jqwik.api.Combinators;
+import net.minecraft.core.NonNullList;
+import net.minecraft.world.item.crafting.CraftingBookCategory;
+import net.minecraft.world.item.crafting.Ingredient;
+
+/**
+ * {@link Arbitrary} implementations for recipes.
+ */
+public final class RecipeArbitraries {
+    public static Arbitrary<RecipeProperties> recipeProperties() {
+        return Combinators.combine(
+            Arbitraries.strings().ofMinLength(1).withChars("abcdefghijklmnopqrstuvwxyz_"),
+            Arbitraries.of(CraftingBookCategory.values())
+        ).as(RecipeProperties::new);
+    }
+
+    public static Arbitrary<ShapelessRecipeSpec> shapelessRecipeSpec() {
+        return Combinators.combine(
+            recipeProperties(),
+            MinecraftArbitraries.ingredient().array(Ingredient[].class).ofMinSize(1).map(x -> NonNullList.of(Ingredient.EMPTY, x)),
+            MinecraftArbitraries.nonEmptyItemStack()
+        ).as(ShapelessRecipeSpec::new);
+    }
+
+    public static Arbitrary<ShapedTemplate> shapedTemplate() {
+        return Combinators.combine(Arbitraries.integers().between(1, 3), Arbitraries.integers().between(1, 3))
+            .as(IntIntImmutablePair::new)
+            .flatMap(x -> MinecraftArbitraries.ingredient().array(Ingredient[].class).ofSize(x.leftInt() * x.rightInt())
+                .map(i -> new ShapedTemplate(x.leftInt(), x.rightInt(), NonNullList.of(Ingredient.EMPTY, i)))
+            );
+    }
+
+    public static Arbitrary<ShapedRecipeSpec> shapedRecipeSpec() {
+        return Combinators.combine(
+            recipeProperties(),
+            shapedTemplate(),
+            MinecraftArbitraries.nonEmptyItemStack()
+        ).as(ShapedRecipeSpec::new);
+    }
+}

projects/common/src/test/java/dan200/computercraft/shared/recipe/RecipeEqualities.java

@@ -0,0 +1,34 @@
+// SPDX-FileCopyrightText: 2023 The CC: Tweaked Developers
+//
+// SPDX-License-Identifier: MPL-2.0
+
+package dan200.computercraft.shared.recipe;
+
+import dan200.computercraft.test.core.StructuralEquality;
+import dan200.computercraft.test.shared.MinecraftEqualities;
+
+/**
+ * {@link StructuralEquality} implementations for recipes.
+ */
+public final class RecipeEqualities {
+    private RecipeEqualities() {
+    }
+
+    public static final StructuralEquality<ShapelessRecipeSpec> shapelessRecipeSpec = StructuralEquality.all(
+        StructuralEquality.at("properties", ShapelessRecipeSpec::properties),
+        StructuralEquality.at("ingredients", ShapelessRecipeSpec::ingredients, MinecraftEqualities.ingredient.list()),
+        StructuralEquality.at("result", ShapelessRecipeSpec::result, MinecraftEqualities.itemStack)
+    );
+
+    public static final StructuralEquality<ShapedTemplate> shapedTemplate = StructuralEquality.all(
+        StructuralEquality.at("width", ShapedTemplate::width),
+        StructuralEquality.at("height", ShapedTemplate::height),
+        StructuralEquality.at("ingredients", ShapedTemplate::ingredients, MinecraftEqualities.ingredient.list())
+    );
+
+    public static final StructuralEquality<ShapedRecipeSpec> shapedRecipeSpec = StructuralEquality.all(
+        StructuralEquality.at("properties", ShapedRecipeSpec::properties),
+        StructuralEquality.at("ingredients", ShapedRecipeSpec::template, shapedTemplate),
+        StructuralEquality.at("result", ShapedRecipeSpec::result, MinecraftEqualities.itemStack)
+    );
+}

projects/common/src/test/java/dan200/computercraft/shared/recipe/ShapedRecipeSpecTest.java

@@ -0,0 +1,32 @@
+// SPDX-FileCopyrightText: 2023 The CC: Tweaked Developers
+//
+// SPDX-License-Identifier: MPL-2.0
+
+package dan200.computercraft.shared.recipe;
+
+import dan200.computercraft.test.shared.NetworkSupport;
+import dan200.computercraft.test.shared.WithMinecraft;
+import net.jqwik.api.Arbitrary;
+import net.jqwik.api.ForAll;
+import net.jqwik.api.Property;
+import net.jqwik.api.Provide;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+
+@WithMinecraft
+public class ShapedRecipeSpecTest {
+    static {
+        WithMinecraft.Setup.bootstrap(); // @Property doesn't run test lifecycle methods.
+    }
+
+    @Property
+    public void testRoundTrip(@ForAll("recipe") ShapedRecipeSpec spec) {
+        var converted = NetworkSupport.roundTrip(spec, ShapedRecipeSpec::toNetwork, ShapedRecipeSpec::fromNetwork);
+        assertThat("Recipes are equal", converted, RecipeEqualities.shapedRecipeSpec.asMatcher(ShapedRecipeSpec.class, spec));
+    }
+
+    @Provide
+    Arbitrary<ShapedRecipeSpec> recipe() {
+        return RecipeArbitraries.shapedRecipeSpec();
+    }
+}

projects/common/src/test/java/dan200/computercraft/shared/recipe/ShapelessRecipeSpecTest.java

@@ -0,0 +1,32 @@
+// SPDX-FileCopyrightText: 2023 The CC: Tweaked Developers
+//
+// SPDX-License-Identifier: MPL-2.0
+
+package dan200.computercraft.shared.recipe;
+
+import dan200.computercraft.test.shared.NetworkSupport;
+import dan200.computercraft.test.shared.WithMinecraft;
+import net.jqwik.api.Arbitrary;
+import net.jqwik.api.ForAll;
+import net.jqwik.api.Property;
+import net.jqwik.api.Provide;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+
+@WithMinecraft
+public class ShapelessRecipeSpecTest {
+    static {
+        WithMinecraft.Setup.bootstrap(); // @Property doesn't run test lifecycle methods.
+    }
+
+    @Property
+    public void testRoundTrip(@ForAll("recipe") ShapelessRecipeSpec spec) {
+        var converted = NetworkSupport.roundTrip(spec, ShapelessRecipeSpec::toNetwork, ShapelessRecipeSpec::fromNetwork);
+        assertThat("Recipes are equal", converted, RecipeEqualities.shapelessRecipeSpec.asMatcher(ShapelessRecipeSpec.class, spec));
+    }
+
+    @Provide
+    Arbitrary<ShapelessRecipeSpec> recipe() {
+        return RecipeArbitraries.shapelessRecipeSpec();
+    }
+}

projects/common/src/test/java/dan200/computercraft/shared/turtle/upgrades/TurtleToolSerialiserTest.java

@@ -8,16 +8,13 @@ import dan200.computercraft.api.turtle.ITurtleUpgrade;
 import dan200.computercraft.api.turtle.TurtleToolDurability;
 import dan200.computercraft.test.core.StructuralEquality;
 import dan200.computercraft.test.shared.MinecraftArbitraries;
+import dan200.computercraft.test.shared.MinecraftEqualities;
+import dan200.computercraft.test.shared.NetworkSupport;
 import dan200.computercraft.test.shared.WithMinecraft;
-import io.netty.buffer.Unpooled;
 import net.jqwik.api.*;
 import net.minecraft.core.registries.Registries;
-import net.minecraft.network.FriendlyByteBuf;
-import net.minecraft.world.item.ItemStack;
-import org.hamcrest.Description;
 
 import static org.hamcrest.MatcherAssert.assertThat;
-import static org.junit.jupiter.api.Assertions.assertEquals;
 
 @WithMinecraft
 class TurtleToolSerialiserTest {
@@ -32,11 +29,9 @@ class TurtleToolSerialiserTest {
      */
     @Property
     public void testRoundTrip(@ForAll("tool") TurtleTool tool) {
-        var buffer = new FriendlyByteBuf(Unpooled.directBuffer());
-        TurtleToolSerialiser.INSTANCE.toNetwork(buffer, tool);
-
-        var converted = TurtleToolSerialiser.INSTANCE.fromNetwork(tool.getUpgradeID(), buffer);
-        assertEquals(buffer.readableBytes(), 0, "Whole packet was read");
+        var converted = NetworkSupport.roundTripSerialiser(
+            tool.getUpgradeID(), tool, TurtleToolSerialiser.INSTANCE::toNetwork, TurtleToolSerialiser.INSTANCE::fromNetwork
+        );
 
         if (!equality.equals(tool, converted)) {
             System.out.println("Break");
@@ -58,22 +53,10 @@ class TurtleToolSerialiserTest {
         ).as(TurtleTool::new);
     }
 
-    private static final StructuralEquality<ItemStack> stackEquality = new StructuralEquality<>() {
-        @Override
-        public boolean equals(ItemStack left, ItemStack right) {
-            return ItemStack.isSameItemSameTags(left, right) && left.getCount() == right.getCount();
-        }
-
-        @Override
-        public void describe(Description description, ItemStack object) {
-            description.appendValue(object).appendValue(object.getTag());
-        }
-    };
-
     private static final StructuralEquality<TurtleTool> equality = StructuralEquality.all(
         StructuralEquality.at("id", ITurtleUpgrade::getUpgradeID),
-        StructuralEquality.at("craftingItem", ITurtleUpgrade::getCraftingItem, stackEquality),
-        StructuralEquality.at("tool", x -> x.item, stackEquality),
+        StructuralEquality.at("craftingItem", ITurtleUpgrade::getCraftingItem, MinecraftEqualities.itemStack),
+        StructuralEquality.at("tool", x -> x.item, MinecraftEqualities.itemStack),
         StructuralEquality.at("damageMulitiplier", x -> x.damageMulitiplier),
         StructuralEquality.at("allowEnchantments", x -> x.allowEnchantments),
         StructuralEquality.at("consumeDurability", x -> x.consumeDurability),

projects/common/src/testFixtures/java/dan200/computercraft/test/shared/MinecraftArbitraries.java

@@ -17,6 +17,7 @@ import net.minecraft.tags.TagKey;
 import net.minecraft.world.item.Item;
 import net.minecraft.world.item.ItemStack;
 import net.minecraft.world.item.Items;
+import net.minecraft.world.item.crafting.Ingredient;
 
 import java.util.List;
 
@@ -44,6 +45,10 @@ public final class MinecraftArbitraries {
         return Arbitraries.oneOf(List.of(Arbitraries.just(ItemStack.EMPTY), nonEmptyItemStack()));
     }
 
+    public static Arbitrary<Ingredient> ingredient() {
+        return nonEmptyItemStack().list().ofMinSize(1).map(x -> Ingredient.of(x.stream()));
+    }
+
     public static Arbitrary<BlockPos> blockPos() {
         // BlockPos has a maximum range that can be sent over the network - use those.
         var xz = Arbitraries.integers().between(-3_000_000, -3_000_000);

projects/common/src/testFixtures/java/dan200/computercraft/test/shared/MinecraftEqualities.java

@@ -0,0 +1,39 @@
+// SPDX-FileCopyrightText: 2023 The CC: Tweaked Developers
+//
+// SPDX-License-Identifier: MPL-2.0
+
+package dan200.computercraft.test.shared;
+
+import dan200.computercraft.test.core.StructuralEquality;
+import net.minecraft.world.item.ItemStack;
+import net.minecraft.world.item.crafting.Ingredient;
+import org.hamcrest.Description;
+
+/**
+ * {@link StructuralEquality} implementations for Minecraft types.
+ */
+public class MinecraftEqualities {
+    public static final StructuralEquality<ItemStack> itemStack = new StructuralEquality<>() {
+        @Override
+        public boolean equals(ItemStack left, ItemStack right) {
+            return ItemStack.isSameItemSameTags(left, right) && left.getCount() == right.getCount();
+        }
+
+        @Override
+        public void describe(Description description, ItemStack object) {
+            description.appendValue(object).appendValue(object.getTag());
+        }
+    };
+
+    public static final StructuralEquality<Ingredient> ingredient = new StructuralEquality<>() {
+        @Override
+        public boolean equals(Ingredient left, Ingredient right) {
+            return left.toJson().equals(right.toJson());
+        }
+
+        @Override
+        public void describe(Description description, Ingredient object) {
+            description.appendValue(object.toJson());
+        }
+    };
+}

projects/common/src/testFixtures/java/dan200/computercraft/test/shared/NetworkSupport.java

@@ -0,0 +1,56 @@
+// SPDX-FileCopyrightText: 2023 The CC: Tweaked Developers
+//
+// SPDX-License-Identifier: MPL-2.0
+
+package dan200.computercraft.test.shared;
+
+import io.netty.buffer.Unpooled;
+import net.minecraft.network.FriendlyByteBuf;
+import net.minecraft.resources.ResourceLocation;
+import net.minecraft.world.item.crafting.RecipeSerializer;
+
+import java.util.function.BiConsumer;
+import java.util.function.BiFunction;
+import java.util.function.Function;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+/**
+ * Support methods for working with Minecraft's networking code.
+ */
+public final class NetworkSupport {
+    private NetworkSupport() {
+    }
+
+    /**
+     * Attempt to serialise and then deserialise a value.
+     *
+     * @param value The value to serialise.
+     * @param write Serialise this value to a buffer.
+     * @param read  Deserialise this value from a buffer.
+     * @param <T>   The type of the value to round trip.
+     * @return The converted value, for checking equivalency.
+     */
+    public static <T> T roundTrip(T value, BiConsumer<T, FriendlyByteBuf> write, Function<FriendlyByteBuf, T> read) {
+        var buffer = new FriendlyByteBuf(Unpooled.directBuffer());
+        write.accept(value, buffer);
+
+        var converted = read.apply(buffer);
+        assertEquals(buffer.readableBytes(), 0, "Whole packet was read");
+        return converted;
+    }
+
+    /**
+     * Attempt to serialise and then deserialise a value from a {@link RecipeSerializer}-like interface.
+     *
+     * @param id    The id of this value.
+     * @param value The value to serialise.
+     * @param write Serialise this value to a buffer.
+     * @param read  Deserialise this value from a buffer.
+     * @param <T>   The type of the value to round trip.
+     * @return The converted value, for checking equivalency.
+     */
+    public static <T> T roundTripSerialiser(ResourceLocation id, T value, BiConsumer<FriendlyByteBuf, T> write, BiFunction<ResourceLocation, FriendlyByteBuf, T> read) {
+        return roundTrip(value, (x, b) -> write.accept(b, x), b -> read.apply(id, b));
+    }
+}

projects/core/src/testFixtures/java/dan200/computercraft/test/core/StructuralEqualities.java

@@ -100,6 +100,29 @@ final class StructuralEqualities {
         }
     }
 
+    record ListEquality<T>(StructuralEquality<T> equality) implements StructuralEquality<List<T>> {
+        @Override
+        public boolean equals(List<T> left, List<T> right) {
+            if (left.size() != right.size()) return false;
+            for (var i = 0; i < left.size(); i++) {
+                if (!equality.equals(left.get(i), right.get(i))) return false;
+            }
+            return true;
+        }
+
+        @Override
+        public void describe(Description description, List<T> object) {
+            description.appendText("[");
+            var separator = false;
+            for (var value : object) {
+                if (separator) description.appendText(", ");
+                separator = true;
+                equality.describe(description, value);
+            }
+            description.appendText("]");
+        }
+    }
+
     static final class EqualityMatcher<T> extends TypeSafeMatcher<T> {
         private final StructuralEquality<T> equality;
         private final T equalTo;

projects/core/src/testFixtures/java/dan200/computercraft/test/core/StructuralEquality.java

@@ -36,6 +36,15 @@ public interface StructuralEquality<T> {
      */
     void describe(Description description, T object);
 
+    /**
+     * Lift this equality to a list of values.
+     *
+     * @return A equality for a list of values.
+     */
+    default StructuralEquality<List<T>> list() {
+        return new StructuralEqualities.ListEquality<>(this);
+    }
+
     /**
      * Convert this equality instance to a {@link Matcher}.
      *

projects/fabric/build.gradle.kts

@@ -91,6 +91,8 @@ dependencies {
     testImplementation(libs.byteBuddyAgent)
     testImplementation(libs.bundles.test)
     testRuntimeOnly(libs.bundles.testRuntime)
+
+    testFixturesImplementation(testFixtures(project(":core")))
 }
 
 sourceSets.main { resources.srcDir("src/generated/resources") }

projects/forge/build.gradle.kts

@@ -165,6 +165,8 @@ dependencies {
     testModImplementation(testFixtures(project(":core")))
     testModImplementation(testFixtures(project(":forge")))
 
+    testFixturesImplementation(testFixtures(project(":core")))
+
     "cctJavadoc"(libs.cctJavadoc)
 }