mirror of
https://github.com/GTNewHorizons/ForgeMultipart.git
synced 2026-11-12 16:37:38 +00:00
refactor: port MultipartRenderer to Java
Kept in the TickScheduler$ shape: the companion holds the implementation because Forge registers that instance itself, and the plain class holds static forwarders. Both halves are load-bearing for different reasons -- guidenh resolves renderWorldBlock as a static first and falls back to MODULE$ -- and neither has a bytecode consumer, only reflective names. The emitted class list is unchanged. All five rendering callbacks keep their signatures, MODULE$ keeps its type and flags, and both classes keep class-level @SideOnly(CLIENT). Only three Scala forwarders for inherited TileEntitySpecialRenderer members and the companion clinit's ACC_PUBLIC differ. The render id is still claimed in the singleton's initialiser, matching the reference's first-access timing. The breaking-overlay branch moved to a private helper that checks the erased Tuple2 and the boxed index before unboxing, which is what the reference's pattern match compiled to. 125 plain-JVM tests and 22 Forge server tests pass, none changed by the port. This class needs a client, so it adds nothing to either suite and everything to the manual checklist.
This commit is contained in:
1 parent
da472f1fd3
commit
fa976c2002
7 files changed
+236
-109
No files matched your search
@@ -539,3 +539,9 @@ These ranges were formed before designing the characterization suite and should
|
||||
- Proved the substitution was an identity before making it. The functional test asserted that each alias returned the very instance its proxy holds, so replacing `logger` with `MultipartProxy.logger` / `MicroblockProxy.logger` cannot change behavior. `ConfigContent` and `MultipartGenerator` are descriptor-identical afterwards.
|
||||
- Noted the first case where the migration's answer is deletion. Zero downstream references plus an impossible target name means there is nothing to preserve, and inventing a `MultipartLogger` type to hold a one-line forwarder would add API rather than remove it.
|
||||
- Emptied the low-risk queue. Everything remaining is the medium group or later, so the next piece of work should be chosen deliberately rather than taken off the top of a list.
|
||||
- Ported `MultipartRenderer` in the `TickScheduler$` shape, with the companion holding the implementation and the plain class holding static forwarders. Forge registers the singleton itself and guidenh resolves the static first, so both halves are load-bearing for different reasons and the emitted class list is unchanged.
|
||||
- Applied the reflective-name rule for the second time, and this is the case it was written for: `MultipartRenderer` has no bytecode consumer at all, only guidenh string constants, and both the class and its companion had to survive.
|
||||
- Found a Scala-to-Java trap that is not silent but will recur: uniform access makes `renderer.hasOverrideBlockTexture` read the same whether it is a field or a no-arg method. It is a method, and the port did not compile until it grew parentheses. Every remaining renderer conversion will meet this.
|
||||
- Dropped the three static forwarders Scala generated for inherited `TileEntitySpecialRenderer` members, the same category as the twelve `KeyBinding` forwarders dropped from `ControlKeyHandler`.
|
||||
- Dropped an unused `ThreadSafeISBRH` import. It annotated nothing and appears nowhere else; if the intent was to mark this ISBRH thread-safe for Angelica then the annotation was never applied in the reference either, which is worth checking independently of the migration.
|
||||
- Recorded that this port adds nothing to the automated suites and everything to the manual checklist. Static and dynamic rendering, the breaking overlay and the render id all need a client.
|
||||
@@ -1168,3 +1168,62 @@ no Scala consumer did.
|
||||
- Test churn, both expected for a removal rather than a port: `PackageObjectCharacterizationTest` was deleted with its
|
||||
subject, and `ProxyLoggerFunctionalTest` lost only its alias-identity assertion. What survives is the assertion worth
|
||||
keeping — that both proxies publish a non-null logger during preInit, which is what the inlined call sites now read.
|
||||
|
||||
## 2026-08-15 — MultipartRenderer Java port
|
||||
|
||||
The client rendering callbacks. No jar links against any member, but guidenh reflects on **both** `MultipartRenderer`
|
||||
and `MultipartRenderer$` by name, resolving `renderWorldBlock` as a static first and falling back to the companion's
|
||||
`MODULE$`. Both classes are therefore kept, in the `TickScheduler$` shape: the companion holds the implementation
|
||||
because Forge registers that instance itself, and the plain class is static forwarders.
|
||||
|
||||
### Observable behavior
|
||||
|
||||
No known divergence. The render id is still claimed in the singleton's initialiser, which preserves the reference's
|
||||
timing: a Scala object runs its body on first access, and the first access is the client proxy's `postInit`.
|
||||
`renderWorldBlock` still returns false for a non-multipart tile, for an empty part list, and after drawing a breaking
|
||||
overlay; `renderTileEntityAt` still returns early unless the tile has dynamic parts; `getRenderId` still reads
|
||||
`TileMultipart.renderID`.
|
||||
|
||||
The reference wrote the dynamic-part guard with a non-short-circuiting `|`. The port uses `||`. Both operands are
|
||||
side-effect-free getters, so this cannot differ.
|
||||
|
||||
The breaking-overlay branch moved into a private helper. The reference nested a pattern match inside an `if`; the hit
|
||||
data is a `scala.Tuple2` that arrives erased, so the port checks the tuple and then the boxed index before unboxing,
|
||||
which is what the match compiled to. `BlockMultipart` already reads the same data the same way.
|
||||
|
||||
### Supported JVM API
|
||||
|
||||
The emitted class list is **unchanged** — nothing added, nothing removed. All five rendering callbacks keep their
|
||||
exact signatures on `MultipartRenderer`, `MODULE$` keeps its type and flags on `MultipartRenderer$`, and both classes
|
||||
keep class-level `@SideOnly(Side.CLIENT)`, so both are still stripped on a dedicated server.
|
||||
|
||||
`MultipartRenderer$` still extends `TileEntitySpecialRenderer` and implements `ISimpleBlockRenderingHandler`. Forge
|
||||
rejects the registration outright if either is missing, and the characterization asserts both.
|
||||
|
||||
### Differences that are not API
|
||||
|
||||
- `MultipartRenderer` loses `func_147498_b`, `func_147496_a` and `func_147497_a`, the forwarders Scala generates for
|
||||
inherited `TileEntitySpecialRenderer` members. Same category as the twelve `KeyBinding` forwarders dropped from
|
||||
`ControlKeyHandler`: artifacts of the object-forwarder mechanism, not API, and unreferenced.
|
||||
- `MultipartRenderer$`'s class initialiser loses `ACC_PUBLIC`, as every converted companion has.
|
||||
- The unused `com.gtnewhorizons.angelica.api.ThreadSafeISBRH` import is dropped. It appears nowhere else in the
|
||||
codebase and annotated nothing; no behavior depended on it. Worth a second look independently — if the intent was to
|
||||
mark this ISBRH thread-safe for Angelica, that annotation was never actually applied in the reference either.
|
||||
|
||||
### Scala's uniform access hid a field-versus-method difference
|
||||
|
||||
`renderer.hasOverrideBlockTexture` reads identically in Scala whether it is a field or a no-arg method. It is a method
|
||||
on `RenderBlocks`, and the port did not compile until it became `hasOverrideBlockTexture()`. This is a compile-time
|
||||
failure rather than a silent one, but it will recur in every remaining renderer conversion.
|
||||
|
||||
### Validation
|
||||
|
||||
- Complete plain-JVM suite: 125 tests, 0 failures, 0 errors.
|
||||
- Clean `spotlessApply checkstyleTest build`: passing.
|
||||
- Java 8 Forge dedicated-server suite: 22 tests, 0 failures, 0 errors. It does not exercise this class and cannot:
|
||||
the server never loads it.
|
||||
- Class list and members diffed against a reference jar built from the pre-port `src/main`.
|
||||
- No characterization assertion changed with the port.
|
||||
- What stays manual, and it is everything this class actually does: static block rendering, dynamic part rendering,
|
||||
the breaking overlay on the struck part, inventory rendering, and the render id being claimed exactly once. None of
|
||||
it can run without a client, so all of it is on the checklist.
|
||||
@@ -10,7 +10,7 @@ what breaks, and what is left. The other documents hold the reasoning.
|
||||
| `JAVA_MIGRATION_DIVERGENCES.md` | Every intentional difference from the reference, one entry per port |
|
||||
| `JAVA_MIGRATION_MANUAL_CHECKS.md` | What no automated test can cover, and must be checked by hand in a client |
|
||||
|
||||
Branch: `algent/java`. Base: `master`. 52 commits so far.
|
||||
Branch: `algent/java`. Base: `master`. 54 commits so far.
|
||||
|
||||
## The one rule that matters
|
||||
|
||||
@@ -56,7 +56,7 @@ awk -F'"' '/<testsuite /{t+=$4;f+=$8;e+=$10} END{print "tests="t" failures="f" e
|
||||
grep -o 'tests="[0-9]*" skipped="[0-9]*" failures="[0-9]*" errors="[0-9]*"' run/server/junit-out/TEST-*.xml
|
||||
```
|
||||
|
||||
Current baseline: **120 plain-JVM tests, 22 Forge server tests, all passing.**
|
||||
Current baseline: **125 plain-JVM tests, 22 Forge server tests, all passing.**
|
||||
|
||||
### ABI diff against the reference
|
||||
|
||||
@@ -163,6 +163,10 @@ server takes. It made `ControlKeyModifer.isControlDown` testable. The client bra
|
||||
no Java source can declare or name it. Both were removed by inlining their single member. Any future one has the same
|
||||
three options: leave it in Scala, invent a Java type to hold its members, or inline it.
|
||||
|
||||
**Scala's uniform access hides field versus no-arg method.** `renderer.hasOverrideBlockTexture` reads the same in
|
||||
Scala either way; it is a method on `RenderBlocks` and Java needs the parentheses. It fails at compile time rather than
|
||||
silently, but expect it in every remaining renderer conversion.
|
||||
|
||||
**Java 8 target.** No `List.of`, no `var`, no switch expressions in main or test sources.
|
||||
|
||||
**Two test classes sharing global registry state** must guard their registrations, and the registries' error paths call
|
||||
@@ -179,9 +183,9 @@ All eight load-bearing `$class` helpers from the inventory, both registries, and
|
||||
Plus the six marker interfaces: `TSlottedPart`, `IRandomDisplayTick`, `INeighborTileChange`, `TRandomUpdateTick`,
|
||||
`ISidedHollowConnect`, `IMicroMaterialRender`, plus `MultipartHelper`, `TileCache`, `PacketScheduler` and the `ControlKeyModifer` pair.
|
||||
|
||||
Both `package.scala` objects are gone, removed rather than ported.
|
||||
Both `package.scala` objects are gone, removed rather than ported. `MultipartRenderer` is done.
|
||||
|
||||
70 Java files, 49 Scala files, ~6,467 Scala lines left (non-blank; that is the metric this figure has always used).
|
||||
72 Java files, 48 Scala files, ~6,376 Scala lines left (non-blank; that is the metric this figure has always used).
|
||||
|
||||
## What is left, and in what order
|
||||
|
||||
|
||||
@@ -0,0 +1,121 @@
|
||||
package codechicken.multipart;
|
||||
|
||||
import net.minecraft.block.Block;
|
||||
import net.minecraft.client.Minecraft;
|
||||
import net.minecraft.client.renderer.RenderBlocks;
|
||||
import net.minecraft.client.renderer.tileentity.TileEntitySpecialRenderer;
|
||||
import net.minecraft.tileentity.TileEntity;
|
||||
import net.minecraft.util.MovingObjectPosition;
|
||||
import net.minecraft.world.IBlockAccess;
|
||||
import net.minecraftforge.client.MinecraftForgeClient;
|
||||
|
||||
import codechicken.lib.raytracer.ExtendedMOP;
|
||||
import codechicken.lib.render.CCRenderState;
|
||||
import codechicken.lib.vec.Vector3;
|
||||
import cpw.mods.fml.client.registry.ISimpleBlockRenderingHandler;
|
||||
import cpw.mods.fml.client.registry.RenderingRegistry;
|
||||
import cpw.mods.fml.relauncher.Side;
|
||||
import cpw.mods.fml.relauncher.SideOnly;
|
||||
import scala.Tuple2;
|
||||
|
||||
/**
|
||||
* Internal class for rendering callbacks. Should be moved to the handler package.
|
||||
* <p>
|
||||
* This is the Scala companion singleton, and it is not a mere forwarder: Forge registers this instance itself, through
|
||||
* both {@code RenderingRegistry.registerBlockHandler} and {@code ClientRegistry.bindTileEntitySpecialRenderer}, so the
|
||||
* implementation has to live here. guidenh also reads {@code MODULE$} as its fallback path.
|
||||
* <p>
|
||||
* Claiming the render id in the constructor preserves the reference's timing: a Scala object runs its body on first
|
||||
* access, and this class is first touched from the client proxy's postInit.
|
||||
*/
|
||||
@SideOnly(Side.CLIENT)
|
||||
public final class MultipartRenderer$ extends TileEntitySpecialRenderer implements ISimpleBlockRenderingHandler {
|
||||
|
||||
public static final MultipartRenderer$ MODULE$ = new MultipartRenderer$();
|
||||
|
||||
private MultipartRenderer$() {
|
||||
TileMultipart.renderID_$eq(RenderingRegistry.getNextAvailableRenderId());
|
||||
}
|
||||
|
||||
@Override
|
||||
public void renderTileEntityAt(TileEntity t, double x, double y, double z, float f) {
|
||||
TileMultipartClient client = (TileMultipartClient) t;
|
||||
TileMultipart tile = (TileMultipart) t;
|
||||
if (tile.jPartList().isEmpty() || !client.hasDynamicParts()) {
|
||||
return;
|
||||
}
|
||||
|
||||
CCRenderState state = CCRenderState.instance();
|
||||
state.resetInstance();
|
||||
state.pullLightmapInstance();
|
||||
state.useNormals = true;
|
||||
|
||||
client.renderDynamic(new Vector3(x, y, z), f, MinecraftForgeClient.getRenderPass());
|
||||
}
|
||||
|
||||
@Override
|
||||
public int getRenderId() {
|
||||
return TileMultipart.renderID();
|
||||
}
|
||||
|
||||
@Override
|
||||
public boolean renderWorldBlock(IBlockAccess world, int x, int y, int z, Block block, int modelId,
|
||||
RenderBlocks renderer) {
|
||||
TileEntity t = world.getTileEntity(x, y, z);
|
||||
if (!(t instanceof TileMultipartClient)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
TileMultipartClient client = (TileMultipartClient) t;
|
||||
TileMultipart tile = (TileMultipart) t;
|
||||
if (tile.jPartList().isEmpty()) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if (renderer.hasOverrideBlockTexture()) {
|
||||
drawBreakingPart(tile, x, y, z, renderer);
|
||||
return false;
|
||||
}
|
||||
|
||||
CCRenderState state = CCRenderState.instance();
|
||||
state.resetInstance();
|
||||
state.lightMatrix.locate(world, x, y, z);
|
||||
boolean b = client.renderStatic(world, new Vector3(x, y, z), renderer);
|
||||
state.lightMatrix.access = null;
|
||||
return b;
|
||||
}
|
||||
|
||||
/**
|
||||
* The hit data is a Scala Tuple2 whose first member is the struck part's index. It arrives erased, so both the
|
||||
* tuple and the index are checked before use, exactly as the reference's pattern match did.
|
||||
*/
|
||||
private static void drawBreakingPart(TileMultipart tile, int x, int y, int z, RenderBlocks renderer) {
|
||||
MovingObjectPosition hit = Minecraft.getMinecraft().objectMouseOver;
|
||||
if (hit == null || hit.blockX != x || hit.blockY != y || hit.blockZ != z) {
|
||||
return;
|
||||
}
|
||||
|
||||
Object data = ExtendedMOP.getData(hit);
|
||||
if (!(data instanceof Tuple2)) {
|
||||
return;
|
||||
}
|
||||
|
||||
Object index = ((Tuple2<?, ?>) data)._1();
|
||||
if (!(index instanceof Integer)) {
|
||||
return;
|
||||
}
|
||||
|
||||
int i = (Integer) index;
|
||||
if (i >= 0 && i < tile.jPartList().size()) {
|
||||
tile.jPartList().get(i).drawBreaking(renderer);
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
public void renderInventoryBlock(Block block, int meta, int modelId, RenderBlocks renderer) {}
|
||||
|
||||
@Override
|
||||
public boolean shouldRender3DInInventory(int modelId) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,40 @@
|
||||
package codechicken.multipart;
|
||||
|
||||
import net.minecraft.block.Block;
|
||||
import net.minecraft.client.renderer.RenderBlocks;
|
||||
import net.minecraft.tileentity.TileEntity;
|
||||
import net.minecraft.world.IBlockAccess;
|
||||
|
||||
import cpw.mods.fml.relauncher.Side;
|
||||
import cpw.mods.fml.relauncher.SideOnly;
|
||||
|
||||
/**
|
||||
* Static forwarders for {@link MultipartRenderer$}, which holds the implementation. guidenh resolves renderWorldBlock
|
||||
* here first and only falls back to the companion's MODULE$, so these must keep their exact signatures.
|
||||
*/
|
||||
@SideOnly(Side.CLIENT)
|
||||
public final class MultipartRenderer {
|
||||
|
||||
private MultipartRenderer() {}
|
||||
|
||||
public static void renderTileEntityAt(TileEntity t, double x, double y, double z, float f) {
|
||||
MultipartRenderer$.MODULE$.renderTileEntityAt(t, x, y, z, f);
|
||||
}
|
||||
|
||||
public static int getRenderId() {
|
||||
return MultipartRenderer$.MODULE$.getRenderId();
|
||||
}
|
||||
|
||||
public static boolean renderWorldBlock(IBlockAccess world, int x, int y, int z, Block block, int modelId,
|
||||
RenderBlocks renderer) {
|
||||
return MultipartRenderer$.MODULE$.renderWorldBlock(world, x, y, z, block, modelId, renderer);
|
||||
}
|
||||
|
||||
public static void renderInventoryBlock(Block block, int meta, int modelId, RenderBlocks renderer) {
|
||||
MultipartRenderer$.MODULE$.renderInventoryBlock(block, meta, modelId, renderer);
|
||||
}
|
||||
|
||||
public static boolean shouldRender3DInInventory(int modelId) {
|
||||
return MultipartRenderer$.MODULE$.shouldRender3DInInventory(modelId);
|
||||
}
|
||||
}
|
||||
@@ -1,103 +0,0 @@
|
||||
package codechicken.multipart
|
||||
|
||||
import codechicken.lib.render.CCRenderState
|
||||
import codechicken.lib.vec.Vector3
|
||||
import cpw.mods.fml.client.registry.ISimpleBlockRenderingHandler
|
||||
import cpw.mods.fml.client.registry.RenderingRegistry
|
||||
import net.minecraft.block.Block
|
||||
import net.minecraft.client.renderer.{Tessellator, RenderBlocks}
|
||||
import net.minecraft.client.renderer.tileentity.TileEntitySpecialRenderer
|
||||
import net.minecraft.tileentity.TileEntity
|
||||
import net.minecraft.world.IBlockAccess
|
||||
import net.minecraft.client.Minecraft
|
||||
import net.minecraftforge.client.ForgeHooksClient
|
||||
import net.minecraftforge.client.MinecraftForgeClient
|
||||
import cpw.mods.fml.relauncher.SideOnly
|
||||
import cpw.mods.fml.relauncher.Side
|
||||
import codechicken.lib.raytracer.ExtendedMOP
|
||||
import codechicken.lib.lighting.LightMatrix
|
||||
import com.gtnewhorizons.angelica.api.ThreadSafeISBRH
|
||||
|
||||
/** Internal class for rendering callbacks. Should be moved to the handler
|
||||
* package
|
||||
*/
|
||||
@SideOnly(Side.CLIENT)
|
||||
object MultipartRenderer
|
||||
extends TileEntitySpecialRenderer
|
||||
with ISimpleBlockRenderingHandler {
|
||||
TileMultipart.renderID_$eq(RenderingRegistry.getNextAvailableRenderId)
|
||||
|
||||
override def renderTileEntityAt(
|
||||
t: TileEntity,
|
||||
x: Double,
|
||||
y: Double,
|
||||
z: Double,
|
||||
f: Float
|
||||
) {
|
||||
val tmpart = t.asInstanceOf[TileMultipartClient]
|
||||
if (tmpart.partList.isEmpty | !tmpart.hasDynamicParts)
|
||||
return
|
||||
|
||||
val state = CCRenderState.instance
|
||||
state.resetInstance()
|
||||
state.pullLightmapInstance()
|
||||
state.useNormals = true
|
||||
|
||||
val pos = new Vector3(x, y, z)
|
||||
tmpart.renderDynamic(pos, f, MinecraftForgeClient.getRenderPass)
|
||||
}
|
||||
|
||||
override def getRenderId = TileMultipart.renderID
|
||||
|
||||
override def renderWorldBlock(
|
||||
world: IBlockAccess,
|
||||
x: Int,
|
||||
y: Int,
|
||||
z: Int,
|
||||
block: Block,
|
||||
modelId: Int,
|
||||
renderer: RenderBlocks
|
||||
): Boolean = {
|
||||
val t = world.getTileEntity(x, y, z)
|
||||
if (!t.isInstanceOf[TileMultipartClient])
|
||||
return false
|
||||
|
||||
val tmpart = t.asInstanceOf[TileMultipartClient]
|
||||
if (tmpart.partList.isEmpty)
|
||||
return false
|
||||
|
||||
if (renderer.hasOverrideBlockTexture) {
|
||||
val hit = Minecraft.getMinecraft.objectMouseOver
|
||||
if (
|
||||
hit != null && hit.blockX == x && hit.blockY == y && hit.blockZ == z && ExtendedMOP
|
||||
.getData(hit)
|
||||
.isInstanceOf[(_, _)]
|
||||
) {
|
||||
val hitInfo: (Int, _) = ExtendedMOP.getData(hit)
|
||||
if (
|
||||
hitInfo._1.isInstanceOf[
|
||||
Int
|
||||
] && hitInfo._1 >= 0 && hitInfo._1 < tmpart.partList.size
|
||||
)
|
||||
tmpart.partList.apply(hitInfo._1).drawBreaking(renderer)
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
val state = CCRenderState.instance
|
||||
state.resetInstance()
|
||||
state.lightMatrix.locate(world, x, y, z)
|
||||
val b = tmpart.renderStatic(world, new Vector3(x, y, z), renderer)
|
||||
state.lightMatrix.access = null
|
||||
b
|
||||
}
|
||||
|
||||
override def renderInventoryBlock(
|
||||
block: Block,
|
||||
meta: Int,
|
||||
modelId: Int,
|
||||
renderer: RenderBlocks
|
||||
) {}
|
||||
|
||||
def shouldRender3DInInventory(modelId: Int) = false
|
||||
}
|
||||
@@ -101,7 +101,7 @@ class MultipartProxy_clientImpl extends MultipartProxy_serverImpl {
|
||||
@SideOnly(Side.CLIENT)
|
||||
override def postInit() {
|
||||
super.postInit()
|
||||
RenderingRegistry.registerBlockHandler(MultipartRenderer)
|
||||
RenderingRegistry.registerBlockHandler(MultipartRenderer$.MODULE$)
|
||||
PacketCustom.assignHandler(MultipartCPH.channel, MultipartCPH)
|
||||
PacketCustom.assignHandler(MultipartCPH.registryChannel, MultipartCPH)
|
||||
|
||||
@@ -112,7 +112,7 @@ class MultipartProxy_clientImpl extends MultipartProxy_serverImpl {
|
||||
@SideOnly(Side.CLIENT)
|
||||
override def onTileClassBuilt(t: Class[_ <: TileEntity]) {
|
||||
super.onTileClassBuilt(t)
|
||||
ClientRegistry.bindTileEntitySpecialRenderer(t, MultipartRenderer)
|
||||
ClientRegistry.bindTileEntitySpecialRenderer(t, MultipartRenderer$.MODULE$)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user