GTNewHorizons / GT-New-Horizons-Modpack

New Modpack with Gregtech, Thaumcraft and Witchery
https://www.gtnewhorizons.com/
Other
891 stars 280 forks source link

Advanced Stocking Input Hatches Are Still Very Broken #16267

Closed srdr2k3 closed 1 month ago

srdr2k3 commented 2 months ago

Your GTNH Discord Username

@srdra

Your Pack Version

2.6.0

Your Server

Private Server

Java Version

Java 17

Type of Server

Vanilla Forge

Your Expectation

Advanced Stocking Hatches should be correctly auto-stocking the fluid to the machine without crashing it.

The Reality

Even with a closed subnet (1 Drive with a single ME Quantum Fluid Cell connected to 16 Advanced Stocking Input Hatches, set to 20 tick refresh rate, which is in turn connected to 64 LPFs as quads), the hatch manages to crash and void the craft. The Subnet:

image

image

Your Proposal

Make it not crash.

Final Checklist

norbby42 commented 2 months ago

I've been trying to get a reliable testcase for Stocking Input Hatches (both advanced and non-advanced) still causing a machine critical shutdown on 2.6.0 (with Glease's internal state mutation cleanup from Apr 15th).

It's difficult/annoying because it's not guaranteed.


I spent a little while digging through the implementation to see if any there were any logic holes in the implementation that stood out to me. One did:

https://github.com/GTNewHorizons/GT5-Unofficial/blob/c3eb8081589b53356f7db89fcdc29dc386e999c3/src/main/java/gregtech/common/tileentities/machines/GT_MetaTileEntity_Hatch_Input_ME.java#L387

When updating the storedInformationFluids array in updateAllInformationSlots(), there is a case where we could potentially NOT get a result from AE2 (due to an exception), but not update/clear the cached values. This would cause shadowStoredFluids when updated in getStoredFluids() to be desynced from the current AE2 network state and have the last known amount (which could be more than AE2 actually has, later causing an extraction failure).

I don't know how this would manifest (AE2 is a bit of an obscure spiderweb), but maybe it's something to consider? Hoping someone who has more experience with these systems can take a look. I could be quacking up entirely the wrong tree.

fluffle commented 1 month ago

I an pretty sure it is not voiding fluids. I have a couple of large scale fluid solidifier implementations that use cell movement and they trigger this all the time. Critically, the universal solidifier connected to AE for on demand crafting does not see crafts getting stuck or failing which means all the items going into the system produce correct outputs, even if machines crash-stop constantly. I have to admit I just stuck machine covers on the LPFs, set to "disable with redstone, safe mode off", to ensure they all stayed operational. So far I have not observed any negative effects from this, apart from all the sad machine noises.

I suspect this is occurring when two machines attempt to take the same fluid from AE on the same tick, but I know even less about the AE codebase than the previous commenter ;-)

My half-baked thought was that maybe all machines did one part of recipe calculation (with SIMULATE) and then later in the same tick did the actual extraction (with MODULATE) expecting it to succeed because the SIMULATE did.

elias098 commented 1 month ago

have played around a bit with the debugger and it does seem like storedInformationFluids and shadowStoredFluids does get desynced, with shadowStoredFluids having entries that should be null. One solution could be to set it to null when it begins processing

elias098 commented 1 month ago

i dont know enough about the codebase to know if it would break anything but it might be better to set shadowStoredFluids during startRecipeProcessing and clear it during endRecipeProcessing