Cleanup 1.17 entity rewriter, fix DataItem amount check

api/src/main/java/com/viaversion/viaversion/api/minecraft/item/DataItem.java

@@ -66,7 +66,7 @@ public class DataItem implements Item {
 
     @Override
     public void setAmount(int amount) {
-        if (amount > Byte.MAX_VALUE && amount < Byte.MIN_VALUE) {
+        if (amount > Byte.MAX_VALUE || amount < Byte.MIN_VALUE) {
             throw new IllegalArgumentException("Invalid item amount: " + amount);
         }
         this.amount = (byte) amount;

api/src/main/java/com/viaversion/viaversion/api/rewriter/RewriterBase.java

@@ -38,13 +38,13 @@ public abstract class RewriterBase<T extends Protocol> implements Rewriter<T> {
     }
 
     /**
-     * To be overriden. Called when initializing the EntityRewriter.
+     * To be overriden. Called when registering the rewriter.
      */
     protected void registerPackets() {
     }
 
     /**
-     * To be overriden. Called when initializing the EntityRewriter.
+     * To be overriden. Called when registering the rewriter.
      */
     protected void registerRewrites() {
     }

common/src/main/java/com/viaversion/viaversion/protocols/protocol1_17to1_16_4/Protocol1_17To1_16_4.java

@@ -31,7 +31,6 @@ import com.viaversion.viaversion.api.type.Type;
 import com.viaversion.viaversion.data.entity.EntityTrackerBase;
 import com.viaversion.viaversion.protocols.protocol1_16_2to1_16_1.ClientboundPackets1_16_2;
 import com.viaversion.viaversion.protocols.protocol1_16_2to1_16_1.ServerboundPackets1_16_2;
-import com.viaversion.viaversion.protocols.protocol1_17to1_16_4.metadata.MetadataRewriter1_17To1_16_4;
 import com.viaversion.viaversion.protocols.protocol1_17to1_16_4.packets.EntityPackets;
 import com.viaversion.viaversion.protocols.protocol1_17to1_16_4.packets.InventoryPackets;
 import com.viaversion.viaversion.protocols.protocol1_17to1_16_4.packets.WorldPackets;
@@ -41,13 +40,13 @@ import com.viaversion.viaversion.rewriter.SoundRewriter;
 import com.viaversion.viaversion.rewriter.StatisticsRewriter;
 import com.viaversion.viaversion.rewriter.TagRewriter;
 
-public class Protocol1_17To1_16_4 extends AbstractProtocol<ClientboundPackets1_16_2, ClientboundPackets1_17, ServerboundPackets1_16_2, ServerboundPackets1_17> {
+public final class Protocol1_17To1_16_4 extends AbstractProtocol<ClientboundPackets1_16_2, ClientboundPackets1_17, ServerboundPackets1_16_2, ServerboundPackets1_17> {
 
     public static final MappingData MAPPINGS = new MappingDataBase("1.16.2", "1.17", true);
     private static final String[] NEW_GAME_EVENT_TAGS = {"minecraft:ignore_vibrations_sneaking", "minecraft:vibrations"};
-    private final EntityRewriter metadataRewriter = new MetadataRewriter1_17To1_16_4(this);
+    private final EntityRewriter entityRewriter = new EntityPackets(this);
     private final ItemRewriter itemRewriter = new InventoryPackets(this);
-    private TagRewriter tagRewriter;
+    private final TagRewriter tagRewriter = new TagRewriter(this);
 
     public Protocol1_17To1_16_4() {
         super(ClientboundPackets1_16_2.class, ClientboundPackets1_17.class, ServerboundPackets1_16_2.class, ServerboundPackets1_17.class);
@@ -55,13 +54,11 @@ public class Protocol1_17To1_16_4 extends AbstractProtocol<ClientboundPackets1_1
 
     @Override
     protected void registerPackets() {
-        metadataRewriter.register();
+        entityRewriter.register();
         itemRewriter.register();
 
-        EntityPackets.register(this);
         WorldPackets.register(this);
 
-        tagRewriter = new TagRewriter(this);
         registerClientbound(ClientboundPackets1_16_2.TAGS, new PacketRemapper() {
             @Override
             public void registerMap() {
@@ -239,7 +236,7 @@ public class Protocol1_17To1_16_4 extends AbstractProtocol<ClientboundPackets1_1
 
     @Override
     public void init(UserConnection user) {
-        user.addEntityTracker(this.getClass(), new EntityTrackerBase(user, Entity1_17Types.PLAYER));
+        addEntityTracker(user, new EntityTrackerBase(user, Entity1_17Types.PLAYER));
         user.put(new InventoryAcknowledgements());
     }
 
@@ -250,7 +247,7 @@ public class Protocol1_17To1_16_4 extends AbstractProtocol<ClientboundPackets1_1
 
     @Override
     public EntityRewriter getEntityRewriter() {
-        return metadataRewriter;
+        return entityRewriter;
     }
 
     @Override

common/src/main/java/com/viaversion/viaversion/protocols/protocol1_17to1_16_4/metadata/MetadataRewriter1_17To1_16_4.java

@@ -1,66 +0,0 @@
-/*
- * This file is part of ViaVersion - https://github.com/ViaVersion/ViaVersion
- * Copyright (C) 2016-2021 ViaVersion and contributors
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- */
-package com.viaversion.viaversion.protocols.protocol1_17to1_16_4.metadata;
-
-import com.viaversion.viaversion.api.minecraft.entities.Entity1_16_2Types;
-import com.viaversion.viaversion.api.minecraft.entities.Entity1_17Types;
-import com.viaversion.viaversion.api.minecraft.entities.EntityType;
-import com.viaversion.viaversion.api.minecraft.metadata.types.MetaType1_17;
-import com.viaversion.viaversion.protocols.protocol1_17to1_16_4.Protocol1_17To1_16_4;
-import com.viaversion.viaversion.rewriter.EntityRewriter;
-
-public class MetadataRewriter1_17To1_16_4 extends EntityRewriter {
-
-    public MetadataRewriter1_17To1_16_4(Protocol1_17To1_16_4 protocol) {
-        super(protocol);
-        mapTypes(Entity1_16_2Types.values(), Entity1_17Types.class);
-    }
-
-    @Override
-    protected void registerRewrites() {
-        filter().handler((event, meta) -> {
-            meta.setMetaType(MetaType1_17.byId(meta.metaType().typeId()));
-
-            if (meta.metaType() == MetaType1_17.POSE) {
-                int pose = meta.value();
-                if (pose > 5) {
-                    // Added LONG_JUMP at 6
-                    meta.setValue(pose + 1);
-                }
-            }
-        });
-        registerMetaTypeHandler(MetaType1_17.ITEM, MetaType1_17.BLOCK_STATE, MetaType1_17.PARTICLE);
-
-        // Ticks frozen added with id 7
-        filter().filterFamily(Entity1_17Types.ENTITY).addIndex(7);
-
-        filter().filterFamily(Entity1_17Types.MINECART_ABSTRACT).index(11).handler((event, meta) -> {
-            // Convert to new block id
-            int data = (int) meta.getValue();
-            meta.setValue(protocol.getMappingData().getNewBlockStateId(data));
-        });
-
-        // Attachment position removed
-        filter().type(Entity1_17Types.SHULKER).removeIndex(17);
-    }
-
-    @Override
-    public EntityType typeFromId(int type) {
-        return Entity1_17Types.getTypeFromId(type);
-    }
-}

common/src/main/java/com/viaversion/viaversion/protocols/protocol1_17to1_16_4/packets/EntityPackets.java

@@ -18,7 +18,10 @@
 package com.viaversion.viaversion.protocols.protocol1_17to1_16_4.packets;
 
 import com.viaversion.viaversion.api.data.entity.EntityTracker;
+import com.viaversion.viaversion.api.minecraft.entities.Entity1_16_2Types;
 import com.viaversion.viaversion.api.minecraft.entities.Entity1_17Types;
+import com.viaversion.viaversion.api.minecraft.entities.EntityType;
+import com.viaversion.viaversion.api.minecraft.metadata.types.MetaType1_17;
 import com.viaversion.viaversion.api.protocol.packet.ClientboundPacketType;
 import com.viaversion.viaversion.api.protocol.packet.PacketWrapper;
 import com.viaversion.viaversion.api.protocol.remapper.PacketRemapper;
@@ -28,16 +31,21 @@ import com.viaversion.viaversion.api.type.types.version.Types1_17;
 import com.viaversion.viaversion.protocols.protocol1_16_2to1_16_1.ClientboundPackets1_16_2;
 import com.viaversion.viaversion.protocols.protocol1_17to1_16_4.ClientboundPackets1_17;
 import com.viaversion.viaversion.protocols.protocol1_17to1_16_4.Protocol1_17To1_16_4;
-import com.viaversion.viaversion.protocols.protocol1_17to1_16_4.metadata.MetadataRewriter1_17To1_16_4;
+import com.viaversion.viaversion.rewriter.EntityRewriter;
 
-public class EntityPackets {
+public final class EntityPackets extends EntityRewriter<Protocol1_17To1_16_4> {
 
-    public static void register(Protocol1_17To1_16_4 protocol) {
+    public EntityPackets(Protocol1_17To1_16_4 protocol) {
-        MetadataRewriter1_17To1_16_4 metadataRewriter = protocol.get(MetadataRewriter1_17To1_16_4.class);
+        super(protocol);
-        metadataRewriter.registerTrackerWithData(ClientboundPackets1_16_2.SPAWN_ENTITY, Entity1_17Types.FALLING_BLOCK);
+        mapTypes(Entity1_16_2Types.values(), Entity1_17Types.class);
-        metadataRewriter.registerTracker(ClientboundPackets1_16_2.SPAWN_MOB);
+    }
-        metadataRewriter.registerTracker(ClientboundPackets1_16_2.SPAWN_PLAYER, Entity1_17Types.PLAYER);
+
-        metadataRewriter.registerMetadataRewriter(ClientboundPackets1_16_2.ENTITY_METADATA, Types1_14.METADATA_LIST, Types1_17.METADATA_LIST);
+    @Override
+    public void registerPackets() {
+        registerTrackerWithData(ClientboundPackets1_16_2.SPAWN_ENTITY, Entity1_17Types.FALLING_BLOCK);
+        registerTracker(ClientboundPackets1_16_2.SPAWN_MOB);
+        registerTracker(ClientboundPackets1_16_2.SPAWN_PLAYER, Entity1_17Types.PLAYER);
+        registerMetadataRewriter(ClientboundPackets1_16_2.ENTITY_METADATA, Types1_14.METADATA_LIST, Types1_17.METADATA_LIST);
 
         protocol.registerClientbound(ClientboundPackets1_16_2.DESTROY_ENTITIES, null, new PacketRemapper() {
             @Override
@@ -116,4 +124,37 @@ public class EntityPackets {
         // The parent class of the other entity move packets that is never actually used has finally been removed from the id list
         protocol.cancelClientbound(ClientboundPackets1_16_2.ENTITY_MOVEMENT);
     }
+
+    @Override
+    protected void registerRewrites() {
+        filter().handler((event, meta) -> {
+            meta.setMetaType(MetaType1_17.byId(meta.metaType().typeId()));
+
+            if (meta.metaType() == MetaType1_17.POSE) {
+                int pose = meta.value();
+                if (pose > 5) {
+                    // Added LONG_JUMP at 6
+                    meta.setValue(pose + 1);
+                }
+            }
+        });
+        registerMetaTypeHandler(MetaType1_17.ITEM, MetaType1_17.BLOCK_STATE, MetaType1_17.PARTICLE);
+
+        // Ticks frozen added with id 7
+        filter().filterFamily(Entity1_17Types.ENTITY).addIndex(7);
+
+        filter().filterFamily(Entity1_17Types.MINECART_ABSTRACT).index(11).handler((event, meta) -> {
+            // Convert to new block id
+            int data = (int) meta.getValue();
+            meta.setValue(protocol.getMappingData().getNewBlockStateId(data));
+        });
+
+        // Attachment position removed
+        filter().type(Entity1_17Types.SHULKER).removeIndex(17);
+    }
+
+    @Override
+    public EntityType typeFromId(int type) {
+        return Entity1_17Types.getTypeFromId(type);
+    }
 }

common/src/main/java/com/viaversion/viaversion/protocols/protocol1_17to1_16_4/packets/InventoryPackets.java

@@ -30,7 +30,7 @@ import com.viaversion.viaversion.protocols.protocol1_17to1_16_4.ServerboundPacke
 import com.viaversion.viaversion.protocols.protocol1_17to1_16_4.storage.InventoryAcknowledgements;
 import com.viaversion.viaversion.rewriter.ItemRewriter;
 
-public class InventoryPackets extends ItemRewriter<Protocol1_17To1_16_4> {
+public final class InventoryPackets extends ItemRewriter<Protocol1_17To1_16_4> {
 
     public InventoryPackets(Protocol1_17To1_16_4 protocol) {
         super(protocol);

common/src/main/java/com/viaversion/viaversion/protocols/protocol1_17to1_16_4/packets/WorldPackets.java

@@ -42,7 +42,7 @@ import java.util.ArrayList;
 import java.util.BitSet;
 import java.util.List;
 
-public class WorldPackets {
+public final class WorldPackets {
 
     public static void register(Protocol1_17To1_16_4 protocol) {
         BlockRewriter blockRewriter = new BlockRewriter(protocol, Type.POSITION1_14);

common/src/main/java/com/viaversion/viaversion/protocols/protocol1_17to1_16_4/types/Chunk1_17Type.java

@@ -33,7 +33,7 @@ import java.util.Arrays;
 import java.util.BitSet;
 import java.util.List;
 
-public class Chunk1_17Type extends Type<Chunk> {
+public final class Chunk1_17Type extends Type<Chunk> {
     private static final CompoundTag[] EMPTY_COMPOUNDS = new CompoundTag[0];
     private final int ySectionCount;
 

common/src/main/java/com/viaversion/viaversion/rewriter/BlockRewriter.java

@@ -24,7 +24,6 @@ import com.viaversion.viaversion.api.protocol.packet.ClientboundPacketType;
 import com.viaversion.viaversion.api.protocol.remapper.PacketRemapper;
 import com.viaversion.viaversion.api.type.Type;
 
-// If any of these methods become outdated, just create a new rewriter overriding the methods
 public class BlockRewriter {
     private final Protocol protocol;
     private final Type<Position> positionType;

common/src/main/java/com/viaversion/viaversion/rewriter/ItemRewriter.java

@@ -30,7 +30,7 @@ import org.checkerframework.checker.nullness.qual.Nullable;
 
 public abstract class ItemRewriter<T extends Protocol> extends RewriterBase<T> implements com.viaversion.viaversion.api.rewriter.ItemRewriter<T> {
 
-    public ItemRewriter(T protocol) {
+    protected ItemRewriter(T protocol) {
         super(protocol);
     }