Creators-of-Create / Create

[Forge Mod] Building Tools and Aesthetic Technology
MIT License
2.81k stars 890 forks source link

Toolbox incorrectly serialized/deserialized when used as MountedStorage on contraption #5083

Open telemenar opened 1 year ago

telemenar commented 1 year ago

Describe the Bug

If you include a toolbox on a contraption (or another other modded storage that extends ItemStackHandler) when the BlockEntity (ToolboxBlockEntity) is used to create a MountedStorage it will use the extended implementation of ItemStackHandler (ToolboxInventory), but only until it is serialized/deserialized.

When the Contraption is deserialized the MountedStorage only uses a plain ItemStackHandler and not the extended version of ItemStackHandler. At which point it will ignore all of the extra handling from the extended class. This can lead to especially strange behavior in the case of toolboxes which have 8 compartments with 4 stacks per compartment because it can result in the contraption while using the base class ItemStackHandler filling every stack with a different item completely ignoring filters.

When the contraption is finally disassembled, the ToolboxInventory has setStackInSlot called for each stack in the base ItemStackHandler as part of the MountedStorage which leads the toolbox in a strange state.

Reproduction Steps

  1. Build a minecart contraption with a toolbox and a drill.
  2. Set each slot in the toolbox to remember some item it will not mine like a torch but then leave 0 stacks of the items in the toolbox.
  3. Assemble the contraption
  4. Pick up the contraption with a wrench (to force serialization)
  5. Place the contraption have it mine 9 different kinds of blocks with the drill (Cobble, Andesite, Diorite, Granite, Cobbled Deepslate, Dirt, Limestone, Calcite, Azurite, etc. ) -- Note the blocks are picked up not dropped
  6. Disassemble the contraption -- Note the toolbox will contain 8 of the blocks but the 9th block will be missing.
  7. Remove each of the blocks. -- Note that the filter for something other than the block is still there.

Expected Result

One of the following should be true:

This expends to other modded storages that implement the getCapability and have an extension of ItemStackHandler.

Screenshots and Videos

No response

Crash Report or Log

No response

Operating System

Windows 11

Mod Version

0.5.1d

Minecraft Version

1.19.2

Forge Version

43.2.14

Other Mods

No response

Additional Context

You may want to check the the ItemStackHandlers are exactly ItemStackHandlers.

Or you could go the route of adding a storage registry like was done in https://github.com/juh9870/MoreMountedStorages but require the mods that wish to add support to add their own handlers and mod specific registry components rather than building the fragile integrations into Create itself.

If you want more detailed pointers into the code let me know I've spent a long time figuring this out.

telemenar commented 1 year ago

Also extensions of ItemStackHandler don't even check the CONTRAPTION_INVENTORY_DENY tag because there is a short circuit in place in MountedStorage.canUseModdedInventory

Allmoz commented 2 months ago

Ahhh... That's why im getting that weird behavior at chunk unload reload with 0.5.1f on 1.18.2