From e0b518830559ea51f90254a0bce0583facb7a569 Mon Sep 17 00:00:00 2001 From: Ghzdude <44148655+ghzdude@users.noreply.github.com> Date: Mon, 13 Jan 2025 16:20:06 -0700 Subject: [PATCH 1/3] improve logging when data packet has data left --- .../api/capability/GregtechDataCodes.java | 21 +++++++++++ .../api/metatileentity/MetaTileEntity.java | 3 ++ .../metatileentity/SyncedTileEntityBase.java | 15 ++------ .../interfaces/ISyncedTileEntity.java | 35 ++++++++++++++----- 4 files changed, 54 insertions(+), 20 deletions(-) diff --git a/src/main/java/gregtech/api/capability/GregtechDataCodes.java b/src/main/java/gregtech/api/capability/GregtechDataCodes.java index 6054957ac0a..b1628643966 100644 --- a/src/main/java/gregtech/api/capability/GregtechDataCodes.java +++ b/src/main/java/gregtech/api/capability/GregtechDataCodes.java @@ -1,5 +1,10 @@ package gregtech.api.capability; +import it.unimi.dsi.fastutil.ints.Int2ObjectArrayMap; +import it.unimi.dsi.fastutil.ints.Int2ObjectMap; + +import java.lang.reflect.Field; + public class GregtechDataCodes { private static int nextId = 0; @@ -180,4 +185,20 @@ public static int assignId() { // ME Parts public static final int UPDATE_AUTO_PULL = assignId(); public static final int UPDATE_ONLINE_STATUS = assignId(); + + // Everything below MUST be last in the class! + public static final Int2ObjectMap NAMES = new Int2ObjectArrayMap<>(); + + static { + try { + for (Field field : GregtechDataCodes.class.getFields()) { + if (field.getType() != Integer.TYPE) continue; + NAMES.put(field.getInt(null), field.getName()); + } + } catch (IllegalAccessException ignored) {} + } + + public static String getNameFor(int id) { + return NAMES.getOrDefault(id, "Unknown DataCode: " + id); + } } diff --git a/src/main/java/gregtech/api/metatileentity/MetaTileEntity.java b/src/main/java/gregtech/api/metatileentity/MetaTileEntity.java index 723df6f6808..480a01dd9db 100644 --- a/src/main/java/gregtech/api/metatileentity/MetaTileEntity.java +++ b/src/main/java/gregtech/api/metatileentity/MetaTileEntity.java @@ -1096,6 +1096,9 @@ public void receiveCustomData(int dataId, @NotNull PacketBuffer buf) { if (cover != null) { cover.readCustomData(internalId, buf); } + if (buf.readableBytes() != 0) + ISyncedTileEntity.handleUnreadPacket(internalId, buf, this); + buf.clear(); // clear data so holder doesn't log error } else if (dataId == UPDATE_SOUND_MUFFLED) { this.muffled = buf.readBoolean(); if (muffled) { diff --git a/src/main/java/gregtech/api/metatileentity/SyncedTileEntityBase.java b/src/main/java/gregtech/api/metatileentity/SyncedTileEntityBase.java index 4e29fe09328..28234604838 100644 --- a/src/main/java/gregtech/api/metatileentity/SyncedTileEntityBase.java +++ b/src/main/java/gregtech/api/metatileentity/SyncedTileEntityBase.java @@ -80,19 +80,10 @@ public final void onDataPacket(@NotNull NetworkManager net, @NotNull SPacketUpda NBTTagCompound entryTag = (NBTTagCompound) entryBase; for (String discriminatorKey : entryTag.getKeySet()) { ByteBuf backedBuffer = Unpooled.copiedBuffer(entryTag.getByteArray(discriminatorKey)); - receiveCustomData(Integer.parseInt(discriminatorKey), new PacketBuffer(backedBuffer)); + int dataId = Integer.parseInt(discriminatorKey); + receiveCustomData(dataId, new PacketBuffer(backedBuffer)); if (backedBuffer.readableBytes() != 0) { - String className = null; - if (this instanceof IGregTechTileEntity gtte) { - MetaTileEntity mte = gtte.getMetaTileEntity(); - if (mte != null) className = mte.getClass().getName(); - } - if (className == null) { - className = this.getClass().getName(); - } - GTLog.logger.error( - "Class {} failed to finish reading receiveCustomData with discriminator {} and {} bytes remaining", - className, discriminatorKey, backedBuffer.readableBytes()); + ISyncedTileEntity.handleUnreadPacket(dataId, backedBuffer, this); } } } diff --git a/src/main/java/gregtech/api/metatileentity/interfaces/ISyncedTileEntity.java b/src/main/java/gregtech/api/metatileentity/interfaces/ISyncedTileEntity.java index 0222ffdbe47..0fad37da0d2 100644 --- a/src/main/java/gregtech/api/metatileentity/interfaces/ISyncedTileEntity.java +++ b/src/main/java/gregtech/api/metatileentity/interfaces/ISyncedTileEntity.java @@ -1,7 +1,13 @@ package gregtech.api.metatileentity.interfaces; +import gregtech.api.capability.GregtechDataCodes; +import gregtech.api.metatileentity.MetaTileEntity; +import gregtech.api.util.GTLog; + import net.minecraft.network.PacketBuffer; +import net.minecraft.tileentity.TileEntity; +import io.netty.buffer.ByteBuf; import org.jetbrains.annotations.NotNull; import java.util.function.Consumer; @@ -25,7 +31,7 @@ public interface ISyncedTileEntity { *

* This method is called Server-Side. *

- * Equivalent to {@link net.minecraft.tileentity.TileEntity#getUpdateTag}. + * Equivalent to {@link TileEntity#getUpdateTag}. * * @param buf the buffer to write data to */ @@ -43,7 +49,7 @@ public interface ISyncedTileEntity { *

* This method is called Client-Side. *

- * Equivalent to {@link net.minecraft.tileentity.TileEntity#handleUpdateTag}. + * Equivalent to {@link TileEntity#handleUpdateTag}. * * @param buf the buffer to read data from */ @@ -62,11 +68,11 @@ public interface ISyncedTileEntity { *

* This method is called Server-Side. *

- * Equivalent to {@link net.minecraft.tileentity.TileEntity#getUpdatePacket} + * Equivalent to {@link TileEntity#getUpdatePacket} * * @param discriminator the discriminator determining the packet sent. * @param dataWriter a consumer which writes packet data to a buffer. - * @see gregtech.api.capability.GregtechDataCodes + * @see GregtechDataCodes */ void writeCustomData(int discriminator, @NotNull Consumer<@NotNull PacketBuffer> dataWriter); @@ -82,10 +88,10 @@ public interface ISyncedTileEntity { *

* This method is called Server-Side. *

- * Equivalent to {@link net.minecraft.tileentity.TileEntity#getUpdatePacket} + * Equivalent to {@link TileEntity#getUpdatePacket} * * @param discriminator the discriminator determining the packet sent. - * @see gregtech.api.capability.GregtechDataCodes + * @see GregtechDataCodes */ default void writeCustomData(int discriminator) { writeCustomData(discriminator, NO_OP); @@ -103,11 +109,24 @@ default void writeCustomData(int discriminator) { *

* This method is called Client-Side. *

- * Equivalent to {@link net.minecraft.tileentity.TileEntity#onDataPacket} + * Equivalent to {@link TileEntity#onDataPacket} * * @param discriminator the discriminator determining the packet sent. * @param buf the buffer containing the packet data. - * @see gregtech.api.capability.GregtechDataCodes + * @see GregtechDataCodes */ void receiveCustomData(int discriminator, @NotNull PacketBuffer buf); + + static void handleUnreadPacket(int discriminator, @NotNull ByteBuf buf, ISyncedTileEntity syncedTile) { + String className = null; + if (syncedTile instanceof IGregTechTileEntity gtte) { + MetaTileEntity mte = gtte.getMetaTileEntity(); + if (mte != null) className = mte.getClass().getSimpleName(); + } else { + className = syncedTile.getClass().getSimpleName(); + } + GTLog.logger.error( + "Class {} failed to finish reading receiveCustomData with discriminator {} and {} bytes remaining", + className, GregtechDataCodes.getNameFor(discriminator), buf.readableBytes()); + } } From 3273f50129f5527ce1037a2d171f066c6ad1f489 Mon Sep 17 00:00:00 2001 From: Ghzdude <44148655+ghzdude@users.noreply.github.com> Date: Mon, 13 Jan 2025 17:05:47 -0700 Subject: [PATCH 2/3] add method to register fields from other classes --- .../api/capability/GregtechDataCodes.java | 36 +++++++++++++++---- 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/src/main/java/gregtech/api/capability/GregtechDataCodes.java b/src/main/java/gregtech/api/capability/GregtechDataCodes.java index b1628643966..78cdbfba678 100644 --- a/src/main/java/gregtech/api/capability/GregtechDataCodes.java +++ b/src/main/java/gregtech/api/capability/GregtechDataCodes.java @@ -1,9 +1,14 @@ package gregtech.api.capability; +import gregtech.common.covers.ender.CoverAbstractEnderLink; +import gregtech.common.metatileentities.multi.multiblockpart.MetaTileEntityFluidHatch; + import it.unimi.dsi.fastutil.ints.Int2ObjectArrayMap; import it.unimi.dsi.fastutil.ints.Int2ObjectMap; +import org.apache.commons.lang3.ArrayUtils; import java.lang.reflect.Field; +import java.lang.reflect.Modifier; public class GregtechDataCodes { @@ -190,15 +195,34 @@ public static int assignId() { public static final Int2ObjectMap NAMES = new Int2ObjectArrayMap<>(); static { + registerFields(GregtechDataCodes.class); + // todo these really should be moved to this class + registerFields(CoverAbstractEnderLink.class, CoverAbstractEnderLink.UPDATE_PRIVATE); + registerFields(MetaTileEntityFluidHatch.class, MetaTileEntityFluidHatch.LOCK_FILL); + } + + public static String getNameFor(int id) { + return NAMES.getOrDefault(id, "Unknown_DataCode:" + id); + } + + /** + * Registers all fields from the passed in class to the name registry. + * Optionally, you can pass in a list of valid ids to check against so that errant ids are not added + * + * @param clazz Class to iterate fields + * @param validIds optional array of valid ids to check against class fields + */ + public static void registerFields(Class clazz, int... validIds) { try { - for (Field field : GregtechDataCodes.class.getFields()) { + for (Field field : clazz.getDeclaredFields()) { if (field.getType() != Integer.TYPE) continue; - NAMES.put(field.getInt(null), field.getName()); + if (!Modifier.isStatic(field.getModifiers())) continue; + if (!Modifier.isFinal(field.getModifiers())) continue; + int id = field.getInt(null); + if (!ArrayUtils.isEmpty(validIds) && !ArrayUtils.contains(validIds, id)) + continue; + NAMES.put(id, field.getName() + ":" + id); } } catch (IllegalAccessException ignored) {} } - - public static String getNameFor(int id) { - return NAMES.getOrDefault(id, "Unknown DataCode: " + id); - } } From 553a0e4bd7a535e4ccddd35d470ba0f7958998ac Mon Sep 17 00:00:00 2001 From: Ghzdude <44148655+ghzdude@users.noreply.github.com> Date: Mon, 13 Jan 2025 18:49:12 -0700 Subject: [PATCH 3/3] add logging to more places log initial data --- .../gregtech/api/cover/CoverSaveHandler.java | 3 ++ .../api/metatileentity/MetaTileEntity.java | 15 ++++-- .../metatileentity/SyncedTileEntityBase.java | 25 ++++------ .../interfaces/ISyncedTileEntity.java | 49 +++++++++++++++---- 4 files changed, 61 insertions(+), 31 deletions(-) diff --git a/src/main/java/gregtech/api/cover/CoverSaveHandler.java b/src/main/java/gregtech/api/cover/CoverSaveHandler.java index fe62aa782a9..e9041d022f3 100644 --- a/src/main/java/gregtech/api/cover/CoverSaveHandler.java +++ b/src/main/java/gregtech/api/cover/CoverSaveHandler.java @@ -1,5 +1,6 @@ package gregtech.api.cover; +import gregtech.api.metatileentity.interfaces.ISyncedTileEntity; import gregtech.api.util.GTLog; import net.minecraft.nbt.NBTTagCompound; @@ -66,6 +67,7 @@ public static void receiveInitialSyncData(@NotNull PacketBuffer buf, @NotNull Co } else { Cover cover = definition.createCover(coverHolder, facing); cover.readInitialSyncData(buf); + ISyncedTileEntity.checkInitialData(buf, cover); coverHolder.addCover(facing, cover); } } @@ -107,6 +109,7 @@ public static void readCoverPlacement(@NotNull PacketBuffer buf, @NotNull CoverH coverHolder.addCover(placementSide, cover); cover.readInitialSyncData(buf); + ISyncedTileEntity.checkInitialData(buf, cover); } coverHolder.scheduleRenderUpdate(); } diff --git a/src/main/java/gregtech/api/metatileentity/MetaTileEntity.java b/src/main/java/gregtech/api/metatileentity/MetaTileEntity.java index 480a01dd9db..611cb125922 100644 --- a/src/main/java/gregtech/api/metatileentity/MetaTileEntity.java +++ b/src/main/java/gregtech/api/metatileentity/MetaTileEntity.java @@ -1043,7 +1043,10 @@ public void receiveInitialSyncData(@NotNull PacketBuffer buf) { MTETrait trait = mteTraitByNetworkId.get(traitNetworkId); if (trait == null) { GTLog.logger.warn("Could not find MTETrait for id: {} at position {}.", traitNetworkId, getPos()); - } else trait.receiveInitialData(buf); + } else { + trait.receiveInitialSyncData(buf); + ISyncedTileEntity.checkInitialData(buf, trait); + } } CoverSaveHandler.receiveInitialSyncData(buf, this); this.muffled = buf.readBoolean(); @@ -1076,10 +1079,14 @@ public void receiveCustomData(int dataId, @NotNull PacketBuffer buf) { scheduleRenderUpdate(); } else if (dataId == SYNC_MTE_TRAITS) { int traitNetworkId = buf.readVarInt(); + int internalId = buf.readVarInt(); MTETrait trait = mteTraitByNetworkId.get(traitNetworkId); if (trait == null) { GTLog.logger.warn("Could not find MTETrait for id: {} at position {}.", traitNetworkId, getPos()); - } else trait.receiveCustomData(buf.readVarInt(), buf); + } else { + trait.receiveCustomData(internalId, buf); + ISyncedTileEntity.checkCustomData(internalId, buf, trait); + } } else if (dataId == COVER_ATTACHED_MTE) { CoverSaveHandler.readCoverPlacement(buf, this); } else if (dataId == COVER_REMOVED_MTE) { @@ -1095,10 +1102,8 @@ public void receiveCustomData(int dataId, @NotNull PacketBuffer buf) { int internalId = buf.readVarInt(); if (cover != null) { cover.readCustomData(internalId, buf); + ISyncedTileEntity.checkCustomData(internalId, buf, cover); } - if (buf.readableBytes() != 0) - ISyncedTileEntity.handleUnreadPacket(internalId, buf, this); - buf.clear(); // clear data so holder doesn't log error } else if (dataId == UPDATE_SOUND_MUFFLED) { this.muffled = buf.readBoolean(); if (muffled) { diff --git a/src/main/java/gregtech/api/metatileentity/SyncedTileEntityBase.java b/src/main/java/gregtech/api/metatileentity/SyncedTileEntityBase.java index 28234604838..9b42003a0dc 100644 --- a/src/main/java/gregtech/api/metatileentity/SyncedTileEntityBase.java +++ b/src/main/java/gregtech/api/metatileentity/SyncedTileEntityBase.java @@ -4,7 +4,6 @@ import gregtech.api.metatileentity.interfaces.IGregTechTileEntity; import gregtech.api.metatileentity.interfaces.ISyncedTileEntity; import gregtech.api.network.PacketDataList; -import gregtech.api.util.GTLog; import net.minecraft.block.state.IBlockState; import net.minecraft.nbt.NBTBase; @@ -82,9 +81,11 @@ public final void onDataPacket(@NotNull NetworkManager net, @NotNull SPacketUpda ByteBuf backedBuffer = Unpooled.copiedBuffer(entryTag.getByteArray(discriminatorKey)); int dataId = Integer.parseInt(discriminatorKey); receiveCustomData(dataId, new PacketBuffer(backedBuffer)); - if (backedBuffer.readableBytes() != 0) { - ISyncedTileEntity.handleUnreadPacket(dataId, backedBuffer, this); - } + + MetaTileEntity mte = null; + if (this instanceof IGregTechTileEntity gtte) + mte = gtte.getMetaTileEntity(); + ISyncedTileEntity.checkCustomData(dataId, backedBuffer, mte == null ? this : mte); } } } @@ -105,18 +106,10 @@ public final void handleUpdateTag(@NotNull NBTTagCompound tag) { byte[] updateData = tag.getByteArray("d"); ByteBuf backedBuffer = Unpooled.copiedBuffer(updateData); receiveInitialSyncData(new PacketBuffer(backedBuffer)); - if (backedBuffer.readableBytes() != 0) { - String className = null; - if (this instanceof IGregTechTileEntity gtte) { - MetaTileEntity mte = gtte.getMetaTileEntity(); - if (mte != null) className = mte.getClass().getName(); - } - if (className == null) { - className = this.getClass().getName(); - } - GTLog.logger.error("Class {} failed to finish reading initialSyncData with {} bytes remaining", - className, backedBuffer.readableBytes()); - } + MetaTileEntity mte = null; + if (this instanceof IGregTechTileEntity gtte) + mte = gtte.getMetaTileEntity(); + ISyncedTileEntity.checkInitialData(backedBuffer, mte == null ? this : mte); } } diff --git a/src/main/java/gregtech/api/metatileentity/interfaces/ISyncedTileEntity.java b/src/main/java/gregtech/api/metatileentity/interfaces/ISyncedTileEntity.java index 0fad37da0d2..97bff58419b 100644 --- a/src/main/java/gregtech/api/metatileentity/interfaces/ISyncedTileEntity.java +++ b/src/main/java/gregtech/api/metatileentity/interfaces/ISyncedTileEntity.java @@ -1,11 +1,13 @@ package gregtech.api.metatileentity.interfaces; import gregtech.api.capability.GregtechDataCodes; -import gregtech.api.metatileentity.MetaTileEntity; +import gregtech.api.cover.Cover; +import gregtech.api.cover.CoverableView; import gregtech.api.util.GTLog; import net.minecraft.network.PacketBuffer; import net.minecraft.tileentity.TileEntity; +import net.minecraft.util.math.BlockPos; import io.netty.buffer.ByteBuf; import org.jetbrains.annotations.NotNull; @@ -117,16 +119,43 @@ default void writeCustomData(int discriminator) { */ void receiveCustomData(int discriminator, @NotNull PacketBuffer buf); - static void handleUnreadPacket(int discriminator, @NotNull ByteBuf buf, ISyncedTileEntity syncedTile) { - String className = null; - if (syncedTile instanceof IGregTechTileEntity gtte) { - MetaTileEntity mte = gtte.getMetaTileEntity(); - if (mte != null) className = mte.getClass().getSimpleName(); - } else { - className = syncedTile.getClass().getSimpleName(); - } + static void checkCustomData(int discriminator, @NotNull ByteBuf buf, Object obj) { + if (buf.readableBytes() == 0) return; + GTLog.logger.error( "Class {} failed to finish reading receiveCustomData with discriminator {} and {} bytes remaining", - className, GregtechDataCodes.getNameFor(discriminator), buf.readableBytes()); + stringify(obj), GregtechDataCodes.getNameFor(discriminator), buf.readableBytes()); + + buf.clear(); // clear to prevent further logging + } + + static void checkInitialData(@NotNull ByteBuf buf, Object obj) { + if (buf.readableBytes() == 0) return; + + GTLog.logger.error("Class {} failed to finish reading initialSyncData with {} bytes remaining", + stringify(obj), buf.readableBytes()); + + buf.clear(); // clear to prevent further logging + } + + static String stringify(Object obj) { + StringBuilder builder = new StringBuilder(obj.getClass().getSimpleName()); + + BlockPos pos = null; + if (obj instanceof TileEntity tileEntity) { + pos = tileEntity.getPos(); // TE pos + } else if (obj instanceof CoverableView view) { + pos = view.getPos(); // MTE pos + } else if (obj instanceof Cover cover) { + pos = cover.getPos(); // Cover pos and side + builder.append("[side=").append(cover.getAttachedSide()).append("]"); + } + + if (pos != null) builder.append(" @ {") + .append(pos.getX()).append("X, ") + .append(pos.getY()).append("Y, ") + .append(pos.getZ()).append("Z}"); + + return builder.toString(); } }