Fluids actually work — placement, physics, and rendering - #31
Merged
Conversation
Water/lava placed into a subgrid via a bucket never worked end to end. Three independent, compounding bugs, found and fixed via live debugging plus decompiling 1.21.1's actual runtime classes: 1. FakeCellGetter#getFluidState was hardcoded to always return Fluids.EMPTY — a leftover stub from before fluid support existed at all. BlockGetter#getFluidState has no vanilla default (abstract); every real implementer derives it from getBlockState(pos).getFluidState() instead. Since FlowingFluid#spreadTo/getNewLiquid (and effectively all of vanilla's own flow-decision logic) read neighbor state exclusively through this method, the fake space looked permanently fluid-free everywhere, including at a piece's own anchor. 2. BucketItem doesn't override useOn(UseOnContext) (confirmed via javap) — it only overrides use(Level, Player, InteractionHand), which vanilla dispatches as a SEPARATE fallback only when useOn PASSes (Minecraft #startUseItem tries useItemOn first, then useItem if that didn't consume the action). Our SubgridEventHandler only ever tried useOn, so a bucket's own useOn always did nothing — and since our RightClickBlock cancellation never set a cancellationResult, vanilla's fallback still ran uncontrolled against the REAL world, placing a real, full-size water block next to/on the SubgridBlock (this is what looked like water "appearing big then disappearing" — real unsupported vanilla water just draining away on its own). Fixed by: (a) setting event.setCancellationResult(SUCCESS) so the vanilla fallback never fires, and (b) handling buckets generically in runItemInteraction via DispensibleContainerItem#emptyContents (filling) and BucketPickup #pickupBlock (emptying into an empty bucket) — the same position-based hooks vanilla's own dispenser code uses instead of a player-raycast, called directly with the fake anchor/hit we already compute. Also fixed a latent gap this newly exercises: runItemInteraction's own touched-cells loop had no self-destruct handling for a piece turning to air (unlike VanillaBlockPiece#applyChanges, which already does), so picking up a water piece would have left a ghost occupying the cell. 3. Even with fluid state and placement correct, water rendered invisible: a fluid's BlockState has RenderShape.INVISIBLE, so renderSingleBlock (what SubgridRenderer already uses for every other piece) is a deliberate no-op for it — fluids render through a completely separate path, BlockRenderDispatcher#renderLiquid, needing a real BlockAndTintGetter (SubgridFluidView, new — local-grid-cell-based, mirrors SubgridBlockEntity#realBlockStateAt) rather than a single BlockState. First attempt (scale via PoseStack around the call) did nothing — renderLiquid takes no PoseStack at all, writing straight into the VertexConsumer via untransformed addVertex(x,y,z), which is why it visually looked like a layer floating near the camera instead of at the subgrid's actual position. Fixed with ScaledVertexConsumer, which routes every position/normal through VertexConsumer's own addVertex(PoseStack.Pose,...)/setNormal(PoseStack.Pose,...) default methods using the renderer's own captured base pose, applying the scale-to-cell-size transform that was missing. Live-tested through step 2 (placement/spread confirmed working — user observed it "flow" between cells) before rendering was investigated and fixed blind (compile + decompiled-source reasoning only, no further live verification available this session). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RwDHChA5dUTFWFKvM19z4S
Was used live to catch the exact tick where the old real-world bucket fallback stomped on the new fake-space placement — no longer needed now that the actual bugs (RightClickBlock cancellation result, useOn/use dispatch) are fixed rather than just diagnosed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RwDHChA5dUTFWFKvM19z4S
… last piece Follow-up to c59d8ef, found on a second read-through: removeAndDrop (added for BucketPickup emptying a water piece back into a bucket) only removes the piece itself, unlike onBlockBreak/handleMinePiece's own cleanup — picking up the last piece in a subgrid left an empty-but-still-present SubgridBlock behind instead of removing it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RwDHChA5dUTFWFKvM19z4S
Second read-through of c59d8ef found two more issues: - SubgridFluidView#getLightEngine() pointed LiquidBlockRenderer's own lighting math at the REAL light engine, but LevelRenderer#getLightColor queries brightness through BlockAndTintGetter#getBrightness/ getRawBrightness (default methods, confirmed via decompiling LevelRenderer) at the fake, tiny local anchor position — not through getLightEngine() directly. That queried real-world light near world origin instead of the subgrid's actual surroundings. Overridden getBrightness/getRawBrightness to substitute the subgrid's one real position instead, same pattern v2.FakeLevel already uses for its own light/biome queries. - FakeCellGetter's Fluids import was dead after c59d8ef removed the only use of Fluids.EMPTY. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RwDHChA5dUTFWFKvM19z4S
… subgrid boundary placePieceCrossBoundary's target cell was blocked by canFit's unconditional "already occupied -> refuse" rule, so only the very first write into an empty neighbor cell ever landed. Every subsequent fluid level update to that same boundary cell (spread/equalize/dry-up happens every tick) was silently dropped, leaving the far side desynchronized from then on. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XgNCHeYEyhW3Uk7qo8nhaG
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Water/lava placed into a subgrid via a bucket never worked end to end. Three independent, compounding bugs, found and fixed via live debugging (with the user, earlier tonight) plus decompiling 1.21.1's actual runtime classes for the parts that had to be finished unattended afterward — see the "Verification" section for exactly which parts got a live check and which didn't.
1.
FakeCellGetter#getFluidStatealways returned emptyHardcoded
Fluids.EMPTY.defaultFluidState()— a leftover stub from before fluid support existed at all.BlockGetter#getFluidStatehas no vanilla default (it's abstract); every real implementer derives it fromgetBlockState(pos).getFluidState()instead. SinceFlowingFluid#spreadTo/getNewLiquid(and effectively all of vanilla's own flow-decision logic) read neighbor state exclusively through this method, the fake space looked permanently fluid-free everywhere, including at a piece's own anchor.2. Buckets were never actually being placed by our own code — and the real vanilla fallback ran uncontrolled
BucketItemdoesn't overrideuseOn(UseOnContext)(confirmed viajavap) — onlyuse(Level, Player, InteractionHand), which vanilla dispatches as a separate fallback only whenuseOnPASSes (Minecraft#startUseItemtriesuseItemOnfirst, thenuseItemif that didn't consume the action).SubgridEventHandleronly ever trieduseOn, so a bucket's own interaction always did nothing on our side — and since ourRightClickBlockcancellation never set acancellationResult, vanilla's fallback still ran uncontrolled against the real world, placing a real, full-size water block next to/on the SubgridBlock. This is what looked like water "appearing big, then disappearing" — that was real, unsupported vanilla water just draining away on its own, not our engine.Fixed by:
event.setCancellationResult(InteractionResult.SUCCESS)so the vanilla fallback never fires.runItemInteractionviaDispensibleContainerItem#emptyContents(filling) andBucketPickup#pickupBlock(emptying into an empty bucket) — the same position-based hooks vanilla's own dispenser code uses instead of a player-raycast, called directly with the fake anchor/hit already computed. No bucket-specific special-casing beyond these two generic vanilla interfaces.runItemInteraction's own touched-cells loop had no self-destruct handling for a piece turning to air (unlikeVanillaBlockPiece#applyChanges, which already does this) — picking up a water piece would have left a ghost occupying the cell. Fixed to match.3. Water was invisible even once placement/physics worked
A fluid's
BlockStatehasRenderShape.INVISIBLE—renderSingleBlock(whatSubgridRendereralready uses for every other piece) is a deliberate no-op for it. Fluids render through a completely separate path,BlockRenderDispatcher#renderLiquid, needing a realBlockAndTintGetter(SubgridFluidView, new — local-grid-cell-based, mirrorsSubgridBlockEntity#realBlockStateAt) rather than a singleBlockState.First attempt (scale via the ambient
PoseStackaround the call) did nothing —renderLiquidtakes noPoseStackat all, writing straight into theVertexConsumervia untransformedaddVertex(x,y,z). This is why it visually looked like a layer floating near the camera instead of at the subgrid's actual position (small raw coordinates being interpreted as already-final render-space positions instead of BE-local ones). Fixed withScaledVertexConsumer, which routes every position/normal throughVertexConsumer's ownaddVertex(PoseStack.Pose,...)/setNormal(PoseStack.Pose,...)default methods using the renderer's own captured base pose — applying the scale-to-cell-size transform that was missing.Also found on review:
SubgridFluidView's lighting pointedLiquidBlockRenderer's internal brightness lookup at the real light engine queried at the fake local anchor position instead of the subgrid's actual real-world position (LevelRenderer#getLightColorreads throughgetBrightness/getRawBrightness, notgetLightEngine()directly) — fixed to substitute the subgrid's real position, same patternv2.FakeLevelalready uses for its own light/biome queries.Verification
./gradlew build(full build) passes clean, reasoned through carefully by decompilingBlockRenderDispatcher/LiquidBlockRenderer/VertexConsumer/LevelRendererto confirm the exact transform/lighting chain — but not live-tested, since this was finished after the user had to step away for the night. Flagging explicitly rather than overclaiming: the math checks out on paper, someone should place water and actually look at it before calling this fully done.Also this session
Reviewed and triaged the full open-issue backlog against current code/live-test results: closed #19 (lamp — confirmed vanilla-accurate behavior, not a bug), #20 (random-tick model — already fixed in current code, matches the issue's own proposed fix), #21 (falling blocks — confirmed fixed), #22 (Mekanism research — conclusion already written up), #24 (sapling crash-risk — confirmed no crash, though leaf rendering is separately still buggy per #26), #25 (bucket/chest-crash — was already fixed, issue said as much), #27 (resync flicker — fixed across PR #29 + #30), #28 (wrap-up — superseded). Also investigated making hoppers work within/between subgrids per a follow-up feature request, found a real item-persistence/open-GUI conflict in the natural implementation, and deliberately did not ship it blind — full writeup on #23 for whoever picks it up next with a live client.