Neoforge Biome/Structure Modifiers crashes on recycled Frozen Registries

[Speiger]

o/ A Interesting crash with server registry recycling happens. (I technically have it fixed with mixins but i still should report it)

When you create a custom server using the world creation screens information, that works just fine. But the moment you try to do that a second time the server will simply crash, because the registries haven't been reset 100%

This leads to the following crash:

Crashlog

``` java.lang.IllegalStateException: Biome Reference{ResourceKey[minecraft:worldgen/biome / minecraft:badlands]=net.minecraft.world.level.biome.Biome@504521ae} already modified at net.neoforged.neoforge.common.world.ModifiableBiomeInfo.applyBiomeModifiers(ModifiableBiomeInfo.java:76) ~[neoforge-20.4.49-beta.jar%23196%23203!/:?] {re:mixin,re:classloading,pl:mixin:APP:chunkpregen.mixins.json:common.forge.BiomeModifierRemoverMixin,pl:mixin:A} at net.neoforged.neoforge.server.ServerLifecycleHooks.lambda$runModifiers$1(ServerLifecycleHooks.java:200) ~[neoforge-20.4.49-beta.jar%23196%23203!/:?] {re:classloading} at java.util.AbstractList$RandomAccessSpliterator.forEachRemaining(AbstractList.java:720) ~[?:?] {} at java.util.stream.ReferencePipeline$Head.forEach(ReferencePipeline.java:762) ~[?:?] {} at net.neoforged.neoforge.server.ServerLifecycleHooks.runModifiers(ServerLifecycleHooks.java:199) ~[neoforge-20.4.49-beta.jar%23196%23203!/:?] {re:classloading} at net.neoforged.neoforge.server.ServerLifecycleHooks.handleServerAboutToStart(ServerLifecycleHooks.java:90) ~[neoforge-20.4.49-beta.jar%23196%23203!/:?] {re:classloading} at net.minecraft.client.server.IntegratedServer.initServer(IntegratedServer.java:72) ~[neoforge-20.4.49-beta.jar%23197!/:?] {re:classloading,pl:runtimedistcleaner:A} at net.minecraft.server.MinecraftServer.runServer(MinecraftServer.java:663) ~[neoforge-20.4.49-beta.jar%23197!/:?] {re:mixin,pl:accesstransformer:B,re:classloading,pl:accesstransformer:B,pl:mixin:APP:chunkpregen.mixins.json:common.server.MinecraftServerMixin,pl:mixin:APP:chunkpregen.mixins.json:common.server.ServerSeedMixin,pl:mixin:A} at net.minecraft.server.MinecraftServer.lambda$spin$2(MinecraftServer.java:255) ~[neoforge-20.4.49-beta.jar%23197!/:?] {re:mixin,pl:accesstransformer:B,re:classloading,pl:accesstransformer:B,pl:mixin:APP:chunkpregen.mixins.json:common.server.MinecraftServerMixin,pl:mixin:APP:chunkpregen.mixins.json:common.server.ServerSeedMixin,pl:mixin:A} at java.lang.Thread.run(Thread.java:833) ~[?:?] {} ```

And sadly there is no API to actually reset the biome/structure modifiers. So a mixin is required to fix this.

To explain why I ran into this problem. I simply implemented a quick seed switch option where you can cycle through seeds a lot more quickly then normally. And when you recycle the registries that have already been picked (you can't change them after the fact) changing the seed has barely any effects on registries themselves. The issue is always appearing, unless you do a 100% registries recreation, which is kinda overkill. That issue exists since 1.19.0

So yeah.

Some infos about neo version: I was using: Neoforge 20.4.49-beta

Edit: This is the code to fix this crash.

Code

```java private void resetRegistries(MinecraftServer server) { final RegistryAccess registries = server.registryAccess(); registries.registryOrThrow(Registries.BIOME).holders().forEach(biomeHolder -> { ((BiomeModifierRemoverMixin)biomeHolder.value().modifiableBiomeInfo()).clearModifiers(null); }); registries.registryOrThrow(Registries.STRUCTURE).holders().forEach(structureHolder -> { ((StructureModifierRemoverMixin)structureHolder.value().modifiableStructureInfo()).clearModifiers(null); }); } ``` It just resets the modifier cache so the modifiers can be reapplied on the next attempt.

[TelepathicGrunt]

Is this reproducible with only one mod that has 1 biome modifier (outside your seed cycling stuff)? Is the steps just to make a world, exit to menu, and then remake the world again?

[Speiger]

@TelepathicGrunt that is reproducable with just forge and 0 biome modifiers. On startup the biome modifiers are compiled and when a existing cache is present it will straight up throw a crash. Because it expects no cache to be present.

You simply can't recycle a server instance with different seeds. You literally have to rebuild the entire registry from scratch everytime you want to change the seed. Even if nothing but the seed changes.

[TelepathicGrunt]

Hello, is this still an issue in 1.21?

[Speiger]

@TelepathicGrunt does ModifiableBiomeInfo#applyBiomeModifiers still throw an exception if it gets called twice instead of invalidating the cache and regenerating it?

Spoilers: Yes

Edit: Sorry I didn't mean it to be that sassy ^^"

[TelepathicGrunt]

This seems like it is intended. As far as I understand it, you change the seed for the game, run the IntegratedServer#initServer again with the registry access of already modified biomes, which triggers NeoForge's handleServerAboutToStart that will now run our modifiers and ServerAboutToStartEvent event as well where other mods could also modify the registry access.

So what this seem like is happening, if a biome modifier ran the first time and added Blaze to the Plains biome, running your current code will take the existing registry access and feed it through the pipelines again and cause Blaze to be added again to the already modified Plains biome. So now Blaze is added at twice the intended rate.

And this is going to also impact other mods too that modify the registryAccess in ServerAboutToStartEvent which is, actually a surprisingly lot of mods! For example, many mods have been injecting their pieces into villages by using ServerAboutToStartEvent for when to modify the vanilla Template Pool https://gist.github.com/TelepathicGrunt/4fdbc445ebcbcbeb43ac748f4b18f342

This means, even if NeoForge removes this crash, your code is still going to duplicate the modifications other mod has done within ServerAboutToStartEvent and that's more of a bug on your side to be honest as those modders have no way to shield themselves from recycling the registry access.

As far as I can tell, it seems Mojang intended for every fresh IntegratedServer to have their own re-created registryAccess data. Even the re-create button in Minecraft does a fresh registryAccess as well.

My question to you is, are you sure you cannot keep the existing IntegratedServer and modify its data in place without needing to spin up a new IntegratedServer? It would work better with other mods beside NeoForge. Like change the seed for IntegratedServer and make sure that new seed is set everywhere it needs to be, then yeet the chunk data and respawn players for a fresh world without needing to tear down and remake RegistryAccess

[Speiger]

Wait so you are saying that BiomeModifier elements that are stored Immutably inside of the Vanilla Registry are being modified as the server loads the world? Let me rephrase this a bit, isn't the Function https://github.com/neoforged/NeoForge/blob/1.21.x/src/main/java/net/neoforged/neoforge/common/world/ModifiableStructureInfo.java#L67 here ment to create a copy of the Structure information that is modified to prevent the original from being modified in the first place is actually useless?

Like why does this code even exist if the function that is here being effectively meaningless. The whole point of my issue is that this functionality seems either:

• 1: Useless and therefor should not exist in the first place (if the modifiers break the original implementation anyways)
• Or 2: Not cause a crash because it just deleting the original modification and just recreate it, since all information is simply immutable anyways. Both the original information and the modifiers are practically create a safe/secure copy that support mods while protecting the original element. (If this is not a given then go back to the 1)

And why I am always spinning up a new server instead of changing the seed? Because once the server is started certain files are simply locked into place, and these files are usually world defining elements that you need to recreate from scratch but can't because they are locked as long as the server runs. The registry itself is never changing and simply a read only map for getting things done. But anyways even if there are self mutating elements inside the registry these elements should always assume: "If there is already a mutation result, discard it and recreate it" (Which the implementation supports too and we have years of proof that it works stable, with all kinds of combinations of mods)

Anyways, if you try to change a seed without actually deleting all files that represent the previous seed then you basically get into real issues, you need to get as close as possible to a delete/recreate as possible. And the one thing i realized is that the registry itself doesn't require a reset to work just fine. Well outside of neo/forges MutableInformation self destructing for no found reason? And if the registry recreation wouldn't take like 5 times longer (Extraration) then creating a fresh world then I would keep it that way.

Anyways, I hope this doesn't seem like a rant, I am just trying to explain my view point ^^" It isn't required that this change happens but I also should make you aware that this has been like in place for a long time just fine, its just neo/forge that is a cause for trouble here and pushing them out of the way basically makes things work without downsides.

[Speiger]

Changed the name since recycled datapacks sounds missleading since the idea being that the registries were already frozen by the time the first server starts up...

[TelepathicGrunt]

I can't speak for the reasons of why the code for the modifiable info classes was setup the way it was. Probably that when it was added, it was also deemed unnecessary to support registry reuse but wanted to provide ability for people to see the unmodified form of the biome/structure if needed.

Though my example with structures was nothing to do with the NeoForge structure modifiers. It was mods, many mods, modifying the TemplatePools directly within ServerAboutToStartEvent. I am fairly certain if you put on a mod that does the village house injection, cycle the seed a ton of times with your mod, and then visit a village. It will be flooded with the modded house because their injection ran multiple times for the already modified TemplatePool. Filling up the list of pieces with the modded piece multiple times. So even if NeoForge changes how it does its modifiers, you still have this hidden incompatibility with other mods and very difficult for players to figure out what is going wrong.

Honestly, I think you are breaking my own mods in some subtle way too by reusing the registry multiple times and feeding it back through the server creation pathway.

[Speiger]

The first aspect is something that should be revisited IMO. And that is what i am asking this entire time.

The second aspect. I will investigate that as soon as i am ready to mod again. And if there is an issue I will try to find a solution to that. I can get creative easily. (Especially since i bug support still all the way back to 1.7.10)

Farmers delight it actually a good mod to test with. Thank you for that potential issue.

[Speiger]

@TelepathicGrunt from looking into the issue you described on my side. Yes that is actually a bug. That should be addressed in the next patch (on all mc versions)

Thank you for pointing that out, because i never noticed that.

[neoforged-releases[bot]]

🚀 This issue has been resolved in NeoForge version 21.1.65, as part of #1545.