Make chunk sections only be considered all air if filled with Blocks.AIR only

[TelepathicGrunt]

A small PR to adjust how LevelChunkSection determines if it is entirely Air or not.

The problem this solves:

If you have a chunk section (16x16x16) filled with Cave Air or a modded block with isAir set to true, LevelChunkSection will return true for hasOnlyAir(). This will cause getBlockState to always return Blocks.AIR despite people setting other kinds of air blocks in the chunk. This only happens when the entire chunk section has blocks whose isAir is true. Thus leading to confusion from players or modders who were so sure they placed a different air block in the chunk section before mining out the rest of it.

It also makes it tougher for modders who has a modded isAir block with behaviors. If they fill a large area with that block, most of that block will be converted automatically to Blocks.AIR. This PR aims to make the hasOnlyAir be true if the chunk section is entirely vanilla air blocks and nothing else. Allowing modded air blocks to not get converted to Blocks.AIR while preserving the vanilla behavior for vanilla blocks. This then would match the expected and resultant behavior being done in the game.

In the code, many of the LevelChunkSection hasOnlyAir calls keeps making the assumption that if that method is true, the spot must be Blocks.AIR. Multiple places makes this assumption. Not good for mods.

Why not replace LevelChunkSection's hasOnlyAir to be hasOnlyEmpy or isEmpty?

Would increase patch size significantly. Figured keeping it small would be more ideal.

Why not add isEmpty to block and blockstate and check that in LevelChunkSection?

Realistically, this would be a useless method. If a modder made a block, then there's some behavior attached to the block or it is acting as some sort of marker. If we let modder mark it as isEmpty, suddenly, that block is replaced with Blocks.AIR in certain chunk sections and the modder's goal with the block is lost. I cannot see any realistic purpose for marking a modded block as isEmpty and having it spontaneously turn into Blocks.AIR. Might as well just use Blocks.AIR in the first place.

isEmpty is now a method added to IBlockExtension and IBlockStateExtension and returns true for AIR, CAVE_AIR, and VOID_AIR by default. Modded isAir blocks will not return true by default unless they override the method and explicitly return true.

Why not a c:air or c:airlike tag?

Many mods are already doing the isAir check thinking it handles everything they need. Adding a tag would require every single one of those mods to go in their code and replace all their isAir checks with a tag check. Seems like uptake would be incredibly slow. By doing this patch instead, their isAir check would keep working out of the box and mods adding air-like blocks can mark their blocks as isAir true without fear of being converted to Blocks.AIR.

What performance impact will this have? Any changes to vanilla worldgen?

Edit: latest changes will have zero impact changes to vanilla blocks. There should be no changes at all to vanilla’s behavior with vanilla blocks. Only modded air blocks will be affected

The only isAir blocks in vanilla are AIR, CAVE_AIR, and VOID_AIR:

• AIR is used everywhere. It's the main default block

• CAVE_AIR is used only in ravines in Overworld and Nether, caves in Nether, and in underground structures/features such as Mineshafts, Dungeons, and Strongholds.

• VOID_AIR is used only for positions outside of the world's build limits. Essentially, it is only when outside the chunk's height ranges. In the case of the Nether, from y = 128 to y = 256, AIR is placed. Above y= 256, VOID_AIR is place.

Before change, what Nether roof has.

After change, what Nether roof has:

So even with this change, nothing changed with the Nether roof so no change to vanilla behavior with vanilla blocks when double checking the Nether.

Hopefully this helps

[neoforged-pr-publishing]

• Publish PR to GitHub Packages

[TelepathicGrunt]

Closest Mojang issue report: https://bugs.mojang.com/browse/MC-232360

[embeddedt]

Your PR description is inconsistent. If isEmpty returns true for all three vanilla air blocks, then it behaves like isAir in a vanilla environment, so the patched code should behave identically to vanilla if there are no modded empty blocks. But then you say:

Maybe a few very rare chunk sections in ravines might now retain their CAVE_AIR instead of converting to AIR. Performance impact? Practically none.

Could you clarify what you mean here?

[embeddedt]

The same change should be made in recalcBlockCounts.

[TelepathicGrunt]

Your PR description is inconsistent. If isEmpty returns true for all three vanilla air blocks, then it behaves like isAir in a vanilla environment, so the patched code should behave identically to vanilla if there are no modded empty blocks. But then you say:

Maybe a few very rare chunk sections in ravines might now retain their CAVE_AIR instead of converting to AIR. Performance impact? Practically none.

Could you clarify what you mean here?

Description was written before the suggestions in discord which was to preserve vanilla behavior so vanilla should see zero changes. Only impacts modded air blocks.

[embeddedt]

Seems fine. The call sites of isEmpty aren't super hot so I don't think any interface overhead is a concern.

feedback accounted for