Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Logging for Unread Packet Data #2698

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
+125 −37
Open

Improve Logging for Unread Packet Data #2698

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e0b5188
improve logging when data packet has data left
ghzdude Jan 13, 2025
3273f50
add method to register fields from other classes
ghzdude Jan 14, 2025
553a0e4
add logging to more places
ghzdude Jan 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions src/main/java/gregtech/api/capability/GregtechDataCodes.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
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 {

private static int nextId = 0;
Expand Down Expand Up @@ -180,4 +190,39 @@ 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<String> 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 : clazz.getDeclaredFields()) {
if (field.getType() != Integer.TYPE) continue;
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) {}
}
}
3 changes: 3 additions & 0 deletions src/main/java/gregtech/api/cover/CoverSaveHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package gregtech.api.cover;

import gregtech.api.metatileentity.interfaces.ISyncedTileEntity;
import gregtech.api.util.GTLog;

import net.minecraft.nbt.NBTTagCompound;
Expand Down Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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();
}
Expand Down
12 changes: 10 additions & 2 deletions src/main/java/gregtech/api/metatileentity/MetaTileEntity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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) {
Expand All @@ -1095,6 +1102,7 @@ public void receiveCustomData(int dataId, @NotNull PacketBuffer buf) {
int internalId = buf.readVarInt();
if (cover != null) {
cover.readCustomData(internalId, buf);
ISyncedTileEntity.checkCustomData(internalId, buf, cover);
}
} else if (dataId == UPDATE_SOUND_MUFFLED) {
this.muffled = buf.readBoolean();
Expand Down
38 changes: 11 additions & 27 deletions src/main/java/gregtech/api/metatileentity/SyncedTileEntityBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -80,20 +79,13 @@ 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));
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());
}
int dataId = Integer.parseInt(discriminatorKey);
receiveCustomData(dataId, new PacketBuffer(backedBuffer));

MetaTileEntity mte = null;
if (this instanceof IGregTechTileEntity gtte)
mte = gtte.getMetaTileEntity();
ISyncedTileEntity.checkCustomData(dataId, backedBuffer, mte == null ? this : mte);
}
}
}
Expand All @@ -114,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);
}
}
64 changes: 56 additions & 8 deletions src/main/java/gregtech/api/metatileentity/interfaces/ISyncedTileEntity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,15 @@
package gregtech.api.metatileentity.interfaces;

import gregtech.api.capability.GregtechDataCodes;
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;

import java.util.function.Consumer;
Expand All @@ -25,7 +33,7 @@ public interface ISyncedTileEntity {
* <p>
* This method is called <strong>Server-Side</strong>.
* <p>
* Equivalent to {@link net.minecraft.tileentity.TileEntity#getUpdateTag}.
* Equivalent to {@link TileEntity#getUpdateTag}.
*
* @param buf the buffer to write data to
*/
Expand All @@ -43,7 +51,7 @@ public interface ISyncedTileEntity {
* <p>
* This method is called <strong>Client-Side</strong>.
* <p>
* Equivalent to {@link net.minecraft.tileentity.TileEntity#handleUpdateTag}.
* Equivalent to {@link TileEntity#handleUpdateTag}.
*
* @param buf the buffer to read data from
*/
Expand All @@ -62,11 +70,11 @@ public interface ISyncedTileEntity {
* <p>
* This method is called <strong>Server-Side</strong>.
* <p>
* 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);

Expand All @@ -82,10 +90,10 @@ public interface ISyncedTileEntity {
* <p>
* This method is called <strong>Server-Side</strong>.
* <p>
* 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);
Expand All @@ -103,11 +111,51 @@ default void writeCustomData(int discriminator) {
* <p>
* This method is called <strong>Client-Side</strong>.
* <p>
* 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 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",
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();
}
}
Loading