TelepathicGrunt

A regex that is more correct and covers some missing reserved names is this:

.\.|(?:CON|PRN|AUX|NUL|CLOCK\$|CONIN\$|CONOUT\$|(?:COM|LPT)[¹²³0-9])(?:\..)?

Would be ideal if this is used instead or a form of it.

This affects 1.20.4. Specifically, the issue is because AIR, CAVE_AIR, and VOID_AIR all will return true for isAir method. In LevelChunkSection, if all blocks in a chunk section would return true for isAir, then that chunk section's hasOnlyAir method now returns true which will make the chunk's getBlockState now return only AIR. Effectively erasing the existence of CAVE_AIR and VOID_AIR from the chunks. (Chunk sections are a 16x16x16 area in a chunk)

1. So to reproduce easily, stand in center of a chunk and set a large area to air so there's no solid block interfering.
   /fill ~-16 ~-16 ~16 ~16 ~16 ~16 air

2. Then set block at feet to cave air.
   /setblock ~ ~ ~ minecraft:cave_air

3. Then try and test if the spot is cave air. (This will fail because chunk section is now all isAir true and will see all spots as AIR. CAVE_AIR is now hidden away)
   /execute if block ~ ~ ~ minecraft:cave_air

4. Set the block next to you to stone.
   /setblock ~1 ~ ~ minecraft:stone

5. Then set block at feet to cave air.
   /setblock ~ ~ ~ minecraft:cave_air

6. Then try and test if the spot is cave air. (This will succeed because that stone block breaks the hasOnlyAir method and forces the game to not think the chunk section is entirely AIR. Thus preserving the CAVE_AIR)
   /execute if block ~ ~ ~ minecraft:cave_air

This is definitely a bug in my opinion as now it is impossible to check for CAVE_AIR or VOID_AIR blocks if a chunk section contains only those 3 air blocks as it all gets converted straight to AIR in a confusing manner without player input. Any datapack that could be using CAVE_AIR or VOID_AIR as markers will hit this bug and break.

A solution is to swap the isAir check in LevelChunkSection to be a .is(Blocks.AIR) check instead since so much of the codebase make the assumption that hasOnlyAir means all the spots in the chunk section must be AIR when that assumption is not always true.

This issue is still present in 1.19.4 Minecraft with Sugar Cane not showing it’s true age in F3 screen. I would like to recommend that this gets looked at again by Mojang since Grum’s comment is about 7 years old and standards might had changed since then. Ideally, blockstate changes on server should be synced to the client to help promote easier F3 debugging of issues and/or allow resource packs to properly work. Example of a sugar cane resourcepack that tries to change the look of sugar cane to match their age is in this other issue report that was closed as a duplicate of this current one: MC-135830

Can confirm with 1.19.2 and 1.19.4 and affects structures generated in the world as well. This is the cause of Woodland Mansions unable to spawn Red/Brown Mushrooms in its 1x2_a7 piece despite the NBT file actually having the mushrooms: MC-160169

If you open up the Minecraft jar and look at the worldgen JSON files it ships with it, under data/minecraft/worldgen/template_pool/ancient_cities/structures.json, you will find in there that the city uses a "minecraft:list_pool_element" type to intentionally combine the three nbt files. Likely an attempt at combining them with a rot processor like Outpost towers do. however, they all run "minecraft:ancient_city_generic_degradation" processor list which only rots blocks under #minecraft:ancient_city_replaceable block tag which does not include Blue Wool or Light Blue Wool. Which means all three NBT camp files will combine always with no change.

Maybe the bug is a missing rot processor for Blue Wool and Light Blue Wool for this template pool entry. It is intended they all spawn in the same spot. The missing rotting is likely the actual bug part. That or they decided against the rotting and just replaced it with the normal processor to disable the wool rotting.

Quick way to see what I am talking about: https://github.com/misode/mcmeta/blob/ce568022daf9a26bf6625e96fd56fb3e0f068c32/data/minecraft/worldgen/template_pool/ancient_city/structures.json#L108-L134

Are you sure these are even suppose to have water in them? Water is such an odd choice. I believe they were suppose to have Grass Block in them with a flower on top. The trapdoors surround the Grass Block to make it look like a small garden. It fits and looks much better than water

Folks, there are precedence for Mojang fixing code that is not reproducible at all in vanilla or are impossible to trigger in vanilla. Example: MC-194811

I made that bug report I linked above because while it affected modded, Mojang should’ve been aware of it in case Mojang tries to remove or rename a structure type in the future. I essentially helped saved some headache for Mojang by bringing up a potential issue when they make future changes even though it was impossible to replicate back then in 1.16.5 vanilla (no custom structure types is able to be made by datapack)

So I’m not sure I agree that a bug report has to have gameplay impact unless you count modded gameplay being impacted.

The logspam is still present in 1.19.3.  The logger line is in the HangingEntity$readAdditionalSaveData method. It appears it is trying to check if the TileX, TileY, TileZ  position saved in the item frame's nbt is closer than 16 blocks to the new position the item frame is placed at. Issue is when generating structures, this saved nbt position will not match/be close to the new position which trigger the logger to spam about invalid position when the position is indeed valid. I think just removing this logger line would be good enough or at least bump it from Error level down to Debug level so users do not see this spam in the regular log files.

seems to be fixed in 1.19. Unable to reproduce anymore

We will need to wait until a future minecraft version. As of 1.18-pre5, Mojang removed the structure section from Biome json so it is currently impossible to change what biomes a structure spawns in by datapack. This is part of Mojang's refactoring of structures. Once they add back the ability to do custom structures and change where structures spawn, then we can test to see if the issue still persist. 

@WeslyMC , Did you make a new datapack specifically for 21w44a that adds ocean monument to a non-ocean/river biome and did /locate in a dimension that only has that biome? I know worldgen changed a bit do I am assuming the attached 1.17.1 datapack will not work in 21w44a so a new datapack has to be made

I would like to propose we split this bug report into two as the 1 block hole and 3 block high hole are caused by separate issues and needs different fixes. My bug report here handles the 1 block hole which is caused by an air block in an nbt file MC-240221

This bug report can focus more on the 3 block high hole as that's caused by the minecraft:woodland_mansion/small_wall.nbt piece being asymmetrical and not placed right. The 2021-10-31_16.44.20.png I attached to this bug report shows it better. Notice how the small_wall reached the column of cobblestone that marks the end of the 3rd floor's wall on the left side of the mansion's wall always. The 3 block high hole is always on the right side of the mansion wall and that's because the small wall starts 1 block to the left of the right side's cobblestone column.

The solution to filling in the 3 block high hole is to extend the small wall nbt to the right by 1 so it is symmetrical. Then, adjust the code to offset it to the new correct starting position so it is still aligned with the rest of the wall's patterns.

In WoodlandMansionPieces$MansionPiecePlacer.createRoof method, the small roof if statements should be changed to this to this for properly offsetting the newly resized small wall nbt piece. I had to do quite a bit of trial and error to get it working but this offset + the fixed nbt file did solve the 3 block high hole issue.

if (!WoodlandMansionPieces.MansionGrid.isHouse(simpleGrid, l + 1, k)) {

  blockPos15 = blockPos7.relative((Direction)rotation.rotate(Direction.EAST), 7);

  blockPos15 = blockPos15.relative(rotation.rotate(Direction.NORTH), 1);

  list.add(new WoodlandMansionPieces.WoodlandMansionPiece(this.structureManager, "small_wall", blockPos15, rotation));

}

if (!WoodlandMansionPieces.MansionGrid.isHouse(simpleGrid, l - 1, k)) {

  blockPos15 = blockPos7.relative((Direction)rotation.rotate(Direction.WEST), 1);

  blockPos15 = blockPos15.relative((Direction)rotation.rotate(Direction.SOUTH), 7);

  list.add(new WoodlandMansionPieces.WoodlandMansionPiece(this.structureManager, "small_wall", blockPos15, rotation.getRotated(Rotation.CLOCKWISE_180)));

}

if (!WoodlandMansionPieces.MansionGrid.isHouse(simpleGrid, l, k - 1)) {

  blockPos15 = blockPos7.relative((Direction)rotation.rotate(Direction.WEST), 1);

  blockPos15 = blockPos15.relative((Direction)rotation.rotate(Direction.NORTH), 1);

  list.add(new WoodlandMansionPieces.WoodlandMansionPiece(this.structureManager, "small_wall", blockPos15, rotation.getRotated(Rotation.COUNTERCLOCKWISE_90)));

}

if (!WoodlandMansionPieces.MansionGrid.isHouse(simpleGrid, l, k + 1)) {

  blockPos15 = blockPos7.relative((Direction)rotation.rotate(Direction.EAST), 7);

  blockPos15 = blockPos15.relative((Direction)rotation.rotate(Direction.SOUTH), 7);

  list.add(new WoodlandMansionPieces.WoodlandMansionPiece(this.structureManager, "small_wall", blockPos15, rotation.getRotated(Rotation.CLOCKWISE_90)));

}

@Avoma Err. I would split MC-110098 into two separate bug reports. The 3 block high hole is caused by piece placement ( from what I can tell) which is an entirely different problem. This single block hole is caused by an air block in an nbt piece.

Please do not mark this as duplicate and instead, make that bug report focus on the 3 block high hole while this one focuses on the single block hole.

The fix is correct that a woodland_mansion:corridor_floor_hight.nbt piece needs to be made that is 3 blocks taller. In code, within the WoodlandMansionPieces$MansionPiecePlacer.createMansion method,

list.add(new WoodlandMansionPieces.WoodlandMansionPiece(this.structureManager, "corridor_floor", blockPos3, rotation));

should be replaced with 

list.add(new WoodlandMansionPieces.WoodlandMansionPiece(this.structureManager, (floorIndex == 0 ? "corridor_floor" : "corridor_floor_high"), blockPos3, rotation));

That way the second and third floors of the mansion can use the taller corridor floor piece that can replace the terrain in the 3 block taller hallways. 

Here is a datapack that lowered the world's min y to -192. Here, no blocks are placed above sea level

image:

The experimental worldgen datapack being shared on Mojang's website compensates for this bug by setting 384 as the height. In retrospect, it really means height from bottom of world and not max y. Even though this is more of a feature request, I'm putting this here for any other user that may try to lower the world and face this issue and come here. Hopefully height will be changed to height_range or max_y instead to stop the confusion

I'm gonna agree with Yung that the workaround of restricting the pool weight to 150 is not a good solution. In fact, 150 is way too low and broke a lot of datapacks and my own mods as we use weights around 1000-3000 to try and get the piece ratios correct. So now we have to duplicate the pool entries a ton of times with 150 as the weight to get back to the same old ratios. I think raising the limit to 5000 would be best as the issue described above tends to occur when people try for several 1 in a million ratio for their pieces.

Of course, the best solution would be to just change net/minecraft/world/level/levelgen/feature/structures/StructureTemplatePool's rawTemplates field to be a list of WeighedRandom$WeighedRandomItem that each entry holds the element entry + weight, add an int totalWeight field that is calculated in the constructors for StructureTemplatePool, and get rid of the templates field entirely. Then simply just change getRandomTemplate to be return net/minecraft/util/WeighedRandom.getRandomItem(random, rawTemplates, totalWeight);. Basically, Minecraft already has a class dedicated to getting a random element from a weighted list and changing the pool class to use it would be ideal. Hopefully this helps! 

still present in 1.16.5. To help clarify the fix, go to wall_base nbt, make the left side jigsaw block's target_name say `minecraft:side_wall_left` and the right side `minecraft:side_wall_right`. Go side_wall_0 and side_wall_1 nbt files and now make their left jigsaw block's name field have `minecraft:side_wall_right` and the right jigsaw block have `minecraft:side_wall_left`. This will make it so that the two side walls can appear on either side of the base wall but facing the same direction as the base wall. 

@osfanjoshua Counterpoint, all these seemingly separate bugs are all caused by a single bug, the world using a hardcoded value for sealevel instead of the chunkgenerator's sealevel that the world already has. This disconnection between the hardcoded sealevel and actual sealevel used for worldgen cascades and causes numerous amount of issues down the line.

It seems far to unproductive to create a bug report for every single issue caused by a single bug and have the mojang dev hunt down every single one of these reports to mark as resolved  when they fix the one root cause. 

This is the same issue that plagues MC-169698. The other report shows it happens with Igloos during world generation

MC-169698

From my testing and debugging, this appears to be specifically caused by the json biome having any structures added to it and the user loads up an already generated chunk with that biome. The instructions about exiting and re-entering the dimension is not needed. Just fly forward in the dimension for a few seconds, turn around, and fly back and the issue will present itself. 

I was helping someone with their mod and they were using json biomes/json dimensions for their mod and they got this issue because their biome has structures in it. When I ran the debugger, their biome source was using the correct dynamic registry instance and was returning the correct biome instance. But within SChunkDataPacket class, it had the exact same dynamic registry so that was good. But the biome being returned from the chunk is a completely new biome instance that is not present in the dynamic registry at all. And that is what causes the massive lag spike as the game starts writing the error to the log like crazy. So as far as I can tell, the biome source is not the issue here. The chunk receives the correct biome as well as it doesn't error upon first generation. Rather, the issue seem to lie in how the chunk either saves the biome to memory or retrieves the biome from memory. But only when structures are present in the biome. 

Very odd situation but hopefully this digging I did helps