Queue world-data hook registration for Minecraft 1.16.5

.github/workflows/ci.yml

@@ -141,9 +141,7 @@ jobs:
           ./gradlew "${gradle_args[@]}"
           first="$RUNNER_TEMP/basemetals-first"
           mkdir -p "$first"
-          cp build/libs/BaseMetals-3.1.0.116051.jar "$first/"
-          cp build/libs/BaseMetals-3.1.0.116051-sources.jar "$first/"
-          cp build/libs/BaseMetals-3.1.0.116051-javadoc.jar "$first/"
+          cp build/libs/*.jar "$first/"
           cp build/release/SHA256SUMS "$first/"
 
       - name: Rebuild and prove byte-identical archives
@@ -159,9 +157,9 @@ jobs:
             -Dorg.gradle.java.installations.auto-download=false
           )
           ./gradlew "${gradle_args[@]}"
-          cmp "$RUNNER_TEMP/basemetals-first/BaseMetals-3.1.0.116051.jar" build/libs/BaseMetals-3.1.0.116051.jar
-          cmp "$RUNNER_TEMP/basemetals-first/BaseMetals-3.1.0.116051-sources.jar" build/libs/BaseMetals-3.1.0.116051-sources.jar
-          cmp "$RUNNER_TEMP/basemetals-first/BaseMetals-3.1.0.116051-javadoc.jar" build/libs/BaseMetals-3.1.0.116051-javadoc.jar
+          for jar in build/libs/*.jar; do
+            cmp "$RUNNER_TEMP/basemetals-first/${jar##*/}" "$jar"
+          done
           cmp "$RUNNER_TEMP/basemetals-first/SHA256SUMS" build/release/SHA256SUMS
 
       - name: Run exact-OreSpawn dedicated-server probe
@@ -185,9 +183,7 @@ jobs:
           if-no-files-found: error
           retention-days: 30
           path: |
-            build/libs/BaseMetals-3.1.0.116051.jar
-            build/libs/BaseMetals-3.1.0.116051-sources.jar
-            build/libs/BaseMetals-3.1.0.116051-javadoc.jar
+            build/libs/*.jar
             build/release/SHA256SUMS
             CHANGELOG.txt
 

CHANGELOG.txt

@@ -1,3 +1,10 @@
+Base Metals 3.1.1.116051
+
+- Register the old-world hook through Forge's queued common setup, before any
+  world loads. This avoids changing a shared list while mods load in parallel.
+- Keep saved IDs, migration rules, ore generation and gameplay unchanged.
+- Let CI collect and compare the three audited jars without fixed filenames.
+
 Base Metals 3.1.0.116051
 
 - Bring the 1.15.2 release to Minecraft 1.16.5 and Forge 36.2.34.

README.md

@@ -12,11 +12,15 @@ shields, bows, crossbows, arrows, and bolts. The mod also includes Crack
 Hammers, scythes, metal anvils, the human detector, villager trades,
 advancements, and extra loot to find in chests.
 
-This is Base Metals `3.1.0.116051` for Minecraft 1.16.5. It brings the 1.15.2
+This is Base Metals `3.1.1.116051` for Minecraft 1.16.5. It brings the 1.15.2
 release forward while keeping the same blocks, items and recipes. MMDLib and
 Additional Loot Tables are no longer needed. OreSpawn places the ores, and
 Base Metals supplies the metals and their recipes.
 
+Version 3.1.1 registers the old-world hook through Forge's queued common setup.
+This avoids changing Forge's shared hook list while mods load in parallel.
+Saved IDs, migration rules and gameplay are unchanged.
+
 Ores use vanilla stone, netherrack and end-stone textures beneath Kiri's ore
 overlays, so resource packs can change their surroundings without making the
 ores look out of place. Smith trades and village chest loot use the
@@ -33,7 +37,7 @@ are not yet included in the 3.0.1 builds for Minecraft 1.13–1.15.
 - Java 8
 - OreSpawn `[4.1.0.116051,5.0.0)` on both client and server
 
-Put `BaseMetals-3.1.0.116051.jar` and OreSpawn `4.1.0.116051` (or a later
+Put `BaseMetals-3.1.1.116051.jar` and OreSpawn `4.1.0.116051` (or a later
 compatible OreSpawn 4 build for 1.16.5) in the `mods` directory. MMDLib and
 Additional Loot Tables are not dependencies. Mineralogy is optional.
 
@@ -210,7 +214,7 @@ and passes its temporary repository through `orespawnVerificationRepository`.
 Normal development does not need a local OreSpawn checkout or repository.
 
 Release artifacts use Maven coordinate
-`zone.moddev.mc.basemetals:BaseMetals:3.1.0.116051`.
+`zone.moddev.mc.basemetals:BaseMetals:3.1.1.116051`.
 
 Report defects through the
 [Base Metals issue tracker](https://github.com/MinecraftModDevelopmentMods/BaseMetals/issues).

docs/MIGRATION.md

@@ -4,9 +4,11 @@ Minecraft 1.13 introduced the flattening, which replaced numeric block and item
 identifiers with names and palette-based block states. Third-party numeric
 blocks cannot be reconstructed by Mojang's vanilla fixer alone, so Base Metals
 installs a narrowly scoped migration hook before Forge reads an old world.
+The hook is registered through queued common setup, after parallel mod loading.
+Only registration timing has changed; saved IDs and migration rules are unchanged.
 
 Always upgrade a copy and retain the original 1.10/1.12 world. Install Base
-Metals `3.1.0.116051`, OreSpawn `4.1.0.116051`, and any other 1.16.5 mods needed
+Metals `3.1.1.116051`, OreSpawn `4.1.0.116051`, and any other 1.16.5 mods needed
 by that world. Do not carry MMDLib, Additional Loot Tables, the OreSpawn 3
 plugin, or Base Metals' old fallback generator into the new instance.
 

docs/VERSIONS.md

@@ -1,6 +1,6 @@
 # Supported versions
 
-Base Metals `3.1.0.116051` targets Minecraft `1.16.5`, Forge `36.2.34`, and
+Base Metals `3.1.1.116051` targets Minecraft `1.16.5`, Forge `36.2.34`, and
 Java 8. The Gradle build itself runs on Java `17.0.1+12` and uses ForgeGradle
 `7.0.34` with Gradle `9.6.1`; those build-time runtimes do not change the Java
 8 requirement of the released mod.
@@ -17,4 +17,4 @@ file before staging it in a temporary Maven repository.
 The schema-3 provider supports the OreSpawn versions in the runtime range above.
 
 Release artifacts use Maven coordinate
-`zone.moddev.mc.basemetals:BaseMetals:3.1.0.116051`.
+`zone.moddev.mc.basemetals:BaseMetals:3.1.1.116051`.

gradle.properties

@@ -40,7 +40,7 @@ mcp_version=20210115.111550
 mod_id=basemetals
 mod_name=Base Metals
 mod_license=LGPL-2.1
-mod_version=3.1.0.116051
+mod_version=3.1.1.116051
 
 # Maven coordinates and credits.
 mod_group=zone.moddev.mc.basemetals

gradle/release/artifacts.gradle

@@ -131,7 +131,7 @@ tasks.register('verifyReleaseConfiguration') {
                 loaderCode: project.loader_code.toString(), javaVersion: project.java_version.toString(),
                 gradleJavaVersion: project.gradle_java_version.toString(),
                 curseForgeProjectId: project.curseforge_project_id.toString()]
-        def expected = [modVersion: '3.1.0.116051', modGroup: expectedMavenGroup,
+        def expected = [modVersion: '3.1.1.116051', modGroup: expectedMavenGroup,
                 minecraftVersion: '1.16.5', forgeVersion: '36.2.34',
                 forgeInstallerSha256: '53B73DBBCC486F38726E76CE923B13381044984C83C762847C7193340F97130F',
                 mappingChannel: 'official', mappingVersion: '1.16.5',
@@ -204,7 +204,7 @@ tasks.register('verifyReleaseArtifacts') {
     dependsOn tasks.named('assemble')
     doLast {
         File libs = file("${buildDir}/libs")
-        def actualArtifacts = fileTree(libs) { include 'BaseMetals-*.jar' }.files.collect { it.name }.sort()
+        def actualArtifacts = fileTree(libs) { include '*.jar' }.files.collect { it.name }.sort()
         if (actualArtifacts != expectedReleaseNames.sort()) {
             throw new GradleException("Expected exactly ${expectedReleaseNames.sort()}, found ${actualArtifacts}")
         }
@@ -256,7 +256,7 @@ tasks.register('verifyReleaseArtifacts') {
                 throw new GradleException('Release manifest contains the wrong identity')
             }
             String modsToml = zip.getInputStream(zip.getEntry('META-INF/mods.toml')).getText('UTF-8')
-            if (modsToml.contains('${') || !modsToml.contains('version="3.1.0.116051"')
+            if (modsToml.contains('${') || !modsToml.contains('version="3.1.1.116051"')
                     || !modsToml.contains('versionRange="[4.1.0.116051,5.0.0)"')
                     || !modsToml.contains('loaderVersion="[36,)"')
                     || !modsToml.contains('versionRange="[36,)"')

src/clientIntegrationTest/resources/META-INF/mods.toml

@@ -11,7 +11,7 @@ description='''Build-only packaged-client checks for Base Metals 1.16.5.'''
 [[dependencies.basemetalsclientprobe]]
 modId="basemetals"
 mandatory=true
-versionRange="[3.1.0.116051]"
+versionRange="[3.1.1.116051]"
 ordering="AFTER"
 side="CLIENT"
 

src/integrationTest/resources/META-INF/mods.toml

@@ -11,7 +11,7 @@ description='''Build-only lifecycle checks for Base Metals 1.16.5.'''
 [[dependencies.basemetalsprobe]]
 modId="basemetals"
 mandatory=true
-versionRange="[3.1.0.116051]"
+versionRange="[3.1.1.116051]"
 ordering="AFTER"
 side="BOTH"
 

src/main/java/zone/moddev/mc/basemetals/BaseMetals.java

@@ -18,6 +18,7 @@
 import net.minecraftforge.fml.ModLoadingContext;
 import net.minecraftforge.fml.common.Mod;
 import net.minecraftforge.fml.config.ModConfig;
+import net.minecraftforge.fml.event.lifecycle.FMLCommonSetupEvent;
 import net.minecraftforge.fml.javafmlmod.FMLJavaModLoadingContext;
 
 @Mod(BaseMetals.MOD_ID)
@@ -27,14 +28,19 @@ public final class BaseMetals {
 
     public BaseMetals() {
         ModTags.initialize();
-        LegacyWorldDataHook.register();
         ContentModeNetwork.register();
         ModContent.initializeFluids();
         ContentModeLootCondition.register();
         ModEntities.initialize();
         ModLoadingContext.get().registerConfig(ModConfig.Type.COMMON, BaseMetalsConfig.SPEC);
         FMLJavaModLoadingContext.get().getModEventBus().addListener(BaseMetalsConfig::onConfigLoading);
+        FMLJavaModLoadingContext.get().getModEventBus().addListener(this::commonSetup);
         MinecraftForge.EVENT_BUS.register(new BaseMetalsEvents());
         DistExecutor.runWhenOn(Dist.CLIENT, () -> ClientSetup::register);
     }
+
+    private void commonSetup(FMLCommonSetupEvent event) {
+        // Forge's shared hook list cannot be mutated safely during parallel mod loading.
+        event.enqueueWork(LegacyWorldDataHook::register);
+    }
 }

src/main/resources/META-INF/mods.toml

@@ -5,7 +5,7 @@ issueTrackerURL="https://github.com/MinecraftModDevelopmentMods/BaseMetals/issue
 
 [[mods]]
 modId="basemetals"
-version="3.1.0.116051"
+version="3.1.1.116051"
 displayName="Base Metals"
 displayURL="https://www.curseforge.com/minecraft/mc-mods/base-metals"
 authors="Cyanobacterium, jriwanek, dshadowwolf, GetDizzy611, SkyBlade1978, the MMD Team"

src/test/java/zone/moddev/mc/basemetals/Native116ContractTest.java

@@ -9,9 +9,49 @@
 import java.nio.file.Files;
 import java.nio.file.Paths;
 import java.util.Properties;
+import java.util.regex.Pattern;
 import org.junit.jupiter.api.Test;
 
 class Native116ContractTest {
+    @Test
+    void keepsPatchVersionMetadataAndAuditsInSync() throws Exception {
+        Properties properties = new Properties();
+        try (InputStream input = Files.newInputStream(Paths.get("gradle.properties"))) {
+            properties.load(input);
+        }
+        String version = properties.getProperty("mod_version");
+        assertEquals("3.1.1.116051", version);
+        assertTrue(source("src/main/resources/META-INF/mods.toml").contains("version=\"" + version + "\""));
+        for (String sourceSet : new String[] {"integrationTest", "clientIntegrationTest"}) {
+            assertTrue(source("src/" + sourceSet + "/resources/META-INF/mods.toml")
+                    .contains("versionRange=\"[" + version + "]\""), sourceSet);
+        }
+        String artifacts = source("gradle/release/artifacts.gradle");
+        assertTrue(artifacts.contains("modVersion: '" + version + "'"));
+        assertTrue(artifacts.contains("contains('version=\"" + version + "\"')"));
+        for (String doc : new String[] {"README.md", "docs/VERSIONS.md", "docs/MIGRATION.md"}) {
+            String text = source(doc);
+            assertTrue(text.contains(version), doc);
+            assertFalse(text.contains("3.1.0.116051"), doc);
+        }
+        assertTrue(source("CHANGELOG.txt").startsWith("Base Metals " + version));
+    }
+
+    @Test
+    void collectsAndComparesOnlyTheStrictlyAuditedJarsInCi() throws Exception {
+        String ci = source(".github/workflows/ci.yml");
+        assertFalse(Pattern.compile("BaseMetals-\\d+\\.\\d+\\.\\d+\\.\\d+(?:-(?:sources|javadoc))?\\.jar")
+                .matcher(ci).find(), "CI must not pin artifact filenames to a previous patch");
+        assertTrue(ci.contains("cp build/libs/*.jar \"$first/\""));
+        assertTrue(ci.contains("for jar in build/libs/*.jar; do"));
+        assertTrue(ci.contains("cmp \"$RUNNER_TEMP/basemetals-first/${jar##*/}\" \"$jar\""));
+        assertTrue(ci.contains("            build/libs/*.jar"));
+        assertTrue(ci.indexOf("verifyReleaseArtifacts") < ci.indexOf("cp build/libs/*.jar"));
+        String artifacts = source("gradle/release/artifacts.gradle");
+        assertTrue(artifacts.contains("fileTree(libs) { include '*.jar' }"));
+        assertTrue(artifacts.contains("actualArtifacts != expectedReleaseNames.sort()"));
+    }
+
     @Test
     void targetsForge36InEveryModDescriptor() throws Exception {
         for (String sourceSet : new String[] {"main", "integrationTest", "clientIntegrationTest"}) {

src/test/java/zone/moddev/mc/basemetals/migration/PersistenceRegistrationThreadTest.java

@@ -0,0 +1,111 @@
+package zone.moddev.mc.basemetals.migration;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+import java.io.InputStream;
+import org.junit.jupiter.api.Test;
+import org.objectweb.asm.ClassReader;
+import org.objectweb.asm.Handle;
+import org.objectweb.asm.Opcodes;
+import org.objectweb.asm.tree.AbstractInsnNode;
+import org.objectweb.asm.tree.ClassNode;
+import org.objectweb.asm.tree.InvokeDynamicInsnNode;
+import org.objectweb.asm.tree.MethodInsnNode;
+import org.objectweb.asm.tree.MethodNode;
+
+class PersistenceRegistrationThreadTest {
+    private static final String ENTRY_POINT = "zone/moddev/mc/basemetals/BaseMetals";
+    private static final String HOOK = "zone/moddev/mc/basemetals/migration/LegacyWorldDataHook";
+    private static final String SETUP_EVENT = "net/minecraftforge/fml/event/lifecycle/FMLCommonSetupEvent";
+    private static final String SETUP_DESCRIPTOR = "(L" + SETUP_EVENT + ";)V";
+
+    @Test
+    void neverRegistersTheSharedWorldHookDirectly() throws Exception {
+        for (MethodNode method : entryPoint().methods) {
+            for (AbstractInsnNode instruction : method.instructions.toArray()) {
+                if (instruction instanceof MethodInsnNode) {
+                    MethodInsnNode call = (MethodInsnNode) instruction;
+                    assertFalse(call.owner.equals(HOOK) && call.name.equals("register"),
+                            "Do not mutate Forge's shared hook ArrayList during parallel mod loading: "
+                                    + method.name);
+                }
+            }
+        }
+    }
+
+    @Test
+    void subscribesQueuedCommonSetupOnceOnTheModEventBus() throws Exception {
+        int listeners = 0;
+        for (MethodNode method : entryPoint().methods) {
+            for (AbstractInsnNode instruction : method.instructions.toArray()) {
+                if (!(instruction instanceof MethodInsnNode)) continue;
+                MethodInsnNode call = (MethodInsnNode) instruction;
+                if (call.owner.equals("net/minecraftforge/eventbus/api/IEventBus")
+                        && call.name.equals("addListener")
+                        && references(previousCode(call), ENTRY_POINT, "commonSetup", SETUP_DESCRIPTOR)) {
+                    assertEquals("<init>", method.name);
+                    listeners++;
+                }
+            }
+        }
+        assertEquals(1, listeners, "The mod event bus must receive common setup exactly once");
+    }
+
+    @Test
+    void registersTheHookOnceThroughNativeEnqueueWork() throws Exception {
+        int registrations = 0;
+        for (MethodNode method : entryPoint().methods) {
+            for (AbstractInsnNode instruction : method.instructions.toArray()) {
+                if (!(instruction instanceof InvokeDynamicInsnNode)
+                        || !references(instruction, HOOK, "register", "()V")) continue;
+                assertEquals("commonSetup", method.name);
+                assertEquals(SETUP_DESCRIPTOR, method.desc);
+                assertEquals("()Ljava/lang/Runnable;", ((InvokeDynamicInsnNode) instruction).desc);
+                AbstractInsnNode next = nextCode(instruction);
+                assertNotNull(next);
+                assertEquals(Opcodes.INVOKEVIRTUAL, next.getOpcode());
+                MethodInsnNode enqueue = (MethodInsnNode) next;
+                assertEquals(SETUP_EVENT, enqueue.owner);
+                assertEquals("enqueueWork", enqueue.name);
+                assertEquals("(Ljava/lang/Runnable;)Ljava/util/concurrent/CompletableFuture;", enqueue.desc);
+                registrations++;
+            }
+        }
+        assertEquals(1, registrations, "Queue one registration after parallel setup, before world loading");
+    }
+
+    private static ClassNode entryPoint() throws Exception {
+        ClassNode node = new ClassNode();
+        try (InputStream input = PersistenceRegistrationThreadTest.class.getResourceAsStream(
+                "/" + ENTRY_POINT + ".class")) {
+            assertNotNull(input);
+            new ClassReader(input).accept(node, 0);
+        }
+        return node;
+    }
+
+    private static boolean references(AbstractInsnNode instruction, String owner, String name, String descriptor) {
+        if (!(instruction instanceof InvokeDynamicInsnNode)) return false;
+        for (Object argument : ((InvokeDynamicInsnNode) instruction).bsmArgs) {
+            if (!(argument instanceof Handle)) continue;
+            Handle handle = (Handle) argument;
+            if (handle.getOwner().equals(owner) && handle.getName().equals(name)
+                    && handle.getDesc().equals(descriptor)) return true;
+        }
+        return false;
+    }
+
+    private static AbstractInsnNode previousCode(AbstractInsnNode instruction) {
+        do { instruction = instruction.getPrevious(); }
+        while (instruction != null && instruction.getOpcode() < 0);
+        return instruction;
+    }
+
+    private static AbstractInsnNode nextCode(AbstractInsnNode instruction) {
+        do { instruction = instruction.getNext(); }
+        while (instruction != null && instruction.getOpcode() < 0);
+        return instruction;
+    }
+}