architectury-api icon indicating copy to clipboard operation
architectury-api copied to clipboard

RegistrySupplier crashing on save with Status Effects

Open fzzyhmstrs opened this issue 1 year ago • 3 comments

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.

fzzyhmstrs avatar Nov 16 '24 15:11 fzzyhmstrs

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());
}

fzzyhmstrs avatar Nov 16 '24 16:11 fzzyhmstrs

Same issue, RegistrySupplier is another implementation of Holder<T>, but in Minecraft, Holder.Reference is excepted.

qyl27 avatar Dec 19 '24 14:12 qyl27

Can confirm

tsuyumori avatar May 08 '25 16:05 tsuyumori