Rework Pump Machine logic to fix issues

[eragaxshim]

Hi everyone! New contributor here. Not new to Java or modding or software, but new to GTCEu. This is a (somewhat) ambitious overhaul of the internal logic for the Pump (the machine, not the cover).

If you want me to make an issue first, I understand, but I was planning on making a patch for my own world so I jumped straight into implementing it.

What

Pumps have some behavior quirks in GTCEu Modern. Partly this is due to clear bugs, partly this is because GT pumps have always behaved maybe not as you would expect (I remember issues on GCTE and earlier also having occasional problems). This implements a brand new Pump Machine logic. It will be slightly slower than the previous implementation, but it's still optimized.

Bugs solved by this PR

The previous implementation contained a porting bug. In 1.12 GCTEu there are mining pipes and it checks for transparent blocks to move the pump head down. In modern, it only lowers the pump head if there is liquid right beneath it. Consequently, the mining pipe is never actually set even though there is code for it. This means that when rejoining a world, the pump will generally stop working as it lost the pump head information and now there is air between the liquid and the pump, so it will do nothing.

From the previous implementation and the implementation in 1.12, I could not really glean what the intended behavior was. Is it only supposed to work if there are source blocks right below? What if there is a layer of source blocks one layer higher than where the pipe is, should those be included? It was quite unclear. Trying to ask around and search on the Discord didn't give me many hints either.

Implementation Details

The previous queue of blocks to check and known fluid source blocks is replaced by the "Pump Queue", which consists of 1 or more paths from the pump head (the source block below the mining pipe) towards source blocks. It does a depth-first(ish) search, biased up and away from the pump head. To prevent weird order of removing source blocks, it first only checks if it can find sources in the layer above the pump head.

The main rule is: For a source block to be mineable by the pump, it must be connected to the liquid block below the pump head through other liquid blocks of the same type, those can be either source or non-source. This means that a source block much higher up, but that flows down and connects to the block below the pump head would also be mind. In fact, it would probably be mined first, "cleaning up" the vein. The oil vein is mined top-down. Only source blocks at the layer below the pump head or above can be pumped.

The implementation is somewhat complicated for performance reasons. It will try to find enough paths so that it has 5 pump cycles queued up without having to do another search. At pump time, it will go through the queued up paths to see if they are still valid. It will first try to only find source blocks above the pump head. If there are none, it will also look at the layer below the pump head (but nothing below that). The pump queue is updated whenever it's empty. If it empties before a pump cycle did the amount of pumps it wants, it will also rebuild.

Currently the highest tier pump will only pump once every 20 ticks and as such only do a search once every 20 ticks. See comments in the code for some more details. For a normal vein, the iterations it does to find enough paths are only a few dozen at most (a standard oil vein always less than 10 or so), so it's not too bad at all. I also tested it in the Nether, where pumps up lava from the Nether nicely. You can also really see the paths there.

Outcome

Pump has overhauled logic, allowing veins to be consistently pumped with a single pump.

Additional Information

I can provide additional info if necessary. Since I'm relatively unaware of a lot of GTCEu internals, I hope there's not any glaring issues.

Potential Compatibility Issues

None, only PumpMachine is changed. If this refers also to porting it to 1.21, it will require some tiny changes due to FluidHandler and stuff now being inside GTCEu itself (instead of ldlib).

Unanswered questions

• How is range supposed to work? The way it was implemented is that while it says the working area is 64x64 (which to me says a square of 64x64 blocks) in the tooltip, it actually is a circle of radius 64. Is that intended?
• Should the queue be persisted? Right now it's just a field in the class.
• Can the previous data structures in the class (fluidSourceBlocks and blocksToCheck and initializedQueue) be safely removed? I assume since they aren't saved it's totally fine but I haven't removed them just to be sure.
• The pump speed might need some balancing. Lower tiers are fine, but EV will now pump 2 blocks every 20 ticks. However, it only advances the pump head every 40 ticks. So while going down an oil vein it is: 2 blocks -> 2 blocks -> final block on layer -> advance pump, so 100 ticks for 5 blocks or nearly twice as slow as what you would expect. I just made pretty arbitrary choices here, any opinions?

[eragaxshim]

The above problem was fixed. I ran 40+ Advanced Pump III pumps in the nether right next to each other. Also a quick performance metric from the pie chart (let me know if there are easy ways to get more accurate measurements). On the old version those 40 pumps in the Nether were at like 13% of the block entity tick, while after this update it's 16%. So slightly slower, but some of them don't even work anymore (and definitely do not if not placed directly above the lava).

[screret]

How is range supposed to work? The way it was implemented is that while it says the working area is 64x64 (which to me says a square of 64x64 blocks) in the tooltip, it actually is a circle of radius 64. Is that intended?

it should probably be the diameter.

Should the queue be persisted? Right now it's just a field in the class.
Can the previous data structures in the class (fluidSourceBlocks and blocksToCheck and initializedQueue) be safely removed? I assume since they aren't saved it's totally fine but I haven't removed them just to be sure.

just remove the old fields, I don't think the queue needs to be saved.

The pump speed might need some balancing. Lower tiers are fine, but EV will now pump 2 blocks every 20 ticks. However, it only advances the pump head every 40 ticks. So while going down an oil vein it is: 2 blocks -> 2 blocks -> final block on layer -> advance pump, so 100 ticks for 5 blocks or nearly twice as slow as what you would expect. I just made pretty arbitrary choices here, any opinions?

hm. fine? need 2nd opinions on this.

[eragaxshim]

@screret thanks for the review! I think it should all be fixed now. I've been playing with this patch on my own world for the past few days and didn't encounter any issues.

[screret]

wait why'd you change long distance networks
separate PR please

[eragaxshim]

wait why'd you change long distance networks separate PR please

Me being an idiot with Git, removed it already

[krossgg]

Wait for FluidStack migration before merge

[eragaxshim]

Wait for FluidStack migration before merge

What's that? Anything I can do to help?

[krossgg]

My comment was more directed towards devs rather than you.
No. It's essentially done. See #1975.

[eragaxshim]

@krossgg with pipenet delayed and #1975 waiting on that, is it possible to merge this? Changing out the FluidStack logic would not be a lot of work and I can do that when the time comes