Change config treatment to avoid undefined behavior

Also changed event listener registration on neoforge to avoid race conditions, as well as fixing worldgen in neoforge

common/src/main/java/dev/lukebemish/excavatedvariants/api/data/DetailedStone.java

@@ -8,41 +8,62 @@
 import com.mojang.datafixers.util.Either;
 import com.mojang.datafixers.util.Pair;
 import com.mojang.serialization.Codec;
+import com.mojang.serialization.DataResult;
 import com.mojang.serialization.codecs.RecordCodecBuilder;
 import dev.lukebemish.excavatedvariants.api.RegistryKeys;
 import dev.lukebemish.excavatedvariants.impl.platform.Services;
 import net.minecraft.resources.ResourceKey;
 import net.minecraft.resources.ResourceLocation;
 
-import java.util.List;
-import java.util.Map;
-import java.util.Objects;
-import java.util.function.Function;
+import java.util.*;
 import java.util.stream.Collectors;
 
 public final class DetailedStone {
-    private static final Codec<DetailedStone> CODEC = RecordCodecBuilder.create(instance -> instance.group(
-            ResourceKey.codec(RegistryKeys.STONE).fieldOf("stone").forGetter(DetailedStone::getStone),
-            Codec.STRING.listOf().fieldOf("required_mods").forGetter(s -> s.modIds),
-            Codec.BOOL.optionalFieldOf("generating", true).forGetter(DetailedStone::isGenerating)
-    ).apply(instance, DetailedStone::new));
-    private static final Codec<Either<ResourceKey<Stone>, DetailedStone>> EITHER_CODEC = Codec.either(ResourceKey.codec(RegistryKeys.STONE), CODEC);
+    private static final Codec<PartialDetailedStone> CODEC = RecordCodecBuilder.create(instance -> instance.group(
+            ResourceKey.codec(RegistryKeys.STONE).fieldOf("stone").forGetter(PartialDetailedStone::stone),
+            Codec.STRING.listOf().optionalFieldOf("required_mods").forGetter(PartialDetailedStone::requiredMods),
+            Codec.BOOL.optionalFieldOf("generating", false).forGetter(PartialDetailedStone::generating)
+    ).apply(instance, PartialDetailedStone::new));
+    private record PartialDetailedStone(ResourceKey<Stone> stone, Optional<List<String>> requiredMods, boolean generating) {
+        public DetailedStone toDetailedStone(ResourceLocation key) {
+            return new DetailedStone(stone, requiredMods.orElse(List.of(key.getNamespace())), generating);
+        }
+    }
+    private static final Codec<Either<ResourceKey<Stone>, PartialDetailedStone>> EITHER_CODEC = Codec.either(ResourceKey.codec(RegistryKeys.STONE), CODEC);
     public static final Codec<Map<ResourceLocation, DetailedStone>> MAP_CODEC = Codec.unboundedMap(ResourceLocation.CODEC, EITHER_CODEC)
             .xmap(
                     map ->
                             map.entrySet().stream()
-                                    .map(e -> new Pair<>(e.getKey(), e.getValue().map(k -> new DetailedStone(k, List.of(e.getKey().getNamespace()), true), Function.identity())))
+                                    .map(e -> new Pair<>(e.getKey(), e.getValue().map(k -> new DetailedStone(k, List.of(e.getKey().getNamespace()), false), partial -> partial.toDetailedStone(e.getKey()))))
                                     .collect(Collectors.toMap(Pair::getFirst, Pair::getSecond)),
                     map ->
                             map.entrySet().stream()
                                     .collect(Collectors.toMap(Map.Entry::getKey, m -> {
-                                        if (m.getValue().modIds.size() == 1 && m.getValue().modIds.get(0).equals(m.getKey().getNamespace()) && m.getValue().isGenerating()) {
+                                        if (m.getValue().modIds.size() == 1 && m.getValue().modIds.get(0).equals(m.getKey().getNamespace()) && !m.getValue().isGenerating()) {
                                             return Either.left(m.getValue().getStone());
                                         } else {
-                                            return Either.right(m.getValue());
+                                            return Either.right(new PartialDetailedStone(
+                                                    m.getValue().getStone(),
+                                                    Optional.of(m.getValue().modIds),
+                                                    m.getValue().isGenerating()
+                                            ));
                                         }
                                     }))
-            );
+            ).flatXmap(map -> {
+                if (map.values().stream().map(s -> Set.copyOf(s.requiredMods())).collect(Collectors.toSet()).size() == 1 && map.values().stream().noneMatch(DetailedStone::isGenerating)) {
+                    HashMap<ResourceLocation, DetailedStone> newMap = new HashMap<>();
+                    for (Map.Entry<ResourceLocation, DetailedStone> entry : map.entrySet()) {
+                        DetailedStone detailedStone = entry.getValue();
+                        DetailedStone newStone = new DetailedStone.Builder().setModIds(detailedStone.requiredMods()).setStone(detailedStone.getStone()).setGenerating(true).build();
+                        newMap.put(entry.getKey(), newStone);
+                    }
+                    return DataResult.success(newMap);
+                } else if (map.values().stream().filter(DetailedStone::isGenerating).map(s -> Set.copyOf(s.requiredMods())).collect(Collectors.toSet()).size() == 1) {
+                    return DataResult.success(map);
+                } else {
+                    return DataResult.error(() -> "A generating ore block must be present, and all generating ore blocks (or candidates for implicit detection) must require the same mods");
+                }
+            }, DataResult::success);
     private final ResourceKey<Stone> stone;
     private final List<String> modIds;
     private final boolean generating;
@@ -53,6 +74,10 @@ private DetailedStone(ResourceKey<Stone> stone, List<String> modIds, boolean gen
         this.generating = generating;
     }
 
+    public List<String> requiredMods() {
+        return this.modIds;
+    }
+
     public boolean hasRequiredMods() {
         return Services.PLATFORM.getModIds().containsAll(this.modIds);
     }
@@ -68,7 +93,7 @@ public boolean isGenerating() {
     public static class Builder {
         private ResourceKey<Stone> stone;
         private List<String> modIds;
-        private boolean generating = true;
+        private boolean generating = false;
 
         /**
          * @param stone the represented stone of this variant/stone pairing

common/src/main/java/dev/lukebemish/excavatedvariants/api/data/Ore.java

@@ -24,6 +24,7 @@
 import org.jetbrains.annotations.Contract;
 
 import java.util.*;
+import java.util.stream.Collectors;
 
 public final class Ore {
     public static final Codec<Ore> CODEC = RecordCodecBuilder.create(i -> i.group(
@@ -48,7 +49,23 @@ public final class Ore {
 
     private Ore(List<ResourceLocation> tags, Map<ResourceLocation, DetailedStone> blocks, Map<String, String> translations, Set<ResourceKey<GroundType>> types) {
         this.tags = tags;
-        this.blocks = new HashMap<>(blocks);
+        Set<Set<String>> modLists = new HashSet<>();
+        // sanity check on the required mods for generating variants
+        Map<ResourceLocation, DetailedStone> blocksCopy = new HashMap<>(blocks);
+        for (var entry : blocks.entrySet()) {
+            if (entry.getValue().isGenerating()) {
+                modLists.add(Set.copyOf(entry.getValue().requiredMods()));
+                if (modLists.size() > 1) {
+                    ExcavatedVariants.LOGGER.error(
+                            "Ore " + getKeyOrThrow().location() + " has multiple generating blocks with different required mods, and will be disabled: " +
+                                    modLists.stream().map(s -> "["+String.join(", ", s)+"]").collect(Collectors.joining("; "))
+                    );
+                    blocksCopy = Collections.emptyMap();
+                    break;
+                }
+            }
+        }
+        this.blocks = blocksCopy;
         this.translations = translations;
         this.types = types;
 
@@ -103,7 +120,7 @@ public void addPossibleVariant(Stone stone, ResourceLocation output) {
         if (ModLifecycle.getLifecyclePhase() != ModLifecycle.PRE_REGISTRATION) {
             throw new IllegalStateException("Cannot add possible variant except during pre-registration");
         }
-        blocks.put(output, new DetailedStone.Builder().setModIds(List.of(ExcavatedVariants.MOD_ID)).setStone(stone.getKeyOrThrow()).build());
+        blocks.put(output, new DetailedStone.Builder().setModIds(List.of(ExcavatedVariants.MOD_ID)).setStone(stone.getKeyOrThrow()).setGenerating(false).build());
     }
 
     @ApiStatus.Internal

common/src/main/java/dev/lukebemish/excavatedvariants/impl/worldgen/OreGenMapSavedData.java

@@ -5,22 +5,32 @@
 
 package dev.lukebemish.excavatedvariants.impl.worldgen;
 
-import com.mojang.datafixers.util.Pair;
 import dev.lukebemish.excavatedvariants.impl.ExcavatedVariants;
+import it.unimi.dsi.fastutil.objects.Object2IntMap;
+import it.unimi.dsi.fastutil.objects.Object2IntMaps;
+import it.unimi.dsi.fastutil.objects.Object2IntOpenHashMap;
 import net.minecraft.nbt.CompoundTag;
 import net.minecraft.world.level.ServerLevelAccessor;
 import net.minecraft.world.level.saveddata.SavedData;
 import org.jspecify.annotations.NonNull;
 
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.HashMap;
-import java.util.Map;
+import java.util.HashSet;
+import java.util.Set;
 
 public class OreGenMapSavedData extends SavedData {
     public static final String DATA_KEY = ExcavatedVariants.MOD_ID + ":ore_replacement";
-    public final Map<Pair<Integer, Integer>, Integer> edgeCount = Collections.synchronizedMap(new HashMap<>());
-    public final Map<Pair<Integer, Integer>, Boolean> ranMap = Collections.synchronizedMap(new HashMap<>());
+    public final Object2IntMap<ChunkKey> edgeCount;
+    public final Set<ChunkKey> ranMap = Collections.synchronizedSet(new HashSet<>());
+
+    public OreGenMapSavedData() {
+        Object2IntMap<ChunkKey> edgeMap = new Object2IntOpenHashMap<>();
+        edgeMap.defaultReturnValue(0);
+        this.edgeCount = Object2IntMaps.synchronize(edgeMap);
+    }
+
+    public record ChunkKey(int x, int z) {}
 
     private static OreGenMapSavedData load(CompoundTag tag) {
         OreGenMapSavedData data = new OreGenMapSavedData();
@@ -29,13 +39,12 @@ private static OreGenMapSavedData load(CompoundTag tag) {
         int[] edge3 = tag.getIntArray("edge_3");
         int[] ran1 = tag.getIntArray("ran_1");
         int[] ran2 = tag.getIntArray("ran_2");
-        int[] ran3 = tag.getIntArray("ran_3");
-        if (edge1.length == edge2.length && edge1.length == edge3.length && ran1.length == ran2.length && ran1.length == ran3.length) {
+        if (edge1.length == edge2.length && edge1.length == edge3.length && ran1.length == ran2.length) {
             for (int i = 0; i < edge1.length; i++) {
-                data.edgeCount.put(new Pair<>(edge1[i], edge2[i]), edge3[i]);
+                data.edgeCount.put(new ChunkKey(edge1[i], edge2[i]), edge3[i]);
             }
             for (int i = 0; i < ran1.length; i++) {
-                data.ranMap.put(new Pair<>(ran1[i], ran2[i]), ran3[i] != 0);
+                data.ranMap.add(new ChunkKey(ran1[i], ran2[i]));
             }
         }
         return data;
@@ -56,29 +65,26 @@ public static OreGenMapSavedData getOrCreate(ServerLevelAccessor world) {
 
     @Override
     @NonNull
-    public CompoundTag save(CompoundTag tag) {
+    public CompoundTag save(@NonNull CompoundTag tag) {
         ArrayList<Integer> edge1 = new ArrayList<>();
         ArrayList<Integer> edge2 = new ArrayList<>();
         ArrayList<Integer> edge3 = new ArrayList<>();
         ArrayList<Integer> ran1 = new ArrayList<>();
         ArrayList<Integer> ran2 = new ArrayList<>();
-        ArrayList<Integer> ran3 = new ArrayList<>();
-        for (Pair<Integer, Integer> p : edgeCount.keySet()) {
-            edge1.add(p.getFirst());
-            edge2.add(p.getSecond());
-            edge3.add(edgeCount.get(p));
+        for (ChunkKey p : edgeCount.keySet()) {
+            edge1.add(p.x());
+            edge2.add(p.z());
+            edge3.add(edgeCount.getInt(p));
         }
-        for (Pair<Integer, Integer> p : ranMap.keySet()) {
-            ran1.add(p.getFirst());
-            ran2.add(p.getSecond());
-            ran3.add(Boolean.TRUE.equals(ranMap.get(p)) ? 1 : 0);
+        for (ChunkKey p : ranMap) {
+            ran1.add(p.x());
+            ran2.add(p.z());
         }
         tag.putIntArray("edge_1", edge1.stream().mapToInt(Integer::intValue).toArray());
         tag.putIntArray("edge_2", edge2.stream().mapToInt(Integer::intValue).toArray());
         tag.putIntArray("edge_3", edge3.stream().mapToInt(Integer::intValue).toArray());
         tag.putIntArray("ran_1", ran1.stream().mapToInt(Integer::intValue).toArray());
         tag.putIntArray("ran_2", ran2.stream().mapToInt(Integer::intValue).toArray());
-        tag.putIntArray("ran_3", ran3.stream().mapToInt(Integer::intValue).toArray());
         return tag;
     }
 }

common/src/main/java/dev/lukebemish/excavatedvariants/impl/worldgen/OreReplacer.java

@@ -5,7 +5,6 @@
 
 package dev.lukebemish.excavatedvariants.impl.worldgen;
 
-import com.mojang.datafixers.util.Pair;
 import dev.lukebemish.excavatedvariants.api.data.Ore;
 import dev.lukebemish.excavatedvariants.api.data.Stone;
 import dev.lukebemish.excavatedvariants.impl.ExcavatedVariants;
@@ -19,6 +18,7 @@
 import net.minecraft.world.level.levelgen.feature.Feature;
 import net.minecraft.world.level.levelgen.feature.FeaturePlaceContext;
 import net.minecraft.world.level.levelgen.feature.configurations.NoneFeatureConfiguration;
+import org.jspecify.annotations.NonNull;
 import org.jspecify.annotations.Nullable;
 
 public class OreReplacer extends Feature<NoneFeatureConfiguration> {
@@ -33,7 +33,7 @@ public OreReplacer() {
     }
 
     @Override
-    public boolean place(FeaturePlaceContext<NoneFeatureConfiguration> ctx) {
+    public boolean place(@NonNull FeaturePlaceContext<NoneFeatureConfiguration> ctx) {
         if (!ExcavatedVariants.getConfig().attemptWorldgenReplacement)
             return true;
         return modifyUnmodifiedNeighboringChunks(ctx.level(), ctx.origin());
@@ -43,25 +43,25 @@ public boolean modifyUnmodifiedNeighboringChunks(WorldGenLevel level, BlockPos p
         OreGenMapSavedData data = OreGenMapSavedData.getOrCreate(level);
         int minY = level.getMinBuildHeight();
         int maxY = level.getMaxBuildHeight();
-        if (data.edgeCount.containsKey(new Pair<>(pos.getX(), pos.getZ())) && data.edgeCount.get(new Pair<>(pos.getX(), pos.getZ())) == 8) {
-            ChunkAccess chunkAccess = level.getChunk(pos);
-            modifyChunk(chunkAccess, minY, maxY);
-            data.edgeCount.put(new Pair<>(pos.getX(), pos.getZ()), 9);
-        }
         BlockPos.MutableBlockPos newPos = new BlockPos.MutableBlockPos(pos.getX(), pos.getY(), pos.getZ());
         for (int i = 0; i < xs.length; i++) {
             newPos.setX(pos.getX() + xs[i] * 16);
             newPos.setZ(pos.getZ() + zs[i] * 16);
-            Pair<Integer, Integer> chunkPos = new Pair<>(newPos.getX(), newPos.getZ());
-            if (!data.edgeCount.containsKey(chunkPos)) data.edgeCount.put(chunkPos, 0);
-            data.edgeCount.put(chunkPos, data.edgeCount.get(chunkPos) + 1);
-            if (data.edgeCount.get(chunkPos) == 8 && data.ranMap.containsKey(chunkPos) && data.ranMap.get(chunkPos)) {
+            OreGenMapSavedData.ChunkKey chunkPos = new OreGenMapSavedData.ChunkKey(newPos.getX(), newPos.getZ());
+            data.edgeCount.put(chunkPos, data.edgeCount.getInt(chunkPos) + 1);
+            if (data.edgeCount.getInt(chunkPos) == 8 && data.ranMap.contains(chunkPos)) {
                 ChunkAccess chunkAccess = level.getChunk(newPos);
                 modifyChunk(chunkAccess, minY, maxY);
                 data.edgeCount.put(chunkPos, 9);
             }
         }
-        data.ranMap.put(new Pair<>(pos.getX(), pos.getZ()), true);
+        OreGenMapSavedData.ChunkKey chunkPos = new OreGenMapSavedData.ChunkKey(pos.getX(), pos.getZ());
+        data.ranMap.add(chunkPos);
+        if (data.edgeCount.getInt(chunkPos) == 8) {
+            ChunkAccess chunkAccess = level.getChunk(pos);
+            modifyChunk(chunkAccess, minY, maxY);
+            data.edgeCount.put(chunkPos, 9);
+        }
         return true;
     }
 

common/src/main/resources/defaultresources/globaldata/excavated_variants/excavated_variants/ore/create/zinc_ore.json

@@ -6,7 +6,10 @@
     "en_us": "Zinc Ore"
   },
   "blocks": {
-    "create:zinc_ore": "excavated_variants:minecraft/stone",
+    "create:zinc_ore": {
+      "stone": "excavated_variants:minecraft/stone",
+      "generating": true
+    },
     "create:deepslate_zinc_ore": "excavated_variants:minecraft/deepslate",
     "spelunkery:granite_zinc_ore": {
       "stone": "excavated_variants:minecraft/granite",

common/src/main/resources/defaultresources/globaldata/excavated_variants/excavated_variants/ore/minecraft/coal_ore.json

@@ -6,7 +6,10 @@
     "en_us": "Coal Ore"
   },
   "blocks": {
-    "minecraft:coal_ore": "excavated_variants:minecraft/stone",
+    "minecraft:coal_ore": {
+      "stone": "excavated_variants:minecraft/stone",
+      "generating": true
+    },
     "minecraft:deepslate_coal_ore": "excavated_variants:minecraft/deepslate",
     "darkerdepths:limestone_coal_ore": "excavated_variants:darkerdepths/limestone",
     "darkerdepths:aridrock_coal_ore": "excavated_variants:darkerdepths/aridrock",

common/src/main/resources/defaultresources/globaldata/excavated_variants/excavated_variants/ore/minecraft/copper_ore.json

@@ -6,8 +6,11 @@
     "en_us": "Copper Ore"
   },
   "blocks": {
+    "minecraft:copper_ore": {
+      "stone": "excavated_variants:minecraft/stone",
+      "generating": true
+    },
     "minecraft:deepslate_copper_ore": "excavated_variants:minecraft/deepslate",
-    "minecraft:copper_ore": "excavated_variants:minecraft/stone",
     "unearthed:beige_limestone_copper_ore": "excavated_variants:unearthed/beige_limestone",
     "unearthed:limestone_copper_ore": "excavated_variants:unearthed/limestone",
     "unearthed:rhyolite_copper_ore": "excavated_variants:unearthed/rhyolite",

common/src/main/resources/defaultresources/globaldata/excavated_variants/excavated_variants/ore/minecraft/diamond_ore.json

@@ -6,7 +6,10 @@
     "en_us": "Diamond Ore"
   },
   "blocks": {
-    "minecraft:diamond_ore": "excavated_variants:minecraft/stone",
+    "minecraft:diamond_ore": {
+      "stone": "excavated_variants:minecraft/stone",
+      "generating": true
+    },
     "minecraft:deepslate_diamond_ore": "excavated_variants:minecraft/deepslate",
     "darkerdepths:limestone_diamond_ore": "excavated_variants:darkerdepths/limestone",
     "darkerdepths:aridrock_diamond_ore": "excavated_variants:darkerdepths/aridrock",

common/src/main/resources/defaultresources/globaldata/excavated_variants/excavated_variants/ore/minecraft/emerald_ore.json

@@ -6,7 +6,10 @@
     "en_us": "Emerald Ore"
   },
   "blocks": {
-    "minecraft:emerald_ore": "excavated_variants:minecraft/stone",
+    "minecraft:emerald_ore": {
+      "stone": "excavated_variants:minecraft/stone",
+      "generating": true
+    },
     "minecraft:deepslate_emerald_ore": "excavated_variants:minecraft/deepslate",
     "unearthed:phyllite_emerald_ore": "excavated_variants:unearthed/phyllite",
     "unearthed:white_granite_emerald_ore": "excavated_variants:unearthed/white_granite",

common/src/main/resources/defaultresources/globaldata/excavated_variants/excavated_variants/ore/minecraft/gold_ore.json

@@ -6,7 +6,10 @@
     "en_us": "Gold Ore"
   },
   "blocks": {
-    "minecraft:gold_ore": "excavated_variants:minecraft/stone",
+    "minecraft:gold_ore": {
+      "stone": "excavated_variants:minecraft/stone",
+      "generating": true
+    },
     "minecraft:deepslate_gold_ore": "excavated_variants:minecraft/deepslate",
     "darkerdepths:limestone_gold_ore": "excavated_variants:darkerdepths/limestone",
     "darkerdepths:aridrock_gold_ore": "excavated_variants:darkerdepths/aridrock",

common/src/main/resources/defaultresources/globaldata/excavated_variants/excavated_variants/ore/minecraft/iron_ore.json

@@ -6,7 +6,10 @@
     "en_us": "Iron Ore"
   },
   "blocks": {
-    "minecraft:iron_ore": "excavated_variants:minecraft/stone",
+    "minecraft:iron_ore": {
+      "stone": "excavated_variants:minecraft/stone",
+      "generating": true
+    },
     "minecraft:deepslate_iron_ore": "excavated_variants:minecraft/deepslate",
     "darkerdepths:limestone_iron_ore": "excavated_variants:darkerdepths/limestone",
     "darkerdepths:aridrock_iron_ore": "excavated_variants:darkerdepths/aridrock",

common/src/main/resources/defaultresources/globaldata/excavated_variants/excavated_variants/ore/minecraft/lapis_ore.json

@@ -6,7 +6,10 @@
     "en_us": "Lapis Lazuli Ore"
   },
   "blocks": {
-    "minecraft:lapis_ore": "excavated_variants:minecraft/stone",
+    "minecraft:lapis_ore": {
+      "stone": "excavated_variants:minecraft/stone",
+      "generating": true
+    },
     "minecraft:deepslate_lapis_ore": "excavated_variants:minecraft/deepslate",
     "darkerdepths:aridrock_lapis_ore": "excavated_variants:darkerdepths/aridrock",
     "darkerdepths:limestone_lapis_ore": "excavated_variants:darkerdepths/limestone",