FakeDomi / FastChest

Makes Chests use static models instead of BERs.
MIT License
75 stars 11 forks source link

[Suggestion] Tag-based compatibility #44

Closed noobanidus closed 3 weeks ago

noobanidus commented 3 weeks ago

Obviously as Lootr containers inherit from ChestBlockEntity, your mixin that disables the renderer being returned also captures Lootr containers.

My suggestion for future compatibility would be to take advantage of block entity type tags to create an exclusion list. This would allow people to configure it via data pack, and mods to include default tags.

It's also simple to implement as noted in the following code snippet from Lootr (although please note that I'm using mojmap here):

    boolean trapped = blockEntity.getType().builtInRegistryHolder().is(LootrTags.BlockEntity.TRAPPED);

I imagine you could then have:

    @Inject(method = "get", at = @At("HEAD"), cancellable = true)
    private <E extends BlockEntity> void removeBlockEntityRenderer(E blockEntity, CallbackInfoReturnable<BlockEntityRenderer<E>> cir)
    {
        if (Config.simplifiedChest)
        {
            if (!blockEntity.getType().buildInRegistryHolder().is(MyTags.EXCLUSION) && (blockEntity instanceof ChestBlockEntity || blockEntity instanceof EnderChestBlockEntity))
            {
                cir.setReturnValue(null);
            }
        }
    }
FakeDomi commented 3 weeks ago

Yeah I originally did not target all block entities for this reason. I didn't see any mods extending ChestBlockEntity but not ChestBlock (where the regular block model is enabled) in the wild, so I assumed it was safe enough to do it this way. Oops.

Tags seem to be the best solution for this.

FakeDomi commented 3 weeks ago

Done, FastChest hackery is opt-in now in version 1.7