Homogenize and/or holdersets when serializing to prevent serializing to NBT from crashing (#9048) Fixes #9043

src/main/java/net/minecraftforge/common/extensions/IForgeHolderSet.java

@@ -5,6 +5,8 @@
 
 package net.minecraftforge.common.extensions;
 
+import net.minecraft.core.HolderSet.ListBacked;
+
 public interface IForgeHolderSet<T>
 {
     /**
@@ -23,4 +25,36 @@ public interface IForgeHolderSet<T>
     {
         // noop by default, mutable/composite holdersets must override
     }
+
+    /**
+     * What format this holderset serializes to in json/nbt/etc
+     */
+    default public SerializationType serializationType()
+    {
+        // handle vanilla holderset types
+        return this instanceof ListBacked<T> listBacked
+            ? listBacked.unwrap().map(
+                // serializes as tag name if this holderset is named 
+                tag -> SerializationType.STRING, 
+                list -> list.size() == 1
+                    // if list has exactly one element then we have to check what kind, otherwise it's a list 
+                    ? list.get(0).unwrap().map(
+                        // if holder has a key bound then it's serialized as that string, otherwise it's inlined as an object
+                        key -> key == null ? SerializationType.OBJECT : SerializationType.STRING,
+                        value -> SerializationType.OBJECT)
+                    : SerializationType.LIST)
+            : SerializationType.UNKNOWN; // unsupported holderset impl, could be anything
+    }
+    
+    /**
+     * What format a holderset serializes to in json/nbt/etc
+     */
+    public static enum SerializationType
+    {
+        /** Unhandled/unsupported holderset implementation, could serialize as potentially anything **/
+        UNKNOWN,
+        STRING,
+        LIST,
+        OBJECT
+    }
 }

src/main/java/net/minecraftforge/registries/holdersets/AndHolderSet.java

@@ -36,7 +36,7 @@ public class AndHolderSet<T> extends CompositeHolderSet<T>
     {
         return HolderSetCodec.create(registryKey, holderCodec, forceList)
             .listOf()
-            .xmap(AndHolderSet::new, AndHolderSet::getComponents)
+            .xmap(AndHolderSet::new, CompositeHolderSet::homogenize)
             .fieldOf("values")
             .codec();
     }

src/main/java/net/minecraftforge/registries/holdersets/CompositeHolderSet.java

@@ -19,7 +19,6 @@ import com.mojang.datafixers.util.Either;
 
 import net.minecraft.core.Holder;
 import net.minecraft.core.HolderSet;
-import net.minecraft.core.Registry;
 import net.minecraft.tags.TagKey;
 import net.minecraft.util.RandomSource;
 
@@ -168,4 +167,77 @@ public abstract class CompositeHolderSet<T> implements ICustomHolderSet<T>
     {
         return this.getList().iterator();
     }
+    
+    /**
+     * Maps the sub-holdersets of this composite such that,
+     * if the list contains more than one element, and is non-homogenous,
+     * each element of the list will serialize as an object.
+     * Prevents crashes from trying to serialize non-homogenous lists to NBT.
+     * 
+     * Lists are considered non-homogenous if it contains more than one serialization type of holderset.
+     * Holdersets may be serialized as strings, lists, or maps.
+     * @see {@link #isHomogenous}
+     * @return List of holdersets with homogenous serialization behavior.
+     * Returns a new List if size > 1 and serialization would be non-homogenous,
+     * otherwise returns the composite's existing List.
+     */
+    public List<HolderSet<T>> homogenize()
+    {
+        List<HolderSet<T>> components = this.getComponents();
+        
+        if (this.isHomogenous())
+        {
+            return components;
+        }
+        
+        List<HolderSet<T>> outputs = new ArrayList<>();
+        for (HolderSet<T> holderset : components)
+        {
+            if (holderset instanceof ICustomHolderSet<T>) // serializes to object already
+            {
+                outputs.add(holderset);
+            }
+            else // serializes to string or list
+            {
+                outputs.add(new OrHolderSet<>(List.of(holderset)));
+            }
+        }
+        return outputs;
+    }
+    
+    /**
+     * @return True if all of our sub-holdersets have the same {@link SerializationType} (string, list, or object).
+     * False if we have more than one holderset AND if either we have more than one serialization type among them,
+     * or any holderset is {@link SerializationType.UNKNOWN}.
+     */
+    public boolean isHomogenous()
+    {
+        final List<HolderSet<T>> holderSets = this.getComponents();
+        
+        if (holderSets.size() < 2)
+        {
+            return true;
+        }
+        
+        // Get the first holderset and check if any subsequent holdersets are different
+        SerializationType firstType = holderSets.get(0).serializationType();
+        
+        // If any holderset has an unknown type, then we cannot assume we are homogenous
+        if (firstType == SerializationType.UNKNOWN)
+        {
+            return false;
+        }
+        
+        final int size = holderSets.size();
+        for (int i = 1; i < size; i++)
+        {
+            final SerializationType type = holderSets.get(i).serializationType();
+            if (type != firstType) // don't need to check type == UNKNOWN again because type != firstType covers it
+            {
+                return false;
+            }
+        }
+        
+        return true;
+    }
 }

src/main/java/net/minecraftforge/registries/holdersets/ICustomHolderSet.java

@@ -16,4 +16,10 @@ public interface ICustomHolderSet<T> extends HolderSet<T>
      * {@return HolderSetType registered to {@link ForgeRegistries.HOLDER_SET_TYPES}}
      */
     HolderSetType type();
+
+    @Override
+    default SerializationType serializationType()
+    {
+        return SerializationType.OBJECT;
+    }
 }

src/main/java/net/minecraftforge/registries/holdersets/OrHolderSet.java

@@ -37,7 +37,7 @@ public class OrHolderSet<T> extends CompositeHolderSet<T>
     {
         return HolderSetCodec.create(registryKey, holderCodec, forceList)
             .listOf()
-            .xmap(OrHolderSet::new, OrHolderSet::getComponents)
+            .xmap(OrHolderSet::new, CompositeHolderSet::homogenize)
             .fieldOf("values")
             .codec();
     }