PneumaticCraft: Repressurized

PneumaticCraft: Repressurized

50M Downloads

[1.16.3] ConfiguredFeatures in pnc-repressurized are not registered

TelepathicGrunt opened this issue ยท 6 comments

commented

Hello! I was test running my mod called Blame to see how well it works on modpacks and it seemed to have found that PneumaticCraft does not register their oil lakes ConfiguredFeatures. This can be an issue for mod compatibility as under certain conditions, unregistered ConfiguredFeatures can basically prevent other mod's registered ConfiguredFeatures 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:
image

Specifically, when you call .withConfiguration on a Feature, you create a ConfiguredFeature. This is what should be registered to the WorldgenRegisties at mod init (you can do it in your mod constructor so you have your config ready too)

oilLake = Feature.LAKE
.withConfiguration(new BlockStateFeatureConfig(ModBlocks.OIL.get().getDefaultState()))
.withPlacement(ModDecorators.OIL_LAKE.get().configure(new ChanceConfig(PNCConfig.Common.General.oilGenerationChance)));

Here's an example from my mod RepurposedStructures of me registering all my ConfiguredFeatures.
https://github.com/TelepathicGrunt/RepurposedStructures/blob/a4e3365e3867b8510952ebf658c415de6e412927/src/main/java/com/telepathicgrunt/repurposedstructures/RSConfiguredFeatures.java#L184-L185

I hope this helps!

commented

I came up with this:

public class EventHandlerWorldGen {
    public static ConfiguredFeature<?,?> OIL_LAKES;

    public static void registerConfiguredFeatures(RegistryEvent.Register<Feature<?>> event) {
        Registry<ConfiguredFeature<?, ?>> registry = WorldGenRegistries.CONFIGURED_FEATURE;

        OIL_LAKES = Feature.LAKE
                .withConfiguration(new BlockStateFeatureConfig(ModBlocks.OIL.get().getDefaultState()))
                .withPlacement(ModDecorators.OIL_LAKE.get().configure(new ChanceConfig(ConfigHelper.getOilLakeChance())));
        Registry.register(registry, RL("oil_lakes"), OIL_LAKES);
    }

    public static void onBiomeLoading(BiomeLoadingEvent event) {
        if (!PNCConfig.Common.General.oilWorldGenBlacklist.contains(event.getName())) {
            event.getGeneration().withFeature(GenerationStage.Decoration.LAKES, OIL_LAKES);
        }
    }
}

registered with (at construction time) :

modBus.addGenericListener(Feature.class, EventPriority.LOW, EventHandlerWorldGen::registerConfiguredFeatures);
forgeBus.addListener(EventPriority.HIGH, EventHandlerWorldGen::onBiomeLoading);

which appears to work. Looks good?

Note that I can't statically init the ConfiguredFeature, because of the reference to ModBlocks.OIL.get(), which isn't available until my blocks are registered. Hence I init it just before the ConfiguredFeature is about to be added to the vanilla registry.

Also, I do my registration via DeferredRegister in general so ModDecorators.OIL_LAKE gets implicitly registered that way - the LOW priority on the registerConfiguredFeatures() method should ensure it runs after that decorator is registered.

commented

Yeah it looks good! If it works on server and client, then you're good to go. Though you can register the ConfiguredFeature at any time before the world is clicked on to enter. Doesn't have to be in the registry events. So if the registry events are still giving you issue with the deferred and all, you can just call registerConfiguredFeatures directly in the PneumaticCraftRepressurized() constructor instead.

commented

That would be too early, since it depends on both a mod-registered block and mod-registered decorator - neither of which are going to be present at construction time. The registry event seems to make the most sense to me, since I know the things it depends on should be present by then.

I've tested this successfully in a single-player world, but I'll also test on dedicated server before comitting.

commented

Oh sorry. I forgot the constructor is before registry events. I was thinking of the FMLCommonSetupEvent. I think I need a break lol.

commented

Thanks, this is useful. We actually had a bit of a conversation about this on the Forge discord a couple of weeks back without any definite consensus on this registration so it's good to have some more clarification.

See https://discordapp.com/channels/313125603924639766/725850371834118214/759043292314468382 and https://discordapp.com/channels/313125603924639766/725850371834118214/759051776811859998 for reference.

commented

Fixed in 2.4.2 release