Fabric API

Fabric API

108M Downloads

foundInOverworld, foundInNether, foundInEnd helper methods are not safe for modded biome sources

TelepathicGrunt opened this issue ยท 8 comments

commented

All of Fabric team noticed that most modded ores will not spawn in BYG biomes. BYG is using Terrablender. Terrablender uses its own biome source. And Fabric API is using a field that's very specific to the vanilla overworld biome source which is bad for mod compat and is breaking modpacks using Terrablender. Instead, Fabric API should be using the overworld's biome source's possibleBiomes field that they hold always which should have all biomes that can actually spawn in overworld. Or ideally, use the existing minecraft:is_overworld biome tag instead. This change should also be done for the nether and end helpers as well so all three helper methods functions properly regardless of the biome source used for the dimension.

/**
* Returns a biome selector that will match all biomes that would normally spawn in the Overworld,
* assuming Vanilla's default biome source is used.
*/
public static Predicate<BiomeSelectionContext> foundInOverworld() {
return context -> context.canGenerateIn(DimensionOptions.OVERWORLD);
}
/**
* Returns a biome selector that will match all biomes that would normally spawn in the Nether,
* assuming Vanilla's default multi noise biome source with the nether preset is used.
*
* <p>This selector will also match modded biomes that have been added to the nether using {@link NetherBiomes}.
*/
public static Predicate<BiomeSelectionContext> foundInTheNether() {
return context -> context.canGenerateIn(DimensionOptions.NETHER);
}
/**
* Returns a biome selector that will match all biomes that would normally spawn in the End,
* assuming Vanilla's default End biome source is used.
*/
public static Predicate<BiomeSelectionContext> foundInTheEnd() {
return context -> context.canGenerateIn(DimensionOptions.END);
}

These helpers seem to have issues as a somewhat similar issue happened in the past: #1703

commented

The link you posted is from before biome tags were added.
It was discussed there that the method name is somewhat misleading.
It also discusses how the chunk generator / biome source don't exist when the biome modification happens.

It would be a breaking change to use minecraft:is_overworld in that method.
Probably no one would actually complain though. :-)

Maybe the method could be deprecated? With some javadoc saying to use:
BiomeSelectors.tag(BiomeTags.IS_OVERWORLD);

There appear to be some major changes in this area for the next minecraft release: #2632

commented

Well it says specifically "assuming Vanilla's default biome source is used.". Since when these methods were created, it wasn't really possible to do otherwise.

warjort is completely right.

commented

After some more digging the current running theory is that the Javadoc is incorrect, but the impl is correct.

TerraBelender injects its biomes seeming after the biome modifications have been applied here: https://github.com/Glitchfiend/TerraBlender/blob/e8c651c459a304fcd39d29d53d5407e5f5cedbe2/Fabric/src/main/java/terrablender/mixin/MixinMinecraftServer.java#L18-L19

As far as I can tell (using github, can validate it with a debugger in 30 seconds) this is after this: https://github.com/FabricMC/fabric/blob/1.18.2/fabric-biome-api-v1/src/main/java/net/fabricmc/fabric/impl/biome/modification/BiomeSelectionContextImpl.java#L105 Thus any mod using canGenerateIn wont be seeing the TB biomes.

Im happy to be wrong, but fabric seems ok to me. May be nice to figure out if there are any APIs you folks are missing so you dont need to reinvent the wheel?

commented

Hi, sorry to randomly inject, but I'm updating my mods from 22w43a to 22w44a and I wanted to confirm that I am using the correct method to inject my ores into biomes correctly. I was getting a little confused about the discussion.

My code currently:

`
public static void addOres()
{
// Inject into Biomes
BiomeModifications.addFeature(overworldSelector(), GenerationStep.Feature.UNDERGROUND_ORES, RegistryKey.of(Registry.PLACED_FEATURE_KEY, new Identifier(Gobber2.MOD_ID, "ore_lucky_block_overworld")));
BiomeModifications.addFeature(overworldSelector(), GenerationStep.Feature.UNDERGROUND_ORES, RegistryKey.of(Registry.PLACED_FEATURE_KEY, new Identifier(Gobber2.MOD_ID, "ore_gobber_overworld")));
BiomeModifications.addFeature(netherSelector(), GenerationStep.Feature.UNDERGROUND_ORES, RegistryKey.of(Registry.PLACED_FEATURE_KEY, new Identifier(Gobber2.MOD_ID, "ore_lucky_block_nether")));
BiomeModifications.addFeature(netherSelector(), GenerationStep.Feature.UNDERGROUND_ORES, RegistryKey.of(Registry.PLACED_FEATURE_KEY, new Identifier(Gobber2.MOD_ID, "ore_gobber_nether")));
BiomeModifications.addFeature(endSelector(), GenerationStep.Feature.UNDERGROUND_ORES, RegistryKey.of(Registry.PLACED_FEATURE_KEY, new Identifier(Gobber2.MOD_ID, "ore_lucky_block_end")));
BiomeModifications.addFeature(endSelector(), GenerationStep.Feature.UNDERGROUND_ORES, RegistryKey.of(Registry.PLACED_FEATURE_KEY, new Identifier(Gobber2.MOD_ID, "ore_gobber_end")));
}

public static Predicate<BiomeSelectionContext> overworldSelector()
{
    return context -> context.getBiomeRegistryEntry().isIn(BiomeTags.IS_OVERWORLD);
}

public static Predicate<BiomeSelectionContext> netherSelector()
{
    return context -> context.getBiomeRegistryEntry().isIn(BiomeTags.IS_NETHER);
}

public static Predicate<BiomeSelectionContext> endSelector()
{
    return context -> context.getBiomeRegistryEntry().isIn(BiomeTags.IS_END);
}

`

commented

This is really off-topic, please don't.

commented

Well it says specifically "assuming Vanilla's default biome source is used.". Since when these methods were created, it wasn't really possible to do otherwise.

warjort is completely right.

To correct myself here by the way: The javadoc is actually incorrect. The method uses the real biome source used by the currently loading world, not just the Vanilla one.

commented

I fixed MI's ore gen in TerraBlender biomes by using BiomeSelectors.tag(ConventionalBiomeTags.IN_OVERWORLD) instead of BiomeSelectors.foundInOverworld(). So this is a real issue, and the fix is easy, although I can't explain why it works.

commented

I have opened a PR fixing this in TerraBlender here: Glitchfiend/TerraBlender#96