Low-Drag-MC / LDLib-MultiLoader

GNU General Public License v3.0
12 stars 14 forks source link

LDLib causes crash when ModernFix dynamic resources is enabled #6

Closed embeddedt closed 1 year ago

embeddedt commented 1 year ago

is there a reason why LDLib attempts to mutate the shapesCache map in ItemModelShaperMixin instead of just modifying the return value like it does for the equivalent BlockModelShaperMixin? This breaks with ModernFix dynamic resources as shapesCache is replaced with a dummy map used for mod compatibility, and cannot be written to.

I think you could just maintain a Map<IRenderer, BakedModel> field inside ItemModelShaper and then use computeIfAbsent on that map to construct a new BakedModel instance once for each IRenderer.

Yefancy commented 1 year ago

thanks. does it look good to your mod?

@Mixin(ItemModelShaper.class)
public class ItemModelShaperMixin {
    @Unique
    static Map<IRenderer, BakedModel> SHAPES_CACHE = new HashMap<>();

    @Inject(method = "getItemModel(Lnet/minecraft/world/item/ItemStack;)Lnet/minecraft/client/resources/model/BakedModel;", at = @At("HEAD"), cancellable = true)
    public void injectGetModel(ItemStack stack, CallbackInfoReturnable<BakedModel> cir) {
        if (stack.getItem() instanceof IItemRendererProvider provider) {
            IRenderer renderer = provider.getRenderer(stack);
            cir.setReturnValue(SHAPES_CACHE.computeIfAbsent(renderer, r -> new BakedModel() {
                @Override
                public List<BakedQuad> getQuads(@Nullable BlockState state, @Nullable Direction direction, RandomSource random) {
                    return renderer.renderModel(null, null, state, direction, random);
                }

                @Override
                public boolean useAmbientOcclusion() {
                    return renderer.useAO();
                }

                @Override
                public boolean isGui3d() {
                    return true;
                }

                @Override
                public boolean usesBlockLight() {
                    return renderer.useBlockLight(stack);
                }

                @Override
                public boolean isCustomRenderer() {
                    return false;
                }

                @Override
                public TextureAtlasSprite getParticleIcon() {
                    return renderer.getParticleTexture();
                }

                @Override
                public ItemTransforms getTransforms() {
                    return ItemTransforms.NO_TRANSFORMS;
                }

                @Override
                public ItemOverrides getOverrides() {
                    return ItemOverrides.EMPTY;
                }
            }));

        }
    }
}
embeddedt commented 1 year ago

Yes, that looks like it should work fine.