Architectury API (Fabric/Forge/NeoForge)

Architectury API (Fabric/Forge/NeoForge)

Mods
170M Downloads

RegistrySupplier crashing on save with Status Effects

fzzyhmstrs opened this issue ยท 1 comments

commented

When exiting world or closing game while a player has an active modded status effect registered via Arch API, the game crashes with the following error.
MC: 1.21.1
Arch: 13.0.8
Env: Intellij Fabric runClient

java.lang.IllegalStateException: Unregistered holder in ResourceKey[minecraft:root / minecraft:mob_effect]: minecraft:mob_effect@simplyswords:ribbonwrath
	at com.mojang.serialization.DataResult$Error.getOrThrow(DataResult.java:287)
	at com.mojang.serialization.DataResult.getOrThrow(DataResult.java:81)
	at net.minecraft.entity.effect.StatusEffectInstance.writeNbt(StatusEffectInstance.java:332)
	at net.minecraft.entity.LivingEntity.writeCustomDataToNbt(LivingEntity.java:728)
	at net.minecraft.entity.player.PlayerEntity.writeCustomDataToNbt(PlayerEntity.java:838)
	at net.minecraft.server.network.ServerPlayerEntity.writeCustomDataToNbt(ServerPlayerEntity.java:392)
	at net.minecraft.entity.Entity.writeNbt(Entity.java:2281)
	at net.minecraft.server.integrated.IntegratedPlayerManager.savePlayerData(IntegratedPlayerManager.java:29)
	at net.minecraft.server.PlayerManager.saveAllPlayerData(PlayerManager.java:643)
	at net.minecraft.server.MinecraftServer.saveAll(MinecraftServer.java:610)
	at net.minecraft.server.integrated.IntegratedServer.saveAll(IntegratedServer.java:302)
	at net.minecraft.server.integrated.IntegratedServer.tick(IntegratedServer.java:98)
	at net.minecraft.server.MinecraftServer.runServer(MinecraftServer.java:747)
	at net.minecraft.server.MinecraftServer.method_29739(MinecraftServer.java:288)
	at java.base/java.lang.Thread.run(Thread.java:1583)

looking at the writeNbt bit of the stack, it becomes apparent that the MC registry entry codec "validates" by doing an instanceof check.
writeNbt -> CODEC -> StatusEffect.ENTRY_CODEC -> getEntryCodec() -> validateReference

private DataResult<RegistryEntry.Reference<T>> validateReference(RegistryEntry<T> entry) {
    return entry instanceof RegistryEntry.Reference<T> reference
	? DataResult.success(reference)
	: DataResult.error(() -> "Unregistered holder in " + this.getKey() + ": " + entry);
}

RegistrySupplier is not an instance of Reference, so this fails and the game crashes from getOrThrow(). Neoforged seems to get around this issue by holding the underlying Reference inside the DeferredHolder, and then passing that delegate into the validation instead of itself. Arch doesn't do this (or anything to avoid this hard instanceof check). I've only tested in fabric, but I would guess this problem would also be in Arch neoforge.

commented

Confirmed that the RegistrySupplier is causing the issues; when I introduce a workaround that provides the Reference instead of the Supplier, all works as intended.

public static RegistryEntry<StatusEffect> getReference(RegistrySupplier<StatusEffect> input) {
    return EFFECT.getRegistrar().getHolder(input.getId());
}