Fix custom ingredient serialization with allowEmpty (#3389)

* Fix custom ingredient serialization with allowEmpty * Remove custom First codec * Fix checkstyle
FabricMC · Nov 2, 2023 · 6ed720c · 6ed720c
1 parent 8ed13ef
commit 6ed720c
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 47 deletions.
diff --git a/...api-v1/src/main/java/net/fabricmc/fabric/impl/recipe/ingredient/CustomIngredientImpl.java b/...api-v1/src/main/java/net/fabricmc/fabric/impl/recipe/ingredient/CustomIngredientImpl.java
@@ -23,10 +23,8 @@
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.stream.Stream;
 
-import com.mojang.datafixers.util.Pair;
 import com.mojang.serialization.Codec;
 import com.mojang.serialization.DataResult;
-import com.mojang.serialization.DynamicOps;
 import org.jetbrains.annotations.Nullable;
 
 import net.minecraft.item.ItemStack;
@@ -56,9 +54,6 @@ public class CustomIngredientImpl extends Ingredient {
 			serializer -> DataResult.success(serializer.getIdentifier())
 	);
 
-	public static final Codec<CustomIngredient> ALLOW_EMPTY_INGREDIENT_CODECS = CODEC.dispatch(TYPE_KEY, CustomIngredient::getSerializer, serializer -> serializer.getCodec(true));
-	public static final Codec<CustomIngredient> DISALLOW_EMPTY_INGREDIENT_CODECS = CODEC.dispatch(TYPE_KEY, CustomIngredient::getSerializer, serializer -> serializer.getCodec(false));
-
 	public static void registerSerializer(CustomIngredientSerializer<?> serializer) {
 		Objects.requireNonNull(serializer.getIdentifier(), "CustomIngredientSerializer identifier may not be null.");
 
@@ -137,33 +132,4 @@ public boolean isEmpty() {
 	private <T> T coerceIngredient() {
 		return (T) customIngredient;
 	}
-
-	public static <T> Codec<T> first(Codec<T> first, Codec<T> second) {
-		return new First<>(first, second);
-	}
-
-	// Decode/encode the first codec, if that fails return the result of the second.
-	record First<T>(Codec<T> first, Codec<T> second) implements Codec<T> {
-		@Override
-		public <T1> DataResult<Pair<T, T1>> decode(DynamicOps<T1> ops, T1 input) {
-			DataResult<Pair<T, T1>> firstResult = first.decode(ops, input);
-
-			if (firstResult.result().isPresent()) {
-				return firstResult;
-			}
-
-			return second.decode(ops, input);
-		}
-
-		@Override
-		public <T1> DataResult<T1> encode(T input, DynamicOps<T1> ops, T1 prefix) {
-			DataResult<T1> firstResult = first.encode(input, ops, prefix);
-
-			if (firstResult.result().isPresent()) {
-				return firstResult;
-			}
-
-			return second.encode(input, ops, prefix);
-		}
-	}
 }
diff --git a/...ipe-api-v1/src/main/java/net/fabricmc/fabric/mixin/recipe/ingredient/IngredientMixin.java b/...ipe-api-v1/src/main/java/net/fabricmc/fabric/mixin/recipe/ingredient/IngredientMixin.java
@@ -16,6 +16,7 @@
 
 package net.fabricmc.fabric.mixin.recipe.ingredient;
 
+import com.mojang.datafixers.util.Either;
 import com.mojang.serialization.Codec;
 import org.spongepowered.asm.mixin.Mixin;
 import org.spongepowered.asm.mixin.injection.At;
@@ -25,6 +26,7 @@
 import net.minecraft.network.PacketByteBuf;
 import net.minecraft.recipe.Ingredient;
 import net.minecraft.util.Identifier;
+import net.minecraft.util.dynamic.Codecs;
 
 import net.fabricmc.fabric.api.recipe.v1.ingredient.CustomIngredient;
 import net.fabricmc.fabric.api.recipe.v1.ingredient.CustomIngredientSerializer;
@@ -35,9 +37,18 @@
 public class IngredientMixin implements FabricIngredient {
 	@Inject(method = "createCodec", at = @At("RETURN"), cancellable = true)
 	private static void injectCodec(boolean allowEmpty, CallbackInfoReturnable<Codec<Ingredient>> cir) {
-		final Codec<CustomIngredient> customIngredientCodec = allowEmpty ? CustomIngredientImpl.ALLOW_EMPTY_INGREDIENT_CODECS : CustomIngredientImpl.DISALLOW_EMPTY_INGREDIENT_CODECS;
-		Codec<Ingredient> ingredientCodec = customIngredientCodec.xmap(CustomIngredient::toVanilla, FabricIngredient::getCustomIngredient);
-		cir.setReturnValue(CustomIngredientImpl.first(cir.getReturnValue(), ingredientCodec));
+		Codec<CustomIngredient> customIngredientCodec = CustomIngredientImpl.CODEC.dispatch(
+				CustomIngredientImpl.TYPE_KEY,
+				CustomIngredient::getSerializer,
+				serializer -> serializer.getCodec(allowEmpty));
+
+		cir.setReturnValue(Codecs.either(customIngredientCodec, cir.getReturnValue()).xmap(
+				either -> either.map(CustomIngredient::toVanilla, ingredient -> ingredient),
+				ingredient -> {
+					CustomIngredient customIngredient = ingredient.getCustomIngredient();
+					return customIngredient == null ? Either.right(ingredient) : Either.left(customIngredient);
+				}
+		));
 	}
 
 	@Inject(

diff --git a/...pi-v1/src/testmod/java/net/fabricmc/fabric/test/recipe/ingredient/SerializationTests.java b/...pi-v1/src/testmod/java/net/fabricmc/fabric/test/recipe/ingredient/SerializationTests.java
@@ -21,6 +21,7 @@
 import com.google.gson.JsonElement;
 import com.google.gson.JsonParseException;
 import com.google.gson.JsonParser;
+import com.mojang.serialization.Codec;
 import com.mojang.serialization.JsonOps;
 
 import net.minecraft.item.Items;
@@ -31,7 +32,7 @@
 import net.minecraft.util.Util;
 
 import net.fabricmc.fabric.api.gametest.v1.FabricGameTest;
-import net.fabricmc.fabric.impl.recipe.ingredient.builtin.AllIngredient;
+import net.fabricmc.fabric.api.recipe.v1.ingredient.DefaultCustomIngredients;
 
 public class SerializationTests {
 	/**
@@ -64,19 +65,29 @@ public void testArrayDeserialization(TestContext context) {
 	}
 
 	/**
-	 * Check that we can serialise a custom ingredient.
+	 * Check that we can serialise and deserialize a custom ingredient.
 	 */
 	@GameTest(templateName = FabricGameTest.EMPTY_STRUCTURE)
 	public void testCustomIngredientSerialization(TestContext context) {
-		String ingredientJson = """
-				{"ingredients":[{"item":"minecraft:stone"}],"fabric:type":"fabric:all"}
-				""".trim();
+		for (boolean allowEmpty : List.of(false, true)) {
+			String ingredientJson = """
+					{"ingredients":[{"item":"minecraft:stone"}],"fabric:type":"fabric:all"}
+					""".trim();
+
+			Ingredient ingredient = DefaultCustomIngredients.all(
+					Ingredient.ofItems(Items.STONE)
+			);
+			JsonElement json = ingredient.toJson(allowEmpty);
+			context.assertTrue(json.toString().equals(ingredientJson), "Unexpected json: " + json);
+			// Make sure that we can deserialize it
+			Codec<Ingredient> ingredientCodec = allowEmpty ? Ingredient.ALLOW_EMPTY_CODEC : Ingredient.DISALLOW_EMPTY_CODEC;
+			Ingredient deserialized = Util.getResult(
+					ingredientCodec.parse(JsonOps.INSTANCE, json), JsonParseException::new
+			);
+			context.assertTrue(deserialized.getCustomIngredient() != null, "Custom ingredient was not deserialized");
+			context.assertTrue(deserialized.getCustomIngredient().getSerializer() == ingredient.getCustomIngredient().getSerializer(), "Serializer did not match");
+		}
 
-		var ingredient = new AllIngredient(List.of(
-				Ingredient.ofItems(Items.STONE)
-		));
-		String json = ingredient.toVanilla().toJson(false).toString();
-		context.assertTrue(json.equals(ingredientJson), "Unexpected json: " + json);
 		context.complete();
 	}
 }