Ender IO Forestry

Ender IO Forestry

954k Downloads

Many items no longer go into armor inventory that used to including non-inventories

choekstr opened this issue · 30 comments

commented

Issue Description:

I am no longer able to store items in the Stellar Chestplate with Inventory III that previous version without the new upgrade mechanics did. I appreciate the changes and while I don't like them there might be items like the following that are erroneously denied:

  • Scannable's scanner (Cannot put a bag into the armor Inventory!?!)
  • Chest Transporter's [Empty] Diamond Chest Transporter (Item too big for Inventory)
  • Thermal's satchel (Item too big for Inventory) <- IS allowed in Dark Armor Chest with Inv III
  • Dank Null (Cannot put a bag into the armor Inventory)
  • EIO Dark & Stellar Armor (Item too big for Inventory) <- all other armors allowed

What happens:

Couple different error messages and not allowing non-inventories, and recently blacklisted inventories to not be placed in armor inventory.

What you expected to happen:

Items previously allowed should still be allowed and non-inventories, or items that have minimal tool specific and only the tool modifiers themselves inventories, should be allowed. The drill from Actually Additions has an inventory and tool modifiers but IS allowed unlike the scanner from Scannables.

Steps to reproduce:

  1. Open Stellar or Dark armor inventory
  2. Attempt to put in items
  3. Receive deny message and item won't be allowed
    ...

Affected Versions (Do not use "latest"):

  • EnderIO: EnderIO-1.12.2-5.0.50.jar
  • EnderCore: EnderCore-1.12.2-0.5.65.jar
  • Minecraft: 1.12.2
  • Forge: forge-14.23.5.2838
  • SpongeForge? no
  • Optifine? no
  • Server

Your most recent log file where the issue was present:

NA

commented

@HalJordan Tinker tools use a lot of NBT data to store things like materials and upgrades of the tool. This makes these item big. All items which need more than 500 bytes of storage are not allowed.

commented

I get the bag in a bag thing, but that doesn't explain why Tinker's Construct tools are blacklisted...

commented

But no other bags restrict tinker tools. I assume EnderIO has challenges other bags and satchels don't?

commented
commented

yeah, we understand the issue. I was merely suggestion we add code checks to be smart about it instead of throwing the baby out with the bathwater.

commented

The check is smart - it rejects all items which them self have an inventory and items which are bigger than 500 bytes.

commented

Maybe set an upper limit that says if you have max int amount of items in your inventory / NBT data coming into my container, you aren't allowed. However if you are a simple tool with normal NBT data that MC supports, allow it. Seems like we could have conditionals here to be smart about what's allowed in instead of an all or nothing approach?

commented
commented

Yeah but even that could brick your world

commented

hmm, it would seem we in the modded community are able to throw all kinds of complex items into a vanilla chest with no issue and that holds 27 slots compared to 6 in a dark armor helm. or a double chest with double the storage. Oh well, I have made my point and initial ask so I think the ask is clear and the stance is clear that is has a theoretical chance that has been mitigated by draconic means :)

commented

If you store too much data in a chunk (eg in chests etc) - the chunk will fail to save. I think this limit is at 2 MB. You can find videos on youtube showing how to use this for duping items by reloading the world repeatedly.

But when the limit on the player inventory is exceeded - you will no longer be able to load into the world.

commented
commented

Ok, so isn't the issue about nesting inventories, but taking a dark helm with inventory III on it gives 6 slots. So 500 bytes x 6, or 3000 bytes, 3k, of NBT data would cause issues to store in dark armor, the same as tens of thousands of nested items in bags within bags x 74 like Henry exampled earlier?

I am so out of my element here and you guys are the expert, but wouldn't just using TAG_Short give 2^15th or 65535 NBT values -32k through +32k), or using TAG_Long give 2^63rd or roughly 9 Quintilian stored tags? I assume world corruption comes way before 9 Quintilian but it seems there could be room for having something like, say, tinker's tool that can go in chests or backpacks or drawers just fine without world corruption, right?

commented
commented

ae doesn't store the items on the disks, it has its own database and the disks just have an ID.

Also, let me repeat (now that I'm at a real keyboard) that the problem is not a single item. It is that the only check we can add is at the single-item level. The issue is that the combined player data, all the player's inventory slots together plus the current potion effects plus entity data plus capability data and a couple other things, has a size limit. And there's no way of safeguarding for that, so the only thing we can do is to prevent single items from becoming huge.

Our choice on how to do that was to (a) prevent nesting inventories as that is a simple rule and even makes sense in-game, and (b) prevent items that are already huge by themselves to be stored. However, "huge" is relative. I picked 500 bytes (as stored on disk) out of a hat. If someone can find out how big the total limit is, we can divide it by 32 (number of inventory slots) and then by the maximum number of storage slots in a ds item and use that number instead.

re the "9 Quintilian" comment: Your train of thought took a very wrong turn there? I'm not sure, but I think you mixed up storing the size of something with storing the "something" itself.

commented

I agree with the thought to prevent an inventory against becoming too large that would cause player data corruption. Although i have always known about this, i have never encountered a problem even when i heavily abused the ironBackpacks nesting function(minecraft 1.7?).

As i see it, the mod developers have taken it upon themselves to try to prevent reaching this limit which is good in itself but in my opinion, it doesnt matter at all. If players cant use the enderIO inventory for their items, they will use one from a different mod which likely would increase their total slots by more than if the equipment inventory was enough.

When mentioning AE, their disks is quite small due to that they only have to store 63 different items ID's along with metadata and amount. A better example would be Refined Storage or better yet; Reborn Storage. These disks can be HUGE are the only items i myself consider a risk to corrupt your player data.

commented

Henry there are 36 inventory and 4 armor slots for vanilla..

commented

ok, I calculated 3x9 instead of 4x9, but you missed the offhand slot... ;)

41, then.

commented

you can corrupt your world with ae. fill a diamond chest with drives and pick it up with a dolly from jabba or carry-on. insta world corruption. i did it with as low as 16 drives but that is off topic. if people corrupt there world with another mod then its that mods problem to fix not enderio. enderio should fix there own stuff and not worry about what other mods are doing and how they are doing it.

commented

Ultimately it becomes a function of moving on to something else as weightmaker put it. If we can no longer can use Inventory of dark armor for tools, which tinker's are a big part, we will just choose a different solution, not the end of the world at all. There are plenty of options that still work to store tools and items. I just wish we could leverage wonderful enderio armor and all the great features and upgrades and also to leverage inventory unfettered.

Since I've not done anything stupid and never corrupted my world I see it as a non issue but fully appreciate the challenge of being blamed for world corruption. Aroma backup and resulting restores fixes all misdoings :)

commented

Very easy case to build, I just disabled the protection and do this: https://streamable.com/4y8x7

I didn't even get to putting this Dark Plate into another Dark Plate because the game became unresponsive and ran out of memory. Not quite the error I expected:
crash-2019-08-09_07.54.58-server.txt That would be more along the lines of e.g. ForestryMC/ForestryMC#1337

commented

This is to prevent your world becoming corrupted by an item that is too big---which can easily happen if you put a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into a bag into and I only added the error messages on build 50.

commented

Did you see much corruption from this in the past when it wasn't restricted and needed to block anything with an inventory, except for some that have inventory, and some others that don't but should be allowed?

I get what the intent is for the change, but was this in response to corruption of worlds? I mean I can put a satchel into a vanilla chest or a crate but I can't put it into an armor inventory? Seems like a fix for something that isn't a problem and very few others are preventing as it's not a common issue.

commented

Don't mistake my questioning with lack of respect as I infinitely trust your judgement and the rest of the team's guidance, but it seems like 54 backpacks in an armor inventory is a bit of a corner case. If Soaryn proofing is the intent then I agree, disabling functionality that most other mods have (nested inventories) because of a rare abuse case is in order but I would maintain this is an exception case.

In the end I just enjoyed a nice clean inventory with my satchel and dank null in my dark/end/stellar chest inventory and now have to keep those in my off hand and inventory. it's a pain but will just hope it's reversed decision at some time in the future.

Thanks for entertaining my comment.

commented

However, "huge" is relative. I picked 500 bytes (as stored on disk) out of a hat. If someone can find out how big the total limit is, we can divide it by 32 (number of inventory slots) and then by the maximum number of storage slots in a ds item and use that number instead.

Relative indeed. I am also interested in keeping commonly used Tinkers' tools in the Armor. I try not to flood issue posts with pages of data and so posted my research to the pastebin linked below.

TLDR ~ Increasing the 500 byte limit to 1,000 bytes is perfectly reasonable for normal gameplay.

https://pastebin.com/N6e1CCXr

commented

Is it possible to keep a running tally of the size of all items in the armor's inventory? I know we can limit it to refuse items over bytes, so could we maybe store (upper limit - N) as an NBT tag on the armor itself? For each item that gets put in, N gets incremented, and when it's taken out it gets decremented, until it hits the limit. That way players could store one or two huge items, or a dozen medium items, without hitting the upper limit we're worried about.

commented

I'm a bit concerned about people not understanding why an item seemingly randomly goes in or is rejected...

commented

"...players could store one or two huge items, or a dozen medium items, without hitting the upper limit we're worried about."

My question is rather why folks are trying to make the armor unnecessarily OP, or try to turn it into a one-size-fits-all sort of setup?

The EnderIO Armor, while nice, is not meant to replace existing portable storage from other mods. it doesn’t have the capacity of storage crates, nor the versatility of a dank null, nor the autonomous nature of some satchels/bag to collect or resupply items, and it’s not meant to.

It’s always felt like it’s more meant to clean up those other inventories by being a dedicated tool-bag. How many wrenches does one need between two-hundred mods? A sure minimum of four, not counting wands that wrench things (Botania, Thaumcraft foci, etc). What about going from land to sea to space? The helmet has six slots that are great for storing other frequently used headgear. Personally I use the chest-plate for common tools, and for the dozen or so Inventory Pets I normally carry, along with their feed bag(s).

Anything that doesn’t stack and would otherwise be eating valuable space in a backpack, satchel, bag, belt, etc. can be stored within EnderIO Armor. As evidenced in my earlier research, even an unnecessary creative mode Tinker’s construct tool can be crammed in without too much grief. I then fail to understand what would constitute “one or two huge items” that would be necessary or worth putting into the armor. As mentioned previously, the limits are mainly there to prevent folks from filling the armor with AE2 drives or other similar storage that tends to break the game very quickly by exceeding Java’s internal string-length handler for the single NBT tag (remember, Minecraft java edition is only built on 32-bit java, and has many built in limitations as a result).

Could an NBT.size tag be added to the armor? Might take a week or two of coding and debugging to get it working perfectly and accurately, but yes, it could. Is there any practical need or motive to do so? Not really given the overall design and function of the armor as is.

commented

That's pretty reasonable. I just want to be able to store my Building Gadget, Copy Paste Gadget, Scanner, Explosive Dark Steel Pickaxe and so on in my chestpiece, so i can easily get to it with a keybind and don't have to carry around a satchel. The modpack i'm using is still on EIO 5.1.55 so the limit means that i can't store any of those items in there. I don't know for sure if the proposed changes will allow all these items through, but i thought it was worth chiming in with this idea to see if it'd be considered useful, allowing for slightly more versatility in what can be stored without risking corruption.

commented

I just want to be able to store my Building Gadget, Copy Paste Gadget, Scanner, Explosive Dark Steel Pick-axe and so on in my chest piece

I'm also still on 5.1.55, still waiting to see when the next stable GitHub or Curseforge version will be released.

Looking at the code commit Henry posted, there is now a config option for per-slot storage options, allowing a user selectable range between 500 and 5,000 bytes per slot, along with a warning on how changing it from default could allow you to accidentally brick your world. To fit the copy/paste or building gadget would require some testing as the reason for it's size is the mod stores it's UNDO history in each device. A brief check on my world and my building gadget uses around 1,400 bytes, and my copy-paste gadget currently uses 400 bytes.

Assuming your modpack has craft-tweaker, a quick and dirty way to check NBT size is to run the command "/ct hand" on whatever you're holding, which will automatically copy the result to your computer clipboard. Just paste this result into any document editor, highlight it, and check the document statistics for "character count". One character is roughly one byte, so it's a rough estimate but once the new version comes out it should allow you to judge how far you need to adjust the config. Personally I still wouldn't recommend going beyond 2,500 bytes per slot in a single player world if at all possible unless you have a fairly good computer to handle it. A fully upgraded, enchanted Dark pick should also fit within a 1,000 to 1,200 limit.