Conflict with Filled to the Brim
Ultimushadow opened this issue ยท 13 comments
I am testing with Filled to the Brim (will be referring to it as FTTB) 1.2.5, Supplementaries 2.5.21, Fabric 0.86.1 for 1.20.1
FTTB: https://github.com/JacktheDevel0per/FilledToTheBrim
Here's the crash report:
crash-2023-07-27_04.06.53-client.txt
Context:
FTTB is made to allow shulker boxes to store empty shulker boxes inside them. It uses a datapack to determine what items are allowed to be stored when empty. Supplementaries has it's own shulker blacklist tag that interferes with this, but it can be remedied through datapack as well. The issue comes when the player attempts to add items to a Sack or Safe item.
Expected Behavior:
Empty shulker boxes are able to be placed inside shulker boxes. If added to the FTTB datapack tag, Safe and Sack items should also be allowed in shulker boxes provided they are empty as well. Shulker boxes with items inside should not allowed to be placed into shulker boxes, same goes for Sack and Safe items. When attempting to put empty shulker boxes into a sack or safe, nothing significant should happen. When attempting to put shulker boxes that have items in them into a sack or safe, nothing significant should happen.
What actually happens:
Nothing is able to be placed into a shulker box without modifying Supplementaries shulker blacklist tag. However, altering the tag does allow empty shulker boxes to be placed into a shulker box. If the Safe or Sack items are added to the FTTB tag, regardless of the shulker blacklist tag, they are unable to be placed into shulker boxes. The worst effect is that, regardless of tags, regardless of whether these containers have items, attempting to place anything into a Sack or Safe, including regular items, results in a crash similar to the one linked.
Additional notes:
Firstly, I believe FTTB is at fault entirely, as using Essential Addons with the Carpet mod also allows shulker recursion (although it does not discriminate based on empty containers) which will immediately cause all shulker, safe and sack items to be able to be placed into container items without even changing the shulker blacklist tag. I wrote an issue report to FTTB a while ago but the mod author has not responded. The issue can be found here: fooeyround/FilledToTheBrim#1 but this report has more information anyways.
I'm writing this out here on the off chance there's something Supplementaries can do to mitigate the issue. I think the concept of being able to carry empty containers is more than reasonable so I was pretty excited for FTTB until I found this incompatibility.
Looks to me that FTTB is at fault for not checking for null level in the block entity. Altho that's something that can happen very rarely the field is nullable and should be checked as cases like this could exist. Having a dummy entity that isn't placed in world is how I manage to invoke it's instance methods allowing for dynamic compatibility with all mods such as this and as you can see it's indact working since it's consequently calling the mod injecyed mixin which unfortunately contains that unsafe level access
That's what I thought, thanks for confirming. Guess we'll have to wait to see if FTTB is continued, I'll have to drop it from my pack in the meantime. I'll close this for now as it's already mentioned in FTTB
I may have resolved this, I may still need to make a "compatibility pack". It would be annoying for other devs to need to add compatibility with the tags. I am guessing you use but don't override the canInsert or something. You might want to use nbt but you might not be able to. (And performance)
I can't do any testing right now, but does this work?
For the tag conflicts, I'll need to look at it later... not sure what a good do for that is.
This will also in turn make filled to the brim "dumber" with this mod, as the magenta box rule, or rather the checks for shulker color overrides will not work and default to Shulker Box, this might change in the future but I am unsure.
Sorry I can't test either I'm on vacation. Also what do you mean when you talk about tags? Thought this had nothing to do with tags
The issue that this was open for was a missing null check. I since then also patched just it be sure so it shouldn't crash there either
I need to add a generalized check for inventories. Do you know I d I can access your inventories through vanilla systems?
I fixed the crash and am going to make a release. There is still a conflict with some tags that I need to fix with custom data packs.
I really don't want to have to have api for other mods to use. I might make an optional one (1 interface with isEmpty and more nest stuff (optional) if. I can't with vanilla already.)
Do you implement Inventory? If so I can just use that...
Yeah all tiles with inventories Dom still not quite sure what the issue was beside the original crash