TerraformersMC / Biolith

A biome placement mod focusing on configurability and consistent distribution of modded biomes
Other
9 stars 2 forks source link

replaceOverworld Doesn't Work w/ JSON/Modded Biomes #1

Closed sylv256 closed 1 year ago

sylv256 commented 1 year ago

I've run into this issue before when attempting to implement something similar to this API, and honestly, I think it's because the JSON-based biomes are registered before the Overworld biome parameter thing calls its bootstrap method. I haven't confirmed this is exactly the case, but I suspect it is.

https://mclo.gs/D3fPHbh

java.lang.ExceptionInInitializerError
    at org.quiltmc.qsl.tag.impl.client.ClientQuiltTagsMod.onInitializeClient(ClientQuiltTagsMod.java:35)
    at net.minecraft.client.MinecraftClient.handler$zon000$quilt_base$onInit(MinecraftClient.java:7518)
    at net.minecraft.client.MinecraftClient.<init>(MinecraftClient.java:470)
    at net.minecraft.client.main.Main.main(Main.java:198)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:568)
    at org.quiltmc.loader.impl.game.minecraft.MinecraftGameProvider.launch(MinecraftGameProvider.java:527)
    at org.quiltmc.loader.impl.launch.knot.Knot.launch(Knot.java:82)
    at org.quiltmc.loader.impl.launch.knot.KnotClient.main(KnotClient.java:28)
    at net.fabricmc.devlaunchinjector.Main.main(Main.java:86)
Caused by: java.lang.IllegalStateException: Errors during registry creation
    at net.minecraft.registry.RegistrySetBuilder$BuildState.thrownOnError(RegistrySetBuilder.java:137)
    at net.minecraft.registry.RegistrySetBuilder.build(RegistrySetBuilder.java:269)
    at net.minecraft.registry.VanillaDynamicRegistries.createLookup(VanillaDynamicRegistries.java:96)
    at org.quiltmc.qsl.tag.impl.client.ClientTagRegistryManager.<clinit>(ClientTagRegistryManager.java:74)
    ... 12 more
    Suppressed: java.lang.IllegalStateException: Unreferenced key: ResourceKey[minecraft:worldgen/biome / wwizardry:forgotten_fields]
        at net.minecraft.registry.RegistrySetBuilder$BuildState.reportRemainingUnreferencedValues(RegistrySetBuilder.java:128)
        at net.minecraft.registry.RegistrySetBuilder.build(RegistrySetBuilder.java:268)
        ... 14 more
gniftygnome commented 1 year ago

Wow ... are you testing Biolith for biome placement? That's really cool but also sort of scary for me since it is extremely alpha code at the moment. Except for the surface rule injector and the surface builders (both of which are being used by Terraformers' biome mods). It also breaks with TerraBlender, which is why the next alpha automatically defers to TB if both are installed. I have to figure out if there's a way to coexist (not going to be easy).

I'm hoping to be able to pick Biolith up again pretty soon and I'll try to understand what's going on here and figure out a fix (I gather you are adding biomes both via json and via Biolith which causes it to crash?).

sylv256 commented 1 year ago

Yeah, I'll have to do some investigations. I'll have to see if somehow registering the specific biome during the net.minecraft.world.gen.BiomeData#bootstrap (QM) registry calls will make it work.

all i wanted was to add a biome to the overworld come on mojang

gniftygnome commented 1 year ago

I haven't seen this issue in Fabric so it's possible I need to make some adjustment for Quilt. I do want Biolith to work in Quilt but I have not tested it there yet.

gniftygnome commented 1 year ago

I got to thinking about this and I realized this problem would have to impact our mods too, so I fired up a test instance of Quilt and got the same error. This issue is specific to Quilt and I am going to ask the Quilt community for advice about what exactly is going on and how to fix it.

gniftygnome commented 1 year ago

When vanilla's OverworldBiomeParameters are returned, JSON biome registry entries have not yet been merged. That's true of Fabric as well as Quilt, but Quilt immediately rifles through the parameters and tries to reconcile them with Biome tags in some way which requires the entries to exist. I don't think that's a very good idea, but it is what it is. In order to work in Quilt, Biolith needs to wait until some later, less convenient stage to inject its biome parameters. This is going to take some thinking.

sylv256 commented 1 year ago

What intrigues me is that adding a modded sub-biome doesn't apply any UNDERGROUND_DECORATIONS features... https://github.com/SylvKT/Wandering-Wizardry/blob/e26c02aab55a3c30bb5d7478864286fdd1919458/src/main/java/io/github/sweetberrycollective/wwizardry/biome/WanderingBiomes.java https://github.com/SylvKT/Wandering-Wizardry/blob/e26c02aab55a3c30bb5d7478864286fdd1919458/src/main/resources/data/wwizardry/worldgen/biome/forgotten_fields.json I don't know half of what's going on here. Maybe I did something wrong because normal biomes have it (see: single-biome world).

Anyways, I'll figure something out.

sylv256 commented 1 year ago

Relevant image

gniftygnome commented 1 year ago

Well, to my mind, running tasks that demand all biome keys in the worldgen parameters have a defined registry entry prior to the point where JSON biomes have their registry entries created seems like an odd choice. It's at best a pretty fragile decision to assume nobody will have injected modded biomes into the parameters.

falkreon commented 1 year ago

I'm really not confident in my analysis, I don't have very much knowledge of the QSL internals yet. So take my words here with a grain of salt please!

sylv256 commented 1 year ago

Looks like it's already there, I appear to be wrong though. Where I point out doesn't seem to be where the real problem happens.

sylv256 commented 1 year ago

The QSL issue has been made. We just need to wait for https://github.com/QuiltMC/quilt-standard-libraries/issues/292 to be fixed.

gniftygnome commented 1 year ago

It's more likely I'll work around this. I suspect Quilt has some design decisions making it very difficult to change their minds so I wasn't even going to try. :)

LambdAurora commented 1 year ago

Should be fixed in the next release of QSL/QFAPI.

gniftygnome commented 1 year ago

Very nice, looks good to me. Thanks!

gniftygnome commented 1 year ago

The lack of locate-ability of Forgotten Fields is my fault. The mod is using Biolith's BiomePlacement.addSubOverworld() which, it turns out, I forgot to test after refactoring the biome parameters code. Terrestria is not yet using this functionality.

There will be a Biolith 0.0.1-alpha.4 release in a few minutes which resolves this oversight. I have found a Forgotten Fields in a test world and it is skulkalicious.