Dismantle DynamicRegistries#getDynamicRegistries into two more specific methods of DynamicRegistries

[FirstMegaGame4]

Let's give some context:

I was dealing with FabricDynamicRegistryProvider in order to generate LevelStem json file objects. But the provider failed to retrieve them. That's something that logically should not happen, and modders are often making mixins to the FabricDynamicRegistryProvider class in order to allow it to load the LevelStem registry (which is a bad practice).

After trying to understand why LevelStem entries were not retrieved, it seemed that it's because DynamicRegistries#getDynamicRegistries was specifically not giving the minecraft:dimension registry (registry for LevelStem objects).

I felt wrong to me, as DynamicRegistries#getDynamicRegistries should, from my point of view, return the data of ALL dynamic registries (if we define "dynamic registries" as registries loaded from data packs, which is what I am assuming).

After looking at the implementation, it seemed that getDynamicRegistries was pulling the registries from RegistryDataLoader.WORLDGEN_REGISTRIES. It excludes the minecraft:dimension registry, as this one is part of RegistryDataLoader.DIMENSION_REGISTRIES.

After looking a bit further, it seemed that WORLDGEN_REGISTRIES and DIMENSION_REGISTRIES do not have the same use cases in the Minecraft code.

- Firstly, DIMENSION_REGISTRIES are not data generated in vanilla. You can't find the dimension data directory in the game's jar.
- Secondly, only WORLDGEN_REGISTRIES are being cloned in RegistryDataLoader#createLookup.
- Thirdly, WORLDGEN_REGISTRIES are being loaded before DIMENSION_REGISTRIES in WorldLoader#load.

So, these behaviors needs to be respected. Current registration of custom/fabric dynamic registries involve putting them in the same situation than the ones from WORLDGEN_REGISTRIES.

As such, to allow FabricDynamicRegistryProvider & other use cases to truly get every dynamic registries from DynamicRegistries#getDynamicRegistries, this PR aims to allow this behavior, and to change the implementation of the registry module to only patch RegistryDataLoader and WorldLoader with registries acting like WORLDGEN_REGISTRIES, and not what should be "all dynamic registries".

Other few questions:
- Should Custom/Fabric Dynamic Registries be able to choose between the behavior of WORLDGEN_REGISTRIES and DIMENSION_REGISTRIES?
- Behavior of WORLDGEN_REGISTRIES and DIMENSION_REGISTRIES involves a sort of "load order", should Fabric API provide a way for Custom/Fabric Dynamic Registries to set a "load order"/"load steps" between different registries?

Check this comment.

Fixes #3838.

[modmuss50]

Looks like some of the datagen is having issues (see the check_resources job):

Caused by: java.lang.NullPointerException: No cloner for minecraft:dimension
	at knot//net.minecraft.core.RegistrySetBuilder.createLazyFullPatchedRegistries(RegistrySetBuilder.java:367)
	at knot//net.minecraft.core.RegistrySetBuilder.lambda$createLazyFullPatchedRegistries$0(RegistrySetBuilder.java:356)

Would it be possible to expand the tests somewhere to cover your usecase to ensure it doesnt regress in the future.

[FirstMegaGame4]

After looking a bit more, calling dynamic registries pulled from RegistryDataLoader.WOLRDGEN_REGISTRIES, WORLDGEN_LIKE_REGISTRIES, is misleading.

Because a better name for them would be BOOTSTRAPPING_REGISTRIES: minecraft:dimension does not bootstrap by itself, while others do.

That's what was causing the data generation crash, since FabricDataGenHelper would pull all dynamic registries in order to load them, but would fail since minecraft:dimension does not bootstrap by itself.

Thus, I had another idea: since my initial idea of having DynamicRegistries#getDynamicRegistries return all of them remains unchanged, I think it should also provide a list of bootstrapping ones, if any advanced system needs to load bootstrapping registries (just like fabric-datagen does). As such, I also have added DynamicRegistries#getBootstrappingRegistries, which would act just like the previous behavior of DynamicRegistries#getDynamicRegistries.

Does that seem right?

[maityyy]

First of all, fixes #3838

I understand that the separation between Dynamic Registries and Dimension Registries might seem strange and annoying, but this is a feature of the game, not a problem of FAPI. What I mean is, there's no need to significantly tweak the original game behavior, all we need is:

1. Better documentation (I don't use the Fabric Wiki, so I might be wrong)
2. Mention this game behavior in the fabric-datagen module's README.md to avoid misunderstandings from modders in the future
3. Perhaps a separate FabricDimensionProvider based on FabricCodecDataProvider. I've always used FabricCodecDataProvider and don't see any limitations. See my Generating "dimension" files is not possible. #3838 (comment)

FabricCodecDataProvider is much simpler solution and does not create any technical burden for FAPI

[FirstMegaGame4]

I agree that there is no need for further tweaking the original game behavior about the Level Stem registry. That's not a goal of the pull request here, it does not want nor does that.

I agree about the documentation thing, specifying the difference between world registries and dimension registries is a need. (They do load after, they do not bootstrap by themselves, they are not patched.)

Here, the pull request "only" changes the specification of DynamicRegistries#getDynamicRegistries to consider all of them, and adds DynamicRegistries#getBootstrappingRegistries for previous use cases.

Which silently solves the issue of the FabricDynamicRegistryProvider.

[maityyy]

After discussing this on Discord, we agreed that this PR cleans up the code much more than it seemed at first glance, WorldPreset requires a LevelStem object directly, which means it has to be created twice. Well, there's no problem moving new LevelStem into a static method for FabricCodecDataProvider solution and for BootstrapContext<WorldPreset>, but the PR is fairly straightforward, and the only requirement is not to change the behavior of existing methods.

[FirstMegaGame4]

By such, this PR will now put back the original behavior of DynamicRegistries#getDynamicRegistries, and deprecate it in order to have a better named method for returning all "registries loaded from datapacks".

[FirstMegaGame4]

Let's summarize what this PR does currently:

• Adds DynamicRegistries#getWorldRegistries, which allows getting every vanilla registry that you can fill from datapacks.
• Adds DynamicRegistries#getBootstrappingRegistries, which allows getting every vanilla registry that is bootstrapped by the vanilla game. Those are all being patched, and loaded before the minecraft:dimension registry.
• Deprecates DynamicRegistries#getDynamicRegistries in favor of the two above. To match the previous behavior, it simply redirects to DynamicRegistries#getBootstrappingRegistries.

Let's then enumerate the internal changes for those:

• WorldLoaderMixin, RegistryPatchGeneratorMixin & FabricDataGenHelper are making use of DynamicRegistries#getBootstrappingRegistries.
• FabricDynamicRegistryProvider makes use of DynamicRegistries#getWorldRegistries, to allow modders registering their level stem objects from it, and be able to share their holders across multiple registry bootstraps.

Now, the last thing I'm wondering, is if FabricDynamicRegistryProvider should provide blank holders for those three:

• ResourceKey[minecraft:dimension / minecraft:overworld]
• ResourceKey[minecraft:dimension / minecraft:the_nether]
• ResourceKey[minecraft:dimension / minecraft:the_end]

After all, it is a tool for modders, they might (why, idk) be using those.

Edit: I suppose they can just use FabricDynamicRegistryProvider$Entries#ref with the vanilla keys avalaible in LevelStem, I'm not sure if it requires the holders to be present in the registry?

[maityyy]

Now, the last thing I'm wondering, is if FabricDynamicRegistryProvider should provide blank holders

The only right solution would be to write the code for Mojang, which is a bad idea for many reasons. We shouldn't provide empty holders either, that's misleading.

[maityyy]

The code looks good. But I'm still not sure about the idea itself, since we can't provide a Holder for vanilla entries (overworld, the_nether, the_end). In any case, NeoForge also allows you to generate a LevelStem using RegistrySetBuilder, so we can allow that too.