From 57fd0ecb2e4b6c6e2a435fc44f35f3139d543f4e Mon Sep 17 00:00:00 2001 From: Joseph B Date: Mon, 6 Feb 2023 14:48:44 -0500 Subject: [PATCH] Homogenize and/or holdersets when serializing to prevent serializing to NBT from crashing (#9048) Fixes #9043 --- .../common/extensions/IForgeHolderSet.java | 34 +++++++++ .../registries/holdersets/AndHolderSet.java | 2 +- .../holdersets/CompositeHolderSet.java | 74 ++++++++++++++++++- .../holdersets/ICustomHolderSet.java | 6 ++ .../registries/holdersets/OrHolderSet.java | 2 +- 5 files changed, 115 insertions(+), 3 deletions(-) diff --git a/src/main/java/net/minecraftforge/common/extensions/IForgeHolderSet.java b/src/main/java/net/minecraftforge/common/extensions/IForgeHolderSet.java index efa633a06b..b3dd00eaf8 100644 --- a/src/main/java/net/minecraftforge/common/extensions/IForgeHolderSet.java +++ b/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 { /** @@ -23,4 +25,36 @@ public interface IForgeHolderSet { // 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 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 + } } diff --git a/src/main/java/net/minecraftforge/registries/holdersets/AndHolderSet.java b/src/main/java/net/minecraftforge/registries/holdersets/AndHolderSet.java index 570e72c916..ea22e477a2 100644 --- a/src/main/java/net/minecraftforge/registries/holdersets/AndHolderSet.java +++ b/src/main/java/net/minecraftforge/registries/holdersets/AndHolderSet.java @@ -36,7 +36,7 @@ public class AndHolderSet extends CompositeHolderSet { return HolderSetCodec.create(registryKey, holderCodec, forceList) .listOf() - .xmap(AndHolderSet::new, AndHolderSet::getComponents) + .xmap(AndHolderSet::new, CompositeHolderSet::homogenize) .fieldOf("values") .codec(); } diff --git a/src/main/java/net/minecraftforge/registries/holdersets/CompositeHolderSet.java b/src/main/java/net/minecraftforge/registries/holdersets/CompositeHolderSet.java index b513e30450..6ea64f7fbd 100644 --- a/src/main/java/net/minecraftforge/registries/holdersets/CompositeHolderSet.java +++ b/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 implements ICustomHolderSet { 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> homogenize() + { + List> components = this.getComponents(); + + if (this.isHomogenous()) + { + return components; + } + + List> outputs = new ArrayList<>(); + for (HolderSet holderset : components) + { + if (holderset instanceof ICustomHolderSet) // 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> 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; + } } \ No newline at end of file diff --git a/src/main/java/net/minecraftforge/registries/holdersets/ICustomHolderSet.java b/src/main/java/net/minecraftforge/registries/holdersets/ICustomHolderSet.java index da05d2ee38..9f33348a65 100644 --- a/src/main/java/net/minecraftforge/registries/holdersets/ICustomHolderSet.java +++ b/src/main/java/net/minecraftforge/registries/holdersets/ICustomHolderSet.java @@ -16,4 +16,10 @@ public interface ICustomHolderSet extends HolderSet * {@return HolderSetType registered to {@link ForgeRegistries.HOLDER_SET_TYPES}} */ HolderSetType type(); + + @Override + default SerializationType serializationType() + { + return SerializationType.OBJECT; + } } diff --git a/src/main/java/net/minecraftforge/registries/holdersets/OrHolderSet.java b/src/main/java/net/minecraftforge/registries/holdersets/OrHolderSet.java index b178e92708..3d87005ce1 100644 --- a/src/main/java/net/minecraftforge/registries/holdersets/OrHolderSet.java +++ b/src/main/java/net/minecraftforge/registries/holdersets/OrHolderSet.java @@ -37,7 +37,7 @@ public class OrHolderSet extends CompositeHolderSet { return HolderSetCodec.create(registryKey, holderCodec, forceList) .listOf() - .xmap(OrHolderSet::new, OrHolderSet::getComponents) + .xmap(OrHolderSet::new, CompositeHolderSet::homogenize) .fieldOf("values") .codec(); }