Minecraft Version

1.21.x

Version

1.114.0

Details

There is a conflict that is occurring with an upcoming Cobblemon release (1.6 on mc version 1.21.1) that causes block updates to stop under certain conditions. The easiest way to start having issues is by leaving and rejoining a world. Based on the stack trace my assumption is the Cobblemon and CC mixins by the ItemStackComponetizationFixMixin files (both have the same name) conflict in a weird way.

I am one of the devs for Cobblemon, however mixins are not my strong suit and so it's possible this is an issue on Cobblemon's side. If that is the case please let me know and I will look into it.

Thank you for your time.

Cobblemon Mixin:
https://gitlab.com/cable-mc/cobblemon/-/blob/main/common/src/main/java/com/cobblemon/mod/common/mixin/ItemStackComponentizationFixMixin.java?ref_type=heads

Cobblemon jar I used:
https://gitlab.com/cable-mc/cobblemon/-/jobs/8279005115/artifacts/raw/Cobblemon-fabric-1.6.0b8390+1.21.1-HEAD-c75c5e4.jar

Log Snippet:

[21:11:17] [Server thread/ERROR] (Minecraft) Failed to read chunk [17, 1]
 java.util.concurrent.CompletionException: java.lang.IllegalArgumentException: Data fixer not registered for: computercraft:turtle_normal in block_entity
	at java.base/java.util.concurrent.CompletableFuture.encodeThrowable(CompletableFuture.java:315) ~[?:?]
	at java.base/java.util.concurrent.CompletableFuture.completeThrowable(CompletableFuture.java:320) ~[?:?]
	at java.base/java.util.concurrent.CompletableFuture$UniApply.tryFire(CompletableFuture.java:649) ~[?:?]
	at java.base/java.util.concurrent.CompletableFuture$Completion.run(CompletableFuture.java:482) ~[?:?]
	at net.minecraft.util.thread.ProcessorMailbox.pollTask(ProcessorMailbox.java:91) ~[minecraft-merged-7a2f2bea21-1.21.1-loom.mappings.1_21_1.layered+hash.652182843-v2.jar:?]
	at net.minecraft.util.thread.ProcessorMailbox.pollUntil(ProcessorMailbox.java:146) ~[minecraft-merged-7a2f2bea21-1.21.1-loom.mappings.1_21_1.layered+hash.652182843-v2.jar:?]
	at net.minecraft.util.thread.ProcessorMailbox.run(ProcessorMailbox.java:102) ~[minecraft-merged-7a2f2bea21-1.21.1-loom.mappings.1_21_1.layered+hash.652182843-v2.jar:?]
	at net.minecraft.server.TickTask.run(TickTask.java:18) ~[minecraft-merged-7a2f2bea21-1.21.1-loom.mappings.1_21_1.layered+hash.652182843-v2.jar:?]
	at net.minecraft.util.thread.BlockableEventLoop.doRunTask(BlockableEventLoop.java:162) ~[minecraft-merged-7a2f2bea21-1.21.1-loom.mappings.1_21_1.layered+hash.652182843-v2.jar:?]
	at net.minecraft.util.thread.ReentrantBlockableEventLoop.doRunTask(ReentrantBlockableEventLoop.java:23) ~[minecraft-merged-7a2f2bea21-1.21.1-loom.mappings.1_21_1.layered+hash.652182843-v2.jar:?]
	at net.minecraft.server.MinecraftServer.doRunTask(MinecraftServer.java:864) ~[minecraft-merged-7a2f2bea21-1.21.1-loom.mappings.1_21_1.layered+hash.652182843-v2.jar:?]
	at net.minecraft.server.MinecraftServer.doRunTask(MinecraftServer.java:173) ~[minecraft-merged-7a2f2bea21-1.21.1-loom.mappings.1_21_1.layered+hash.652182843-v2.jar:?]
	at net.minecraft.util.thread.BlockableEventLoop.pollTask(BlockableEventLoop.java:136) ~[minecraft-merged-7a2f2bea21-1.21.1-loom.mappings.1_21_1.layered+hash.652182843-v2.jar:?]
	at net.minecraft.server.MinecraftServer.pollTaskInternal(MinecraftServer.java:846) ~[minecraft-merged-7a2f2bea21-1.21.1-loom.mappings.1_21_1.layered+hash.652182843-v2.jar:?]
	at net.minecraft.server.MinecraftServer.pollTask(MinecraftServer.java:840) ~[minecraft-merged-7a2f2bea21-1.21.1-loom.mappings.1_21_1.layered+hash.652182843-v2.jar:?]
	at net.minecraft.util.thread.BlockableEventLoop.managedBlock(BlockableEventLoop.java:145) ~[minecraft-merged-7a2f2bea21-1.21.1-loom.mappings.1_21_1.layered+hash.652182843-v2.jar:?]
	at net.minecraft.server.MinecraftServer.managedBlock(MinecraftServer.java:810) ~[minecraft-merged-7a2f2bea21-1.21.1-loom.mappings.1_21_1.layered+hash.652182843-v2.jar:?]
	at net.minecraft.server.MinecraftServer.waitUntilNextTick(MinecraftServer.java:815) ~[minecraft-merged-7a2f2bea21-1.21.1-loom.mappings.1_21_1.layered+hash.652182843-v2.jar:?]
	at net.minecraft.server.MinecraftServer.runServer(MinecraftServer.java:702) ~[minecraft-merged-7a2f2bea21-1.21.1-loom.mappings.1_21_1.layered+hash.652182843-v2.jar:?]
	at net.minecraft.server.MinecraftServer.method_29739(MinecraftServer.java:281) ~[minecraft-merged-7a2f2bea21-1.21.1-loom.mappings.1_21_1.layered+hash.652182843-v2.jar:?]
	at java.base/java.lang.Thread.run(Thread.java:1583) [?:?]
Caused by: java.lang.IllegalArgumentException: Data fixer not registered for: computercraft:turtle_normal in block_entity
	at com.mojang.datafixers.schemas.Schema.getChoiceType(Schema.java:109) ~[datafixerupper-8.0.16.jar:?]
	at dan200.computercraft.shared.util.ComponentizationFixers.fixTurtleBlockEntity(ComponentizationFixers.java:154) ~[computercraft-1.114.0-64b5720b4b825f21.jar:?]
	at dan200.computercraft.shared.util.ComponentizationFixers.makeBlockEntityRewrites(ComponentizationFixers.java:150) ~[computercraft-1.114.0-64b5720b4b825f21.jar:?]
	at net.minecraft.util.datafix.fixes.ItemStackComponentizationFix.modifyReturnValue$zzd000$computercraft$wrapMakeRule(ItemStackComponentizationFix.java:780) ~[minecraft-merged-7a2f2bea21-1.21.1-loom.mappings.1_21_1.layered+hash.652182843-v2.jar:?]
	at net.minecraft.util.datafix.fixes.ItemStackComponentizationFix.makeRule(ItemStackComponentizationFix.java:678) ~[minecraft-merged-7a2f2bea21-1.21.1-loom.mappings.1_21_1.layered+hash.652182843-v2.jar:?]
	at com.mojang.datafixers.DataFix.getRule(DataFix.java:119) ~[datafixerupper-8.0.16.jar:?]
	at com.mojang.datafixers.DataFixerUpper.lambda$getRule$1(DataFixerUpper.java:124) ~[datafixerupper-8.0.16.jar:?]
	at it.unimi.dsi.fastutil.longs.Long2ObjectOpenHashMap.computeIfAbsent(Long2ObjectOpenHashMap.java:454) ~[fastutil-8.5.12.jar:?]
	at it.unimi.dsi.fastutil.longs.Long2ObjectMaps$SynchronizedMap.computeIfAbsent(Long2ObjectMaps.java:466) ~[fastutil-8.5.12.jar:?]
	at com.mojang.datafixers.DataFixerUpper.getRule(DataFixerUpper.java:116) ~[datafixerupper-8.0.16.jar:?]
	at com.mojang.datafixers.DataFixerUpper.update(DataFixerUpper.java:78) ~[datafixerupper-8.0.16.jar:?]
	at net.minecraft.util.datafix.DataFixTypes.update(DataFixTypes.java:75) ~[minecraft-merged-7a2f2bea21-1.21.1-loom.mappings.1_21_1.layered+hash.652182843-v2.jar:?]
	at net.minecraft.util.datafix.DataFixTypes.update(DataFixTypes.java:83) ~[minecraft-merged-7a2f2bea21-1.21.1-loom.mappings.1_21_1.layered+hash.652182843-v2.jar:?]
	at net.minecraft.world.level.chunk.storage.EntityStorage.modifyExpressionValue$zcb000$cobblemon$doEntityFix(EntityStorage.java:536) ~[minecraft-merged-7a2f2bea21-1.21.1-loom.mappings.1_21_1.layered+hash.652182843-v2.jar:?]
	at net.minecraft.world.level.chunk.storage.EntityStorage.method_31731(EntityStorage.java:73) ~[minecraft-merged-7a2f2bea21-1.21.1-loom.mappings.1_21_1.layered+hash.652182843-v2.jar:?]
	at java.base/java.util.concurrent.CompletableFuture$UniApply.tryFire(CompletableFuture.java:646) ~[?:?]
	... 18 more

Cobblemon declares its own set of datafixer schemas and runs its own set of datafixer transformers. This means any mod that expects to be using a vanilla schema (which I think is a reasonable assumption) will not work.

I understand the motivation of what cobblemon is doing, but this feels like it's going to cause more pain than its worth. Separate mod datafixer is really something which needs support at the mod-loader level.

Cobblemon still USES the vanilla schema - it just also has its own. The issue thats happening here is that CC expects that the vanilla schema is the ONLY schema (that uses ItemComponentizationFix) - which I am not sure is a reasonable assumption.

I took a look at it, and (correct me if I'm wrong), it looks like CC is hooking into ItemComponentizationFix to do things besides fixing the item components. Specifically, it is moving the LeftUpgrade and RightUpgrade to LeftUpgradeNbt and RightUpgradeNbt.

From what I understand from digging through the MC source for a few hours (which SUCKED btw) this isn't really consistent with how Minecraft does data fixes. ItemComponentizationFix is supposed to do one thing, update the ITEM_STACK type from data version x to version y.

We use that in Cobblemon to update OUR block entities that contain items. We define a schema that defines types for our block entities, and in those definitions, we say "this type has an ITEM_STACK type at this nbt/json field." We also mixin to the fix to fix our items, so any mod using that fix to fix THEIR BEs has our items fixed.

So the problem is, that when Cobblemon goes to fix our block entities, the ItemComponentizationFix has a mixin that says "get this turtle block entity and do this fix as well". That turtle block entity doesn't exist in our schema, so errors abound.

So basically, the issue is that ItemComponentizationFix (which should be able to be used by any schema with an ITEM_STACK type) now can't be used by schemas without a BLOCK_ENTITY type that also contains the turtle id.

This made me discover an issue though - Cobblemon's DFU shouldn't be running on new chunks but it is. If we fix that, our mods should at least be compatible on new versions. Just cant update an old world that had them both

I appreciate your response.

You can actually see this with Cobblemon's Guilded chest — because other fixers are not being applied (e.g. ItemStackCustomNameToOverrideComponentFix), ominous banners in those chests aren't correctly translated

Thank you for bringing this up! Did not know this was a thing.

This is partly why I say this needs to be solved on a mod loader level. It would be conceptually much cleaner if everyone could do what Cobblemon does, and define your own mod-specific schema, but any such fixers need to run interleaved with vanilla's own datafixers, rather than in an entirely separate pass

Yeah I definitely get what you are saying here now. I think our solution is probably just going to be to pick and choose what fixes run on our schema, but I definitely won’t blame others for not wanting to do that - it is kind of hacky and really annoying.

I've not found a good approach for injecting custom fixers

Yeah iirc this was the main reason we went with our own schema. We needed custom fixes for some Pokemon stuff and the choices seemed to be an unholy amalgamation of bad mixins or our own schema lol. Then when we discovered we needed to fix our BEs as well, it just made sense to tack the fixes on.

Like I said before, we had an issue where our DFU was running on newly genned chunks, and we fixed that. Our mods can now run together. The only issues are when introducing Cobblemon to a world that previously contained CC, and when trying to convert a world that had Cobblemon and CC.

As for fixing those problems, I think the easiest (even if it is “wrong”) fix is to just move your fix into another fix less liable to be used by modded schemas. I think adding a whole new fix would be better, but I agree with you that injecting fixes is not very clean lol.

CC expects that the vanilla schema is the ONLY schema that uses ItemComponentizationFix. [...] ItemComponentizationFix is supposed to do one thing, update the ITEM_STACK type from data version x to version y.

I think this is the crux of it really. While some of vanilla's fixers are generic (it would be very silly for any mod to mix in to WriteAndReadFix!), ItemComponentizationFix isn't. Its explicitly written to update from schema V3818 to V3820. If you try to run it on another schema, you'll get something which sort of works, but not completely.

You can actually see this with Cobblemon's Guilded chest — because other fixers are not being applied (e.g. ItemStackCustomNameToOverrideComponentFix), ominous banners in those chests aren't correctly translated (they lose their pattern, and use minecraft:custom_name rather than minecraft:item_name).

This is partly why I say this needs to be solved on a mod loader level. It would be conceptually much cleaner if everyone could do what Cobblemon does, and define your own mod-specific schema, but any such fixers need to run interleaved with vanilla's own datafixers, rather than in an entirely separate pass. You probably could maintain a mirror of all of vanilla's fixers, but that sounds like a maintenance nightmare. It is going to be better behaved just to mixin to vanilla's fixers, however gross it is :s.

From what I understand from digging through the MC source for a few hours (which SUCKED btw) this isn't really consistent with how Minecraft does data fixes.

Yeah, I don't disagree with that. This was originally done because the turtle upgrade fixers should also be running the componentisation transform on the upgrade data (though doesn't, because upgrade data was very unstructured). I probably could move that into a separate fixer — I've not found a good approach for injecting custom fixers, but it is possible:

@Mixin(DataFixers.class)
abstract class DataFixersMixin {
    @ModifyExpressionValue(
        method = "addFixers",
        slice = @Slice(
            from = @At(value = "CONSTANT", args = "intValue=4071"),
            to = @At(value = "CONSTANT", args = "intValue=4071", shift = At.Shift.BY, by = 3)
        ),
        at = @At(
            value = "INVOKE",
            target = "Lcom/mojang/datafixers/DataFixerBuilder;addSchema(ILjava/util/function/BiFunction;)Lcom/mojang/datafixers/schemas/Schema;"
        ),
        allow = 1
    )
    @SuppressWarnings("UnusedMethod")
    private static Schema createSchema4071(Schema schema, @Local DataFixerBuilder builder) {
        if (schema.getVersionKey() != DataFixUtils.makeKey(4071)) {
            throw new IllegalStateException("Unexpected schema version");
        }

        builder.addFixer(...);
        return schema;
    }
}

You probably could maintain a mirror of all of vanilla's fixers [..] I think our solution is probably just going to be to pick and choose what fixes run on our schema

Actually, thinking about this further, I'm not sure this works — if other mods start trying to inject their own datafixers (e.g. to rename an item), none of these will be picked up by your version.

I'm also a little unsure about how pulling out the ITEM_STACK type from another schema would work. My understanding is that it won't have a reference to the (block) entities you've defined in your schema, so if vanilla components reference a Cobblemon entity (e.g. via the minecraft:entity_data), then the Pokemon datafixers will not be run.

Trust me, just mixin to V1460 and BlockPosFormatAndRenamesFix, it's much easier :p.

Also a really big downside of mixining to V1460 and BlockPosFormatAndRenamesFix is that we can't use DFUs betweeen updates on the same MC version.

Trust me, just mixin to V1460 and BlockPosFormatAndRenamesFix, it's much easier :p.

Unfortunately this isn't an option for us right now lol - we are pretty close to releasing our next version, and this would require substantial enough changes that we would have to probably push it back - which I doubt our team will want to do.

I'm also a little unsure about how pulling out the ITEM_STACK type from another schema would work. My understanding is that it won't have a reference to the (block) entities you've defined in your schema, so if vanilla components reference a Cobblemon entity (e.g. via the minecraft:entity_data), then the Pokemon datafixers will not be run.

I don't think this is true. I think the ITEM_STACK would have a reference to TypeReference.BLOCK_ENTITY which would be our block entities. (Though not vanilla's, so I guess a modded item in a Cobblemon block entity that has minecraft:entity_data of a non cobblemon mob will cause issues)

Actually, thinking about this further, I'm not sure this works — if other mods start trying to inject their own datafixers (e.g. to rename an item), none of these will be picked up by your version.

This is true. And very unfortunate. I guess there is only so much we can do to allow people to port up their worlds.