Help us fix spark transfers

Wormbo

As you may have noticed, development speed for Botania has recently come to a bit of a stop. While I'm sure it will pick up speed again in the near future, there is currently a reasonably annoying bug in Botania's spark-based mana transfer logic, as described in "Spark Network seizes up when charging items that drain mana constantly".
Long story short: If a transfer completely fills the target pool, but some small constant mana draining effect removes some mana from the pool, the transfer still stops and usually won't restart without manual intervention.
So, if you know how to write code in Java (ideally for Minecraft) and want to give fixing this a try, please let us know. As far as I'm aware, a fix may require rewriting a lot of the spark transfer logic, which is why I haven't gotten around to figuring it out myself yet.

If you are unfamiliar with how spark transfers are supposed to work, here's a quick rundown:

Sparks are entities attached to certain mana-containing or mana-using blocks. (Mostly mana pools and devices that are expensive to run, like the Terrestrial Agglomeration Plate.)
At the base level, devices can instruct their spark to pull mana from all other nearby sparks.
Sparks can also be augmented to make mana flow between blocks on its own. The five possible "augmentation states" are "none" (no augment), "recessive" (pushes mana away from the block), "dominant" (pulls mana towards the block), "dispersive" (pushes mana into mana-holding items in nearby players' inventories), and "isolated" (prevents interactions with recessive and dominant sparks).
Recessive sparks automatically initiate transfers towards nearby unaugmented or dispersive sparks.
Dominant sparks automatically initiate transfers from nearby unaugmented sparks only.
Spark transfers are always lossless. If the target block is almost full, an appropriately reduced amount of mana is transferred during that tick. The split between multiple transfers from an almost empty block or to an almost full block can be defined by the implementation in any way, but ideally evens out eventually.
Each individual spark transfer runs individually from other transfers that the source or destination sparks participate in. There can only be one transfer between any pair of sparks. Requests from "device block" have higher priority than transfers between different augmentation states.

williewillus

have you tried reverting the PR that you suspected introduced the issue and seeing if it's fixed?
I'd rather it be correct and slower than fast and wrong

Wormbo

williewillus Might be a workaround for 1.20.1 for the time being, but each recessive or dominant spark getting nearby sparks every tick might be a significant performance impact, especially in Botania's already entity-heavy build style.

cjlarose

I can take a look at this. This issue affects my own builds on 1.20.1. I have some professional Java experience and a good amount of experience contributing to projects on GitHub (https://github.com/cjlarose), though not a lot of Minecraft mod development experience.

I'll see if I can fix the spark transfer issues while preserving the performance benefits from https://github.com/VazkiiMods/Botania/pull/4459.

Do you know of any reliable steps to reproduce the issue or a world folder that reproduces the issue in 1.20.1? If not, I can start there and see if I can reliably reproduce it. I've seen it firsthand in my own survival worlds, but it wasn't clear to me what combination of factors made it break.

Wormbo

Thank you for the offer. Someone on the Discord took up the offer and provided a PR already.

The main issue was that when chunks with sparks are loaded again, the order in which sparks are ticked is not necessarily the same every time. In certain situations, transfers towards dominant sparks could stop because the source sparks found the target spark's pool to be full, but before the target spark gets ticked, another source (e.g. a flower or a player's trinket via a bound mana mirror) may have drained some mana again. Then it's the dominant spark's turn and it sees that it's still not full, so it doesn't see any reasons to update transfers.

The PR fixed this by introducing pull transfers in addition to the existing push transfers. Dominant sparks now are the ones handling the mana transfer towards themselves, so they will have the same direct control over the transfers as recessive sparks pushing to unaugmented and dispersive sparks.

cjlarose

No worries! Thanks for the context, @Wormbo !