[1.16.3] Classloading the DynamicRegistries before the world is made and ConfiguredFeatures/StructureFeature not registered
TelepathicGrunt opened this issue ยท 4 comments
Versions
- Minecraft: 1.16.3
- Forge: 34.1.9
- Vampirism: Vampirism-1.16.3-1.6.0-beta.1
So I was testing out Vampirism with some of my mods and my mod Blame found there's a few issues with Vampirism that could significantly harm mod compatibility.
The first issue is Vampirism's ClientEventHandler.onGuiInit is classloading the DynamicRegistries class before any world is made. This can be really bad as when DynamicRegistries is loaded the first time, it copies the entire content of WorldGenRegistries into itself and will never copy it again until you restart the whole game.
What this means if is any more registers their biomes, ConfiguredFeatures, ConfiguredStructures (MCP calls them StructureFeatures), etc after ClientEventHandler.onGuiInit classloaded DynamicRegistries, their registered stuff will break in nasty ways. I'm talking stuff like "Unknown Biome Id: -1" spam and biomes not spawning at all. Yeah. On a server, ClientEventHandler.onGuiInit probably isn't ran so dedicated servers are fine. But Integrated Servers in singleplayer will be affected. The solution is to not classload DynamicRegistries until a world is made and only then, is it 100% safe to reference the DynamicRegistries class. It is hard to wrap our heads around this and it caused so many issues over on Fabric until we realized what was going on and got people to stop using that class at mod init lol.
Now the second issue is the configured forms of vampirism:hunter_camp and vampirism:vampire_dungeon are not registered. This can be an issue for mod compatibility as under certain conditions, unregistered ConfiguredFeatures and ConfiguredStructures can basically prevent other mod's registered ConfiguredFeatures/ConfiguredStructures from spawning if in the same generation stage.
By that I mean, if mod A adds an unregistered CF to the ore generation stage and the biome's codec reaches it first, it will choke and basically nuke mob B's registered CFs afterwards. Here's a case where BetterCaves forgot to register their CF and caused several CFs from Oh The Biomes You'll Go to stop spawning in the world: YUNG-GANG/YUNGs-Better-Caves#75
Here's a more detailed explanation of why this happens in the biome's codec:
Specifically, when you call .withConfiguration on a Feature, you create a ConfiguredFeature. This is what should be registered to the WorldgenRegistries.
Here's an example from my mod RepurposedStructures of me registering all my ConfiguredFeatures in case you plan on adding more and want to see one way of organizing everything.
https://github.com/TelepathicGrunt/RepurposedStructures/blob/a4e3365e3867b8510952ebf658c415de6e412927/src/main/java/com/telepathicgrunt/repurposedstructures/RSConfiguredFeatures.java#L184-L185
I hope this helps!
Here's the log with Blame showing the problems found. It's pretty useful for this kind of stuff as I even caught myself forgetting to register ConfiguredFeatures until I used Blame on my own mods lol.
https://hastebin.com/liluwihiye.sql
^^second part could be fixed like d47e199 , but must be tested
The ClientEventHandler.onGuiInit part is only there, because the game crashes for Optifine user while creating a new world
@Cheaterpaul maybe it is reasonable to move from a static registration to registering those in a function called right after the features themselves have been registered. Just to make sure we do not accidentally register them to early due to an access to that class. Not sure.
@TelepathicGrunt Thank you for your notification about this issue as well as the detailed explanation. However, I haven't completely understood the first issue yet.
As far as I can tell from our code and the stacktrace you provided Vampirism classloads DynamicRegistries during init of the WorldSelectionScreen. This might be a little bit before Vanilla does, but latest when hitting the "create new world" button it is accessed (via CreateWorldScreen#func_243425_a) which is still before any world has been created.
So this should only affect anything registered to the WorldGenRegistries between opening the load world screen to the create new world screen, right? I don't see any reason to register something here. I assume everything should be registered during startup of the game since it is only executed once. Everything later (e.g. some config GUI) can also be accessed after a world has already been loaded (and therefore the registries have been copied anyway).
The reason we are accessing the DynamicRegistries in the first place is because Optifine (with shaders) sometimes messes with dynamic registries (probably by statically initializing them too early). We could of course move our access behind a Optifine loaded check, but I would like to understand the original issue first.
Ah I see you were the one to make that optifine biome breaking issue report. Yeah Optifine is causing a headache and I just check too and the place where you load it might be fine. Most people have been registering their configured stuff in the mod init so that should be good. The only other place I could see people registering is in BiomeLoadEvent but they will need to register to the DynamicRegistries directly at that point so your classloading shouldn't be an issue.
I can go ahead and make Blame ignore vampirism's DynamicRegistries's loading as there shouldn't any problem with it. (it would be really odd for someone to register to WorldGenRegistries between the Show Worlds button and clicking on a world itself.