Inventory Tweaks [1.12 only]

Inventory Tweaks [1.12 only]

127M Downloads

Inventory handling not well handled for custom/more slots

Funwayguy opened this issue ยท 6 comments

commented

@Kobata, I've been making a mod that add's additional slots to the vanilla player's inventory and there are 2 frankly very annoying issues in 1.7.10

  1. When sorting items from one slot to another it is using a 'while(slot.isEmpty())' loop in numerous places without checking whether the slot is even valid to be accessed. This is a major issue when the slot is set to show a specific item stack that cannot be picked up and to replace it if empty (causes an infinite loop an locks up the users game). This could be solved by both checking if the slot can in fact be accessed and only passing over each slot once using a 'for' loop.

  2. All of your inventory handling seems to use an assumed static size and positions for vanilla inventories and takes no consideration for other mods who wish to change such sizes/positions. A simple solution would be to query the size of the given inventory and slot positions as well as what kind of slots they are and generate a map it that way. This kind of handling would also enable automatic compatibility for most modded inventories without having to specify them in the code.

If you're interested in which mod I'm developing that encountered these issues:
https://github.com/Funwayguy/InfiniteInvo

PS: For the record I have not had any other modded inventories have issues with my mod yet so I am almost certain it is not a compatibility issue from my side.

commented
  1. should be handled by https://github.com/Kobata/inventory-tweaks/blob/develop/src/main/java/invtweaks/InvTweaksContainerManager.java#L108 (and similar checks in the same method) as it became an issue with a few mods back in 1.5/1.6

  2. is not possible using vanilla, and is the reason for the annotation API. (And also why it refuses to do anything if it doesn't recognize the container -- if you're completely overwriting a vanilla container type it will break, but that's your fault)

commented
  1. This is not the case when Space + Clicking from a player's inventory to a chest. It has been tried and checked. https://github.com/Kobata/inventory-tweaks/blob/c2e714001c50dbe2b2cebf5e2d9c253e43a8926c/src/main/java/invtweaks/InvTweaksHandlerShortcuts.java#L424

  2. I do believe this is possible considering all of the positional information for slots is easily accessible using vanilla methods. It would only be a matter of using said positions to determine which slot is in which row/column and how big those rows/columns are. As for knowing if the slots are natively sortable would be as simple as doing a 'instanceof' check on the Slot.

commented

The selection of sections does not rely on any positional information, because the position of the slots isn't what it's looking for, but a more conceptual 'purpose' of the slot -- there have been mod GUIs that use very non-standard layouts.

commented

a49ed9d should fix the first one, there was a condition to stop it but it had a typo.

commented

Thank you for the fix. The other issue I can at least work around.

commented

Thinking about it, looking back at history and not seeing any major changes in that code in forever, I'm not entirely convinced the condition there was actually wrong, but I need to make that code less confusing anyway.

It /should/ have already been aborting if it failed to move, and had no possible 'backup' slots to use.