Some Structuren BoundingBoxes have a weird offset

fantahund commented 5 months ago

Expected behavior

Some structures return an incorrect position of the bounding box. The following structures usually return the bounding box too high: Cold Ocean Ruins, WarmOcean Ruins, Beached Shipwreck, Shipwreck, Buried Treasure The following structures usually return bounding boxes that are too low: Jungle Pyramid, Igloo All other structures seem to be correct.

The problem only seems to occur in newly created chunks. After a server restart, the bounding box is displayed correctly.

Observed/Actual behavior

Newly loaded chunk: Alt text

After a server restart: Alt text

Steps/models to reproduce

Install the StructureBoundingBoxTestPlugin.
Go to the structure described in the problem above. And see at which position the bounding box with diamond blocks is displayed.
Restart the server and go to the same structure and the bounding box seems to be in the right place.

Plugin and Datapack List

[12:07:15 INFO]: Server Plugins (1): [12:07:15 INFO]: Bukkit Plugins: [12:07:15 INFO]: - StructureBoundingBoxTestPlugin

[12:07:54 INFO]: There are 3 data pack(s) enabled: [vanilla (built-in)], [file/bukkit (world)], [paper (built-in)] [12:07:54 INFO]: There are no more data packs available

Paper version

[12:08:25 INFO]: Checking version, please wait... [12:08:25 INFO]: This server is running Paper version 1.21-37-master@dd49fba (2024-06-24T02:08:49Z) (Implementing API version 1.21-R0.1-SNAPSHOT) You are running the latest version

Other

No response

Kadeluxe commented 5 months ago

Just discovered this bug on 1.20 too so it's not 1.21 specific.

jugh3ad commented 4 months ago

This is vanilla behavior for the way some structures are created during initial chunks creation. The initial bounding box has a hardcoded Y level. After the terrain is generated the structure is moved to the appropriate Y level of the structure.

lynxplay commented 4 months ago

As jugh3ad correctly pointed out, that is simply how some structure pieces generate. We could make this interaction a little bit less painful by clearing the structure BB cache after the StructureStart is done with postProcessing, but I am hoping for some more input from the team here.

E.g.

diff --git a/src/main/java/net/minecraft/world/level/levelgen/structure/StructureStart.java b/src/main/java/net/minecraft/world/level/levelgen/structure/StructureStart.java
index c92a2c5bb..fbbc0f0e0 100644
--- a/src/main/java/net/minecraft/world/level/levelgen/structure/StructureStart.java
+++ b/src/main/java/net/minecraft/world/level/levelgen/structure/StructureStart.java
@@ -121,6 +121,7 @@ public final class StructureStart {
             }
             // CraftBukkit end

+            this.cachedBoundingBox = null; // Paper - invalidate cached bounding box after post processing pieces.
             this.structure.afterPlace(world, structureAccessor, chunkGenerator, random, chunkBox, chunkPos, this.pieceContainer);
         }
     }

would fix the bounding box after place. It'll still be fucked during the respective async events, but I don't think we really want to edit anything there.

PaperMC / Paper