search

FabricMC / fabric

Essential hooks for modding with Fabric.
Apache License 2.0
2.31k stars 404 forks source link

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

Closed alcatrazEscapee closed 2 months ago

alcatrazEscapee commented 3 months ago

Minecraft Version: 1.21 Fabric Loader Version:: 0.15.11 Fabric API Version: 0.100.1+1.21

This was an issue I originally encountered with NeoForge in a multiloader workspace, and reported here: https://github.com/neoforged/NeoForge/issues/1204 The exact same issue is present in Fabric, except using Fabric's biome modification API. The full diagnosis is in the other issue, and it applies almost 1-1 with fabric.

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.

Root Cause

SerializableRegistries.stream() (Yarn) encodes registry elements to be synced, but it first checks the list of known packs vs. the pack that defined the biome, and if they are present, the element's content won't be synced. Assuming Fabric's biome modifications apply similarly to Forge (i.e. applied only on server during startup, and then trusted to sync), this does not sync the content of any modified biomes.

I have written a mixin (Official Mappings) which I can confirm has fixed this in my (Fabric) mod dev workspace, which does solve this issue, although this is a huge hack in "just synchronize all biomes and don't bother checking".

Patbox commented 3 months ago

Actual correct fix would be changing the RegistryEntryInfo that is attached to modified entries to something that either strips pack info or uses some sort of hash (through stripping would be safest option).

lukebemish commented 2 months ago

The fix on the neo end for the equivalent issue is https://github.com/neoforged/NeoForge/pull/1227 -- which is basically exactly what Patbox suggested, removing the KnownPack attached to the entry for any entry that's changed.