Biome modifiers don't sync modifications to client, climate/effect modifications are unworkable.

[alcatrazEscapee]

Minecraft Version: 1.21
NeoForge Version: 21.0.37-beta

All testing happened on singleplayer, integrated server.

Overview

I have experienced this in a neo mod dev workspace, which I will try and convince you of via the following debugger screenshots:

Biome as visible to the server - climate settings are modified, modified biome info is present:

Biome as visible to the client - everything is defaulted:

Why is this a problem?

Modifications that affect structure or feature generation are perfectly fine to be server-only, since they aren't synced to the client typically. However climate and special effects are mostly/only used on client - it's effectively impossible to use a biome modifier to change the rendering of rain in a biome for example, or the water/fog colors.

Diagnosis

Using the same debugger from above, I can confirm that the coremod is applied to the network codec fields, and so would in theory try and sync the biome if timed properly:

This is reading the modified biome info, not the original. I have tried to breakpoint the flow of biome modifier application and synchronization:

1. Biome modifiers are applied from ServerLifecycleHooks.handleServerAboutToStart()
   • At this point, biome modifications are visible in ServerLifecycleHooks.getCurrentServer().registryAccess()
2. Biomes (along with all registries) are are grabbed to be synced in SynchronizeRegistriesTask.sendRegistries()
   • this.registries also has modifications visible at this time
   • This calls into RegistrySynchronization.packRegistry to pack individual registries, which we encounter a problem. This appears to check that is the biome loaded from Set<KnownPack>. I do not fully understand the purpose of this code, but what I think happens here, is vanilla realizes the biome in server (i.e. minecraft:badlands) is loaded from the "minecraft:core:1.21" pack, which is identical on client. and so it doesn't sync it, because it is (assumed to be) unmodified.

3. ClientConfigurationPacketListenerImpl.handleRegistryData() is called with the packed data
   • Note that the data received here is empty data - nothing has been sent:

4. RegistryDataCollector.collectGameRegistries() receives empty data, sets up registries, and the modifications are effectively voided at this point

Fix?

I have no idea what this network registry sync code is doing but, probably, add a special case for RegistrySynchronization.packRegistry() to always sync biomes that have a biome modifier.

I can confirm, at least in my mod dev environment, that this mixin fixes the issue. However, this is a completely brute-force approach called "just sync all biomes" forcibly.

[lukebemish]

Ah yeah, this has to do with how the new KnownPack keys work -- basically, the server will avoid syncing info to the client if the client already has it, so it doesn't sync the biome (fully) if the client already has that biome and the pack it came from. The correct solution is probably to yeet the KnownPack entry from where that's stored for any biome that's had a modifier applied.

[sciwhiz12]

To make @lukebemish's idea more concrete: perhaps we can add an interface, let's call it SyncAware, which is present on registrable types that may be possibly modified on the server-side that would differ in value from the client. That interface would then be called in RegistrySynchronization, either in lieu of or in addition to the KnownPack check, to determine whether to synchronize the entry to the client or not.

For NeoForge, we would then implement that interface onto Biome and Structure (since both have modifiers attached to them) such that it would force a sync if the modifiable info field they contain (respectively, modifiableBiomeInfo and modifiableStructureInfo) is present.

What do you think?

[lukebemish]

Uh... I'm not sold on that approach. It seems overengineered. Fixing this -- or any equivalent issue that arises -- is as simple as stripping the KnownPack data if we modify datapack registry entries, from those entries. We don't need a whole new system in parallel to the vanilla one...

[sciwhiz12]

A bit of a shotgun approach I think (and probably why I didn't think it up), but it would likely work as well (if possibly doing some needless synchronization for unmodified entries).

Do you have the time to look into it, or shall I?

[lukebemish]

I wouldn't remove that from every entry, I'd only remove it from entries targeted by modifiers. I'll probably have some time to give it a poke in the next day or so.