Skip to content

Commit

Permalink
[1.20.4] Reimplement ID offset on modded EntityDataSerializers (#425)
Browse files Browse the repository at this point in the history
This reimplements the ID offset previously applied to modded
`EntityDataSerializer`s which was lost during the registry rework.

Previously this was achieved by setting a minimum ID on the registry.
Due to the modified vanilla registries not supporting that, the IDs of
modded serializers now also start at 0 which leads to the client using
the vanilla serializer corresponding to the ID of the modded one and
potentially blowing up while deserializing the data item, i.e. by
reading past the buffer's length.
  • Loading branch information
XFactHD authored Dec 25, 2023
1 parent 1c1212a commit 8998c21
Show file tree
Hide file tree
Showing 3 changed files with 129 additions and 4 deletions.
Original file line number Diff line number Diff line change
@@ -1,11 +1,24 @@
--- a/net/minecraft/network/syncher/EntityDataSerializers.java
+++ b/net/minecraft/network/syncher/EntityDataSerializers.java
@@ -157,16 +_,16 @@
@@ -156,17 +_,29 @@
FriendlyByteBuf::writeQuaternion, FriendlyByteBuf::readQuaternion
);

+ private static final org.slf4j.Logger LOGGER = com.mojang.logging.LogUtils.getLogger();
+ private static final StackWalker STACK_WALKER = StackWalker.getInstance(StackWalker.Option.RETAIN_CLASS_REFERENCE);
+ /**
+ * @deprecated NeoForge: Use {@link net.neoforged.neoforge.registries.NeoForgeRegistries#ENTITY_DATA_SERIALIZERS} instead
+ */
+ @Deprecated
public static void registerSerializer(EntityDataSerializer<?> p_135051_) {
- SERIALIZERS.add(p_135051_);
+ if (SERIALIZERS.add(p_135051_) >= 256) throw new RuntimeException("Vanilla DataSerializer ID limit exceeded");
+ if (!STACK_WALKER.getCallerClass().equals(EntityDataSerializers.class)) {
+ // TODO 1.20.5: throw an exception in dev instead
+ LOGGER.error("Modded EntityDataSerializers must be registered to NeoForgeRegistries.ENTITY_DATA_SERIALIZERS instead to prevent ID mismatches between client and server!", new Throwable());
+ }
+
+ if (SERIALIZERS.add(p_135051_) >= net.neoforged.neoforge.common.CommonHooks.VANILLA_SERIALIZER_LIMIT)
+ throw new RuntimeException("Vanilla EntityDataSerializer ID limit exceeded");
}

@Nullable
Expand Down
9 changes: 7 additions & 2 deletions src/main/java/net/neoforged/neoforge/common/CommonHooks.java
Original file line number Diff line number Diff line change
Expand Up @@ -895,19 +895,24 @@ public static boolean hasNoElements(Ingredient ingredient) {
@Deprecated(forRemoval = true, since = "1.20.1") // Tags use a codec now This was never used in 1.20
public static <T> void deserializeTagAdditions(List<TagEntry> list, JsonObject json, List<TagEntry> allList) {}

public static final int VANILLA_SERIALIZER_LIMIT = 256;

@Nullable
public static EntityDataSerializer<?> getSerializer(int id, CrudeIncrementalIntIdentityHashBiMap<EntityDataSerializer<?>> vanilla) {
EntityDataSerializer<?> serializer = vanilla.byId(id);
if (serializer == null) {
return NeoForgeRegistries.ENTITY_DATA_SERIALIZERS.byId(id);
return NeoForgeRegistries.ENTITY_DATA_SERIALIZERS.byId(id - VANILLA_SERIALIZER_LIMIT);
}
return serializer;
}

public static int getSerializerId(EntityDataSerializer<?> serializer, CrudeIncrementalIntIdentityHashBiMap<EntityDataSerializer<?>> vanilla) {
int id = vanilla.getId(serializer);
if (id < 0) {
return NeoForgeRegistries.ENTITY_DATA_SERIALIZERS.getId(serializer);
id = NeoForgeRegistries.ENTITY_DATA_SERIALIZERS.getId(serializer);
if (id >= 0) {
return id + VANILLA_SERIALIZER_LIMIT;
}
}
return id;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
/*
* Copyright (c) NeoForged and contributors
* SPDX-License-Identifier: LGPL-2.1-only
*/

package net.neoforged.neoforge.debug.entity;

import io.netty.buffer.Unpooled;
import net.minecraft.client.renderer.entity.EntityRenderer;
import net.minecraft.client.renderer.entity.EntityRendererProvider;
import net.minecraft.gametest.framework.GameTest;
import net.minecraft.nbt.CompoundTag;
import net.minecraft.network.FriendlyByteBuf;
import net.minecraft.network.protocol.game.ClientboundSetEntityDataPacket;
import net.minecraft.network.syncher.EntityDataAccessor;
import net.minecraft.network.syncher.EntityDataSerializer;
import net.minecraft.network.syncher.SynchedEntityData;
import net.minecraft.resources.ResourceLocation;
import net.minecraft.world.entity.Entity;
import net.minecraft.world.entity.EntityType;
import net.minecraft.world.entity.MobCategory;
import net.minecraft.world.level.Level;
import net.neoforged.neoforge.common.CommonHooks;
import net.neoforged.neoforge.registries.DeferredHolder;
import net.neoforged.neoforge.registries.NeoForgeRegistries;
import net.neoforged.testframework.DynamicTest;
import net.neoforged.testframework.TestFramework;
import net.neoforged.testframework.annotation.ForEachTest;
import net.neoforged.testframework.annotation.OnInit;
import net.neoforged.testframework.annotation.TestHolder;
import net.neoforged.testframework.gametest.EmptyTemplate;
import net.neoforged.testframework.registration.RegistrationHelper;

@ForEachTest(groups = EntityDataSerializerTest.GROUP)
public class EntityDataSerializerTest {
public static final String GROUP = "level.entity.data_serializer";

private static final RegistrationHelper REG_HELPER = RegistrationHelper.create("neotests_custom_entity_data_serializer");
private static final DeferredHolder<EntityDataSerializer<?>, EntityDataSerializer<Byte>> TEST_SERIALIZER = REG_HELPER
.registrar(NeoForgeRegistries.Keys.ENTITY_DATA_SERIALIZERS)
.register("test_serializer", () -> EntityDataSerializer.simple((buf, b) -> buf.writeByte(b), FriendlyByteBuf::readByte));

@OnInit
static void register(final TestFramework framework) {
REG_HELPER.register(framework.modEventBus());
}

@GameTest
@EmptyTemplate(floor = true)
@TestHolder(description = "Tests if custom EntityDataSerializers are properly handled")
static void customEntityDataSerializer(final DynamicTest test, final RegistrationHelper reg) {
var testEntity = reg.entityTypes().registerType("serializer_test_entity", () -> EntityType.Builder.of(TestEntity::new, MobCategory.CREATURE)
.sized(1, 1)).withRenderer(() -> TestEntityRenderer::new);

test.onGameTest(helper -> {
var entity = helper.spawn(testEntity.get(), 1, 2, 1);
var items = entity.getEntityData().packDirty();
if (items == null) {
helper.fail("Expected dirty entity data, got none");
return;
}
var pkt = new ClientboundSetEntityDataPacket(entity.getId(), items);
FriendlyByteBuf buf = new FriendlyByteBuf(Unpooled.buffer());
pkt.write(buf);
helper.assertTrue(buf.readVarInt() == entity.getId(), "Entity ID didn't match"); // Drop entity ID
buf.readByte(); // Drop item ID
int expectedId = NeoForgeRegistries.ENTITY_DATA_SERIALIZERS.getId(TEST_SERIALIZER.get()) + CommonHooks.VANILLA_SERIALIZER_LIMIT;
helper.assertTrue(buf.readVarInt() == expectedId, "Serializer ID didn't match");
buf.readByte(); // Drop data
buf.readByte(); // Drop EOF marker
helper.assertTrue(buf.readableBytes() == 0, "Buffer not empty");

helper.succeed();
});
}

private static class TestEntity extends Entity {
private static final EntityDataAccessor<Byte> DATA_TEST_VALUE = SynchedEntityData.defineId(TestEntity.class, TEST_SERIALIZER.value());

public TestEntity(EntityType<? extends Entity> entityType, Level level) {
super(entityType, level);
entityData.set(DATA_TEST_VALUE, (byte) 1);
}

@Override
protected void defineSynchedData() {
entityData.define(DATA_TEST_VALUE, (byte) 0);
}

@Override
protected void readAdditionalSaveData(CompoundTag tag) {}

@Override
protected void addAdditionalSaveData(CompoundTag tag) {}
}

private static class TestEntityRenderer extends EntityRenderer<TestEntity> {
public TestEntityRenderer(EntityRendererProvider.Context context) {
super(context);
}

@Override
public ResourceLocation getTextureLocation(TestEntity entity) {
return null;
}
}
}

0 comments on commit 8998c21

Please sign in to comment.