Remove block entity blocks from the octree and remove the unused invisible field.

[leMaik]

This builds upon #1376. While working on that PR, I found that we don't remove block entities from the octree yet, even if they have no actual block model (ie. intersect always returns false), which is the case for all block entity blocks except decorated pots.

This PR leads to fewer blocks in the octree, thus possibly smaller octrees and more merged nodes. E.g. signs are air now and not sign blocks that need intersection.

[NotStirred]

Looks great 👍 just some bikesheddable random thoughts

[NotStirred]

An additional thought, is it possible for a user to need both the entity tag and the ability to return multiple entities?

[leMaik]

The closest we have is the decorated pot, which uses the block entity information to update the block tag (creating a new Block instance) and then uses an entity to create the upper part (which is higher than 1x1x1 and thus can't be in the octree).

[NotStirred]

My understanding was that isBlockEntity represented this behaviour already, you either isBlockEntity and get replaced with the result, or hasEntity and don't.

[NotStirred]

Does it make any sense to just have this api hasEntity createEntities isReplacedByEntities, removing the second path?

Or have I just misunderstood the idea?

[leMaik]

My understanding was that isBlockEntity represented this behaviour already

It didn't do that yet (that's what this PR does) but it's indeed what I would have expected.

Does it make any sense to [remove toBlockEntity]?

Maybe. The only reason it exists is because we need block entity data for some entities but not for other entities where that block entity data isn't even available.

[NotStirred]

It just feels a bit weird that the callee decides which method gets called by two booleans, obviously not blocking ^>^

It would then have to be Collection<Entity> createEntities(Vector3 position, Optional<CompoundTag> entityTag)

[leMaik]

I absolutely agree. The current interface let's the block decide what to implement bit the callee decides which method to call. That only works well because there is only one callee.

[leMaik]

@NotStirred The problem is that we don't have the tile entity data when we instantiate the block. We load a chunk's tile entities after all blocks have been loaded and then either create block entities or create a new tag (and from that, a new block) with the tile entity data.

Collection<Entity> createEntities(Vector3 position, Optional<CompoundTag> entityTag)

If we were to do this, we would need to load the tile entities first, put them into some Map<Position, CompoundTag> and then get the correct entity tag while iterating through the blocks. The interface wouldn't be good though. If the tile entity is there, it isn't optional – it is required to create the entities.

A better way to think about createEntities and createBlockEntity is probably to think about block instances as factories for the entities that correspond to their block type. And of course, the callee of a factory knows which method to call (and the factory has methods to tell the callee what it supports).