BlockType#isOccluding is incorrectly handled

[ShaneBeee]

Expected behavior

BlockType#isOccluding should return whether the block is occluding

Observed/Actual behavior

I was alerted in the Skript community it was not working on an Observer block.
So I did a quick test:

from CraftBlockType:

@Override
public boolean isOccluding() {
    return this.getHandle().defaultBlockState().isRedstoneConductor(EmptyBlockGetter.INSTANCE, BlockPos.ZERO);
}

I would assume this should be:

@Override
public boolean isOccluding() {
    return this.getHandle().defaultBlockState().canOcclude();
}

Steps/models to reproduce

someone in the Skript community pointed this out to me, see pic/notes above.

Plugin and Datapack List

[22:24:23 INFO]: ℹ Server Plugins (9):
[22:24:23 INFO]: Paper Plugins (4):
[22:24:23 INFO]: - CorePlugin, SkBee, SkBriggy, SkNMS
[22:24:23 INFO]: Bukkit Plugins (5):
[22:24:23 INFO]: - BeeConomy, PermissionsEx, Skript, skript-reflect, Vault

Paper version

Paper version 26.1.2-5-main@d61d8df

Other

No response

[Malfrador]

This seems to be mostly a naming/inconsistency issue, partially on Mojang's side.

• CraftBlockData#isOccluding returns the block state canOcclude. That method is used for the light calculation, and for water droplets.
• CraftBlockType#isOccluding returns the block state isRedstoneConductor. That is the method used for entity spawning, chest opening, bat sleeping and redstone connectivity, so most actual occluding checks. Terribly named NMS method there. The Javadocs are also wrong here, claiming its used for lighting - its not.

Both don't reliably return the same value. e.g. for a Redstone block, BlockData#canOcclude will return true, while BlockType#canOcclude will return false, as the block itself is not a conductor. Same is true for observers and presumably other blocks.

Not sure how to solve this the best way, this is a bit of a mess.

[ShaneBeee]

id suggest changing it to return defaultBlockState().canOcclude (to match BlockData)
and create a separate method for BlockType#isRedstoneConductor

[Malfrador]

That would be a pretty significant behaviour break, e.g. for plugins using that method to check if a mob can spawn in a location.

[ShaneBeee]

that's fair yeah. quite the pickle here now ;)

[ShaneBeee]

another suggestion is deprecate both BlockType/BlockData#isOccluding...
and then introduce canOcclude and isRedstoneConductor