Skip to content

Apply babbaj's review of the Java pathfinder - #20

Merged
0Mattias merged 2 commits into
mainfrom
claude/pathfinder-review-cleanup
Sep 12, 2026
Merged

0Mattias merged 2 commits into
mainfrom
claude/pathfinder-review-cleanup

Conversation

@0Mattias

Copy link
Copy Markdown
Owner

babbaj reviewed the port on cabaletta/baritone#5117. His points were about the port's seams rather than its arithmetic, so they apply here unchanged, and this brings back the review commit from the fork's java-nether-pathfinder branch (0Mattias/baritone a8641949) — the one the pull request now carries.

  • the port's own BlockPos is gone; it uses Minecraft's
  • the int constants for the dimension and the cache-miss mode are enums, and hasChunkFromJava is hasChunkFromCaller
  • the wrappers that existed to pack arguments for JNI — isVisible, isVisibleMulti, the batch raytrace, the long-packed path segment — are gone; callers use Raytracer.raytrace and List<BlockPos> directly
  • Raytracer.raytrace returns the hit position or null instead of a boolean plus an out-array
  • the search and the region reader time themselves with currentTimeMillis, and Chunk calls its 16-block slices sections
  • BaritoneRegion checks CachedRegion.CACHED_REGION_MAGIC, now public
  • elytraCustomAllocator is removed rather than kept as a deprecated no-op

Cheesecake's own differences survive it: NetherPathfinderContext keeps the UnusableRays guards and the read/write locking, so the per-ray calls that replace the batch wrappers sit inside them; the region reader still reads a region's blocks in the order Baritone writes them; package-info keeps its own wording. The pathfinder package is otherwise byte for byte the branch's, modulo the package name and the licence header, which is how it should stay while the pull request is open.

All 102 unit tests pass locally, including the oracle tests that hold the port to the native library's recorded answers, and CancelTest comes across with them.

🤖 Generated with Claude Code

0Mattias and others added 2 commits September 12, 2026 16:24
The port was offered to upstream as cabaletta/baritone#5117 and babbaj
reviewed it there. His points were about the port's seams rather than its
arithmetic, so they apply here unchanged: the class kept Minecraft at
arm's length in places where it did not have to, and it still carried the
shims the FFI needed and plain Java does not.

Taken from the review commit on the fork's java-nether-pathfinder branch
(0Mattias/baritone a8641949), which is what the pull request now carries:

  - the port's own BlockPos is gone; it uses Minecraft's, as the rest of
    Baritone does, and keeps only the floor helper as static methods
  - the int constants for the dimension and the cache-miss mode are
    enums, and hasChunkFromJava is hasChunkFromCaller, since both sides
    of the call are Java now
  - the wrappers that existed to pack arguments for JNI -- isVisible,
    isVisibleMulti, the batch raytrace, the long-packed path segment --
    are gone; callers use Raytracer.raytrace and List<BlockPos> directly
  - Raytracer.raytrace returns the hit position or null instead of a
    boolean plus an out-array, and its inner loop computes the child
    node's arguments before the recursive step rather than inside it
  - the search and the region reader time themselves with
    currentTimeMillis, and Chunk calls its 16-block slices sections
  - BaritoneRegion checks CachedRegion.CACHED_REGION_MAGIC, now public,
    rather than duplicating the number
  - elytraCustomAllocator is removed rather than kept as a deprecated
    no-op: it only ever chose the native library's allocator, and there
    is no native library any more

Cheesecake's own differences survive it. NetherPathfinderContext keeps the
UnusableRays guards around every ray and the read/write locking, so the
per-ray calls that replace the batch wrappers sit inside them; the region
reader keeps reading a region's blocks in the order Baritone writes them;
package-info keeps its own wording. The pathfinder package is otherwise
byte for byte the branch's, modulo the package name and the licence
header, which is how it should stay while the pull request is open.

The oracle tests still hold the port to the native library's recorded
answers, and CancelTest comes across with them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The line was written when the Nether pathfinder was babbaj's library
loaded through JNI. It is a Java port in the mod's own source now, so the
word is wrong; what the sentence is actually drawing a line around is
that the elytra pathfinder is a separate one from the walking pathfinder,
with its own copy of the terrain.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@0Mattias
0Mattias merged commit 3c7ac44 into main Sep 12, 2026
8 checks passed
@0Mattias
0Mattias deleted the claude/pathfinder-review-cleanup branch September 12, 2026 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant