Merge 3dbc624aeb into 09c6b9895c

fixes #100

CHANGELOG.md

@@ -3,8 +3,17 @@
 All notable changes to this project will be documented in this file.
 
 ## Unreleased
+- /
 
-- added better logging for cases where items are assigned to multiple unification tags
+## [1.2.2] - 2024-10-23
+
+- fixed crash on empty recipe JSONs
+
+## [1.2.1] - 2024-10-22
+
+- added logging for cases where items are assigned to multiple unification tags
+- added logging for cases where the recipe type can't be found
+- added skipping logic for recipes with invalid recipe types
 - added Turkish translation ([#102](https://github.com/AlmostReliable/almostunified/pull/102))
 - fixed crash when runtime isn't loaded ([#101](https://github.com/AlmostReliable/almostunified/issues/101))
 - fixed newly created custom tags not being considered for unification
@@ -35,6 +44,8 @@ All notable changes to this project will be documented in this file.
 Initial 1.21.1 port.
 
 <!-- Versions -->
+[1.2.2]: https://github.com/AlmostReliable/almostunified/releases/tag/v1.21.1-1.2.2
+[1.2.1]: https://github.com/AlmostReliable/almostunified/releases/tag/v1.21.1-1.2.1
 [1.2.0]: https://github.com/AlmostReliable/almostunified/releases/tag/v1.21.1-1.2.0
 [1.1.0]: https://github.com/AlmostReliable/almostunified/releases/tag/v1.21.1-1.1.0
 [1.0.0]: https://github.com/AlmostReliable/almostunified/releases/tag/v1.21.1-1.0.0

Common/src/main/java/com/almostreliable/unified/config/DuplicateConfig.java

@@ -60,20 +60,35 @@ public final class DuplicateConfig extends Config {
      * @return True if the recipe type is ignored, false otherwise
      */
     private boolean isRecipeTypeIgnored(RecipeLink recipe) {
-        return ignoredRecipeTypesCache.computeIfAbsent(recipe.getType(), type -> {
+        ResourceLocation type = recipe.getType();
+        Boolean ignored = ignoredRecipeTypesCache.get(type);
+        if (ignored == null) {
+            ignored = computeIsRecipeTypeIgnored(recipe);
+            ignoredRecipeTypesCache.put(type, ignored);
+        }
+        return ignored;
+    }
+
+    private boolean computeIsRecipeTypeIgnored(RecipeLink recipe) {
+        ResourceLocation type = recipe.getType();
+        String typeString = type.toString();
         for (Pattern ignorePattern : ignoreRecipeTypes) {
-                if (ignorePattern.matcher(type.toString()).matches()) {
+            if (ignorePattern.matcher(typeString).matches()) {
                 return true;
             }
         }
         return false;
-        });
     }
 
     public JsonCompare.CompareSettings getCompareSettings(ResourceLocation type) {
         return overrideRules.getOrDefault(type, defaultRules);
     }
 
+    public JsonCompare.CompareContext getCompareContext(RecipeLink recipe) {
+        JsonCompare.CompareSettings compareSettings = getCompareSettings(recipe.getType());
+        return JsonCompare.CompareContext.create(compareSettings, recipe);
+    }
+
     public boolean shouldCompareAll() {
         return compareAll;
     }

Common/src/main/java/com/almostreliable/unified/unification/recipe/RecipeJsonImpl.java

@@ -28,7 +28,7 @@ public class RecipeJsonImpl implements RecipeJson {
         try {
             return ResourceLocation.parse(json.get("type").getAsString());
         } catch (Exception e) {
-            throw new IllegalArgumentException("could not detect recipe type");
+            throw new IllegalArgumentException("could not detect recipe type for recipe " + id);
         }
     }
 

Common/src/main/java/com/almostreliable/unified/unification/recipe/RecipeLink.java

@@ -2,6 +2,7 @@ package com.almostreliable.unified.unification.recipe;
 
 import net.minecraft.resources.ResourceLocation;
 
+import com.almostreliable.unified.AlmostUnifiedCommon;
 import com.almostreliable.unified.api.unification.recipe.RecipeData;
 import com.almostreliable.unified.utils.JsonCompare;
 
@@ -11,25 +12,49 @@ import com.google.gson.JsonObject;
 import org.jetbrains.annotations.Nullable;
 
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.HashSet;
+import java.util.Map;
 import java.util.Set;
 import java.util.stream.Collectors;
 
-public class RecipeLink implements RecipeData {
+public final class RecipeLink implements RecipeData {
+    /**
+     * This cache is an optimization to avoid creating many ResourceLocations for just a few different types.
+     * Having fewer ResourceLocation instances can greatly speed up equality checking when these are used as map keys.
+     */
+    private static final Map<String, ResourceLocation> PARSED_TYPE_CACHE = new HashMap<>();
+
     private final ResourceLocation id;
     private final ResourceLocation type;
     private final JsonObject originalRecipe;
     @Nullable private DuplicateLink duplicateLink;
     @Nullable private JsonObject unifiedRecipe;
 
-    public RecipeLink(ResourceLocation id, JsonObject originalRecipe) {
+    private RecipeLink(ResourceLocation id, JsonObject originalRecipe, ResourceLocation type) {
         this.id = id;
         this.originalRecipe = originalRecipe;
+        this.type = type;
+    }
 
+    @Nullable
+    public static RecipeLink of(ResourceLocation id, JsonObject originalRecipe) {
         try {
-            this.type = ResourceLocation.tryParse(originalRecipe.get("type").getAsString());
+            ResourceLocation type = ResourceLocation.parse(originalRecipe.get("type").getAsString());
+            return new RecipeLink(id, originalRecipe, type);
         } catch (Exception e) {
-            throw new IllegalArgumentException("could not detect recipe type");
+            AlmostUnifiedCommon.LOGGER.warn("Could not detect recipe type for recipe '{}', skipping.", id);
+            return null;
+        }
+    }
+
+    public static RecipeLink ofOrThrow(ResourceLocation id, JsonObject originalRecipe) {
+        try {
+            String typeString = originalRecipe.get("type").getAsString();
+            ResourceLocation type = PARSED_TYPE_CACHE.computeIfAbsent(typeString, ResourceLocation::parse);
+            return new RecipeLink(id, originalRecipe, type);
+        } catch (Exception e) {
+            throw new IllegalArgumentException("could not detect recipe type for recipe " + id);
         }
     }
 
@@ -40,19 +65,19 @@ public class RecipeLink implements RecipeData {
      *
      * @param first           first recipe to compare
      * @param second          second recipe to compare
-     * @param compareSettings Settings to use for comparison.
+     * @param compareContext  Settings and context to use for comparison.
      * @return the recipe where rules are applied and the recipes are compared for equality, or null if the recipes are not equal
      */
     @Nullable
-    public static RecipeLink compare(RecipeLink first, RecipeLink second, JsonCompare.CompareSettings compareSettings) {
+    public static RecipeLink compare(RecipeLink first, RecipeLink second, JsonCompare.CompareContext compareContext) {
         JsonObject selfActual = first.getActual();
         JsonObject toCompareActual = second.getActual();
 
         JsonObject compare = null;
         if (first.getType().toString().equals("minecraft:crafting_shaped")) {
-            compare = JsonCompare.compareShaped(selfActual, toCompareActual, compareSettings);
-        } else if (JsonCompare.matches(selfActual, toCompareActual, compareSettings)) {
-            compare = JsonCompare.compare(compareSettings.getRules(), selfActual, toCompareActual);
+            compare = JsonCompare.compareShaped(selfActual, toCompareActual, compareContext);
+        } else if (JsonCompare.matches(selfActual, toCompareActual, compareContext)) {
+            compare = JsonCompare.compare(compareContext.settings().getRules(), selfActual, toCompareActual);
         }
 
         if (compare == null) return null;
@@ -129,10 +154,10 @@ public class RecipeLink implements RecipeData {
      * the master from the link will be used. Otherwise, we will create a new link if needed.
      *
      * @param otherRecipe     Recipe data to check for duplicate against.
-     * @param compareSettings Settings to use for comparison.
+     * @param compareContext  Settings and context to use for comparison.
      * @return True if recipe is a duplicate, false otherwise.
      */
-    public boolean handleDuplicate(RecipeLink otherRecipe, JsonCompare.CompareSettings compareSettings) {
+    public boolean handleDuplicate(RecipeLink otherRecipe, JsonCompare.CompareContext compareContext) {
         DuplicateLink selfDuplicate = getDuplicateLink();
         DuplicateLink otherDuplicate = otherRecipe.getDuplicateLink();
 
@@ -141,7 +166,7 @@ public class RecipeLink implements RecipeData {
         }
 
         if (selfDuplicate == null && otherDuplicate == null) {
-            RecipeLink compare = compare(this, otherRecipe, compareSettings);
+            RecipeLink compare = compare(this, otherRecipe, compareContext);
             if (compare == null) {
                 return false;
             }
@@ -153,7 +178,7 @@ public class RecipeLink implements RecipeData {
         }
 
         if (otherDuplicate != null) {
-            RecipeLink compare = compare(this, otherDuplicate.getMaster(), compareSettings);
+            RecipeLink compare = compare(this, otherDuplicate.getMaster(), compareContext);
             if (compare == null) {
                 return false;
             }
@@ -163,7 +188,7 @@ public class RecipeLink implements RecipeData {
         }
 
         // selfDuplicate != null
-        RecipeLink compare = compare(selfDuplicate.getMaster(), otherRecipe, compareSettings);
+        RecipeLink compare = compare(selfDuplicate.getMaster(), otherRecipe, compareContext);
         if (compare == null) {
             return false;
         }

Common/src/main/java/com/almostreliable/unified/unification/recipe/RecipeTransformer.java

@@ -31,6 +31,7 @@ import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 import java.util.function.Consumer;
 import java.util.stream.Collectors;
@@ -111,7 +112,9 @@ public class RecipeTransformer {
         return recipes
             .entrySet()
             .stream()
-            .map(entry -> new RecipeLink(entry.getKey(), entry.getValue().getAsJsonObject()))
+            .filter(entry -> entry.getValue() instanceof JsonObject jsonObject && !jsonObject.isEmpty())
+            .map(entry -> RecipeLink.of(entry.getKey(), entry.getValue().getAsJsonObject()))
+            .filter(Objects::nonNull)
             .sorted(Comparator.comparing(entry -> entry.getId().toString()))
             .collect(Collectors.groupingByConcurrent(RecipeLink::getType));
     }
@@ -138,7 +141,8 @@ public class RecipeTransformer {
             return false;
         }
 
-        JsonCompare.CompareSettings compareSettings = duplicateConfig.getCompareSettings(curRecipe.getType());
+        JsonCompare.CompareContext compareContext = duplicateConfig.getCompareContext(curRecipe);
+
         boolean foundDuplicate = false;
         for (RecipeLink recipeLink : recipes) {
             if (!curRecipe.getType().equals(recipeLink.getType())) {
@@ -150,7 +154,7 @@ public class RecipeTransformer {
                 continue;
             }
 
-            foundDuplicate |= curRecipe.handleDuplicate(recipeLink, compareSettings);
+            foundDuplicate |= curRecipe.handleDuplicate(recipeLink, compareContext);
         }
 
         return foundDuplicate;

Common/src/main/java/com/almostreliable/unified/utils/JsonCompare.java

@@ -1,5 +1,7 @@
 package com.almostreliable.unified.utils;
 
+import com.almostreliable.unified.unification.recipe.RecipeLink;
+
 import com.google.gson.JsonArray;
 import com.google.gson.JsonElement;
 import com.google.gson.JsonObject;
@@ -8,8 +10,6 @@ import com.google.gson.JsonPrimitive;
 import org.jetbrains.annotations.Nullable;
 
 import java.util.Arrays;
-import java.util.Collection;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
@@ -47,8 +47,8 @@ public final class JsonCompare {
     }
 
     @Nullable
-    public static JsonObject compareShaped(JsonObject first, JsonObject second, CompareSettings compareSettings) {
-        if (!matches(first, second, compareSettings)) return null;
+    public static JsonObject compareShaped(JsonObject first, JsonObject second, CompareContext compareContext) {
+        if (!matches(first, second, compareContext)) return null;
 
         JsonArray firstPattern = JsonUtils.arrayOrSelf(first.get("pattern"));
         JsonArray secondPattern = JsonUtils.arrayOrSelf(second.get("pattern"));
@@ -97,20 +97,18 @@ public final class JsonCompare {
         return keyMap;
     }
 
-    public static boolean matches(JsonObject first, JsonObject second, CompareSettings compareSettings) {
-        Collection<String> ignoredFields = compareSettings.getIgnoredFields();
-        if (ignoredFields.isEmpty() && first.size() != second.size()) {
+    public static boolean matches(JsonObject first, JsonObject second, CompareContext compareContext) {
+        CompareSettings compareSettings = compareContext.settings;
+        if (!compareSettings.hasIgnoredFields() && first.size() != second.size()) {
             return false;
         }
 
-        for (Map.Entry<String, JsonElement> firstEntry : first.entrySet()) {
-            if (ignoredFields.contains(firstEntry.getKey())) continue;
-
-            JsonElement firstElem = firstEntry.getValue();
-            JsonElement secondElem = second.get(firstEntry.getKey());
-
+        for (String field : compareContext.compareFields()) {
+            JsonElement secondElem = second.get(field);
             if (secondElem == null) return false;
 
+            JsonElement firstElem = first.get(field);
+
             // sanitize elements for implicit counts of 1
             if (compareSettings.handleImplicitCounts && needsSanitizing(firstElem, secondElem)) {
                 firstElem = sanitize(firstElem);
@@ -270,6 +268,17 @@ public final class JsonCompare {
         }
     }
 
+    public record CompareContext(CompareSettings settings, List<String> compareFields) {
+        public static CompareContext create(CompareSettings settings, RecipeLink curRecipe) {
+            Set<String> compareFields = curRecipe.getActual().keySet();
+            if (!settings.ignoredFields.isEmpty()) {
+                compareFields = new HashSet<>(compareFields);
+                compareFields.removeAll(settings.ignoredFields);
+            }
+            return new CompareContext(settings, List.copyOf(compareFields));
+        }
+    }
+
     public static class CompareSettings {
 
         public static final String IGNORED_FIELDS = "ignored_fields";
@@ -292,8 +301,8 @@ public final class JsonCompare {
             }
         }
 
-        public Set<String> getIgnoredFields() {
-            return Collections.unmodifiableSet(ignoredFields);
+        public boolean hasIgnoredFields() {
+            return !ignoredFields.isEmpty();
         }
 
         public JsonObject serialize() {

Common/src/test/java/testmod/TestUtils.java

@@ -83,11 +83,11 @@ public class TestUtils {
 
     public static RecipeLink recipe(String jsonStr) {
         var json = json(jsonStr);
-        return new RecipeLink(ResourceLocation.parse("test"), json);
+        return RecipeLink.ofOrThrow(ResourceLocation.parse("test"), json);
     }
 
     public static RecipeLink recipe(JsonObject json) {
-        return new RecipeLink(ResourceLocation.parse("test"), json);
+        return RecipeLink.ofOrThrow(ResourceLocation.parse("test"), json);
     }
 
     public static JsonObject json(String json) {

gradle.properties

@@ -8,7 +8,7 @@ enableAccessWidener = false
 minecraftVersion = 1.21.1
 
 # Mod
-modVersion = 1.2.0
+modVersion = 1.2.2
 modPackage = com.almostreliable.unified
 modId = almostunified
 modName = AlmostUnified