[Split Shulkers] Consider using Mixin inheritance for improved mod compatibility

[RubixDev]

Hi, I'm the author of EnchantedShulkers, and I got a request to make my mod compatible with yours RubixDev/EnchantedShulkers#21. While working on that, I learned about Mixin Inheritance, which is preferred over just overriding entire vanilla methods in mixins. I am mainly talking about the getUpdatePacket() and getUpdateTag() methods in the MixinShulkerBoxBlockEntity class:

MCTweaks/SplitShulkerBoxes/common/src/main/java/cursedflames/splitshulkers/mixin/MixinShulkerBoxBlockEntity.java

Lines 70 to 85 in 1feecdf

	@Override
	public ClientboundBlockEntityDataPacket getUpdatePacket() {
	return ClientboundBlockEntityDataPacket.create(this);
	}
	@Override
	public @NotNull CompoundTag getUpdateTag() {
	var tag = super.getUpdateTag();
	var color = this.getColor();
	var color2 = this.splitshulkers_secondaryColor;
	if (color != color2) {
	secondaryColorToTag(color2, tag);
	}
	return tag;
	}
	}

You can see how I "override" the same two methods on my side here and here (note: I'm using yarn mappings so the names are different).

While a change on your side technically isn't necessary, it would still be good practice to do so and a step forward for better compatibility between your mod and other mods like Shulker+.

---

Additionally you should consider relying less on ci.cancel() calls. Mainly

MCTweaks/SplitShulkerBoxes/common/src/main/java/cursedflames/splitshulkers/mixin/client/MixinBlockEntityWithoutLevelRenderer.java

Lines 50 to 58 in 1feecdf

	@Inject(method = "renderByItem", cancellable = true, at = @At(value = "INVOKE", shift = At.Shift.BEFORE, target = "Lnet/minecraft/world/level/block/ShulkerBoxBlock;getColorFromItem(Lnet/minecraft/world/item/Item;)Lnet/minecraft/world/item/DyeColor;"))
	private void onRenderByItem(ItemStack stack, ItemDisplayContext displayContext, PoseStack poseStack, MultiBufferSource bufferSource, int i, int j, CallbackInfo ci) {
	var color1 = ShulkerBoxBlock.getColorFromItem(stack.getItem());
	var tag = BlockItem.getBlockEntityData(stack);
	var color2 = secondaryColorFromTag(tag, color1);
	var index = 17 * (color1 == null ? 0 : color1.getId()+1) + (color2 == null ? 0 : color2.getId()+1);
	this.blockEntityRenderDispatcher.renderItem(splitshulkers_AllShulkerBoxes[index], poseStack, bufferSource, i, j);
	ci.cancel();
	}

and

MCTweaks/SplitShulkerBoxes/fabric/src/main/java/cursedflames/splitshulkers/mixin/client/MixinShulkerBoxRenderer.java

Lines 28 to 47 in 1feecdf

	@Inject(
	method = "render(Lnet/minecraft/world/level/block/entity/ShulkerBoxBlockEntity;FLcom/mojang/blaze3d/vertex/PoseStack;Lnet/minecraft/client/renderer/MultiBufferSource;II)V",
	at = @At(value = "INVOKE", shift = At.Shift.BEFORE, target = "Lnet/minecraft/client/resources/model/Material;buffer(Lnet/minecraft/client/renderer/MultiBufferSource;Ljava/util/function/Function;)Lcom/mojang/blaze3d/vertex/VertexConsumer;"),
	cancellable = true,
	locals = LocalCapture.CAPTURE_FAILHARD
	)
	private void onRender(ShulkerBoxBlockEntity shulkerBox, float f, PoseStack poseStack, MultiBufferSource multiBufferSource, int i, int j, CallbackInfo ci, Direction direction, Material material) {
	// if (false) return; // TODO check for non-split shulkers
	ci.cancel();
	var color2 = ((SplitShulkerBoxBlockEntity) shulkerBox).splitshulkers_getSecondaryColor();
	var model = (ShulkerModelGetter) this.model;
	// Render base with new color
	Material material2 = color2 == null ? Sheets.DEFAULT_SHULKER_TEXTURE_LOCATION : Sheets.SHULKER_TEXTURE_LOCATION.get(color2.ordinal());
	VertexConsumer vertexConsumer2 = material2.buffer(multiBufferSource, RenderType::entityCutoutNoCull);
	model.getBase().render(poseStack, vertexConsumer2, i, j, 1.0f, 1.0f, 1.0f, 1.0f);
	// Render lid normally
	VertexConsumer vertexConsumer = material.buffer(multiBufferSource, RenderType::entityCutoutNoCull);
	model.getLid().render(poseStack, vertexConsumer, i, j, 1.0f, 1.0f, 1.0f, 1.0f);
	poseStack.popPose();
	}

caused some trouble. For ensuring compatibility between our mods I had to mixin into your mixins in these places to basically just apply the same modifications that I do to the vanilla game.

[CursedFlames]

I've rewritten the mixins to hopefully be less invasive in 1b7ee9a; let me know if there's anything else that could be changed to improve compat.