ClassiCube / MCGalaxy

A Minecraft Classic / ClassiCube server software
GNU General Public License v3.0
162 stars 76 forks source link

Trains exhibit a strong bias toward 0,0 #709

Closed rdebath closed 2 years ago

rdebath commented 2 years ago

Make a 256xNx256 map Place a checkered layer of red and white blocks. Place 2000-3000 train blocks above these. Run for a while, the train blocks should spread out evenly across the map, however, instead they display a marked preference to move toward the 0,0,0 corner.

This appears to be due to a very poor random number generator as this change fixes the issue...

diff --git a/MCGalaxy/Blocks/Physics/TrainPhysics.cs b/MCGalaxy/Blocks/Physics/TrainPhysics.cs
index 8010d19b4..b8cf6beda 100644
--- a/MCGalaxy/Blocks/Physics/TrainPhysics.cs
+++ b/MCGalaxy/Blocks/Physics/TrainPhysics.cs
@@ -25,9 +25,9 @@ namespace MCGalaxy.Blocks.Physics {

         public static void Do(Level lvl, ref PhysInfo C) {
             Random rand = lvl.physRandom;
-            int dirX = rand.Next(1, 10) <= 5 ? 1 : -1;
-            int dirY = rand.Next(1, 10) <= 5 ? 1 : -1;
-            int dirZ = rand.Next(1, 10) <= 5 ? 1 : -1;
+            int dirX = rand.Next(0, 1023) <= 511 ? 1 : -1;
+            int dirY = rand.Next(0, 1023) <= 511 ? 1 : -1;
+            int dirZ = rand.Next(0, 1023) <= 511 ? 1 : -1;
             ushort x = C.X, y = C.Y, z = C.Z;

             for (int dx = -dirX; dx != 2 * dirX; dx += dirX)

The change means that bit 9 rather than bit 4(ish) is used as the random source and the modulus is a power of two.

UnknownShadow200 commented 2 years ago

Whoever wrote train physics originally forgot that 10 was exclusive not inclusive, which caused the gradual bias

rdebath commented 2 years ago

Okaay. Inclusive lower, exclusive upper. You'll notice I have the same issue, but because I chose a larger range (to use the higher bit) it doesn't impact so much.

UnknownShadow200 commented 2 years ago

For consistency with elsewhere I went with rand.Next(1, 100+1)

fun fact: deleting fireworks handler probably also has the same mistake