diff --git a/JAVA_MIGRATION.md b/JAVA_MIGRATION.md index 9719887..d192385 100644 --- a/JAVA_MIGRATION.md +++ b/JAVA_MIGRATION.md @@ -441,3 +441,9 @@ These ranges were formed before designing the characterization suite and should - Recorded the consequence for consumers: Scala code that recompiles must extend `JCuboidPart` or declare the overrides itself. This belongs in the release notes for the first Java release, not only in the divergence log. - Found a build-order constraint: `compileJava` runs before `compileScala`, so ported Java that still references Scala types must live under `src/main/scala` for joint compilation, as `minecraft/McBlockPart.java` already does. Files can move to `src/main/java` once their Scala dependencies are ported. - Established the fixture recipe for the remaining load-bearing bridges: build a reference dev jar at the pre-port commit, compile the consumer with Scala 2.11.5 under Java 8 with `-target:jvm-1.8` against that jar plus the project compile classpath, then store the class base64-encoded under `src/test/resources/compat/`. +- Ported the whole `TNormalOcclusion.scala` unit: `NormalOcclusionTest`, `NormalOcclusionTest$`, `JNormalOcclusion`, `TNormalOcclusion`, `TNormalOcclusion$class` and `NormallyOccludedPart`. The singleton and both trait interfaces are descriptor-identical to the reference; the other three add only private members. +- Found that Scala emits trait super accessors as `ACC_SYNTHETIC`, which `javac` hides completely. An abstract synthetic method cannot be implemented by any Java class or by Scala recompiled against a Java interface, so `codechicken$multipart$TNormalOcclusion$$super$occlusionTest` is emitted as a non-synthetic default returning `true`. Method resolution ignores the flag, so existing call sites are unaffected. Expect the same treatment wherever a remaining trait calls `super`. +- Found the dangerous shape of the linearization problem. `HollowMicroblock` simply lacked an override, which is easy to spot. `PostMicroblock` already had one ending in `super.occlusionTest(npart)`, which used to route through the trait and now reaches `Microblock` directly, silently dropping the box test while still compiling. Existing `super` call sites must be audited on every remaining trait conversion, not just missing overrides. +- Recorded a second source-only divergence: `NormalOcclusionTest` was a Scala `object`, so `NormalOcclusionTest(a, b)` apply sugar no longer compiles and recompiled Scala callers must write `NormalOcclusionTest.apply(a, b)`. Both static forwarders and the `MODULE$` singleton keep their descriptors, so binaries are unaffected. +- Kept `scala.collection.Traversable` in two descriptors, as the ABI inventory predicted. They are drained with `JavaConversions.asJavaIterator(boxes.toIterator())`, which is the one interop route callable from Java. +- Removed two more closure artifacts, `NormalOcclusionTest$$anonfun$apply$1` and its nested `$$anonfun$apply$2`, and corrected the scaladoc that claimed the test "returns true if the test fails". diff --git a/JAVA_MIGRATION_DIVERGENCES.md b/JAVA_MIGRATION_DIVERGENCES.md index 0d436b3..238f732 100644 --- a/JAVA_MIGRATION_DIVERGENCES.md +++ b/JAVA_MIGRATION_DIVERGENCES.md @@ -156,3 +156,90 @@ None removed. The Scala implementation generated no closure classes for this tra - Clean `spotlessApply checkstyleTest build`: passing. - Java 8 Forge dedicated-server suite: 2 tests, 0 failures, 0 errors. The `RenderBlocks` side warning and the `antlr4` jar parse errors in that log are present on the pre-port baseline as well. + +## 2026-08-14 — TNormalOcclusion Java port + +Covers `NormalOcclusionTest`, `NormalOcclusionTest$`, `JNormalOcclusion`, `TNormalOcclusion`, +`TNormalOcclusion$class` and `NormallyOccludedPart`, which all shared one Scala file. + +### Observable behavior + +No known divergence in the occlusion result. The box test still returns true when every pair of boxes fails to +intersect, still gathers the neighbour's normal boxes followed by its partial boxes in that order, and the bridge still +evaluates the box test before the super chain and short-circuits on failure. + +`Traversable` inputs are drained through `JavaConversions.asJavaIterator(boxes.toIterator())` into an `ArrayList` +before testing, rather than traversed with `forall`. Iteration order is preserved. The result of the test does not +depend on order, since `intersects` is side-effect free. + +### Accepted divergence: super accessor becomes a non-synthetic default + +The reference interface declares `codechicken$multipart$TNormalOcclusion$$super$occlusionTest(TMultiPart)Z` as +`ACC_PUBLIC | ACC_ABSTRACT | ACC_SYNTHETIC`. `javac` hides synthetic members completely, so no Java class, and no +Scala class recompiled against a Java interface, could ever implement it. Left abstract it would make the interface +unimplementable outside the original Scala compiler. + +It is therefore emitted as a non-synthetic `default` returning `true`. `true` is the identity for the `&&` chain the +bridge builds, and matches `TMultiPart.occlusionTest`. Classes compiled against the reference supply their own +override carrying their real super chain, so the default is only reached if the deprecated bridge is used on a class +that never had one. Method resolution ignores `ACC_SYNTHETIC`, so existing `invokeinterface` call sites are unaffected. + +Consequently `NormallyOccludedPart` no longer declares that method itself and inherits the default. An old +`invokevirtual` against it still resolves through the interface default and still returns `true`. + +### Accepted divergence: Scala apply sugar on NormalOcclusionTest + +`NormalOcclusionTest` was a Scala `object`, so Scala callers could write `NormalOcclusionTest(boxes1, boxes2)`. It is +now a Java class with static methods, and that sugar no longer compiles; recompiled Scala callers must write +`NormalOcclusionTest.apply(...)`. The in-repo callers in `EdgeMicroblock` and `HollowMicroblock` were updated. + +Binary compatibility is unaffected: both static forwarders keep their reference descriptors, and `NormalOcclusionTest$` +keeps `MODULE$` plus both instance methods, which is what ForgeRelocationFMP and OpenComputers link against. + +### Accepted divergence: NormallyOccludedPart.getType + +The reference emitted two `getType` methods, `()Ljava/lang/String;` and `()Lscala/runtime/Null$;`, because Scala infers +`Null` for `def getType = null`. Only the `String` form remains. The `Null$` form was a compiler artifact of the +inferred type, and no jar in the pack references it. AE2, the only consumer of this class, uses the `(Cuboid6)V` +constructor and the type itself. + +### Trait linearization, as with TCuboidPart + +Both in-repo mixins needed explicit handling, and the two cases differ: + +- `HollowMicroblock` did not override `occlusionTest` at all and relied on the trait. It now declares + `NormalOcclusionTest.apply(this, npart) && super.occlusionTest(npart)`. +- `PostMicroblock` already overrode `occlusionTest` and ended with `super.occlusionTest(npart)`, which previously + routed through `TNormalOcclusion` because it was the last mixin. That call now reaches `Microblock` directly and + would have skipped the box test entirely, so the same expression was substituted at that call site. + +This second shape is the more dangerous one: the class looks correct, still compiles, and silently drops the box test. +Every remaining trait conversion needs its existing `super` call sites audited, not just its missing overrides. + +### Supported JVM API + +- `NormalOcclusionTest$`, `JNormalOcclusion` and `TNormalOcclusion` are descriptor-identical to the reference. +- `NormalOcclusionTest` and `TNormalOcclusion$class` add only a private constructor, and `NormalOcclusionTest` three + private helpers. Both public `apply` descriptors are unchanged, including + `(Lscala/collection/Traversable;Lscala/collection/Traversable;)Z`. +- Scala's `Traversable` is retained in two descriptors, as the ABI inventory anticipated. + +### Compiler artifacts + +Accepted divergence: `NormalOcclusionTest$$anonfun$apply$1` and `NormalOcclusionTest$$anonfun$apply$1$$anonfun$apply$2` +are removed. The Java loops produce no replacement classes. These were closure implementation details of the nested +`forall` and are not retained as supported API. + +### Documentation fix + +The reference scaladoc said the test "returns true if the test fails", which the earlier characterization showed to be +backwards. The Java doc comments now say the test returns true when the parts may coexist. + +### Validation + +- `NormalOcclusionCharacterizationTest`: 12 tests, 0 failures, 0 errors, unchanged from the Scala baseline. +- `TNormalOcclusionBinaryCompatibilityTest`: 1 test, 0 failures, 0 errors, exercising forwarder, `$class`, singleton + and the accessor callback. +- Complete plain-JVM suite: 38 tests, 0 failures, 0 errors. +- Clean `spotlessApply checkstyleTest build`: passing. +- Java 8 Forge dedicated-server suite: 2 tests, 0 failures, 0 errors. diff --git a/src/main/scala/codechicken/microblock/EdgeMicroblock.scala b/src/main/scala/codechicken/microblock/EdgeMicroblock.scala index 3b1bf90..2d6eaad 100644 --- a/src/main/scala/codechicken/microblock/EdgeMicroblock.scala +++ b/src/main/scala/codechicken/microblock/EdgeMicroblock.scala @@ -10,6 +10,7 @@ import Vector3._ import Rotation._ import codechicken.multipart.TMultiPart import codechicken.multipart.TNormalOcclusion +import codechicken.multipart.NormalOcclusionTest import codechicken.multipart.JPartialOcclusion import codechicken.multipart.TileMultipart import codechicken.multipart.MultiPartRegistry @@ -215,7 +216,8 @@ trait PostMicroblock if (npart.asInstanceOf[CommonMicroblock].getSlot >> 1 == getShape) return true - return super.occlusionTest(npart) + // TNormalOcclusion is a Java interface, so its box test must be applied here rather than by the super chain. + return NormalOcclusionTest.apply(this, npart) && super.occlusionTest(npart) } def getResistanceFactor = PostMicroClass.getResistanceFactor diff --git a/src/main/scala/codechicken/microblock/HollowMicroblock.scala b/src/main/scala/codechicken/microblock/HollowMicroblock.scala index 716bd94..7308a49 100644 --- a/src/main/scala/codechicken/microblock/HollowMicroblock.scala +++ b/src/main/scala/codechicken/microblock/HollowMicroblock.scala @@ -3,8 +3,10 @@ package codechicken.microblock import net.minecraft.entity.player.EntityPlayer import net.minecraft.util.MovingObjectPosition import codechicken.multipart.TFacePart +import codechicken.multipart.TMultiPart import codechicken.lib.vec.{Translation, Cuboid6, Rotation, Vector3} import codechicken.multipart.TNormalOcclusion +import codechicken.multipart.NormalOcclusionTest import codechicken.microblock.MicroMaterialRegistry.IMicroMaterial import net.minecraft.client.renderer.RenderBlocks import codechicken.lib.render.{BlockRenderer, RenderUtils, CCRenderState} @@ -341,6 +343,10 @@ trait HollowMicroblock def getBounds: Cuboid6 = FaceMicroClass.aBounds(shape) + // TNormalOcclusion is a Java interface, so its box test must be applied here rather than by the super chain. + override def occlusionTest(npart: TMultiPart): Boolean = + NormalOcclusionTest.apply(this, npart) && super.occlusionTest(npart) + override def getPartialOcclusionBoxes = HollowMicroClass.pBoxes(shape) def getHollowSize = tile match { diff --git a/src/main/scala/codechicken/multipart/JNormalOcclusion.java b/src/main/scala/codechicken/multipart/JNormalOcclusion.java new file mode 100644 index 0000000..f0ebc28 --- /dev/null +++ b/src/main/scala/codechicken/multipart/JNormalOcclusion.java @@ -0,0 +1,13 @@ +package codechicken.multipart; + +import codechicken.lib.vec.Cuboid6; + +/** + * Java interface containing callbacks for normal occlusion testing. Make sure to override occlusionTest as in + * {@link TNormalOcclusion}. + */ +public interface JNormalOcclusion { + + /** Return a list of normal occlusion boxes. */ + Iterable getOcclusionBoxes(); +} diff --git a/src/main/scala/codechicken/multipart/NormalOcclusionTest$.java b/src/main/scala/codechicken/multipart/NormalOcclusionTest$.java new file mode 100644 index 0000000..ba89024 --- /dev/null +++ b/src/main/scala/codechicken/multipart/NormalOcclusionTest$.java @@ -0,0 +1,22 @@ +package codechicken.multipart; + +import codechicken.lib.vec.Cuboid6; +import scala.collection.Traversable; + +/** + * Scala companion singleton. Retained because compiled Scala consumers read MODULE$ and call these instance methods. + */ +public final class NormalOcclusionTest$ { + + public static final NormalOcclusionTest$ MODULE$ = new NormalOcclusionTest$(); + + private NormalOcclusionTest$() {} + + public boolean apply(Traversable boxes1, Traversable boxes2) { + return NormalOcclusionTest.apply(boxes1, boxes2); + } + + public boolean apply(JNormalOcclusion part1, TMultiPart part2) { + return NormalOcclusionTest.apply(part1, part2); + } +} diff --git a/src/main/scala/codechicken/multipart/NormalOcclusionTest.java b/src/main/scala/codechicken/multipart/NormalOcclusionTest.java new file mode 100644 index 0000000..af61acc --- /dev/null +++ b/src/main/scala/codechicken/multipart/NormalOcclusionTest.java @@ -0,0 +1,62 @@ +package codechicken.multipart; + +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; + +import codechicken.lib.vec.Cuboid6; +import scala.collection.JavaConversions; +import scala.collection.Traversable; + +/** + * Simple bounding box based occlusion testing. If any two parts have overlapping bounding boxes, the test fails. + * + * @see TIconHitEffects for notes on the Scala/Java composition setup. + */ +public final class NormalOcclusionTest { + + private NormalOcclusionTest() {} + + /** Performs the test, returns true if the parts may coexist. */ + public static boolean apply(Traversable boxes1, Traversable boxes2) { + return test(collect(boxes1), collect(boxes2)); + } + + /** Performs the test, returns true if the parts may coexist. */ + public static boolean apply(JNormalOcclusion part1, TMultiPart part2) { + List boxes = new ArrayList<>(); + if (part2 instanceof JNormalOcclusion) { + addAll(boxes, ((JNormalOcclusion) part2).getOcclusionBoxes()); + } + if (part2 instanceof JPartialOcclusion) { + addAll(boxes, ((JPartialOcclusion) part2).getPartialOcclusionBoxes()); + } + return test(boxes, part1.getOcclusionBoxes()); + } + + private static boolean test(Iterable boxes1, Iterable boxes2) { + for (Cuboid6 v1 : boxes1) { + for (Cuboid6 v2 : boxes2) { + if (v1.intersects(v2)) { + return false; + } + } + } + return true; + } + + private static List collect(Traversable boxes) { + List result = new ArrayList<>(); + Iterator iterator = JavaConversions.asJavaIterator(boxes.toIterator()); + while (iterator.hasNext()) { + result.add(iterator.next()); + } + return result; + } + + private static void addAll(List target, Iterable boxes) { + for (Cuboid6 box : boxes) { + target.add(box); + } + } +} diff --git a/src/main/scala/codechicken/multipart/NormallyOccludedPart.java b/src/main/scala/codechicken/multipart/NormallyOccludedPart.java new file mode 100644 index 0000000..8487459 --- /dev/null +++ b/src/main/scala/codechicken/multipart/NormallyOccludedPart.java @@ -0,0 +1,34 @@ +package codechicken.multipart; + +import java.util.Collections; + +import codechicken.lib.vec.Cuboid6; + +/** Utility part class for performing 3rd party occlusion tests. */ +public class NormallyOccludedPart extends TMultiPart implements TNormalOcclusion { + + private final Iterable bounds; + + public NormallyOccludedPart(Iterable bounds) { + this.bounds = bounds; + } + + public NormallyOccludedPart(Cuboid6 bound) { + this(Collections.singletonList(bound)); + } + + @Override + public String getType() { + return null; + } + + @Override + public Iterable getOcclusionBoxes() { + return bounds; + } + + @Override + public boolean occlusionTest(TMultiPart npart) { + return NormalOcclusionTest.apply(this, npart) && super.occlusionTest(npart); + } +} diff --git a/src/main/scala/codechicken/multipart/TNormalOcclusion$class.java b/src/main/scala/codechicken/multipart/TNormalOcclusion$class.java new file mode 100644 index 0000000..a6430bd --- /dev/null +++ b/src/main/scala/codechicken/multipart/TNormalOcclusion$class.java @@ -0,0 +1,15 @@ +package codechicken.multipart; + +/** Binary bridge for Scala implementations compiled against the original trait. */ +@Deprecated +public abstract class TNormalOcclusion$class { + + private TNormalOcclusion$class() {} + + public static boolean occlusionTest(TNormalOcclusion part, TMultiPart npart) { + return NormalOcclusionTest.apply(part, npart) + && part.codechicken$multipart$TNormalOcclusion$$super$occlusionTest(npart); + } + + public static void $init$(TNormalOcclusion part) {} +} diff --git a/src/main/scala/codechicken/multipart/TNormalOcclusion.java b/src/main/scala/codechicken/multipart/TNormalOcclusion.java new file mode 100644 index 0000000..7494a85 --- /dev/null +++ b/src/main/scala/codechicken/multipart/TNormalOcclusion.java @@ -0,0 +1,23 @@ +package codechicken.multipart; + +/** + * Normal occlusion for parts that also extend {@link TMultiPart}. + *

+ * A superclass method beats an interface default on the JVM, so implementors must override occlusionTest themselves + * with {@code NormalOcclusionTest.apply(this, npart) && super.occlusionTest(npart)}, as {@link NormallyOccludedPart} + * does. Declaring it default here would silently lose to TMultiPart's version. + */ +public interface TNormalOcclusion extends JNormalOcclusion { + + boolean occlusionTest(TMultiPart npart); + + /** + * Super accessor generated by Scala for classes compiled against the original trait, called by + * {@link TNormalOcclusion$class#occlusionTest}. Those classes supply their own override carrying their real super + * chain; the default here is the identity for that chain and is only reached if the deprecated bridge is used on a + * class that never had one. + */ + default boolean codechicken$multipart$TNormalOcclusion$$super$occlusionTest(TMultiPart npart) { + return true; + } +} diff --git a/src/main/scala/codechicken/multipart/TNormalOcclusion.scala b/src/main/scala/codechicken/multipart/TNormalOcclusion.scala deleted file mode 100644 index 0b91219..0000000 --- a/src/main/scala/codechicken/multipart/TNormalOcclusion.scala +++ /dev/null @@ -1,65 +0,0 @@ -package codechicken.multipart - -import codechicken.lib.vec.Cuboid6 -import scala.collection.JavaConversions._ -import java.lang.Iterable - -/** This suite of 3 classes provides simple functions for standard bounding box - * based occlusion testing. If any two parts have overlapping bounding boxes, - * the test fails - * - * See TIconHitEffects for notes on the Scala|Java composition setup. - */ -object NormalOcclusionTest { - - /** Performs the test, returns true if the test fails - */ - def apply( - boxes1: Traversable[Cuboid6], - boxes2: Traversable[Cuboid6] - ): Boolean = - boxes1.forall(v1 => boxes2.forall(v2 => !v1.intersects(v2))) - - /** Performs the test, returns true if the test fails - */ - def apply(part1: JNormalOcclusion, part2: TMultiPart): Boolean = { - var boxes = Seq[Cuboid6]() - if (part2.isInstanceOf[JNormalOcclusion]) - boxes = boxes ++ part2.asInstanceOf[JNormalOcclusion].getOcclusionBoxes - - if (part2.isInstanceOf[JPartialOcclusion]) - boxes = - boxes ++ part2.asInstanceOf[JPartialOcclusion].getPartialOcclusionBoxes - - return NormalOcclusionTest(boxes, part1.getOcclusionBoxes) - } -} - -/** Java interface containing callbacks for normal occlusion testing. Make sure - * to override occlusionTest as in TNormalOcclusion - */ -trait JNormalOcclusion { - - /** Return a list of normal occlusion boxes - */ - def getOcclusionBoxes: Iterable[Cuboid6] -} - -/** Trait for scala programmers - */ -trait TNormalOcclusion extends TMultiPart with JNormalOcclusion { - override def occlusionTest(npart: TMultiPart): Boolean = - NormalOcclusionTest(this, npart) && super.occlusionTest(npart) -} - -/** Utility part class for performing 3rd party occlusion tests - */ -class NormallyOccludedPart(bounds: Iterable[Cuboid6]) - extends TMultiPart - with TNormalOcclusion { - def this(bound: Cuboid6) = this(Seq(bound)) - - def getType = null - - def getOcclusionBoxes = bounds -}