Fix OreSpawn settings crash on Minecraft 1.15.2 (4.1.1.115021)

.github/workflows/ci.yml

@@ -126,9 +126,9 @@ jobs:
           if-no-files-found: error
           retention-days: 30
           path: |
-            build/libs/OreSpawn-4.1.0.115021.jar
-            build/libs/OreSpawn-4.1.0.115021-sources.jar
-            build/libs/OreSpawn-4.1.0.115021-javadoc.jar
+            build/libs/OreSpawn-4.1.1.115021.jar
+            build/libs/OreSpawn-4.1.1.115021-sources.jar
+            build/libs/OreSpawn-4.1.1.115021-javadoc.jar
             build/release/SHA256SUMS
             CHANGELOG.txt
 

CHANGELOG.txt

@@ -1,3 +1,8 @@
+Version 4.1.1.115021
+
+* Fixed a crash when opening or editing OreSpawn's settings in development runs.
+  The fix has also been checked in a normal Forge installation.
+
 Version 4.1.0.115021
 
 * Add a directory for loaded provider mods and optional add-on settings screens.

README.md

@@ -17,11 +17,14 @@ End" policy used by mods such as Base Metals.
 This is not the unrelated mod that adds mobs and dimensions under the same
 name.
 
-This branch builds target-qualified version `4.1.0.115021` for Minecraft 1.15.2
+This branch builds target-qualified version `4.1.1.115021` for Minecraft 1.15.2
 and Forge. It adds material-group ore arbitration, exact biome replacements,
 and a directory for installed provider mods to the OreSpawn 4.0.16 base. See the
 [versioning policy](docs/VERSIONS.md) for the encoding and release convention.
 
+I've fixed a crash when opening or editing OreSpawn's settings in development
+runs. The fix has also been checked in a normal Forge installation.
+
 ## What Happens When It Is Installed?
 
 OreSpawn is deliberately passive on its own. It does not replace stone, remove

docs/VERSIONS.md

@@ -52,8 +52,8 @@ Examples:
 | Minecraft | Loader | Target | Example full OreSpawn version |
 | --- | --- | ---: | --- |
 | 1.13.2 | Forge | `113021` | `4.1.0.113021` |
-| 1.14.4 | Forge | `114041` | `4.1.0.114041` |
-| 1.15.2 | Forge | `115021` | `4.1.0.115021` |
+| 1.14.4 | Forge | `114041` | `4.1.1.114041` |
+| 1.15.2 | Forge | `115021` | `4.1.1.115021` |
 | 1.16.5 | Forge | `116051` | `4.0.16.116051` |
 | 1.20.6 | Forge | `120061` | `4.0.6.120061` |
 | 1.21.11 | Forge | `121111` | `4.0.6.121111` |
@@ -163,11 +163,14 @@ GameTest harness, so it keeps its ordinary dedicated benchmark auto-stop without
 a compatibility shim. A branch may therefore legitimately skip functional
 version numbers.
 
-The 1.15.2 line now advances to 4.1.0 for material-group ore policies,
+The 1.15.2 line advances to 4.1.0 for material-group ore policies,
 the loaded-mod directory, and exact new-terrain biome replacements. It keeps
-the `114041` target suffix, API major 1, and the target's existing registry
+the `115021` target suffix, API major 1, and the target's existing registry
 identities while provider/global/world schemas advance to 5/8/7.
 
+Version 4.1.1 fixes a crash in the settings screens during development runs.
+The same fix is included in the Forge 1.14.4 and 1.15.2 releases.
+
 This provides three useful guarantees:
 
 1. A functional version is not used to describe two unrelated change sets.

gradle.properties

@@ -19,7 +19,7 @@ mcp_version=20200515.085601
 mod_id=orespawn
 mod_name=MMD OreSpawn
 mod_license=LGPL-2.1
-mod_version=4.1.0.115021
+mod_version=4.1.1.115021
 mod_group=zone.moddev.mc.orespawn
 mod_authors=SkyBlade1978, dshadowwolf, the MMD Team
 mod_description=Configurable, provider-driven terrain, ore, and deposit generation.

gradle/release/artifacts.gradle

@@ -90,7 +90,7 @@ def preparedReleaseDir = providers.gradleProperty('preparedReleaseDir')
 tasks.register('verifyReleaseConfiguration') {
     group = 'verification'
     doLast {
-        if (project.mod_version != '4.1.0.115021'
+        if (project.mod_version != '4.1.1.115021'
                 || project.mod_group != expectedMavenGroup
                 || project.minecraft_version != '1.15.2'
                 || project.forge_version != '31.2.57'
@@ -104,9 +104,9 @@ tasks.register('verifyReleaseConfiguration') {
             throw new GradleException('Unexpected dispatcher or Java target metadata')
         }
         List<String> expectedPublicArtifacts = [
-                'OreSpawn-4.1.0.115021.jar',
-                'OreSpawn-4.1.0.115021-sources.jar',
-                'OreSpawn-4.1.0.115021-javadoc.jar'
+                'OreSpawn-4.1.1.115021.jar',
+                'OreSpawn-4.1.1.115021-sources.jar',
+                'OreSpawn-4.1.1.115021-javadoc.jar'
         ]
         if (base.archivesName.get() != expectedMavenArtifact
                 || expectedReleaseFiles.get().collect { it.toString() } != expectedPublicArtifacts) {
@@ -123,7 +123,7 @@ tasks.register('verifyReleaseConfiguration') {
                 'src/main/java/zone/moddev/mc/orespawn/worldgen/LegacyMineralogyProfileMigration.java',
                 'README.md', 'CHANGELOG.txt'
         ].each { path ->
-            if (!file(path).getText('UTF-8').contains('4.1.0.115021')) {
+            if (!file(path).getText('UTF-8').contains('4.1.1.115021')) {
                 throw new GradleException("Release identity missing from ${path}")
             }
         }

src/main/java/zone/moddev/mc/orespawn/client/TextFieldWidget.java

@@ -14,15 +14,17 @@ class TextFieldWidget extends net.minecraft.client.gui.widget.TextFieldWidget {
 	}
 
 	void setValue(String value) {
-		setText(value);
+		// Official remapping gives the native method our alias name. Keep this
+		// a superclass call so imported jars cannot dispatch back into this bridge.
+		super.setText(value);
 	}
 
 	String getValue() {
-		return getText();
+		return super.getText();
 	}
 
 	void setMaxLength(int length) {
-		setMaxStringLength(length);
+		super.setMaxStringLength(length);
 	}
 
 	void setBounds(int x, int y, int width, int height) {

src/main/java/zone/moddev/mc/orespawn/worldgen/LegacyConfigMigrator.java

@@ -359,7 +359,7 @@ private static void writeReport(Path config, List<String> lines) {
 
 	private static void writeUpgradeReport(Path config, int imported, List<String> detail) {
 		List<String> lines = new ArrayList<>();
-		lines.add("OreSpawn 4.1.0.115021 Upgrade Report");
+		lines.add("OreSpawn 4.1.1.115021 Upgrade Report");
 		lines.add("================================");
 		lines.add("");
 		lines.add("RESULT: Legacy OreSpawn settings were imported into the OS4 profile.");

src/main/java/zone/moddev/mc/orespawn/worldgen/LegacyMineralogyProfileMigration.java

@@ -199,7 +199,7 @@ private static void writeUpgradeReport(Path worldRoot, Path configPath,
         Path report = worldRoot.resolve("serverconfig/orespawn-upgrade-report.txt");
         List<String> missing = missingBlocks(igneous, metamorphic, sedimentary);
         List<String> lines = new ArrayList<>();
-        lines.add("OreSpawn 4.1.0.115021 Upgrade Report");
+        lines.add("OreSpawn 4.1.1.115021 Upgrade Report");
         lines.add("================================");
         lines.add("");
         lines.add("RESULT: Existing Mineralogy " + identity.version + " world detected.");

src/test/java/zone/moddev/mc/orespawn/client/TextFieldRemappingTest.java

@@ -0,0 +1,129 @@
+package zone.moddev.mc.orespawn.client;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.io.InputStream;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.Method;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+import org.objectweb.asm.ClassReader;
+import org.objectweb.asm.ClassVisitor;
+import org.objectweb.asm.ClassWriter;
+import org.objectweb.asm.MethodVisitor;
+import org.objectweb.asm.Opcodes;
+
+import net.minecraft.client.gui.FontRenderer;
+
+/** Exercises the name collision introduced when another mod uses official mappings. */
+public class TextFieldRemappingTest {
+	private static final String WRAPPER = TextFieldWidget.class.getName().replace('.', '/');
+	private static final String NATIVE = "net/minecraft/client/gui/widget/TextFieldWidget";
+	private static final String REMAPPED_NATIVE = OfficialTextField.class.getName().replace('.', '/');
+
+	@ParameterizedTest
+	@ValueSource(strings = { "setValue", "getValue", "setMaxLength" })
+	void eachOfficialNameStillDelegatesToTheNativeWidget(String method) throws Exception {
+		Object field = remappedField();
+		if (method.equals("setValue")) {
+			invoke(field.getClass(), field, method, String.class, "minecraft:stone");
+			assertEquals("minecraft:stone", ((OfficialTextField) field).value);
+		} else if (method.equals("setMaxLength")) {
+			invoke(field.getClass(), field, method, int.class, 1024);
+			assertEquals(1024, ((OfficialTextField) field).maximum);
+		} else {
+			assertEquals("", readValue(field.getClass(), field));
+		}
+	}
+
+	@Test
+	void remappedWidgetPreservesLongValuesAndNativeLimits() throws Exception {
+		Object field = remappedField();
+		Class<?> remapped = field.getClass();
+		String value = "minecraft:stone,minecraft:granite,minecraft:diorite,minecraft:andesite";
+
+		invoke(remapped, field, "setMaxLength", int.class, 1024);
+		invoke(remapped, field, "setValue", String.class, value);
+		assertEquals(value, readValue(remapped, field));
+		invoke(remapped, field, "setMaxLength", int.class, 12);
+		assertEquals(value.substring(0, 12), readValue(remapped, field));
+		invoke(remapped, field, "setValue", String.class, "minecraft:granite");
+		assertEquals("minecraft:gr", readValue(remapped, field));
+	}
+
+	private Object remappedField() throws Exception {
+		// Use the real compiled wrapper, remapping the three native MCP names as
+		// ForgeGradle does when importing an SRG jar into an official-mapped project.
+		ClassWriter writer = new ClassWriter(0);
+		try (InputStream input = TextFieldWidget.class.getResourceAsStream("TextFieldWidget.class")) {
+			new ClassReader(input).accept(new ClassVisitor(Opcodes.ASM6, writer) {
+				@Override
+				public void visit(int version, int access, String name, String signature,
+						String superName, String[] interfaces) {
+					super.visit(version, access, name, signature, REMAPPED_NATIVE, interfaces);
+				}
+
+				@Override
+				public MethodVisitor visitMethod(int access, String name, String descriptor,
+						String signature, String[] exceptions) {
+					return new MethodVisitor(Opcodes.ASM6,
+							super.visitMethod(access, name, descriptor, signature, exceptions)) {
+						@Override
+						public void visitMethodInsn(int opcode, String owner, String name,
+								String descriptor, boolean isInterface) {
+							if (owner.equals(WRAPPER) || owner.equals(NATIVE)) {
+								if (name.equals("setText")) name = "setValue";
+								if (name.equals("getText")) name = "getValue";
+								if (name.equals("setMaxStringLength")) name = "setMaxLength";
+							}
+							super.visitMethodInsn(opcode, owner.equals(NATIVE) ? REMAPPED_NATIVE : owner,
+								name, descriptor, isInterface);
+						}
+					};
+				}
+			}, 0);
+		}
+		Class<?> remapped = new ClassLoader(getClass().getClassLoader()) {
+			Class<?> defineWrapper() {
+				byte[] bytes = writer.toByteArray();
+				return defineClass(TextFieldWidget.class.getName(), bytes, 0, bytes.length);
+			}
+		}.defineWrapper();
+		Constructor<?> constructor = remapped.getDeclaredConstructor(FontRenderer.class,
+				int.class, int.class, int.class, int.class, String.class);
+		constructor.setAccessible(true);
+		return constructor.newInstance(null, 0, 0, 200, 20, "host_blocks");
+	}
+
+	private static void invoke(Class<?> type, Object field, String name, Class<?> parameter,
+			Object value) throws Exception {
+		Method method = type.getDeclaredMethod(name, parameter);
+		method.setAccessible(true);
+		method.invoke(field, value);
+	}
+
+	private static Object readValue(Class<?> type, Object field) throws Exception {
+		Method method = type.getDeclaredMethod("getValue");
+		method.setAccessible(true);
+		return method.invoke(field);
+	}
+
+	/** Small native-widget stand-in: only naming and virtual dispatch matter here. */
+	public static class OfficialTextField {
+		public int x, y, width, height;
+		private int maximum = 32;
+		private String value = "";
+
+		public OfficialTextField(FontRenderer font, int x, int y, int width, int height, String label) { }
+		public void setValue(String value) {
+			this.value = value.substring(0, Math.min(maximum, value.length()));
+		}
+		public String getValue() { return value; }
+		public void setMaxLength(int maximum) {
+			this.maximum = maximum;
+			value = value.substring(0, Math.min(maximum, value.length()));
+		}
+	}
+}