Allow Datapack Registry to be configured by RegistryBuilder

[Argon4W]

Modify the RegistryDataLoader$RegistryData and RegistryBuilder to allow datapack registries to be configured by RegistryBuilder.

[neoforged-pr-publishing]

• Publish PR to GitHub Packages

[CLAassistant]

All committers have signed the CLA.

[Matyrobbrt]

Why do you need to configure dp regs with registrybuilder?
I'm not sure this is the best approach, instead ModifyRegistriesEvent should also fire for data pack registries.

[Argon4W]

Why do you need to configure dp regs with registrybuilder? I'm not sure this is the best approach, instead ModifyRegistriesEvent should also fire for data pack registries.

Sorry but I noticed that the doc of the ModifyRegistriesEvent said that it cannot be used to modify datapack registries.

Also, even if this event works, it cannot add default value for the datapack registry cuz the generated registry is MappedRegistry instead of DefaultedMappedRegistry.

[Matyrobbrt]

Ah, so your usecase is adding default values to the registry? I suppose that's fine, but if you add the consumer to the registry data, please make it the last parameter of the record.

[Argon4W]

Ah, so your usecase is adding default values to the registry? I suppose that's fine, but if you add the consumer to the registry data, please make it the last parameter of the record.

Okay, I just added a new commit to make the consumer the last parameter.

[Shadows-of-Fire]

I don't think letting a datapack registry be a DefaultedMappedRegistry is a good idea, since by virtue of being a data registry, data objects may be removed (including the default object) which will invalidate the nonnull contract of DefaultedMappedRegistry#get(ResourceLocation).

Allowing the configuration through registry builder satisfies requirements from #1158, which is (or rather, might be?) useful, but use of a default key is a footgun.

[Argon4W]

I don't think letting a datapack registry be a DefaultedMappedRegistry is a good idea, since by virtue of being a data registry, data objects may be removed (including the default object) which will invalidate the nonnull contract of DefaultedMappedRegistry#get(ResourceLocation).

Allowing the configuration through registry builder satisfies requirements from #1158, which is (or rather, might be?) useful, but use of a default key is a footgun.

Indeed, the default object may be removed. But it also allow datapacks to change a hardcoded default state, making the datapack behavior more flexible. Also, modifying default object isn’t quite common, and the default value is disabled at default, so if you want to add a default value, you should declare it explicitly and take the risk.

[Argon4W]

I don't think letting a datapack registry be a DefaultedMappedRegistry is a good idea, since by virtue of being a data registry, data objects may be removed (including the default object) which will invalidate the nonnull contract of DefaultedMappedRegistry#get(ResourceLocation).

Allowing the configuration through registry builder satisfies requirements from #1158, which is (or rather, might be?) useful, but use of a default key is a footgun.

BTW, adding default value can prevent the usage of unsafe null value or Optional as default value. Though I can use Holder.direct() and RegistryFileCodec/ByteBufCodecs.holder to achieve similar behavior like the default value, it just feel hacky and the default value has no key to represent

[Shadows-of-Fire]

Eh, well, people can use it at their own risk. It isn't really any different than just calling .holderOrThrow on a datapack registry, which people are almost certainly doing.

[neoforged-releases]

🚀 This PR has been released as NeoForge version 21.1.58.