Fix replay recipe sync registry mismatch

[mezz]

This issue was reported to me:

• [Bug]: Flashback replays of servers can't be opened if JEI is installed mezz/JustEnoughItems#4344
• [Other]: whenever i try to run my flashback from a server i get this, mezz/JustEnoughItems#4376

It was also reported to your discord here, but without enough information:

• https://discord.com/channels/1257203701349875753/1506633809716838480/1506633809716838480

I was able to reproduce it on Fabric with JEI installed:

1. Run a dedicated Minecraft 1.21.11 server with a server-only datapack with this pack.mcmeta:

{
  "pack": {
    "description": "Registry mismatch reproduction",
    "min_format": [94, 1],
    "max_format": 94
  }
}

2. Add data/minecraft/trim_pattern/bolt.json:

{
  "asset_id": "minecraft:bolt",
  "decal": true,
  "description": {
    "text": "Server-only Bolt Armor Trim"
  }
}

3. Do not install this datapack in the client’s local environment. This creates the required difference:
   Server: minecraft:bolt → literal description, decal=true
   Client: minecraft:bolt → vanilla translated description, decal=false
4. Run the client with the broken Flashback build and JEI 27.4.0.24.
5. Connect to the dedicated server and start a Flashback recording.
6. Wait for the player to join fully so the initial snapshot is written.
7. Stop the recording and disconnect from the server.
8. From the client’s replay browser, load the newly created replay.
9. The broken handler resolves the recorded trim-pattern tags against the local vanilla registry. During playback, the vanilla bolt smithing-trim recipe retains the local minecraft:bolt holder.
10. JEI registers the vanilla recipe serializers with Fabric recipe synchronization. When Fabric encodes that recipe against the recorded server registry, the local holder has no ID in that registry, and playback disconnects.

[Moulberry]

So if I'm understanding correctly, the problem is that Minecraft's registry loading code is prioritizing getting the tags from the old pendingTags map instead of getting the fresh tags from the registry that we just loaded?

And the solution is to avoid adding it to pendingTags, to ensure that it can only load the fresh tags?

[mezz]

Yes, sort of, with one nuance: both paths use the same recorded tags. The problem is which of the two registry instances resolves them. To keep the terms clear, I'll call them the local registry that playback starts with and the recorded registry loaded from the replay.

Previously,

1. Flashback created a PendingTags entry against the local registry.
2. After installing the recorded registry, ReplayServer reloaded server datapack resources, including recipes.
3. Minecraft used the PendingTags entry to resolve registry references while deserializing those recipes, so they retained holders from the local registry.
4. Fabric's recipe sync later tried to encode the recipes against the recorded registry and failed.

The fix defers resolving the tags until Flashback knows which registry will remain active. If the recorded registry replaces the local registry, the tags are resolved against the recorded registry. If their elements match and the local registry remains active, the tags are resolved against the local registry instead.

In short, tags must be resolved against whichever registry remains active; otherwise recipes can retain holders that recipe sync cannot encode.

Sorry it was really complicated to figure out so I'm still struggling a bit to explain it clearly, I hope that makes sense!

[Moulberry]

Gotcha, so the tags in the registries are fine it's just the reloadable resources.

I'm confused on one thing though: you said

If the recorded registry replaces the local registry, the tags are resolved against the recorded registry

but the code doesn't seem to do that?

You have if (!synchronizeRegistries) {, so if the registries changed, then the pendingTags list won't have the entry added for the registry

[mezz]

There are two paths:

1. When the recorded registry is installed, its tag payload is included in NetworkedRegistryData, like before:

this.pendingRegistryMap.put(
    resourceKey,
    new RegistryDataLoader.NetworkedRegistryData(entry.elements(), networkPayload)
);

RegistryDataLoader.load resolves and installs that payload while constructing the recorded registry, so no PendingTags entry is needed for that registry. The important change is that we now return early here. Previously, the same payload was also resolved against the local registry and added to pendingTags, which overrode the recorded lookup during resource reload and caused the crash.

2. When the local registry remains active, the recorded registry and its tags are discarded. The saved payload must be resolved and applied against the local registry. You were doing that before, but also in cases where local is not active (causing a crash), so that step has been moved later to when we know local is active and it's safe to do it.

[Moulberry]

RegistryDataLoader.load resolves and installs that payload while constructing the recorded registry, so no PendingTags entry is needed for that registry.

Right, I understand that the tags get put in the registry correctly, but you said the problem was with when the server's resources are reloaded. i.e. inside MinecraftServer#reloadResources, there's TagLoader#loadTagsForExistingRegistries which we override in com.moulberry.flashback.mixin.playback.MixinMinecraftServer to return the pendingTags we loaded earlier.

As you say, RegistryDataLoader.load will install the payload, but it won't add to the pendingTags list, and it's the pendingTags list which we replace TagLoader#loadTagsForExistingRegistries with.

For additional context, the reason why we need to replace TagLoader#loadTagsForExistingRegistries is because TagLoader#loadTagsForExistingRegistries will try to load the tags directly from the integrated server's empty datapacks folder

So, in summary, it seems to me that any custom tag that was recorded won't be used inside MinecraftServer#reloadResources because it never gets added to the pendingTags list.

Consequently, I think this PR will make MinecraftServer#reloadResources use Client: minecraft:bolt → vanilla translated description, decal=false instead of Server: minecraft:bolt → literal description, decal=true

As long as I'm understanding everything correctly here, this should be solveable by removing if (!synchronizeRegistries) {, which will make pendingTags correctly contain references to the newly loaded registry

[mezz]

pendingTags is a sparse override list, not an exhaustive list. TagLoader.buildUpdatedLookups uses PendingTags.lookup() when an entry exists and otherwise uses the active registry’s own lookup, which is already in the recorded registry at reload time.

When resource reload starts, the recorded registry has already been installed and already contains its recorded tags. Because there is no PendingTags entry for that registry, Minecraft uses the recorded registry’s existing lookup, which has the correct data.

Previously, Flashback also added a Registry.PendingTags object for this registry to the pendingTags list after resolving its payload against the local registry. During resource reload, Minecraft selected that object’s lookup instead of the active recorded registry’s lookup, causing the crash.

[Moulberry]

Ah okay. If it's just an override list, can't we just make TagLoader#loadTagsForExistingRegistries an empty list so it always loads from the current registry? That should simplify things a lot, because then we shouldn't need to pass pendingTags to the server at all

[mezz]

That... seems plausible huh. Give me a minute to try it out!

[mezz]

Ok sorry that took a while to test each step, but that led me down a path where I could simplify this to just one line lol

[mezz]

Now that I have a precise fix I'll see if I can clean up a little

[mezz]

Ok I think this is in a good state now.

[Moulberry]

Thank you very much for your help with this. I'll try to merge this and get a new release as soon as I can

[mezz]

Thanks!