[FAPI 0.68.0] `FabricDataGenHelper.createRegistryWrapper` changes bootstrap order.
quiqueck opened this issue ยท 6 comments
I am trying to use the DataGen to create all relevant files for my Biomes and Features by adding custom bootstrap methods to the buildRegistry
entry point:
@Override
public void buildRegistry(RegistrySetBuilder registryBuilder) {
registryBuilder.add(Registries.CONFIGURED_FEATURE, TestConfiguredFeatures::bootstrap);
registryBuilder.add(Registries.PLACED_FEATURE, TestPlacedFeatures::bootstrap);
registryBuilder.add(Registries.BIOME, TestBiomes::bootstrap);
registryBuilder.add(Registries.NOISE_SETTINGS, NoiseDatagen::bootstrap);
}
The PLACED_FEATURE-Bootstrap relies on objects created in the CONFIGURED_FEATURE-bootstrap, and BIOMEs rely on the PLACED_FEATUREs.
If the bootstrap for those Biomes is called before the features are initialised, there may be odd behaviour.
Unfortunately, FabricDataGenHelper.createRegistryWrapper
does not run toe bootstrap sequence in the same order as BuiltinRegistries.REGISTRY_BUILDER
. This is due to the use of an unordered value-set of a HasMap that will populate the RegistryBuilder
:
I think the new RegistryBuilder
should retain the original order that is used in BuiltinRegistries.REGISTRY_BUILDER
for all vanilla Registries.
@quiqueck Is this issue resolved?
If figured it is more of a convenience issue. I was trying to move older code directly into the DataGen phase. That code did rely on having bound RegistryEntry
s which were causing the issue.
Switching the code to RegistryEntry.Reference
s when bootstraping the features/biomes resolved the order dependency. I think it is still an issue, but I did not want to bother you with a convenience change. :)
@quiqueck I think the change is still reasonable. Just for reference (no pun intended) can you show the old and new code? Might include in patch note.
I believe this is working as intenteded using the RegistryEntry.Reference
is the expected way of doing this. If you do have a use case where this doesnt work for please let me know.
@quiqueck I think the change is still reasonable. Just for reference (no pun intended) can you show the old and new code? Might include in patch note.
Not sure if that would help, as it is very specific code :)
But In my case, I have created my own custom RegistryEntry class, that works like a RegistryEntry.Reference
but allows us to store both a key and a value (References can either hold a key (when intrusive) or a value (when used as a forward reference). Our code did both building the feature and adding it to the registry at the same time
This way we can create ConfiguredFeature
s that were not added to the registry yet (with all the needed Info we need in later processing).
So, in the original code for 1.19.2 we had this:
public static RegistryEntry<ConfiguredFeature<?, ?>> register(
Identifier id,
ConfiguredFeature<?, ?> cFeature
) {
return BuiltinRegistries.add(
BuiltinRegistries.CONFIGURED_FEATURE,
id,
cFeature
);
}
This returns a bound RegistryEntry
, where we have access to the configured Feature as a value (which we use for convince lateran in our bootstrap code).
For the new version we have two ways of obtaining the RegistryEntry
. If the configured features registry gets created before the one for placed features, we can simply use this to obtain a bound RegistryEntry (the same way as before):
public static RegistryEntry<ConfiguredFeature<?, ?>> register(
Registerable<ConfiguredFeature<?, ?>> ctx,
Identifier id,
ConfiguredFeature<?, ?> cFeature
) {
RegistryKey<ConfiguredFeature<?, ?>> key = RegistryKey.of(RegistryKeys.CONFIGURED_FEATURE, id);
return ctx.register(key, cFeature);
}
When the placed features a bootstrapped before the configured features, we create a custom Reference holder, that will store the ConfiguredFeature and the reference key like this:
public static RegistryEntry<ConfiguredFeature<?, ?>> getReference(
Identifier id,
ConfiguredFeature<?, ?> cFeature
) {
return FullReferenceHolder.create(
Registries.CONFIGURED_FEATURE,
RegistryKey.of(RegistryKeys.CONFIGURED_FEATURE, id),
cFeature
);
}
FullReferenceHolder
is a custom class that behaves mostly the same as RegistryEntry.Reference
, but (as I said above) it can be created with both a key and a value. If you do not need the value in the resulting RegistryEntry, you can simply create a valid Reference using:
public static RegistryEntry<ConfiguredFeature<?, ?>> getReference(
Registerable<?> ctx,
Identifier id
) {
return ctx.getRegistryLookup(RegistryKeys.CONFIGURED_FEATURE)
.getOrThrow(RegistryKey.of(RegistryKeys.CONFIGURED_FEATURE, id));
}
The Registerable
in that case does not need to be the one for the CONFIGURED_FEATURE
-Registry. The returned RegistryEntry
can be unbound (have a null-value) depending on the order the bootstrap is executed.
Maybe I should also show how we use those Methods This is a simplified Version of what is actually going on in our Code:
public class MyConfiguredFeatures {
public static RegistryEntry<ConfiguredFeature<?, ?>> FEATURE_A
= getReference(someID, createConfiguredFeatureA() );
public static void bootstrap(Registerable <ConfiguredFeature<?, ?>> registerable) {
FEATURE_A = register(
registerable,
FEATURE_A.unwrapKey().orElseThrow().location(),
holder.value()
);
}
}
So when bootstrapping PlacedFeature
, FEATURE_A
would wither be an unbound Reference
or a bound RegistryEntry
. We can use both to create the PlacedFeature
s.
Whenever the bootstrap process of ConfiguredFeature
s runs, it will take the information that is already stored in the RegistryEntry
(remeber our Version of the Reference holds both the key and the value) to register those with the Registry.