Buckets: one emptying entry point, predict water poured beside into containers

Review round 1 on #6783:
- `Behaviours::bucket_empty` takes `beside: Option<BlockPos>` and returns
  where the contents went; Shift only counts with a hit result, as in
  vanilla's `!shiftKeyDown || hitResult == null`.
- The client leaves waterlogging to the server wherever the water lands,
  not only on the clicked block.
- `Hit::beside` replaces three copies of `pos + face`.
- The click cases get their own test, with a case where both blocks refuse.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

crates/horizon-client/src/game.rs

@@ -839,6 +839,16 @@ struct Hit {
     face: [i32; 3],
     point: [f64; 3],
 }
+impl Hit {
+    /// The block the clicked face leads to (`pos.relative(direction)`).
+    fn beside(&self) -> [i32; 3] {
+        [
+            self.pos[0] + self.face[0],
+            self.pos[1] + self.face[1],
+            self.pos[2] + self.face[2],
+        ]
+    }
+}
 /// What a falling block's tick did (`FallingBlockEntity.tick`).
 enum FallStep {
     Falling,
@@ -12636,11 +12646,7 @@ impl Game<'_> {
         } else if content.is_water() && self.res.behaviours().is_liquid_container(clicked) {
             return hit.pos;
         }
-        [
-            hit.pos[0] + hit.face[0],
-            hit.pos[1] + hit.face[1],
-            hit.pos[2] + hit.face[2],
-        ]
+        hit.beside()
     }
     /// The cauldron at `pos`, and whether water is above it
     /// (`CauldronInteractions.isUnderWater`).
@@ -13068,16 +13074,14 @@ impl Game<'_> {
                 return Ok(UseResult::Fail);
             }
             let pos = BlockPos::new(target[0], target[1], target[2]);
-            let beside = BlockPos::new(
-                hit.pos[0] + hit.face[0],
-                hit.pos[1] + hit.face[1],
-                hit.pos[2] + hit.face[2],
-            );
-            // `Player.isShiftKeyDown`: only air takes a fluid under Shift.
+            let [x, y, z] = hit.beside();
+            let beside = Some(BlockPos::new(x, y, z));
+            // `Player.isShiftKeyDown`: under Shift only air takes the fluid;
+            // anything else passes it to the block beside.
             let sneaking = self.keys.contains(&KeyCode::ShiftLeft);
             let creative = self.creative;
             let emptied = self.with_level(|b, level, ticks| {
-                b.bucket_empty_beside(
+                b.bucket_empty(
                     level, ticks, pos, beside, content, sound, sneaking, creative,
                 )
             });

crates/horizon-client/src/game/dispenser.rs

@@ -1247,10 +1247,10 @@ impl Game<'_> {
             horizon_client::items::bucket_contents(&item).expect("filled bucket dispatcher");
         let target_pos = Self::pos_of(target);
         let emptied = self.with_level(|b, level, ticks| {
-            b.bucket_empty(level, ticks, target_pos, content, sound, false, false)
+            b.bucket_empty(level, ticks, target_pos, None, content, sound, false, false)
         });
         self.process_level_events()?;
-        if !emptied {
+        if emptied.is_none() {
             return Ok((self.dispense_default(pos, facing, stack), CLICK));
         }
         if self.dim == super::Dim::Nether

crates/horizon-client/src/game/remote.rs

@@ -7652,24 +7652,22 @@ impl Game<'_> {
         };
         let target = self.bucket_target(content, &hit);
         let pos = horizon_core::position::BlockPos::new(target[0], target[1], target[2]);
-        let beside = horizon_core::position::BlockPos::new(
-            hit.pos[0] + hit.face[0],
-            hit.pos[1] + hit.face[1],
-            hit.pos[2] + hit.face[2],
-        );
+        let [x, y, z] = hit.beside();
+        let beside = Some(horizon_core::position::BlockPos::new(x, y, z));
         let sneaking = self.keys.contains(&KeyCode::ShiftLeft);
         let creative = self.creative;
-        // A water bucket on a liquid container fills it server-side only;
-        // a container that refuses passes it to the block beside, which
-        // this client changes itself.
-        let clicked = self.world.block(hit.pos[0], hit.pos[1], hit.pos[2]);
-        let container = content.is_water() && self.res.behaviours().is_liquid_container(clicked);
         self.with_level(|b, level, ticks| {
             let mut level = DryLevel::new(level);
-            let emptied = b.bucket_empty_beside(
+            let emptied = b.bucket_empty(
                 &mut level, ticks, pos, beside, content, sound, sneaking, creative,
             );
-            let client_change = !(container && emptied == Some(pos));
+            // Water poured into a liquid container (where it went, which may
+            // be the block beside a refusing one) fills it server-side only.
+            let client_change = !(content.is_water()
+                && emptied.is_some_and(|at| {
+                    use horizon_world::level::LevelAccess as _;
+                    b.is_liquid_container(level.block_state(at))
+                }));
             (
                 emptied.is_some(),
                 client_change.then(|| level.changed.into_iter().collect()),

crates/horizon-world/src/level/buckets.rs

@@ -65,7 +65,7 @@ impl<L: LevelAccess + ?Sized> Sim<'_, L> {
     /// `hitResult.getBlockPos().relative(hitResult.getDirection())` (None for
     /// a dispenser, which has no hit result). `sound` is the bucket's empty
     /// sound (mob buckets have their own); `sneaking` is
-    /// `user.isShiftKeyDown()` (false for a dispenser, whose user is null);
+    /// `user.isShiftKeyDown()`, which only counts with a hit result;
     /// `creative` is whether that user is a creative player.
     pub(crate) fn bucket_empty(
         &mut self,
@@ -109,14 +109,14 @@ impl<L: LevelAccess + ?Sized> Sim<'_, L> {
         let place_liquid = may_replace || container;
         // `canPlaceFluidInsideBlock`: air always takes the fluid; a block that
         // merely holds it (a waterloggable block, a replaceable plant) only
-        // when the player is not sneaking (a dispenser has no user and so
-        // no hit result, which always allows it).
-        if !(b.is_air(state) || place_liquid && !sneaking) {
+        // when the player is not sneaking or there is no hit result (a
+        // dispenser, or the retry below).
+        if !(b.is_air(state) || place_liquid && (!sneaking || beside.is_none())) {
             // A refused click (a double slab, or Shift on a waterloggable
             // block) pours into the block beside the clicked face instead,
-            // without a hit result, so Shift no longer matters there.
+            // without a hit result.
             return beside.and_then(|beside| {
-                self.bucket_empty(beside, None, content, sound, false, creative)
+                self.bucket_empty(beside, None, content, sound, sneaking, creative)
             });
         }
         let water = matches!(fluid, FluidKind::Water | FluidKind::FlowingWater);
@@ -184,36 +184,21 @@ impl Behaviours {
     ) -> Option<&'static str> {
         self.sim(level, ticks).bucket_pickup(pos, creative)
     }
-    /// `DispensibleContainerItem.emptyContents(user, level, pos, null)`.
+    /// `DispensibleContainerItem.emptyContents(user, level, pos, hitResult)`:
+    /// where the contents went. `beside` is the block beside the clicked
+    /// face, tried when `pos` refuses (None without a hit result).
     pub fn bucket_empty<L: LevelAccess + ?Sized>(
         &self,
         level: &mut L,
         ticks: &mut Ticks,
         pos: BlockPos,
-        content: BucketContent,
-        sound: &str,
-        sneaking: bool,
-        creative: bool,
-    ) -> bool {
-        self.sim(level, ticks)
-            .bucket_empty(pos, None, content, sound, sneaking, creative)
-            .is_some()
-    }
-    /// `BucketItem.emptyContents(user, level, pos, hitResult)` for a click:
-    /// `beside` is the block beside the clicked face, tried when `pos`
-    /// refuses. Returns where the contents went.
-    pub fn bucket_empty_beside<L: LevelAccess + ?Sized>(
-        &self,
-        level: &mut L,
-        ticks: &mut Ticks,
-        pos: BlockPos,
-        beside: BlockPos,
+        beside: Option<BlockPos>,
         content: BucketContent,
         sound: &str,
         sneaking: bool,
         creative: bool,
     ) -> Option<BlockPos> {
         self.sim(level, ticks)
-            .bucket_empty(pos, Some(beside), content, sound, sneaking, creative)
+            .bucket_empty(pos, beside, content, sound, sneaking, creative)
     }
 }

crates/horizon-world/src/level/tests.rs

@@ -880,15 +880,20 @@ fn bucket_source_removal_drains_spread_water() {
     let mut w = world!();
     w.floor(0, 10, "stone");
     let at = BlockPos::new(0, 1, 0);
-    assert!(w.b().bucket_empty(
-        &mut w.level,
-        &mut w.ticks,
-        at,
-        BucketContent::Fluid(crate::fluid::FluidKind::Water),
-        "minecraft:item.bucket.empty",
-        false,
-        false
-    ));
+    assert!(
+        w.b()
+            .bucket_empty(
+                &mut w.level,
+                &mut w.ticks,
+                at,
+                None,
+                BucketContent::Fluid(crate::fluid::FluidKind::Water),
+                "minecraft:item.bucket.empty",
+                false,
+                false
+            )
+            .is_some()
+    );
     for _ in 0..60 {
         w.tick();
     }
@@ -2214,6 +2219,21 @@ fn placing_and_breaking_report_the_affected_state() {
 fn dispenser_buckets_pick_up_and_empty() {
     let mut w = world!();
     let at = BlockPos::new(0, 10, 0);
+    // A dispenser has no hit result: nothing beside to try.
+    let empty = |w: &mut World, content, sound: &str, creative| {
+        w.f.behaviours
+            .bucket_empty(
+                &mut w.level,
+                &mut w.ticks,
+                at,
+                None,
+                content,
+                sound,
+                false,
+                creative,
+            )
+            .is_some()
+    };
     let water = w.id("water", &[("level", "0")]);
     w.put(at, water);
     let picked = w.b().bucket_pickup(&mut w.level, &mut w.ticks, at, false);
@@ -2246,15 +2266,9 @@ fn dispenser_buckets_pick_up_and_empty() {
     w.put(at, barrier);
     let sound = "minecraft:item.bucket.empty";
     let content = BucketContent::Fluid(crate::fluid::FluidKind::Water);
-    assert!(
-        !w.b()
-            .bucket_empty(&mut w.level, &mut w.ticks, at, content, sound, false, false)
-    );
+    assert!(!empty(&mut w, content, sound, false));
     assert_eq!(w.prop(at, "waterlogged"), "false");
-    assert!(
-        w.b()
-            .bucket_empty(&mut w.level, &mut w.ticks, at, content, sound, false, true)
-    );
+    assert!(empty(&mut w, content, sound, true));
     assert_eq!(w.prop(at, "waterlogged"), "true");
     assert_eq!(
         w.b().bucket_pickup(&mut w.level, &mut w.ticks, at, false),
@@ -2268,86 +2282,69 @@ fn dispenser_buckets_pick_up_and_empty() {
     // Emptying water into air makes a source; stone refuses.
     let air = w.b().air();
     w.put(at, air);
-    assert!(
-        w.b()
-            .bucket_empty(&mut w.level, &mut w.ticks, at, content, sound, false, false)
-    );
+    assert!(empty(&mut w, content, sound, false));
     assert_eq!(w.name(at), "water[level=0]");
     let stone = w.id("stone", &[]);
     w.put(at, stone);
-    assert!(
-        !w.b()
-            .bucket_empty(&mut w.level, &mut w.ticks, at, content, sound, false, false)
-    );
+    assert!(!empty(&mut w, content, sound, false));
     // Water fills a waterloggable slab instead of replacing it.
     let dry = w.id("oak_slab", &[]);
     w.put(at, dry);
-    assert!(
-        w.b()
-            .bucket_empty(&mut w.level, &mut w.ticks, at, content, sound, false, false)
-    );
+    assert!(empty(&mut w, content, sound, false));
     assert_eq!(w.prop(at, "waterlogged"), "true");
-    // Sneak-clicking the slab refuses: only air takes a fluid under Shift.
-    let dry = w.id("oak_slab", &[]);
-    w.put(at, dry);
-    assert!(
-        !w.b()
-            .bucket_empty(&mut w.level, &mut w.ticks, at, content, sound, true, false)
-    );
-    assert_eq!(w.prop(at, "waterlogged"), "false");
-    // A click on the slab pours beside it, where Shift no longer matters.
-    let above = BlockPos::new(at.x, at.y + 1, at.z);
-    assert_eq!(
-        w.b().bucket_empty_beside(
-            &mut w.level,
-            &mut w.ticks,
-            at,
-            above,
-            content,
-            sound,
-            true,
-            false
-        ),
-        Some(above)
-    );
-    assert_eq!(w.prop(at, "waterlogged"), "false");
-    assert_eq!(w.name(above), "water[level=0]");
-    // A double slab holds no water: the click fills the block beside it.
-    let air = w.b().air();
-    w.put(above, air);
-    let double = w.id("oak_slab", &[("type", "double")]);
-    w.put(at, double);
-    assert_eq!(
-        w.b().bucket_empty_beside(
-            &mut w.level,
-            &mut w.ticks,
-            at,
-            above,
-            content,
-            sound,
-            false,
-            false
-        ),
-        Some(above)
-    );
-    assert_eq!(w.prop(at, "type"), "double");
-    assert_eq!(w.name(above), "water[level=0]");
-    w.put(above, air);
     // Powder snow only goes into empty space.
     let air = w.b().air();
     w.put(at, air);
-    assert!(w.b().bucket_empty(
-        &mut w.level,
-        &mut w.ticks,
-        at,
+    assert!(empty(
+        &mut w,
         BucketContent::Powder,
         "minecraft:item.bucket.empty_powder_snow",
-        false,
         false
     ));
     assert_eq!(w.name(at), "powder_snow[]");
 }
 
+#[test]
+fn clicked_bucket_pours_beside_a_refusing_block() {
+    let mut w = world!();
+    let at = BlockPos::new(0, 10, 0);
+    let above = BlockPos::new(0, 11, 0);
+    let sound = "minecraft:item.bucket.empty";
+    let content = BucketContent::Fluid(crate::fluid::FluidKind::Water);
+    let air = w.b().air();
+    let click = |w: &mut World, sneaking: bool| {
+        w.f.behaviours.bucket_empty(
+            &mut w.level,
+            &mut w.ticks,
+            at,
+            Some(above),
+            content,
+            sound,
+            sneaking,
+            false,
+        )
+    };
+    // Sneak-clicking a slab pours beside it, where Shift no longer counts.
+    let slab = w.id("oak_slab", &[]);
+    w.put(at, slab);
+    assert_eq!(click(&mut w, true), Some(above));
+    assert_eq!(w.prop(at, "waterlogged"), "false");
+    assert_eq!(w.name(above), "water[level=0]");
+    // A double slab holds no water: the click fills the block beside it.
+    w.put(above, air);
+    let double = w.id("oak_slab", &[("type", "double")]);
+    w.put(at, double);
+    assert_eq!(click(&mut w, false), Some(above));
+    assert_eq!(w.prop(at, "type"), "double");
+    assert_eq!(w.name(above), "water[level=0]");
+    // Both refusing empties nothing.
+    let stone = w.id("stone", &[]);
+    w.put(above, stone);
+    assert_eq!(click(&mut w, false), None);
+    assert_eq!(w.prop(at, "type"), "double");
+    assert_eq!(w.get(above), stone);
+}
+
 #[test]
 fn creaking_heart_follows_its_logs_and_the_night() {
     let mut w = world!();