From c3e73c9f11fc7e5ce0dbdfeae044c1910939c970 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=99=E3=81=94=E3=81=84=E3=82=B8=E3=83=A3=E3=83=B3?= Date: Thu, 20 Aug 2026 08:41:41 +0200 Subject: [PATCH] fix(worldgen): BlockDirection random sampling parity with vanilla (#2773) --- crates/pumpkin-data/src/block_direction.rs | 138 +++++++++++++++++- .../feature/features/chorus_plant.rs | 2 +- .../feature/features/coral/coral_claw.rs | 11 +- .../feature/features/coral/coral_tree.rs | 8 +- .../generation/feature/features/coral/mod.rs | 2 +- .../feature/features/drip_stone/small.rs | 2 +- .../feature/features/tree/root/mangrove.rs | 2 +- .../feature/features/tree/trunk/bending.rs | 3 +- .../feature/features/tree/trunk/dark_oak.rs | 3 +- .../feature/features/tree/trunk/forking.rs | 5 +- .../features/tree/trunk/upwards_branching.rs | 3 +- 11 files changed, 158 insertions(+), 21 deletions(-) diff --git a/crates/pumpkin-data/src/block_direction.rs b/crates/pumpkin-data/src/block_direction.rs index f6a98028e..b8147f0ab 100644 --- a/crates/pumpkin-data/src/block_direction.rs +++ b/crates/pumpkin-data/src/block_direction.rs @@ -81,11 +81,20 @@ impl BlockDirection { } pub fn random(random: &mut RandomGenerator) -> Self { - Self::all()[random.next_bounded_i32(Self::all().len() as i32 - 1) as usize] + // Vanilla `Direction.getRandom` = `values[nextInt(values.length)]` over all six + // directions, in declaration order (DOWN, UP, NORTH, SOUTH, WEST, EAST). + Self::all()[random.next_bounded_i32(Self::all().len() as i32) as usize] } + /// Vanilla `Direction.Plane.HORIZONTAL.getRandomDirection`: `nextInt(4)` over `[NORTH, EAST, + /// SOUTH, WEST]`. + /// + /// The bound is the full length (a `len() - 1` here made the last direction unreachable) and + /// the order is [`Self::horizontal_worldgen`], not [`Self::horizontal`], so a given draw picks + /// the direction vanilla picks. pub fn random_horizontal(random: &mut RandomGenerator) -> HorizontalFacing { - Self::horizontal()[random.next_bounded_i32(Self::horizontal().len() as i32 - 1) as usize] + let directions = Self::horizontal_worldgen(); + directions[random.next_bounded_i32(directions.len() as i32) as usize] } #[must_use] @@ -168,6 +177,21 @@ impl BlockDirection { ] } + /// The four horizontal directions in vanilla `Direction.Plane.HORIZONTAL` order NORTH, EAST, + /// SOUTH, WEST (matching vanilla's `getRandomDirection(random)` indexes with `nextInt(4)`). + /// Worldgen-parity code that samples or iterates a random horizontal direction MUST use this, + /// not [`Self::horizontal`] (whose `[North, South, West, East]` order would pick a different + /// direction for the same draw). + #[must_use] + pub const fn horizontal_worldgen() -> [HorizontalFacing; 4] { + [ + HorizontalFacing::North, + HorizontalFacing::East, + HorizontalFacing::South, + HorizontalFacing::West, + ] + } + #[must_use] pub const fn flow_directions() -> [Self; 5] { [Self::Down, Self::North, Self::South, Self::West, Self::East] @@ -305,3 +329,113 @@ impl HorizontalFacingExt for HorizontalFacing { } } } + +#[cfg(test)] +mod tests { + use super::*; + use pumpkin_util::random::legacy_rand::LegacyRand; + use pumpkin_util::random::xoroshiro128::Xoroshiro; + + fn legacy(seed: u64) -> RandomGenerator { + RandomGenerator::Legacy(LegacyRand::from_seed(seed)) + } + + fn xoroshiro(seed: u64) -> RandomGenerator { + RandomGenerator::Xoroshiro(Xoroshiro::from_seed(seed)) + } + + /// Vanilla `Direction.getRandom` indexes `values[nextInt(6)]` over the declaration order DOWN, + /// UP, NORTH, SOUTH, WEST, EAST. Pinned against `java.util.Random` with seed 12345. + #[test] + fn random_matches_java_util_random() { + let mut random = legacy(12345); + let drawn: Vec = (0..12) + .map(|_| BlockDirection::random(&mut random)) + .collect(); + use BlockDirection::{Down, East, South, Up, West}; + assert_eq!( + drawn, + [ + Up, West, South, Down, Up, West, Up, Down, Up, South, East, Down + ] + ); + } + + /// Vanilla `Direction.Plane.HORIZONTAL.getRandomDirection` indexes `nextInt(4)` over NORTH, + /// EAST, SOUTH, WEST. Pinned against `java.util.Random` with seed 12345. + #[test] + fn random_horizontal_matches_java_util_random() { + let mut random = legacy(12345); + let drawn: Vec = (0..8) + .map(|_| BlockDirection::random_horizontal(&mut random)) + .collect(); + use HorizontalFacing::{East, North, South, West}; + assert_eq!(drawn, [East, South, West, West, West, North, East, North]); + } + + /// Every production caller reaches these functions through the `Xoroshiro` variant + /// (feature-stage worldgen RNG, fire spread, gourd stems), while the parity tests above use + /// `Legacy`. These sequences are pinned from pumpkin's Xoroshiro implementation (whose + /// generator core is bit-verified against vanilla in `pumpkin-util`) to guard the enum dispatch + /// and the small-bound `next_bounded_i32` path against regressions. + #[test] + fn random_pinned_through_xoroshiro() { + let mut random = xoroshiro(12345); + let drawn: Vec = (0..12) + .map(|_| BlockDirection::random(&mut random)) + .collect(); + use BlockDirection::{Down, East, South, Up, West}; + assert_eq!( + drawn, + [ + Down, West, East, Down, South, West, Up, South, Up, East, West, East + ] + ); + + let mut random = xoroshiro(12345); + let drawn: Vec = (0..8) + .map(|_| BlockDirection::random_horizontal(&mut random)) + .collect(); + use HorizontalFacing as H; + assert_eq!( + drawn, + [ + H::North, + H::West, + H::West, + H::North, + H::South, + H::South, + H::North, + H::South + ] + ); + } + + /// With the old `len() - 1` bound the last entry of each array was unreachable. + #[test] + fn every_direction_is_reachable() { + let mut random = legacy(0); + let mut seen = [false; 6]; + for _ in 0..512 { + seen[BlockDirection::random(&mut random) as usize] = true; + } + assert!(seen.iter().all(|&s| s), "unreachable direction: {seen:?}"); + + let mut random = legacy(0); + let mut seen_horizontal = [false; 4]; + for _ in 0..512 { + let index = match BlockDirection::random_horizontal(&mut random) { + HorizontalFacing::North => 0, + HorizontalFacing::East => 1, + HorizontalFacing::South => 2, + HorizontalFacing::West => 3, + }; + seen_horizontal[index] = true; + } + assert!( + seen_horizontal.iter().all(|&s| s), + "unreachable horizontal direction: {seen_horizontal:?}" + ); + } +} diff --git a/crates/pumpkin-world/src/generation/feature/features/chorus_plant.rs b/crates/pumpkin-world/src/generation/feature/features/chorus_plant.rs index 32ab4d684..d9146297b 100644 --- a/crates/pumpkin-world/src/generation/feature/features/chorus_plant.rs +++ b/crates/pumpkin-world/src/generation/feature/features/chorus_plant.rs @@ -138,7 +138,7 @@ fn grow_tree_recursive( for _ in 0..stems { // Pick a random horizontal direction - let dir = BlockDirection::horizontal()[random.next_bounded_i32(4) as usize]; + let dir = BlockDirection::random_horizontal(random); let target = top.offset(dir.to_offset()); let target_below = target.down(); diff --git a/crates/pumpkin-world/src/generation/feature/features/coral/coral_claw.rs b/crates/pumpkin-world/src/generation/feature/features/coral/coral_claw.rs index d7bf81f44..3602e6a06 100644 --- a/crates/pumpkin-world/src/generation/feature/features/coral/coral_claw.rs +++ b/crates/pumpkin-world/src/generation/feature/features/coral/coral_claw.rs @@ -25,11 +25,14 @@ impl CoralClawFeature { if !CoralFeature::generate_coral_piece(chunk, block_registry, random, block, pos) { return false; } + let direction = BlockDirection::random_horizontal(random); let i = random.next_bounded_i32(2) + 2; - let direction = BlockDirection::horizontal() - [random.next_bounded_i32(BlockDirection::horizontal().len() as i32 - 1) as usize]; - // TODO: Shuffle - let directions = BlockDirection::horizontal().into_iter().take(i as usize); + // TODO: vanilla iterates the first `i` of Util.toShuffledList([direction, + // direction.getClockWise(), direction.getCounterClockWise()], random) — the + // shuffle consumes RNG draws and the opposite of `direction` is never visited. + let directions = BlockDirection::horizontal_worldgen() + .into_iter() + .take(i as usize); 'block0: for direction2 in directions { let mut pos = pos; let j = random.next_bounded_i32(2) + 1; diff --git a/crates/pumpkin-world/src/generation/feature/features/coral/coral_tree.rs b/crates/pumpkin-world/src/generation/feature/features/coral/coral_tree.rs index 4602b332d..b29783d8f 100644 --- a/crates/pumpkin-world/src/generation/feature/features/coral/coral_tree.rs +++ b/crates/pumpkin-world/src/generation/feature/features/coral/coral_tree.rs @@ -32,8 +32,12 @@ impl CoralTreeFeature { } let i = random.next_bounded_i32(3) + 2; - // TODO: Shuffle - let directions = BlockDirection::horizontal().into_iter().take(i as usize); + // TODO: vanilla takes the first `i` of Plane.HORIZONTAL.shuffledCopy(random) — a + // Fisher–Yates that must run over the horizontal_worldgen() [N, E, S, W] base + // order and consumes RNG draws. + let directions = BlockDirection::horizontal_worldgen() + .into_iter() + .take(i as usize); for dir in directions { pos = pos.offset(dir.to_offset()); let times = random.next_bounded_i32(5) + 2; diff --git a/crates/pumpkin-world/src/generation/feature/features/coral/mod.rs b/crates/pumpkin-world/src/generation/feature/features/coral/mod.rs index 44efd8de9..8ae743f87 100644 --- a/crates/pumpkin-world/src/generation/feature/features/coral/mod.rs +++ b/crates/pumpkin-world/src/generation/feature/features/coral/mod.rs @@ -55,7 +55,7 @@ impl CoralFeature { chunk.set_block_state(&pos.0, block_state); } } - for dir in BlockDirection::horizontal() { + for dir in BlockDirection::horizontal_worldgen() { let dir_pos = pos.offset(dir.to_offset()); if random.next_f32() >= 0.2 || GenerationCache::get_block_state(chunk, &dir_pos.0).to_block_id() != Block::WATER diff --git a/crates/pumpkin-world/src/generation/feature/features/drip_stone/small.rs b/crates/pumpkin-world/src/generation/feature/features/drip_stone/small.rs index 80d2a91e3..a6f6f8760 100644 --- a/crates/pumpkin-world/src/generation/feature/features/drip_stone/small.rs +++ b/crates/pumpkin-world/src/generation/feature/features/drip_stone/small.rs @@ -61,7 +61,7 @@ impl SmallDripstoneFeature { random: &mut RandomGenerator, ) { super::gen_dripstone(chunk, pos); - for dir in BlockDirection::horizontal() { + for dir in BlockDirection::horizontal_worldgen() { if random.next_f32() > self.directional_spread { continue; } diff --git a/crates/pumpkin-world/src/generation/feature/features/tree/root/mangrove.rs b/crates/pumpkin-world/src/generation/feature/features/tree/root/mangrove.rs index ec3e06a0d..eb225e96d 100644 --- a/crates/pumpkin-world/src/generation/feature/features/tree/root/mangrove.rs +++ b/crates/pumpkin-world/src/generation/feature/features/tree/root/mangrove.rs @@ -54,7 +54,7 @@ impl MangroveRootPlacer { roots.push(trunk_pos.down()); - for dir in BlockDirection::horizontal() { + for dir in BlockDirection::horizontal_worldgen() { let start = trunk_pos.offset_dir(dir.to_offset(), 1); let mut offshoots: Vec = Vec::new(); if !self.can_grow(chunk, random, start, dir, trunk_pos, &mut offshoots, 0) { diff --git a/crates/pumpkin-world/src/generation/feature/features/tree/trunk/bending.rs b/crates/pumpkin-world/src/generation/feature/features/tree/trunk/bending.rs index 109cfe048..2663d1c15 100644 --- a/crates/pumpkin-world/src/generation/feature/features/tree/trunk/bending.rs +++ b/crates/pumpkin-world/src/generation/feature/features/tree/trunk/bending.rs @@ -31,8 +31,7 @@ impl BendingTrunkPlacer { below_trunk_provider: &BlockStateProvider, trunk_block: &BlockState, ) -> (Vec, Vec) { - let horizontal_directions = BlockDirection::horizontal(); - let direction = horizontal_directions[random.next_bounded_i32(4) as usize]; + let direction = BlockDirection::random_horizontal(random); let log_height = height as i32 - 1; let mut pos = start_pos; diff --git a/crates/pumpkin-world/src/generation/feature/features/tree/trunk/dark_oak.rs b/crates/pumpkin-world/src/generation/feature/features/tree/trunk/dark_oak.rs index aa22c3a12..a0deb9beb 100644 --- a/crates/pumpkin-world/src/generation/feature/features/tree/trunk/dark_oak.rs +++ b/crates/pumpkin-world/src/generation/feature/features/tree/trunk/dark_oak.rs @@ -51,8 +51,7 @@ impl DarkOakTrunkPlacer { below_trunk_provider, ); - let horizontal_directions = BlockDirection::horizontal(); - let lean_direction = horizontal_directions[random.next_bounded_i32(4) as usize]; + let lean_direction = BlockDirection::random_horizontal(random); let lean_height = height as i32 - random.next_bounded_i32(4); let mut lean_steps = 2 - random.next_bounded_i32(3); diff --git a/crates/pumpkin-world/src/generation/feature/features/tree/trunk/forking.rs b/crates/pumpkin-world/src/generation/feature/features/tree/trunk/forking.rs index 65f188363..e1a46c074 100644 --- a/crates/pumpkin-world/src/generation/feature/features/tree/trunk/forking.rs +++ b/crates/pumpkin-world/src/generation/feature/features/tree/trunk/forking.rs @@ -34,8 +34,7 @@ impl ForkingTrunkPlacer { let mut nodes = Vec::new(); let mut logs = Vec::new(); - let horizontal_directions = BlockDirection::horizontal(); - let lean_direction = horizontal_directions[random.next_bounded_i32(4) as usize]; + let lean_direction = BlockDirection::random_horizontal(random); let lean_height = height as i32 - random.next_bounded_i32(4) - 1; let mut lean_steps = 3 - random.next_bounded_i32(3); @@ -69,7 +68,7 @@ impl ForkingTrunkPlacer { let mut branch_tx = start_pos.0.x; let mut branch_tz = start_pos.0.z; - let branch_direction = horizontal_directions[random.next_bounded_i32(4) as usize]; + let branch_direction = BlockDirection::random_horizontal(random); if branch_direction != lean_direction { let branch_pos = lean_height - random.next_bounded_i32(2) - 1; diff --git a/crates/pumpkin-world/src/generation/feature/features/tree/trunk/upwards_branching.rs b/crates/pumpkin-world/src/generation/feature/features/tree/trunk/upwards_branching.rs index a7cc893b2..c9a859765 100644 --- a/crates/pumpkin-world/src/generation/feature/features/tree/trunk/upwards_branching.rs +++ b/crates/pumpkin-world/src/generation/feature/features/tree/trunk/upwards_branching.rs @@ -41,8 +41,7 @@ impl UpwardsBranchingTrunkPlacer { if height_pos < height as i32 - 1 && random.next_f32() < self.place_branch_per_log_probability { - let branch_dir = - BlockDirection::horizontal()[random.next_bounded_i32(4) as usize]; + let branch_dir = BlockDirection::random_horizontal(random); let branch_len = self.extra_branch_length.get(random); let branch_pos = (branch_len - self.extra_branch_length.get(random) - 1).max(0); let branch_steps = self.extra_branch_steps.get(random);