Fix world-data hook registration for Minecraft 1.15.2

.github/workflows/ci.yml

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

CHANGELOG.txt

@@ -1,3 +1,12 @@
+Base Metals 3.0.2.115021
+
+- Fix an intermittent world loading and saving failure when other mods also
+  register world-data hooks. Register our migration hook through Forge's queued
+  setup instead of changing its shared hook list during parallel mod loading.
+- Keep existing world data, migration rules, registries and gameplay unchanged.
+- Collect the audited release jars in CI without repeating their version in
+  copy and comparison commands.
+
 Base Metals 3.0.1.115021
 
 - Bring Base Metals to Minecraft 1.15.2 and Forge 31.2.57.

README.md

@@ -12,7 +12,7 @@ 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.0.1.115021` for Minecraft 1.15.2. It brings the 1.14.4
+This is Base Metals `3.0.2.115021` for Minecraft 1.15.2. It brings the 1.14.4
 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.
@@ -29,7 +29,7 @@ Armorer, Weaponsmith and Toolsmith professions introduced in Minecraft 1.14.
 - Java 8
 - OreSpawn `[4.1.0.115021,5.0.0)` on both client and server
 
-Put `BaseMetals-3.0.1.115021.jar` and OreSpawn `4.1.0.115021` (or a later
+Put `BaseMetals-3.0.2.115021.jar` and OreSpawn `4.1.0.115021` (or a later
 compatible OreSpawn 4 build for 1.15.2) in the `mods` directory. MMDLib and
 Additional Loot Tables are not dependencies. Mineralogy is optional.
 
@@ -204,7 +204,7 @@ the temporary repository through `orespawnVerificationRepository`. Local
 candidate builds remain guarded against publication.
 
 Release artifacts use Maven coordinate
-`zone.moddev.mc.basemetals:BaseMetals:3.0.1.115021`.
+`zone.moddev.mc.basemetals:BaseMetals:3.0.2.115021`.
 
 Report defects through the
 [Base Metals issue tracker](https://github.com/MinecraftModDevelopmentMods/BaseMetals/issues).

docs/MIGRATION.md

@@ -6,7 +6,7 @@ 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.
 
 Always upgrade a copy and retain the original 1.10/1.12 world. Install Base
-Metals `3.0.1.115021`, OreSpawn `4.1.0.115021`, and any other 1.15.2 mods needed
+Metals `3.0.2.115021`, OreSpawn `4.1.0.115021`, and any other 1.15.2 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.0.1.115021` targets Minecraft `1.15.2`, Forge `31.2.57`, and
+Base Metals `3.0.2.115021` targets Minecraft `1.15.2`, Forge `31.2.57`, 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.
@@ -16,4 +16,4 @@ CI stages the same checksum-verified file 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.0.1.115021`.
+`zone.moddev.mc.basemetals:BaseMetals:3.0.2.115021`.

gradle.properties

@@ -40,7 +40,7 @@ mcp_version=20200515.085601
 mod_id=basemetals
 mod_name=Base Metals
 mod_license=LGPL-2.1
-mod_version=3.0.1.115021
+mod_version=3.0.2.115021
 
 # 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.0.1.115021', modGroup: expectedMavenGroup,
+        def expected = [modVersion: '3.0.2.115021', modGroup: expectedMavenGroup,
                 minecraftVersion: '1.15.2', forgeVersion: '31.2.57',
                 forgeInstallerSha256: '84FD49A743978421B26F62AC78222289FE892A46F996CF11CB5E05B7441A7773',
                 mappingChannel: 'snapshot', mappingVersion: '20200514-1.15.1',
@@ -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.0.1.115021"')
+            if (modsToml.contains('${') || !modsToml.contains('version="3.0.2.115021"')
                     || !modsToml.contains('versionRange="[4.1.0.115021,5.0.0)"')) {
                 throw new GradleException('Release mods.toml contains unresolved or incompatible metadata')
             }

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

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

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

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

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

@@ -16,9 +16,11 @@
 import net.minecraftforge.api.distmarker.Dist;
 import net.minecraftforge.common.MinecraftForge;
 import net.minecraftforge.fml.DistExecutor;
+import net.minecraftforge.fml.DeferredWorkQueue;
 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 +29,19 @@ public final class BaseMetals {
     public static final Logger LOGGER = LogManager.getLogger(MOD_ID);
 
     public BaseMetals() {
-        LegacyWorldDataHook.register();
         ContentModeNetwork.register();
         ModContent.initializeFluids();
         LootConditionManager.registerCondition(new ContentModeLootCondition.Serializer());
         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 is not safe to mutate during parallel mod construction.
+        DeferredWorkQueue.runLater(LegacyWorldDataHook::register);
+    }
 }

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

@@ -5,7 +5,7 @@ issueTrackerURL="https://github.com/SkyBlade1978/BaseMetals/issues"
 
 [[mods]]
 modId="basemetals"
-version="3.0.1.115021"
+version="3.0.2.115021"
 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/Native115ContractTest.java

@@ -44,6 +44,16 @@ void pinsThePublishedOreSpawnRelease() throws Exception {
         assertTrue(source("build.gradle").contains("curse.maven:${orespawnModule}:${orespawn_curse_file_id}"));
     }
 
+    @Test
+    void ciCollectsTheAuditedJarsWithoutRepeatingTheReleaseVersion() throws Exception {
+        String workflow = source(".github/workflows/ci.yml");
+        assertTrue(workflow.contains("cp build/libs/*.jar \"$first/\""));
+        assertTrue(workflow.contains("for jar in \"$RUNNER_TEMP\"/basemetals-first/*.jar; do"));
+        assertFalse(workflow.contains("BaseMetals-3.0.1.115021"));
+        assertFalse(workflow.contains("BaseMetals-3.0.2.115021"));
+        assertTrue(workflow.contains("verifyReleaseArtifacts writeReleaseChecksums"));
+    }
+
     @Test
     void keepsTheLocalCandidatePublicationGuard() throws Exception {
         assertTrue(source("gradle/stage-orespawn-release.sh")

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

@@ -0,0 +1,50 @@
+package zone.moddev.mc.basemetals.migration;
+
+import java.io.InputStream;
+import org.junit.jupiter.api.Test;
+import org.objectweb.asm.ClassReader;
+import org.objectweb.asm.Handle;
+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;
+import static org.junit.jupiter.api.Assertions.*;
+
+class PersistenceRegistrationThreadTest {
+    @Test
+    void registersTheSharedWorldHookOnceThroughQueuedCommonSetup() throws Exception {
+        ClassNode node = new ClassNode();
+        try (InputStream input = getClass().getResourceAsStream(
+                "/zone/moddev/mc/basemetals/BaseMetals.class")) {
+            assertNotNull(input);
+            new ClassReader(input).accept(node, 0);
+        }
+        String hook = "zone/moddev/mc/basemetals/migration/LegacyWorldDataHook";
+        int queuedRegistrations = 0;
+        for (MethodNode method : node.methods) {
+            boolean hookRunnable = false;
+            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 hook ArrayList during parallel mod construction");
+                    if (call.owner.equals("net/minecraftforge/fml/DeferredWorkQueue")
+                            && call.name.equals("runLater") && hookRunnable) {
+                        assertEquals("commonSetup", method.name);
+                        queuedRegistrations++;
+                        hookRunnable = false;
+                    }
+                }
+                if (instruction instanceof InvokeDynamicInsnNode) {
+                    for (Object argument : ((InvokeDynamicInsnNode) instruction).bsmArgs) {
+                        if (argument instanceof Handle && ((Handle) argument).getOwner().equals(hook)
+                                && ((Handle) argument).getName().equals("register")) hookRunnable = true;
+                    }
+                }
+            }
+        }
+        assertEquals(1, queuedRegistrations,
+                "Register once after parallel setup, before any world can load");
+    }
+}