diff --git a/JAVA_MIGRATION.md b/JAVA_MIGRATION.md index 51c9b4d..558c9c5 100644 --- a/JAVA_MIGRATION.md +++ b/JAVA_MIGRATION.md @@ -520,3 +520,7 @@ These ranges were formed before designing the characterization suite and should - Found that `MicroblockGenerator$.create` is matched by exact parameter types, with `MicroblockClass`'s fully qualified name string-compared. Widening a parameter or renaming the class breaks the lookup while every call site still links. - Verified against the already-ported `TileMultipart` that `partList_$eq(scala.collection.Seq)`, `loadParts`, `notifyTileChange` and `markRender` all survived. `partList_$eq` is a Scala `var` setter with no Java-facing equivalent and is reflectively load-bearing, so dropping it for a list mutator would have broken guidenh invisibly. It was kept. - Confirmed the MultipartHelper port satisfies guidenh directly: it resolves `createTileFromNBT` as a static on the plain class and only falls back to `MultipartHelper$.MODULE$` if that misses, so the retained companion is belt-and-braces rather than the primary path. +- Ported `TileCache` to Java, the first conversion free to change shape rather than preserve it. It has zero downstream references in bytecode and does not appear in guidenh's reflective names, so `map()` became a `java.util.Map`, `add`/`remove` became `void`, and `apply` returns a nullable `FlaggedTile` instead of a `scala.Option`. No internal caller used any of the discarded return values. +- Dropped `TileCache$`, `TileCache$FlaggedTile$` and the whole case-class machinery on `FlaggedTile`. It is only ever a map value, never compared, hashed, printed or destructured. This is the `IconHitEffects$` case rather than the `MultipartHelper$` one: nothing names it in bytecode or by string. +- Pinned an unreachable branch before removing it. The reference's `findTile` ends in `case _ => null`, but `Some(FlaggedTile(t, rem))` matches whatever the flag holds, so a tile flagged removed is returned like any other and only the warning is suppressed. The functional test asserts that before the port so the Java version cannot quietly start returning null. +- Confirmed the adapter approach works for ports that change collection types. The characterization test named `scala.Option` in exactly one helper, so the port's test diff was two lines and every behavioral assertion carried over untouched. diff --git a/JAVA_MIGRATION_DIVERGENCES.md b/JAVA_MIGRATION_DIVERGENCES.md index 7675283..282a97c 100644 --- a/JAVA_MIGRATION_DIVERGENCES.md +++ b/JAVA_MIGRATION_DIVERGENCES.md @@ -958,3 +958,55 @@ that overrides `convertMulti(T)`, exactly as it did under Scala's `asInstanceOf[ - Public members, descriptors and the full emitted class list diffed against a reference jar built at `0fdf9e5`. - What stays manual: a description packet actually arriving at a watching client. The functional test builds and sends one with no players connected, so the wire format and the client's handling of it are still unasserted. + +## 2026-08-15 — TileCache Java port + +The client-side tile recovery cache. **Zero downstream references**: no jar links against any member, and guidenh's +reflective name list does not include it. That makes this the first port free to change shape rather than preserve it, +on the `IDWriter` precedent. + +### Observable behavior + +No known divergence. `add` still stores the tile unflagged and `remove` still replaces that entry with a flagged one +rather than dropping it, both keyed by a fresh `BlockCoord` built from the tile's own coordinates. `findTile` still +prefers the world's tile, still warns only when the cached entry is not flagged removed, and still throws +`RuntimeException("DC: Client multipart @" + c + " not found")` when the cache has nothing. + +The reference's trailing `case _ => null` in `findTile` is unreachable — `Some(FlaggedTile(t, rem))` matches whatever +the flag is — so a tile flagged removed is returned just like any other, silently. The Java port makes that explicit +with a comment instead of a dead branch, and `stillReturnsATileThatWasFlaggedRemoved` pins it. + +### Changed API + +All of it is unreferenced, and each change removes a Scala type from a signature no Java caller could use naturally: + +| Member | Reference | Port | +| --- | --- | --- | +| `map()` | `scala.collection.mutable.Map` | `java.util.Map` | +| `add`, `remove` | returned `scala.Option` (the displaced entry) | `void` | +| `apply(BlockCoord)` | `scala.Option` | `FlaggedTile`, null when absent | + +`findTile` and `clear` are unchanged. No internal caller used any of the discarded return values: `TileMultipart` calls +`add`/`remove`/`findTile` and `MultipartEventHandler` calls `clear`. + +### Removed API + +- `TileCache$` and `TileCache$FlaggedTile$`, the two companion singletons. Nothing references either in bytecode or by + name, which is the `IconHitEffects$` case rather than the `MultipartHelper$` one. +- `FlaggedTile`'s case-class machinery: `copy`, `copy$default$1`, `copy$default$2`, `canEqual`, `equals`, `hashCode`, + `toString`, `productPrefix`, `productArity`, `productElement`, `productIterator`, and the `scala.Product` and + `scala.Serializable` interfaces. It is only ever a map *value*, never compared, hashed, printed or destructured, so + none of it was load-bearing. `t()` and `removed()` are kept under their generated names. + +### Validation + +- Complete plain-JVM suite: 114 tests, 0 failures, 0 errors. +- Clean `spotlessApply checkstyleTest build`: passing. +- Java 8 Forge dedicated-server suite: 13 tests, 0 failures, 0 errors, 3 of them new. `findTile` needs a world, so all + three branches are covered there rather than headless. +- Class list and members diffed against a reference jar built at `b92d4b7`. Two classes removed, both companions. +- One characterization assertion changed with the port: the test's `entryAt` adapter stopped unwrapping a + `scala.Option`. It was written as the single place the collection types are named precisely so this diff would be two + lines. Every behavioral assertion is unchanged and passed before and after. +- What stays manual: the recovery path itself. It only fires when a client tile is evicted from its chunk before its + update packet arrives, which no test reproduces. diff --git a/JAVA_MIGRATION_HANDOFF.md b/JAVA_MIGRATION_HANDOFF.md index 9ce977e..a3f3d0e 100644 --- a/JAVA_MIGRATION_HANDOFF.md +++ b/JAVA_MIGRATION_HANDOFF.md @@ -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`. 43 commits so far. +Branch: `algent/java`. Base: `master`. 46 commits so far. ## The one rule that matters @@ -56,7 +56,7 @@ awk -F'"' '/ map = new HashMap<>(); + + private TileCache() {} + + public static Map map() { + return map; + } + + public static void add(TileMultipart t) { + map.put(new BlockCoord(t), new FlaggedTile(t, false)); + } + + public static void remove(TileMultipart t) { + map.put(new BlockCoord(t), new FlaggedTile(t, true)); + } + + public static FlaggedTile apply(BlockCoord c) { + return map.get(c); + } + + public static void clear() { + map.clear(); + } + + public static TileMultipart findTile(World world, BlockCoord c) { + TileMultipart inWorld = BlockMultipart.getTile(world, c.x, c.y, c.z); + if (inWorld != null) { + return inWorld; + } + + FlaggedTile entry = map.get(c); + if (entry == null) { + throw new RuntimeException("DC: Client multipart @" + c + " not found"); + } + + // A tile flagged removed is still returned; the flag only says the warning is expected. + if (!entry.removed()) { + MultipartProxy.logger().warn( + "Client multipart @" + c + + " vanished from world but was recovered. If possible causes can be identified, please report to the github issue tracker."); + } + return entry.t(); + } + + public static final class FlaggedTile { + + private final TileMultipart t; + private final boolean removed; + + public FlaggedTile(TileMultipart t, boolean removed) { + this.t = t; + this.removed = removed; + } + + public TileMultipart t() { + return t; + } + + public boolean removed() { + return removed; + } + } +} diff --git a/src/main/scala/codechicken/multipart/TileCache.scala b/src/main/scala/codechicken/multipart/TileCache.scala deleted file mode 100644 index 07402c4..0000000 --- a/src/main/scala/codechicken/multipart/TileCache.scala +++ /dev/null @@ -1,46 +0,0 @@ -package codechicken.multipart - -import codechicken.lib.vec.BlockCoord -import scala.collection.mutable.{Map => MMap} -import net.minecraft.world.World -import codechicken.multipart.handler.MultipartProxy - -/** In order to maintain tight client/server synchronisation without bandwidth - * overhead, all data written must be read. Sometimes the client tile is - * removed from the chunk before the packet arrives. This class maintains a - * reference to all multipart tiles. The original design placed tiles that were - * invalidated into a map that was cleared upon arrival of the update packet. - * Due to other factors that have eluded identification, references to all - * tiles held until they are replaced or the world is reloaded. - */ -object TileCache { - case class FlaggedTile(t: TileMultipart, removed: Boolean) - - val map = MMap[BlockCoord, FlaggedTile]() - - def add(t: TileMultipart) = map.put(new BlockCoord(t), FlaggedTile(t, false)) - def remove(t: TileMultipart) = - map.put(new BlockCoord(t), FlaggedTile(t, true)) - def apply(c: BlockCoord) = map.get(c) - def clear() = map.clear() - - def findTile(world: World, c: BlockCoord) = - BlockMultipart.getTile(world, c.x, c.y, c.z) match { - case null => - apply(c) match { - case Some(FlaggedTile(t, rem)) => - if (!rem) - MultipartProxy.logger.warn( - "Client multipart @" + c + " vanished from world but was recovered. If possible causes can be identified, please report to the github issue tracker." - ) - t - case None => - throw new RuntimeException( - "DC: Client multipart @" + c + " not found" - ) - case _ => - null - } - case t => t - } -} diff --git a/src/test/java/codechicken/multipart/TileCacheCharacterizationTest.java b/src/test/java/codechicken/multipart/TileCacheCharacterizationTest.java index 175fe37..737cbce 100644 --- a/src/test/java/codechicken/multipart/TileCacheCharacterizationTest.java +++ b/src/test/java/codechicken/multipart/TileCacheCharacterizationTest.java @@ -109,8 +109,7 @@ class TileCacheCharacterizationTest { // The two adapters below are the only places the cache's collection types are named. private static TileCache.FlaggedTile entryAt(BlockCoord coord) { - scala.Option entry = TileCache.apply(coord); - return entry.isDefined() ? entry.get() : null; + return TileCache.apply(coord); } private static int cacheSize() {