Remove reflective implementation of ICustomPacket. (#8973)

Make vanilla custom packets able to be sent multiple times. Closes #8969

patches/minecraft/net/minecraft/network/protocol/game/ClientboundCustomPayloadPacket.java.patch

@@ -25,7 +25,7 @@
     }
  
     public ClientboundCustomPayloadPacket(FriendlyByteBuf p_178836_) {
-@@ -43,6 +_,7 @@
+@@ -43,15 +_,17 @@
        } else {
           throw new IllegalArgumentException("Payload may not be larger than 1048576 bytes");
        }
@@ -33,7 +33,10 @@
     }
  
     public void m_5779_(FriendlyByteBuf p_132044_) {
-@@ -52,6 +_,7 @@
+       p_132044_.m_130085_(this.f_132029_);
+-      p_132044_.writeBytes(this.f_132030_.copy());
++      p_132044_.writeBytes(this.f_132030_.slice()); // Use Slice instead of copy, to not update the read index, allowing packet to be sent multiple times.
+    }
  
     public void m_5797_(ClientGamePacketListener p_132041_) {
        p_132041_.m_7413_(this);
@@ -41,3 +44,12 @@
     }
  
     public ResourceLocation m_132042_() {
+@@ -61,4 +_,8 @@
+    public FriendlyByteBuf m_132045_() {
+       return new FriendlyByteBuf(this.f_132030_.copy());
+    }
++
++   @Override public int getIndex() { return Integer.MAX_VALUE; }
++   @Override public ResourceLocation getName() { return m_132042_(); }
++   @org.jetbrains.annotations.Nullable @Override public FriendlyByteBuf getInternalData() { return this.f_132030_; }
+ }

patches/minecraft/net/minecraft/network/protocol/game/ServerboundCustomPayloadPacket.java.patch

@@ -14,7 +14,16 @@
     public void m_5779_(FriendlyByteBuf p_133994_) {
        p_133994_.m_130085_(this.f_133980_);
 -      p_133994_.writeBytes((ByteBuf)this.f_133981_);
-+      p_133994_.writeBytes((ByteBuf)this.f_133981_.copy()); //This may be access multiple times, from multiple threads, lets be safe like the S->C packet
++      p_133994_.writeBytes((ByteBuf)this.f_133981_.slice()); // Use Slice instead of copy, to not update the read index, allowing packet to be sent multiple times.
     }
  
     public void m_5797_(ServerGamePacketListener p_133992_) {
+@@ -43,4 +_,8 @@
+    public FriendlyByteBuf m_179590_() {
+       return this.f_133981_;
+    }
++
++   @Override public int getIndex() { return Integer.MAX_VALUE; }
++   @Override public ResourceLocation getName() { return m_179589_(); }
++   @org.jetbrains.annotations.Nullable @Override public FriendlyByteBuf getInternalData() { return m_179590_(); }
+ }

patches/minecraft/net/minecraft/network/protocol/login/ClientboundCustomQueryPacket.java.patch

@@ -9,3 +9,21 @@
     private static final int f_179804_ = 1048576;
     private final int f_134745_;
     private final ResourceLocation f_134746_;
+@@ -30,7 +_,7 @@
+    public void m_5779_(FriendlyByteBuf p_134757_) {
+       p_134757_.m_130130_(this.f_134745_);
+       p_134757_.m_130085_(this.f_134746_);
+-      p_134757_.writeBytes(this.f_134747_.copy());
++      p_134757_.writeBytes(this.f_134747_.slice()); // Use Slice instead of copy, to not update the read index, allowing packet to be sent multiple times.
+    }
+ 
+    public void m_5797_(ClientLoginPacketListener p_134754_) {
+@@ -48,4 +_,8 @@
+    public FriendlyByteBuf m_179812_() {
+       return this.f_134747_;
+    }
++
++   @Override public int getIndex() { return m_134755_(); }
++   @Override public ResourceLocation getName() { return m_179811_(); }
++   @org.jetbrains.annotations.Nullable @Override public FriendlyByteBuf getInternalData() { return m_179812_(); }
+ }

patches/minecraft/net/minecraft/network/protocol/login/ServerboundCustomQueryPacket.java.patch

@@ -10,3 +10,12 @@
     private static final int f_179821_ = 1048576;
     private final int f_134825_;
     @Nullable
+@@ -46,4 +_,8 @@
+    public FriendlyByteBuf m_179825_() {
+       return this.f_134826_;
+    }
++
++   @Override public int getIndex() { return m_179824_(); }
++   @Override public net.minecraft.resources.ResourceLocation getName() { return net.minecraftforge.network.LoginWrapper.WRAPPER; }
++   @org.jetbrains.annotations.Nullable @Override public FriendlyByteBuf getInternalData() { return m_179825_(); }
+ }

src/main/java/net/minecraftforge/network/ICustomPacket.java

@@ -5,83 +5,24 @@
 
 package net.minecraftforge.network;
 
-import it.unimi.dsi.fastutil.objects.Reference2ReferenceArrayMap;
 import net.minecraft.network.protocol.Packet;
-import net.minecraft.network.FriendlyByteBuf;
-import net.minecraft.network.protocol.login.ServerboundCustomQueryPacket;
-import net.minecraft.network.protocol.login.ClientboundCustomQueryPacket;
-import net.minecraft.network.protocol.game.ServerboundCustomPayloadPacket;
-import net.minecraft.network.protocol.game.ClientboundCustomPayloadPacket;
-import net.minecraft.resources.ResourceLocation;
-import net.minecraftforge.fml.unsafe.UnsafeHacks;
 
-import java.lang.reflect.Field;
-import java.lang.reflect.Modifier;
-import java.util.Arrays;
-import java.util.Optional;
-import java.util.function.Function;
-import java.util.stream.Collectors;
-import java.util.stream.Stream;
+import org.jetbrains.annotations.Nullable;
+
+import net.minecraft.network.FriendlyByteBuf;
+import net.minecraft.resources.ResourceLocation;
 
 public interface ICustomPacket<T extends Packet<?>> {
-    @SuppressWarnings("OptionalUsedAsFieldOrParameterType")
-    enum Fields {
-        CPACKETCUSTOMPAYLOAD(ServerboundCustomPayloadPacket.class),
-        SPACKETCUSTOMPAYLOAD(ClientboundCustomPayloadPacket.class),
-        CPACKETCUSTOMLOGIN(ServerboundCustomQueryPacket.class),
-        SPACKETCUSTOMLOGIN(ClientboundCustomQueryPacket.class),
-        ;
+    /**
+     * Returns a unsafe reference to this packet's internal data.
+     * Any modifications to this buffer will be reflected in the main buffer.
+     */
+    @Nullable
+    FriendlyByteBuf getInternalData();
 
-        static final Reference2ReferenceArrayMap<Class<?>, Fields> lookup;
-        static {
-            lookup = Stream.of(values()).
-                    collect(Collectors.toMap(Fields::getClazz, Function.identity(), (m1, m2)->m1, Reference2ReferenceArrayMap::new));
-        }
+    ResourceLocation getName();
 
-        private final Class<?> clazz;
-
-        final Optional<Field> data;
-        final Optional<Field> channel;
-        final Optional<Field> index;
-
-        Fields(Class<?> customPacketClass)
-        {
-            this.clazz = customPacketClass;
-            Field[] fields = customPacketClass.getDeclaredFields();
-            data = Arrays.stream(fields).filter(f-> !Modifier.isStatic(f.getModifiers()) && f.getType() == FriendlyByteBuf.class).findFirst();
-            channel = Arrays.stream(fields).filter(f->!Modifier.isStatic(f.getModifiers()) && f.getType() == ResourceLocation.class).findFirst();
-            index = Arrays.stream(fields).filter(f->!Modifier.isStatic(f.getModifiers()) && f.getType() == int.class).findFirst();
-        }
-
-        private Class<?> getClazz()
-        {
-            return clazz;
-        }
-    }
-
-    default FriendlyByteBuf getInternalData() {
-        return Fields.lookup.get(this.getClass()).data.map(f->UnsafeHacks.<FriendlyByteBuf>getField(f, this)).orElse(null);
-    }
-
-    default ResourceLocation getName() {
-        return Fields.lookup.get(this.getClass()).channel.map(f->UnsafeHacks.<ResourceLocation>getField(f, this)).orElse(LoginWrapper.WRAPPER);
-    }
-
-    default int getIndex() {
-        return Fields.lookup.get(this.getClass()).index.map(f->UnsafeHacks.getIntField(f, this)).orElse(Integer.MIN_VALUE);
-    }
-
-    default void setData(FriendlyByteBuf buffer) {
-        Fields.lookup.get(this.getClass()).data.ifPresent(f->UnsafeHacks.setField(f, this, buffer));
-    }
-
-    default void setName(ResourceLocation channelName) {
-        Fields.lookup.get(this.getClass()).channel.ifPresent(f->UnsafeHacks.setField(f, this, channelName));
-    }
-
-    default void setIndex(int index) {
-        Fields.lookup.get(this.getClass()).index.ifPresent(f->UnsafeHacks.setIntField(f, this, index));
-    }
+    int getIndex();
 
     default NetworkDirection getDirection() {
         return NetworkDirection.directionFor(this.getClass());

src/main/java/net/minecraftforge/network/LoginWrapper.java

@@ -13,6 +13,7 @@ import net.minecraftforge.network.event.EventNetworkChannel;
 import org.apache.commons.lang3.tuple.Pair;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
+import org.jetbrains.annotations.ApiStatus;
 
 /**
  * Wrapper for custom login packets. Transforms unnamed login channel messages into channels dispatched the same
@@ -21,7 +22,8 @@ import org.apache.logging.log4j.Logger;
 public class LoginWrapper
 {
     private static final Logger LOGGER = LogManager.getLogger();
-    static final ResourceLocation WRAPPER = new ResourceLocation("fml:loginwrapper");
+    @ApiStatus.Internal
+    public static final ResourceLocation WRAPPER = new ResourceLocation("fml:loginwrapper");
     private EventNetworkChannel wrapperChannel;
 
     LoginWrapper() {

src/main/java/net/minecraftforge/network/NetworkDirection.java

@@ -14,7 +14,6 @@ import net.minecraft.network.protocol.game.ServerboundCustomPayloadPacket;
 import net.minecraft.network.protocol.game.ClientboundCustomPayloadPacket;
 import net.minecraft.resources.ResourceLocation;
 import net.minecraftforge.fml.LogicalSide;
-import net.minecraftforge.fml.unsafe.UnsafeHacks;
 import org.apache.commons.lang3.tuple.Pair;
 
 import java.util.function.BiFunction;
@@ -25,15 +24,16 @@ import java.util.stream.Stream;
 
 public enum NetworkDirection
 {
-    PLAY_TO_SERVER(NetworkEvent.ClientCustomPayloadEvent::new, LogicalSide.CLIENT, ServerboundCustomPayloadPacket.class, 1),
-    PLAY_TO_CLIENT(NetworkEvent.ServerCustomPayloadEvent::new, LogicalSide.SERVER, ClientboundCustomPayloadPacket.class, 0),
-    LOGIN_TO_SERVER(NetworkEvent.ClientCustomPayloadLoginEvent::new, LogicalSide.CLIENT, ServerboundCustomQueryPacket.class, 3),
-    LOGIN_TO_CLIENT(NetworkEvent.ServerCustomPayloadLoginEvent::new, LogicalSide.SERVER, ClientboundCustomQueryPacket.class, 2);
+    PLAY_TO_SERVER(NetworkEvent.ClientCustomPayloadEvent::new, LogicalSide.CLIENT, ServerboundCustomPayloadPacket.class, 1, (d, i, n) -> new ServerboundCustomPayloadPacket(n, d)),
+    PLAY_TO_CLIENT(NetworkEvent.ServerCustomPayloadEvent::new, LogicalSide.SERVER, ClientboundCustomPayloadPacket.class, 0, (d, i, n) -> new ClientboundCustomPayloadPacket(n, d)),
+    LOGIN_TO_SERVER(NetworkEvent.ClientCustomPayloadLoginEvent::new, LogicalSide.CLIENT, ServerboundCustomQueryPacket.class, 3, (d, i, n) -> new ServerboundCustomQueryPacket(i, d)),
+    LOGIN_TO_CLIENT(NetworkEvent.ServerCustomPayloadLoginEvent::new, LogicalSide.SERVER, ClientboundCustomQueryPacket.class, 2, (d, i, n) -> new ClientboundCustomQueryPacket(i, n, d));
 
     private final BiFunction<ICustomPacket<?>, Supplier<NetworkEvent.Context>, NetworkEvent> eventSupplier;
     private final LogicalSide logicalSide;
     private final Class<? extends Packet> packetClass;
     private final int otherWay;
+    private final Factory factory;
 
     private static final Reference2ReferenceArrayMap<Class<? extends Packet>, NetworkDirection> packetLookup;
 
@@ -42,12 +42,13 @@ public enum NetworkDirection
                 collect(Collectors.toMap(NetworkDirection::getPacketClass, Function.identity(), (m1,m2)->m1, Reference2ReferenceArrayMap::new));
     }
 
-    NetworkDirection(BiFunction<ICustomPacket<?>, Supplier<NetworkEvent.Context>, NetworkEvent> eventSupplier, LogicalSide logicalSide, Class<? extends Packet> clazz, int i)
+    private NetworkDirection(BiFunction<ICustomPacket<?>, Supplier<NetworkEvent.Context>, NetworkEvent> eventSupplier, LogicalSide logicalSide, Class<? extends Packet> clazz, int i, Factory factory)
     {
         this.eventSupplier = eventSupplier;
         this.logicalSide = logicalSide;
         this.packetClass = clazz;
         this.otherWay = i;
+        this.factory = factory;
     }
 
     private Class<? extends Packet> getPacketClass() {
@@ -75,10 +76,10 @@ public enum NetworkDirection
     @SuppressWarnings("unchecked")
     public <T extends Packet<?>> ICustomPacket<T> buildPacket(Pair<FriendlyByteBuf,Integer> packetData, ResourceLocation channelName)
     {
-        ICustomPacket<T> packet = (ICustomPacket<T>)UnsafeHacks.newInstance(getPacketClass());
-        packet.setName(channelName);
-        packet.setData(packetData.getLeft());
-        packet.setIndex(packetData.getRight());
-        return packet;
+        return this.factory.create(packetData.getLeft(), packetData.getRight(), channelName);
+    }
+
+    private interface Factory<T extends Packet<?>> {
+        ICustomPacket<T> create(FriendlyByteBuf data, Integer index, ResourceLocation channelName);
     }
 }

src/main/java/net/minecraftforge/network/simple/IndexedMessageCodec.java

@@ -135,7 +135,7 @@ public class IndexedMessageCodec
     }
 
     void consume(FriendlyByteBuf payload, int payloadIndex, Supplier<NetworkEvent.Context> context) {
-        if (payload == null) {
+        if (payload == null || !payload.isReadable()) {
             LOGGER.error(SIMPLENET, "Received empty payload on channel {}", Optional.ofNullable(networkInstance).map(NetworkInstance::getChannelName).map(Objects::toString).orElse("MISSING CHANNEL"));
             if (!HandshakeHandler.packetNeedsResponse(context.get().getNetworkManager(), payloadIndex))
             {