[Bug] BlockEntityType mixin is dangerous & slow

[embeddedt]

Describe the bug

Create Casing injects a hook into BlockEntityType.isValid that disables the valid blocks check for specific block entity types. This is very inefficient, as it must check equality against each of the listed types, and is also dangerous as it bypasses the purpose of the check (to protect against a desync of the block entity and the block in the world, which happens under certain conditions).

If ability to dynamically add blocks from addons to the BE type is a goal, a better approach would be to use BlockEntityTypeAddBlocksEvent to add any desired blocks to the BlockEntityType's set of valid blocks at startup.

I've attached a profiler screenshot highlighting that most of the CPU time in the BlockEntityType.isValid method is consumed by this mixin hook. This profiler report was captured from the Craftoria 1.23.0 modpack.

[iglee42]

I see the issue, but do you that if I made this mixin, it's because I don't have access to the blocks to add

You solution is a good idea, just there are many blocks that I need for theses block entity types which I don't have access to

[embeddedt]

If they are all instances of a given class, you can use instanceof and loop through all the blocks in BuiltInRegistries.BLOCK.

[iglee42]

If I follow you correctly you ask to loop on every deferredholder values in the blocks registry to avoid the lag causes by getting the value of ten block entity type deferred holders ?
I hope you see the issue here
And if your solution is better do directly a pull request, you seems to know how to fix the issue so do it github is done for that

[embeddedt]

The loop would be done once during startup, in the BlockEntityTypeAddBlocksEvent event, rather than checking 10 deferred holders on every call to BlockEntityType.isValid, where most of the calls are not even related to your block entity types. So yes, it's substantially more efficient.

I've already done a substantial amount of the legwork here by providing a detailed description, a profiler report, and a proposed solution. I'm also not making any demands for you to fix this immediately. I find the tone of your last sentence to be a bit rude in this context, but I will assume it's just a translation problem.

[iglee42]

It's true that I hadn't thought about the fact that the call to this event was only once compared to isValid
Also I agree that optimisation must be done in Encased but I can't delete this mixin because if I forgot something when doing the fix it can be a cataclysm for players and we know that Encased isn't updated often si I will stay for time and will denigrate the mod itself
Also I think that a smart optimisation can be done by executing the verification only if the return value is false