Bugs found on ATC server

[p455w0rd]

• The inscriber thing: So when trying to automate the inputting of items into an inscriber via pipes/hopper, inscribers will accept an infinite number of item up to that item's max stack size. The inscriber only ever expects 1 item (with stack size of 1), and thus stops functioning when there is more than one item in any given slot. @covers1624 found that the code required to keep this functionality was commented out. I went through Gunther's commits and found that it was he who made this change with no explanation. After uncommenting, we could find no reason for commenting this code out. and inscribers went back to functioning as they should. The specific lines I'm refencing can be found at https://github.com/covers1624/Applied-Energistics-2/blob/rv5-1.12/src/main/java/appeng/tile/misc/TileInscriber.java#L296-L309

• Certus/Quartz Cutting Knife recipes only function if the knife has fully durability. Once a single craft has been completed (durability changes from 50/50 to 49/50), the knife becomes useless.

• When setting any terminal to only show craftable items, the terminal displays no items, (slots appear to be empty) although randomly clicking the empty slots sometimes gives you the option to autocraft and while in the same GUI and mode, you can also straight up randomly pull items out. This shouldn't be possible when viewing craftable-only items

• Shift-clicking item from crafting grid output slot causes severe server lag. This happens to the point on the ATC server the if I rapidly shift-click the results quickly enough, it times everyone on the server out. This also happens with RS, so I'm not placing blame. But something needs to happens here for either mod to be usable.

MC Version: 1.12
AE version: rv5-alpha-?? (Gunther stated that he isn't updating version string yet, so they're all "0")
Forge version: 14.21.1.2415

[GuntherDW]

The main reason I commented out those lines was to check in the first place if the broken code was still broken in 1.12, looks like it is. I haven't gotten back to it just yet.
If you check the issue/PR that was closed in my repo it shows you why the "fix" was not really a decent fix.
GuntherDW#3

I'll see what I can do about the quartz knife recipe.

About the third bug, is that after or before the update from wednesday? There are some issues regarding networking in Netty that have change but I haven't found out what exactly is causing it.
I added a workaround for a couple but still need to check out what exactly is causing it. (Netty has updated between MC versions)

I haven't touched the fourth issue yet, but it is somewhat inherent to minecraft. A lot of crafting recipes are laggy, but because you can keep shift-clicking AE kind of amplifies that and can cause people to lag out indeed.

I don't know if this can be "fixed" without writing a custom crafting handler but I think yueh can explain this better than I can.
I know that autocrafting recipes tend to not lag out the server nearly as much as manual crafting but I do agree that crafting has become an issue in modded.

[p455w0rd]

This is from the most recent ATC update (I tested all these issues just before posting today).

Please explain how you come to the conclusion that the 1.12 code from the #3 issue on your tracker is broken. We have tested and I can assure you it is not. Try it for yourself please.

As far as crafting vs AE autocrafting...this is REALLY odd since the automation happens much quicker than any human is capable of doing manually, yet it performs great. I would really like to know what's going on here. IMO it's definitely outside of this mod, but effects it severely nonetheless.

Thanks for taking the time to properly respond :) <3 @GuntherDW - You are precisely what this mod needed!

[yueh]

Just as a hint. Even if it is slightly more work, limiting one issue to one bug makes managing them way easier. Like being able to close them via commit messages. Also it does not cause it to reference other random issues.

Regarding the crafting. That is pretty simple. It crafts every single step on its own. So up to 64 crafts per item. Up to 9 different items each time and for each single item taken out, it has to completely rebuild the network inventory because some broken inventory reported 9 diamonds, but only stores 1. So we are talking about 576 rebuilds. Dpending on the storage being used (drives are way better than any alternative), it can get expensive. Sure it still happens faster as any human could do it. It probably takes a couple of hours. But still a couple of msecs are to slow for a computer, if you have only 50.

To fix it mostly requires this:

1. Convince every mod dev out there to stop using broken inventories in terms of minecraft (trashcans, enderchests, any false reported stacksizes, any sorts of shared inventories, any pipe like teleporting stuff around, etc)
2. Convince forge to add an IItemHandler designed for performance and not to be a simple and slow as possible. Say use a more reactive approach or at least some observer pattern, so you do not have to rescan 20k slots to validate, if 1 piece of cobble was actually stored here. As well as ensure 1. still applies in regards to broken observer implementations.

[GuntherDW]

fbaff34
Should fix the cable anchor recipe. Apparently 32767 (or Short.MAX_VALUE) is the magic value used for minecraft's crafting ingredients.

[p455w0rd]

I'm using only AE drives for storage. Also, it appears that you ( @yueh ) seem to think I'm referring to the autocrafting as being slow. On the contrary: The autocrafting is where AE shines. It's just pulling items from the network manually (via GUI) that lags and will eventually DoS a server. On the topic of IItemHandler being garbage (the implementation-not the idea) - It is truly horrible. However if this is what's causing the issue with mods like AE2/RS, would the simple solution not be to just not support IItemHandler? IInventory is still a perfectly viable thing that should probably used over IItemHandler with game breaking issues like this...

[covers1624]

Yeah the netty bs is full retarded, I'm going to try and investigate what has changed and maybe poke cpw to see if the change is something known or something that fml can swallow, regarding inscribed insertion, basically what was happening was, the cap insertion was failing as it was supposed to as the item wasn't allowed in, but dumb patch is dumb and it fell in to vanilla insertion logic, first checks if the item is "valid" and it is, it then checks if the item can be inserted and your default override for that bounces to is valid. You can even verify the fix, only takes 2min with a breakpoint.

[JelmarG]

The shift-clicking stacks has been lagging servers for years, that's not new.

[GuntherDW]

The issue is that the ByteBuffer changed between version by the looks of it. The server, when you're in a singleplayer world gets a different "ByteBuf" instance than it would get when you're running a dedicated server, which doesn't support "array()".
That was the main cause of the ValueConfig and JEI packets not working. But something else might be screwing up the crafting UI

On a singleplayer session the ByteBuf is "io.netty.buffer.UnpooledByteBufAllocator.InstrumentedUnpooledUnsafeHeapByteBuf", but when you're running a server that gets set to "io.netty.buffer.PooledUnsafeDirectByteBuf".
This is in "net.minecraft.network.PacketBuffer", "public final ByteBuf buf".

[covers1624]

Yep, CCL has been experiencing the same issue. Think im just gonna try and get ahold of CPW then. Its all netty black magic to me :D

[yueh]

This slightly feels to get out of hands with every posting appearing to hit a completely new and/or different topic. Currently it is still ok, but it can get worse quite fast.

Shift Click Crafting

I am fully aware what you mean @p455w0rd. To shortly go back to autocrafting, this can calculate all requirements off thread and then just have a list with very few different items but still potentially high stacksizes. Extracting them is quite fast as there is pretty much no further validation needed.

But for the manual crafting it is quite a amount of work and it hits so many different areas with performance issues. Which also does not leave us many optimizations. It even affects vanilla crafting benches to some degree. But it is mostly limited by not having an infinite inventory behind it. Thus it is limited to a single stack until you have to manually refill it and giving it some time to settle.

The first in no particular order is getting the items. We have to extract every item piece by piece. So up to 9 extractions for a single crafting operation. Each one has the chance to fully invalidate the network inventory due to large amount of unreliable inventories out there. Like a recipe needing iron ingots and iron blocks. Extracting them from a compacting drawer might make blocks unavailable.
The only option here is to try and combine the same item types into a single extract. But simple compression recipes are not that common and it would leave use with a N+1 problem. E.g. extracting N cobble fails, try N-1, fail, ... try 1, fail. Of course limited to N <= 9, but still.
So for item we have to revalidate the inventory and rescan every attached inventory. Drives got a bit better with 1.10 and later as they (should) now work with deltas and issue a full rescan. But it can be an issue with other inventories. But IItemHandler is certainly better than IInventory. The first at least can support better performing insert/extract out of the box, which the latter does not. So IInventory is actually worse in some areas and in general way more difficult to work with directly. Nevertheless both are nothing of value once it exceeds the use case of chest -> hopper or hopper -> chest. This needs some interfaces designed to handle monitoring an inventory in a very efficient way. Say an observer to notify about changes and not forcing a full rescan of the whole world for every single operation. There are a couple of suggestion on how to "improve" IItemHandler, but you can easily combine all of them into "How to make it slower and more tedious to work with".

The next issue then is crafting a single stack, because that just multiples the operations by up to 64.
Which puts the previous N immediately to <= 576.
But it also adds that we have to lookup the recipe 64 times. Because we cannot assume it is still a valid recipe, say container items like tools losing durability. As there is no straight forward way to detect these cases easily, we have to hit the recipe manager for every single operation. Which is notoriously slow, because it has absolutetly no lookup based system. It is just M handlers iterating over N recipes until one finds a match. (N different for each M). E.g. binnies mod was particularly well know in 1.7.10 to cause ridiculuous performance due to scanning all the different wood recipes. Usually it did take like 1 order of magnitude more time than all other handlers combined. Not sure how the new recipe system performs. Or even the forge "improved" one. But I doubt it would perform better.

And there are probably many other things I have already forgotten. But usually, if we are able to improve one part, another ones gets worse and causes eachother to cancel out. Or some other mods do some funky stuff, so there is no option as to assume everything is broken and have to very conservative, otherwise everything would break left and right.
As long as forge does not step up and improves the recipe manager to something like O(1), O(log n), or O(n) with being the input slots instead of the current worse case of O(#recipeHandlers * #recipesPerHandler). I doubt we can ever fix it. And Forge will probably argue that it is not necessary for vanilla and you could just disable shift clicking should it be an issue...

But without having actual sampler or profiler snapshot. It is just guessing about it. Thus I simply claim the recipe or some other mod is causing it until proven otherwise.

netty

The issue here is probably not netty but the abomination forge uses. Like it does not even use the normal pipeline. If you create a channel, you are not using it to send packets along. You will use it so send packets so forge can create packets from these packets via toBytes/fromBytes. Which completely invalidates the netty pipeline. There is absolutely no option to add a simple logging or transparent compression, etc. Because there is no pipeline or packets.

In terms of singleplayer/multiplayer I certainly would not be surprised should forge use different code paths. Like use the packets to create packets to serialise packets to send packets over network and back for the multiplayer part, but use direct memory access for single player. Due to it still being multithreaded it then has to resort to other threadsafe datatypes, which break down the moment you treat them like packets...

[ben-mkiv]

can't the crafting lag be fixed by introducing a crafting buffer, which first checks inventory, then extracts up to a stack of items of each recipe part, and crafts the stack in a single task.

(this could probably be done by adding "virtual" crafting units to the AE controller/craftingterminal which handle the crafting)

[thiakil]

PacketJEIRecipe & PacketValueConfig can be simply changed to use ByteBufInputStream instead of ByteArrayInputStream, other uses of array() appeared to not be ByteBufs from forge/netty

see thiakil@1c6744f

[thiakil]

For what it's worth, I've created a branch and cherrypicked only the things that can be re-integrated (i.e. not including anything that was necessary for being a fork) @ https://github.com/thiakil/Applied-Energistics-2/tree/rv5-cleanerhistory

As I have said from the beginning I'm happy to help if you want.

My branch cannot be merged without conflicts as I was too far in by the time I knew of Gunther's work, and I also chose to rebase the dangling 1.10 commits of yours instead of cherrypicking them.

As for why you'd want to look, some things I've just done differently (all the Inscriber fixes for example, or the whole 1.12 registry system), it also fixes a number of bugs on this tracker. E.g. Botania flower bags, facade rendering (and no it doesnt mess with non-facade models), slab lighting, off-hand GUI items, BC & TF wrench support, the Buildcraft crash for 1.11, advancements, TOP/HWYLA show custom interface & P2P names, Storing entities in the spatial storage system works fine, and probably a few other things.

I also changed the P2P item & fluid tunnels to fully use caps, and they support the output pulling from the input too. My FE tunnel works the same way, and also should distribute power like the old RF tunnel did. There's also a JEI page for the different tunnels listing their attunement help text.

In reference to the OP of this thread it should only have the bug about shift clicking, as I have yet to try to increase the amount of items it pulls per crafting (the functions already know how many operations are requested, so seems logical it could at least try to extract that many source ingredients).

[yueh]

Closing for now. The first point should already be solved for a long time. The last one will see a major improvement with the next release.