Inventory Controller is massively overpowered

[Vexatos]

From what I've heard the getStackInSlot method of the inventory_controller component returns the full NBT data of the ItemStack. This means you can for example read the entire (!) contents of the cells inside an ME drive from Applied Energistics, it also makes many drivers added by addon mods (For example Computronics' Spatial IO Port Driver or the Routing Track/Detector/Switch Drivers) completely useless. In my opinion the inventory controller is massively overpowered. getStackInSlot should only return the Item type, stack size and metadata, that would be MUCH more fair and wouldn't make adding drivers for things like AE Spatial IO port redundant. I hope you will discuss this more (on IRC or here), thank you for reading my opinion.

[natedogith1]

I believe the nbt tag was added because so many things differentiate based on nbt data. However I agree that it might be a tad over-powered, and while it being gzip-compressed helps, there are pure-lua deflators just a google search away.

[Kubuxu]

We already have created lib for it. I won't say that it is overpowered. It lets us look deeper into items without converters, but there are few major drawbacks; it is slow and it gives you too much data. Great example are TiC's tools. Even after few minutes looking at it you do not know what is going on with its NBT. @Vexatos It won't replace converters as those are able to give much more information, in a nicer format, than only NBT.

[Vexatos]

@Kubuxu I do not think you are supposed to read every item stored in an ME drive, everything inside a TiC tool or the aspects stored inside a Thaumcraft wand. You are just not supposed to be able to. That's what makes it overpowered. Computronics, for instance, only adds Drivers if the mod developers agree with it so it does not become unbalanced (Which is why there is no Botania integration in Computronics, yet you can still read how much mana a mana tablet stores using the Inventory Controller).

[Kubuxu]

If it is such a problem, only solution is encryption with per world/save key. But then you wouldn't be able to work with programs between worlds/saves. For example someone could created database of stuff that he wants to export by AE and it won't work in other world/save.

[fnuecke]

The reason it's just a binary blob right now is really just one of cost-benefit relation. It's by no means trivial to get anything meaningful out of it (yes, there are plain Lua GZIP implementations, but parsing binary NBT data? Not so many, as far as I'm aware, at least not wide-spread).

The problem with "proper" encryption is

• that the resulting binary blob will usually be different each time, meaning no equality checks on the Lua side.
• that the data cannot be easily parsed by components even when implemented in Java. They'd be required to use Arguments.checkItemStack, while right now that's just convenience.

If you can propose some non-trivially breakable, but still deterministic encryption algorithm, go ahead... but as long as it's something like "xor the data with a world-based key", that's just not worth the effort IMHO, since it's way too easy to break (in that example, given you know the data you'd expect and can then just xor it against the data you actually get).

Also, for reference, previous considerations on this in the original issue.

[Kubuxu]

@fnuecke we have created library but we can keep it quiet if you wish.

[Vexatos]

Yup, there is an NBT parser up on OpenPrograms. I understand the issue with encryption etc. Again, I would recommend only passing the (unlocalized and thus unique) Item name, stack size and maybe the metadata.

[Kubuxu]

But then you can't configure export buses.

[Suterusu1337]

It can be overpowered, but then again, anything can be if you are choosing to do so. I doubt much of the player base will know/care about NBT tags in items read by a Computer, or even what to do with said information.

Personally, it has been a miracle worker for me trying to integrate OC into GregTech, as 90%+ of GT information is stored in NBT form. Just trying to interact with the mod would be hard if I had to rely on a mod author to create a driver/mod plugin for everything added, and then being limited on what they thought was OK to include.

The only people who would begin to exploit or understand the true "overpowered"ness of this are the people who truly understand the coding aspect. This isn't a single block to place down to 5x ores or make infinite steam. It's code based, and if a few people are smart enough to figure out how to use it, so be it; they can chose to not use it in their programs. There are far fewer users of computer mods that know what they are doing than players of modded Minecraft.

[Vexatos]

@Suterusu1337 There are addons that integrate every device in GregTech into OC, just saying. And that's exactly what I want. As an addon developer, I only add functions to devices if the mod authors agree with me adding them, this way I won't add any functions that allow exploits or simply break the mods' balance (Again, this is the reason why there is no Thaumcraft/Botania integration in Computronics). You should not be able to read anything you want, you should be able to read anything the mod dev of the thing you want to read from allows you to. This NBT reading breaks a lot of various mods' balance. If you add functions using some addon, that addon will most likely only add functions the developer of the original mod is okay with. This way there won't be any balance/theme breaking the mod author is most likely unhappy with.

[Xfel]

If I look at the source code:

if (stack.hasTagCompound && Settings.get.allowCompressedNBTTags) {
    output += "tag" -> CompressedStreamTools.compress(stack.getTagCompound)
}

It seems like I can disable the NBT stuff in the config.

[Vexatos]

@Xfel That's true, but it is enabled by default and I highly dislike that fact. If this is being kept, that value should be false by default and enabled by server owners if they explicitly want that exploitable feature. I do not think it is fair to various mod authors to have it enabled by default.

[Kubuxu]

@Vexatos But if you set it on false by default you can't set up export buses to export specific:

• TiC's tools
• Safari nets,
• Enchant books,
• Ender chests/tanks and so on.

It is like @fnuecke said cost-benefit trade. If it is such an extreme issue, please come up with a solution other that removing feature.

[Vexatos]

It is exactly that. I do not think you are supposed to set the AE2 export bus to anything you want (And you can set it to anything you want, even items you do not own yet. There is a reason you cannot simply shift-click any item stack into the export bus using NEI). I do not have a better solution other than removing the export bus driver in general as I do not think it is intended to be used this way, you might want to talk with the AE2 developers about that. Anyways, I really do think this cost-benefit trade goes far too much into the cost of balance, I know removing it makes the AE2 export bus driver useless for some things, but that is one benefit for the cost of many mods' balance.

[fnuecke]

While I think the "can't shift-click configure export bus" boils down more to a technical limitation than a gameplay/balance one, I still understand, and partially share your concern. The thing is, that while currently this is pretty much only used by the export bus driver, I imagine there could be many more future uses of this, which would get shut down if this were completely disabled by default...

Again, I'd be happy to add some form of encryption, but I'd really really like it to have deterministic output, to allow equality comparison... I'm aware that this is strongly in conflict with good encryption, but maybe someone is aware of something "good enough" that cannot be trivially cracked?

Otherwise we really would need some alternative way of providing item stacks. I guess one option would be to have the adapter have an actual (ghost) inventory and allow specifying an index in that inventory, instead of a Lua-side item stack representation? Does anyone have any better ideas?

[Vexatos]

@fnuecke Again, I'd remove that feature (Or turn it off by default). If you do not want that at all and cannot be convinced otherwise, I would like you to implement your idea, an actual inventory to check from is, in my opinion, MUCH better than anything inside Lua.

[fnuecke]

Another potential solution we just came up with on IRC, which might be a little unwieldy, but should provide all the power needed while fully encapsulating the actual NBT data: a new component that can be configured with a number of item stacks (either from a robots inventory or an inventory adjacent to an adapter). Instead of passing a table representing an item stack (as it is now), one would pass the address of such a "database" component plus the index of the stack to use.

Here's a preliminary list of features:

• configure(register:number[, invAddr:string], invSlot:number); as mentioned, from robot or inventory adjacent to adapter
• get(register:number); get basic item stack representation sans NBT, i.e only name + damage
• hashOf(register:number); returns a hash for a hash for a configured item stack, useful for communicating which stack is meant over the network / saving to file
• indexOf(hash:string); number of register stack with specified index is stored in, see hashFor
• cloneTo(addr:string); copy configuration to another component of this type, for convenience
• Maybe also allow cloning of configurations via crafting? Not really a fan of this one, personally.
• Multiple tiers with different storage capacity?

Let me know what you think and if I forgot something.

Edit: oh, what's totally not decided yet is what kind of component this would be. An upgrade? A card? A block, even? Opinions welcome. Also, name brainstorming. "Filter", "Database", ...? Ideas?

Edit 2: also, I want to get this done before properly releasing 1.4, so that will be delayed until a solution for this is implemented (with me leaning towards the new component atm).

[rmellema]

Maybe an infochest? since it is sort of a chest that extracts info from the items you put in?

Also, would get return two numbers/strings? Or would this be stuffed together?

[fnuecke]

While the configuration would be the most intuitive, the downside with this component being a block is that it leads to a discrepancy when using it as a block versus using it as a robot upgrade - or, if the block were to save its configuration (which I'm not a huge fan of) it'd be impossible to reconfigure once (internally) built into a robot.

As for get (or whatever it'd be called), I'd imagine it to return what getItemStack does now, minus the NBT (or, if so configured in the settings, including the NBT, I'd still leave that in for those that want it, but defaulting to false).

[Suterusu1337]

How many people are going to be over using this feature outside of its intended purpose? How many people are going to "break" other mods by integrating this into their code?

For the AE2 example... How many regular modded MC players are going to look to OC as their way to "cheat" at other mods?

I think the concern is valid, but the scope is very, very limited. Most people are looking to drivers regardless for easy to use methods, not coming up with their own code to use against NBT tags.

[Vexatos]

@fnuecke I recommend having it an upgrade item which you can right click to open a filter GUI. as for the component name, I'd choose "Database Upgrade" and database as the component name.

[fnuecke]

Suterusu1337, that's indeed how I initially saw that myself (hence the current implementation), but seeing as there's readily available code to extract the NBT information it makes exploiting this much too easy for me not to do something about it. Sure it's incredibly powerful to just write your own "drivers" on the Lua side, but it also means there's no control whatsoever. Which circumvents any concept of "privacy" so to speak - some mods want to have the actual data stored in an item's NBT secret, a simple example being MFR's "mystery" safari nets, another being Thaumcraft which doesn't show aspects unless you researched them (from my limited understanding anyways).

Vexatos, the right-click GUI is a nice idea. It still won't make them reconfigurable once built into a robot, but I suppose that's just fitting into the "build your robots responsibly" vein.

[Vexatos]

@fnuecke Upgrade Containers exist for a reason.

[fnuecke]

Psht, go away, would you. You and your logic ;-)