Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Crash] Deserialize dynamic registry by server codec at client side in singleplayer #4423

Open
Phoupraw opened this issue Feb 7, 2025 · 7 comments

Comments

@Phoupraw
Copy link
Contributor

Phoupraw commented Feb 7, 2025

0.114.0+1.21.1

  1. Create a codec using a server only registry, such as loot table.
Codec<Pair<Identifier,RegistryEntry<LootTable>>> SERVER_CODEC = Codec.mapPair(
  Identifier.CODEC.fieldOf("id"),
  LootTable.ENTRY_CODEC.fieldOf("loot_table")
).codec();
  1. Create a codec without server only registry.
Codec<Pair<Identifier, RegistryEntry<LootTable>>> CLIENT_CODEC = Identifier.CODEC
  .fieldOf("id")
  .xmap(
    id -> Pair.of(id, RegistryEntry.of(LootTable.EMPTY)),
    Pair::getFirst
  )
  .codec();
  1. Create a dynamic registry by DynamicRegistries.registerSynced(RegistryKey<? extends Registry<T>> key, Codec<T> dataCodec, Codec<T> networkCodec, SyncOption... options), where the dataCodec is the codec using a server only registry and the networkCodec is the codec without server only registry.
  2. Create a json file of this dynamic registry.
{
  "id": "test",
  "loot_table": {
    "pools": [
      {
        "rolls": 1,
        "entries": [
          {
            "type": "item",
            "name": "coal"
          }
        ]
      }
    ]
  }
}
  1. Start singleplayer and create a new world.
  2. Crash.
java.lang.IllegalStateException: Failed to parse ...
	at knot//net.minecraft.registry.RegistryLoader.loadFromResource(RegistryLoader.java:244)
	at knot//net.minecraft.registry.RegistryLoader$Loader.loadFromResource(RegistryLoader.java:308)
	at knot//net.minecraft.registry.RegistryLoader.method_56514(RegistryLoader.java:114)
	at knot//net.minecraft.registry.RegistryLoader.method_45120(RegistryLoader.java:134)
	at java.base/java.lang.Iterable.forEach(Iterable.java:75)
	at knot//net.minecraft.registry.RegistryLoader.load(RegistryLoader.java:134)
	at knot//net.minecraft.registry.RegistryLoader.mixinextras$bridge$load$34(RegistryLoader.java)
	at knot//net.minecraft.registry.RegistryLoader.wrapOperation$zgg000$fabric-registry-sync-v0$wrapIsServerCall(RegistryLoader.java:1056)
	at knot//net.minecraft.registry.RegistryLoader.mixinextras$bridge$wrapOperation$zgg000$fabric-registry-sync-v0$wrapIsServerCall$37(RegistryLoader.java)
	at knot//net.minecraft.registry.RegistryLoader.wrapOperation$zhc000$fabric-resource-conditions-api-v1$captureRegistries(RegistryLoader.java:1557)
	at knot//net.minecraft.registry.RegistryLoader.loadFromResource(RegistryLoader.java:114)
	at knot//net.minecraft.server.SaveLoading.loadDynamicRegistryManager(SaveLoading.java:78)
	at knot//net.minecraft.server.SaveLoading.withRegistriesLoaded(SaveLoading.java:87)
	at knot//net.minecraft.server.SaveLoading.load(SaveLoading.java:37)
Caused by: java.lang.IllegalStateException: Registry does not exist: ResourceKey[minecraft:root / minecraft:loot_table]
	at knot//com.mojang.serialization.DataResult$Error.getOrThrow(DataResult.java:287)
	at knot//com.mojang.serialization.DataResult.getOrThrow(DataResult.java:81)
	at knot//net.minecraft.registry.RegistryLoader.parseAndAdd(RegistryLoader.java:205)
	at knot//net.minecraft.registry.RegistryLoader.loadFromResource(RegistryLoader.java:242)
	... 36 more
@Juuxel
Copy link
Member

Juuxel commented Feb 7, 2025

This behaviour comes from vanilla code. The "network codec" must be able to also read any data pack files on the client as vanilla has an optimisation where entries from "known data packs" aren't synced at all but are instead read directly on the client. This uses the network codec to avoid reading unnecessary data that the client doesn't need.

"Network codec" is actually a misnomer and the codecs would be more accurately named "server codec" and "client codec". The optimisation and the codec names were discussed on Fabricord just a week ago: https://discord.com/channels/507304429255393322/566276937035546624/1334925452791513230

@Phoupraw
Copy link
Contributor Author

Phoupraw commented Feb 7, 2025

Not related to "network codec". Actually, the game uses dataCodec to deserialize resource in client side, but dataCodec contains server only registry, so it crashes.

(Though I wrote wrong CLIENT_CODEC. I revised it just now.)

@Juuxel
Copy link
Member

Juuxel commented Feb 7, 2025

Ah, this might actually be a different issue since this is the server side registry load (you can see it from SaveLoading in the stacktrace), which would use your server codec and doesn't involve the client at all.

The issue here is that you're trying to refer to the loot table registry in a "normal" dynamic registry. All the loot-related data types are so-called reloadable dynamic registries which are loaded in their own phase which occurs later (and they can be reloaded if data packs change). Using a RegistryEntry<LootTable> makes no sense there as the loot table registry only gets read later.

I think we could add support for custom reloadable registries in a PR, but the crash seems to be the expected behaviour here.

(The misnomer is still real though, even if unrelated. 😛)

Edit: a good fix would be to use RegistryKey<LootTable>, just a plain LootTable or an Either of them, and to fetch the reloadable registry entries later.

@Phoupraw
Copy link
Contributor Author

Phoupraw commented Feb 7, 2025

I use RegistryEntry<LootTable> rather than RegistryKey<LootTable> because I allow both inline loot table and reference loot table. If accessing loot table registry is impossible in normal dynamic registries deserialization phase, what should I do for it?

@Juuxel
Copy link
Member

Juuxel commented Feb 7, 2025

You can use an Either<RegistryKey<LootTable>, LootTable> (Codec.either). The registry key can be resolved into an actual loot table at runtime when the reloadable registries have been loaded.

@Phoupraw
Copy link
Contributor Author

Phoupraw commented Feb 7, 2025

LootTable.CODEC depends on LootPool.CODEC, and the latter depends on LootPoolEntryTypes.CODEC, and the latter depends on LootTableEntry.CODEC, and the latter depends on loot table registry. Though it doesn't matter in most cases.

Is there anyway to defer the deserialization of my dynamic registry?

@Juuxel
Copy link
Member

Juuxel commented Feb 7, 2025

The only way to really defer it in this case is to use a mixin and insert your registry into the loot pipeline (= make it a reloadable registry).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants