Biomes O' Plenty

Biomes O' Plenty

Mods
151M Downloads

[1.16.1] Possible bug with the reflection used in ModCompatibility.copyModBiomeWeights() method

TelepathicGrunt opened this issue ยท 0 comments

commented

I came across this reflection that BoP does into Forge's BiomeManager and I noticed that it makes an assumption that may cause strange behaviors later.

Specifically, it seems it calls BiomeManager.setupBiomes() to try and get all vanilla biomes in order to filter them out of BiomeManager.getBiomes(type) later. Except, Forge moved the desert biomes into BiomeLayer so the desert index in setupBiomes's returned lists is actually empty

The problematic line of code:

BiomesOPlenty/src/main/java/biomesoplenty/init/ModCompatibility.java

Lines 28 to 63 in f910136

try
{
// An array containing lists of default biome entries for only standard BiomeTypes
List<BiomeManager.BiomeEntry>[] vanillaBiomes = (List<BiomeManager.BiomeEntry>[]) ObfuscationReflectionHelper.findMethod(BiomeManager.class, "setupBiomes").invoke(null);
for (BiomeManager.BiomeType type : BiomeManager.BiomeType.values())
{
// Creates a mutable version of the current biome type's biome array and wraps entries to support .equals()
List<WrappedBiomeEntry> entries = Lists.newArrayList();
List<WrappedBiomeEntry> vanillaEntries = Lists.newArrayList();
for (BiomeManager.BiomeEntry entry : BiomeManager.getBiomes(type))
{
entries.add(new WrappedBiomeEntry(entry));
}
for (BiomeManager.BiomeEntry entry : vanillaBiomes[type.ordinal()])
{
vanillaEntries.add(new WrappedBiomeEntry(entry));
}
//Remove all default biomes from the entries list
entries.removeAll(vanillaEntries);
for (WrappedBiomeEntry wrappedEntry : entries)
{
remapBiomeToBoP(wrappedEntry.biomeEntry.biome, type, wrappedEntry.biomeEntry.weight);
}
}
}
catch (Exception e)
{
BiomesOPlenty.logger.error("An error has occurred whilst copying mod biomes");
e.printStackTrace();
return;
}

Personally, I think this can be cleaned up and that the reflection is really not needed. What I would do instead to get all modded BiomeEntry is the following (warning, a bit pesudocode here):

        for (BiomeManager.BiomeType type  : BiomeManager.BiomeType.values()) {
            // Removes vanilla entries by checking if the biome namespace is not "minecraft"
            // If a mod makes their biome with minecraft namespace, they need a bug report as that's a big no-no. 
            List<BiomeManager.BiomeEntry> moddedBiomesInType = BiomeManager.getBiomes(type).stream()
                    .filter(biomeEntry -> !biomeEntry.biome.getRegistryName().getNamespace().equals("minecraft"))
                    .collect(Collectors.toList());
            
            // calls remapBiomeToBoP on each modded biome and its weight. 
            moddedBiomesInType.stream().forEach(biomeEntry -> remapBiomeToBoP(biomeEntry.biome, type, biomeEntry.weight));
        }

I ignored the WrappedBiomeEntry as it doesn't seem like it is needed at all since only the biome and the weight is used. To check if a biome is modded, just the namespace needs to be checked. Any mod that tried to add a modded biome with the 'minecraft' namespace needs to get an issue report saying to change that. But really, I have not found anyone doing that in a loooong time so I do not believe this practice is widespread enough to really warrant going out of our way to support them.

If you are trying to support mods adding additional vanilla biome entries with different weights, then that does get more tricky as then, you have to get the desert biome entries too. Personally, if you are looking to do this, then it would probably be easier and safer to just define your own list of all vanilla biome entries with the vanilla weight and do something like a if( !hardcodedVanillaBiomeEntryList.contains(biomeManagerBiomeEntry) ) remapBiomeToBoP(biomeManagerBiomeEntry.biome, type, biomeManagerBiomeEntry.weight); which would not need any reflection and you know the vanilla biome entries are untouched.

I hope this helps!