Malorolam / LootBags

Other
21 stars 31 forks source link

Bag Storage - Unlimited lootbags #155

Closed Sunekaer closed 5 years ago

Sunekaer commented 5 years ago

Bug Report You can take unlimited lootbags from the Bag Storage block.

Steps to Reproduce (for bugs)

  1. Right click the block to open its GUI.
  2. Instead of clicking the bag to remove it, hover over it with the mouse pointer and tap an open hotbar key slot. You'll notice the "Stored" value doesn't change.

works with all types of bags from the storage block LootBags-1.12.2-2.5.7 Forge-14.23.4.2765

Malorolam commented 5 years ago

I'll take a look at it.

Spyrox17 commented 5 years ago

quick, everyone, dupe the bags before it gets fixed

drunkripper commented 5 years ago

@Malorolam just because this may be related, If you put a mechanical pipe to the chest as well, it does not reduce the value, so you can have INFINTE legendary loot bags. I was wondering what i have 2.76e7 loot bags because i have like 20 openers hooked up and my mob farm isn't running. If i get a chance i'll look at the inventory code and see if i can spot the issue and make a pull request if you can't get to it.

The bag storage issue seems to only not work with Mekanism logistical transports. Everything else seems to properly lower the amount.

Malorolam commented 5 years ago

It's likely Mekanism doing things weirdly. Looking at the code, it seems that they're using getStackInSlot, which is for querying the item in the slot and not calling decrStackSize or removeStackFromSlot at all. Then they're just telling the tile entity to set that slot to the appropriate reduction (which for me is empty in all cases) instead of asking the tile entity to reduce or empty the slot using the decrStackSize or removeStackFromSlot.

I can try to fix it on my end, since the stack size is only ever 1, either there's a bag there or there isn't.

drunkripper commented 5 years ago

It won't pull the bag if there's not enough in it (if you have 255 "Bag Units" and you try to pull a legendary bag, it won't pull. but if you have 256 it'll pull one but it doesn't get anything. Could you have a fake inventory for machines to pull from possibly? And if you detect that the number has gone down, you can just lower the number by 1 and put another into the hidden inventory? That's just how i would probably attempt to fix it, but there's probably a way more efficient way of doing it as well.)

Malorolam commented 5 years ago

Yes, because the method called checks to see if there can be a bag, so if there isn't enough Bag Value to make that bag it just returns an empty itemstack. And technically the method I'm using to implement the bag storage is a fake inventory, since the bags stored are just an integer, the itemstacks are only created when a bag is queried or removed. The method you described is how the Loot Recycler handles its bags, but I found that didn't really work for the storage since the bag can change.

dcmallo commented 5 years ago

you can also dupe bags by putting bag storage next to a crafting station and shift clicking it out from the crafting station gui.

Malorolam commented 5 years ago

80% sure that's the same issue, but I'll check it out too.

Danekjovax commented 5 years ago

Not sure why it's closed; I'm still observing this dupe bug with Mekanism pipes.

Malorolam commented 5 years ago

It is closed because 2.5.8.5 added this: -Added a targeted config option to fix the dupe bug with Mekanism logistic pipes and Bag Storage bag count more completely, !!HOWEVER!! this involves pulling the stack trace almost every time the Bag Storage inventory is queried, so performance may suffer for it. This is disabled by default.

Additionally, it was determined that the dupe bug is purely because of how Mekanism handles item routing. I cannot fix it any more than I have.

Danekjovax commented 5 years ago

My apologies. I was at work and didn't follow through with Mekanism's support page at the time. So I'll verify we have the latest version of Mekanism. I'll post with them of anything I'm still observing at this point. Thanks for your time.