Write-to-read problem with configs, leading to weird side effects

[IchHabeHunger54]

Minecraft Version: 1.19.2

Forge Version: 43.1.47

Logs: N/A

Steps to Reproduce:
This is a bit of a problem. I have not found a reliable way to reproduce. See the details below.

Description of issue:
There is a write-on-read problem with Forge's config system. When the config is read, the contents read are written to the same file. This can lead to side effects such as config backups being created and the config being reset to normal for no reason at all. As mentioned, this is not reliably reproducable. I can't really pinpoint a reason why, either, but some discussion on Discord suggested that ForgeConfigSpec#correct is broken in one way or another.

[TheCurle]

This is what also causes the corruption issue with Notepad++, and the occasional blanking out to 14 nul values of configs.

I've spent weeks worth of time scouring all the config related functions, i'm certain that the issue lies within NightConfig at this point.

Assigning this to myself for posterity.

[TheElectronWill]

Hi! NightConfig's author here.
I've analysed the problem reported in neoforged/NeoForge#32, probably caused by multiple concurrent events conflicting with each other, and maybe dependent on the editor used to edit the file. I don't know why it hasn't been reported sooner in my repository, but now that I'm aware of it, I'll be able to publish a solution :)

[TheElectronWill]

Update: TheElectronWill/night-config#152 has been merged with a full rewrite of FileConfig, based on concurrent configurations (see the comments on the PR). When the new API is used properly, it should hopefully fix the problem.

[PaintNinja]

That's great, although we're waiting for that to be published in a new release first so that we're able to update and start taking advantage of your fixes.

[TheElectronWill]

I've just released NightConfig v3.7.0 with the aforementioned fixes and more (sorry for the wait)

[SrRapero720]

@TheElectronWill have you done any breaking change on the 3.6.7 -> 3.7.0 transition?
At least to attempt PR a backport

[TheElectronWill]

According to japicmp, there is no breaking change. Replacing NightConfig 3.6.x by NightConfig 3.7.x should "just work" (at the very least, it will compile).

Use 3.7.1 instead of 3.7.0, it fixes a misconfiguration of the Gradle build (TheElectronWill/night-config#173). It should be available in a few hours.

[LexManos]

Binary wise it doesn't look like there are any breaking changes, source wise there is one that effects Forge.
The removal of the throws on addWatch makes the compiler complain about unreachable code. Which is simple enough to update for.

I looked over all your changes last night and it seems fine. Did some testing and with a little tweaking was about to us the bulk operations on my correct function but haven't pushed the changes until I can talk to @PaintNinja because I saw his PR about making the builder simpler and wanted to talk to him about it.

As for backports, it should just be the version bump and couple lines change to not have a try and call it good.

[LexManos]

Found another breaking change in 3.7.1, namely tomls don't work at all as they don't use StampedConfig as their internal implementation.
But seems you've fixed that in 3.7.3. But there is another breaking change I ran into. StampedConfig doesn't support valueMap().
Got a work around, but honestly, these issues mean I can't backport the changes as modders could easily be relying on those behaviors and a backport would mean breaking those mods.

[TheElectronWill]

But seems you've fixed that in 3.7.3. But there is another breaking change I ran into. StampedConfig doesn't support valueMap().

Does Forge actually use it? A search on GitHub and Gitlab seemed to show that it's very rarely used, let alone with StampedConfig/async FileConfigs. In any case, valueMap() on the pre-3.7.0 FileConfig is broken because it cannot provide any form of synchronization, which is one of the causes of config corruption.

[LexManos]

It did in a few places. Which i have had to remove while updating to address a reported issue of 7.3.1 being non functional.

8edb551