search

SwitchCraftCC / Plethora-Fabric

A peripheral provider for ComputerCraft, ported to Fabric
MIT License
16 stars 11 forks source link

Force set stack during item transfers (fixes #31) #67

Closed Allymonies closed 1 year ago

Allymonies commented 1 year ago

InventoryStorage is not designed to support inventories that do not own their slots, such as EquipmentInventoryWrapper. An issue came up that due to the validation on EquipmentInventoryWrapper's setStack, rollbacks during item transfers fail, causing items to be deleted (as pushItems actually does two extractions, and the first is meant to be rolled back). To solve this, I created a new InventoryStorage, DerivedInventoryStorageImpl, and an accompanying DerivedInventorySlotWrapper so that we can call a new function, forceSetStack to ensure rollbacks complete successfully.

Note that, because of isValid, this does NOT allow setting disallowed items in equipment slots (tested this to make sure). However, we still maintain the existing behavior that pullItems can put more than 1 of stackable items into helmet slots (think mob skulls and carved pumpkins). Tested that pushItems now works, and pullItems remains working.

Lemmmy commented 1 year ago

In the end, we realised that simply removing the isValid check in our equipment inventory wrapper is sufficient. It's not expected, per the vanilla inventory contracts, for setStack to perform its own validity check. It was necessary back in 1.12 since the caller didn't do one, but now we finally have Fabric's transfer API in 1.19.4+, all the proper checks are performed.