Clean up Title packets and enforce action types (#1803)

proxy/src/main/java/com/velocitypowered/proxy/connection/client/ConnectedPlayer.java

@@ -135,6 +135,7 @@ import net.kyori.adventure.text.logger.slf4j.ComponentLogger;
 import net.kyori.adventure.text.minimessage.translation.Argument;
 import net.kyori.adventure.text.serializer.legacy.LegacyComponentSerializer;
 import net.kyori.adventure.text.serializer.plain.PlainTextComponentSerializer;
+import net.kyori.adventure.title.Title;
 import net.kyori.adventure.title.Title.Times;
 import net.kyori.adventure.title.TitlePart;
 import net.kyori.adventure.translation.GlobalTranslator;
@@ -486,11 +487,11 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player,
   }
 
   @Override
-  public void showTitle(net.kyori.adventure.title.@NonNull Title title) {
+  public void showTitle(@NonNull Title title) {
     if (this.getProtocolVersion().noLessThan(ProtocolVersion.MINECRAFT_1_8)) {
       GenericTitlePacket timesPkt = GenericTitlePacket.constructTitlePacket(
           GenericTitlePacket.ActionType.SET_TIMES, this.getProtocolVersion());
-      net.kyori.adventure.title.Title.Times times = title.times();
+      Title.Times times = title.times();
       if (times != null) {
         timesPkt.setFadeIn((int) DurationUtils.toTicks(times.fadeIn()));
         timesPkt.setStay((int) DurationUtils.toTicks(times.stay()));

proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/title/GenericTitlePacket.java

@@ -22,10 +22,12 @@ import com.velocitypowered.proxy.protocol.MinecraftPacket;
 import com.velocitypowered.proxy.protocol.ProtocolUtils;
 import com.velocitypowered.proxy.protocol.packet.chat.ComponentHolder;
 import io.netty.buffer.ByteBuf;
+import org.jetbrains.annotations.NotNull;
 
 public abstract class GenericTitlePacket implements MinecraftPacket {
 
   public enum ActionType {
+
     SET_TITLE(0),
     SET_SUBTITLE(1),
     SET_ACTION_BAR(2),
@@ -45,16 +47,7 @@ public abstract class GenericTitlePacket implements MinecraftPacket {
     }
   }
 
-
+  public abstract @NotNull ActionType getAction();
-  private ActionType action;
-
-  protected void setAction(ActionType action) {
-    this.action = action;
-  }
-
-  public final ActionType getAction() {
-    return action;
-  }
 
   public ComponentHolder getComponent() {
     throw new UnsupportedOperationException("Invalid function for this TitlePacket ActionType");
@@ -88,11 +81,9 @@ public abstract class GenericTitlePacket implements MinecraftPacket {
     throw new UnsupportedOperationException("Invalid function for this TitlePacket ActionType");
   }
 
-
   @Override
-  public final void decode(ByteBuf buf, ProtocolUtils.Direction direction,
+  public final void decode(ByteBuf buf, ProtocolUtils.Direction direction, ProtocolVersion version) {
-      ProtocolVersion version) {
+    throw new UnsupportedOperationException("Decode is not implemented");
-    throw new UnsupportedOperationException(); // encode only
   }
 
   /**
@@ -103,21 +94,17 @@ public abstract class GenericTitlePacket implements MinecraftPacket {
    * @return GenericTitlePacket instance that follows the invoker type/version
    */
   public static GenericTitlePacket constructTitlePacket(ActionType type, ProtocolVersion version) {
-    GenericTitlePacket packet = null;
     if (version.noLessThan(ProtocolVersion.MINECRAFT_1_17)) {
-      packet = switch (type) {
+      return switch (type) {
         case SET_ACTION_BAR -> new TitleActionbarPacket();
         case SET_SUBTITLE -> new TitleSubtitlePacket();
         case SET_TIMES -> new TitleTimesPacket();
         case SET_TITLE -> new TitleTextPacket();
-        case HIDE, RESET -> new TitleClearPacket();
+        case HIDE, RESET -> new TitleClearPacket(type);
         default -> throw new IllegalArgumentException("Invalid ActionType");
       };
     } else {
-      packet = new LegacyTitlePacket();
+      return new LegacyTitlePacket(type);
     }
-    packet.setAction(type);
-    return packet;
   }
-
 }

proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/title/LegacyTitlePacket.java

@@ -17,32 +17,44 @@
 
 package com.velocitypowered.proxy.protocol.packet.title;
 
+import com.google.common.base.Preconditions;
 import com.velocitypowered.api.network.ProtocolVersion;
 import com.velocitypowered.proxy.connection.MinecraftSessionHandler;
 import com.velocitypowered.proxy.protocol.ProtocolUtils;
 import com.velocitypowered.proxy.protocol.packet.chat.ComponentHolder;
 import io.netty.buffer.ByteBuf;
-import org.checkerframework.checker.nullness.qual.Nullable;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 
 public class LegacyTitlePacket extends GenericTitlePacket {
 
+  private final ActionType action;
+
   private @Nullable ComponentHolder component;
   private int fadeIn;
   private int stay;
   private int fadeOut;
 
+  public LegacyTitlePacket() {
+    throw new UnsupportedOperationException("Decode is not implemented");
+  }
+
+  public LegacyTitlePacket(ActionType action) {
+    this.action = Preconditions.checkNotNull(action, "action");
+  }
+
   @Override
   public void encode(ByteBuf buf, ProtocolUtils.Direction direction, ProtocolVersion version) {
     if (version.lessThan(ProtocolVersion.MINECRAFT_1_11)
-        && getAction() == ActionType.SET_ACTION_BAR) {
+        && this.action == ActionType.SET_ACTION_BAR) {
       throw new IllegalStateException("Action bars are only supported on 1.11 and newer");
     }
-    ProtocolUtils.writeVarInt(buf, getAction().getAction(version));
+    ProtocolUtils.writeVarInt(buf, this.action.getAction(version));
 
-    switch (getAction()) {
+    switch (this.action) {
       case SET_TITLE, SET_SUBTITLE, SET_ACTION_BAR -> {
         if (component == null) {
-          throw new IllegalStateException("No component found for " + getAction());
+          throw new IllegalStateException("No component found for " + this.action);
         }
         component.write(buf);
       }
@@ -52,14 +64,13 @@ public class LegacyTitlePacket extends GenericTitlePacket {
         buf.writeInt(fadeOut);
       }
       case HIDE, RESET -> {}
-      default -> throw new UnsupportedOperationException("Unknown action " + getAction());
+      default -> throw new UnsupportedOperationException("Unknown action " + this.action);
     }
-
   }
 
   @Override
-  public void setAction(ActionType action) {
+  public @NotNull ActionType getAction() {
-    super.setAction(action);
+    return action;
   }
 
   @Override
@@ -105,7 +116,7 @@ public class LegacyTitlePacket extends GenericTitlePacket {
   @Override
   public String toString() {
     return "GenericTitlePacket{"
-        + "action=" + getAction()
+        + "action=" + action
         + ", component='" + component + '\''
         + ", fadeIn=" + fadeIn
         + ", stay=" + stay

proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/title/TitleActionbarPacket.java

@@ -22,13 +22,15 @@ import com.velocitypowered.proxy.connection.MinecraftSessionHandler;
 import com.velocitypowered.proxy.protocol.ProtocolUtils;
 import com.velocitypowered.proxy.protocol.packet.chat.ComponentHolder;
 import io.netty.buffer.ByteBuf;
+import org.jetbrains.annotations.NotNull;
 
 public class TitleActionbarPacket extends GenericTitlePacket {
 
   private ComponentHolder component;
 
-  public TitleActionbarPacket() {
+  @Override
-    setAction(ActionType.SET_ACTION_BAR);
+  public @NotNull ActionType getAction() {
+    return ActionType.SET_ACTION_BAR;
   }
 
   @Override

proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/title/TitleClearPacket.java

@@ -21,30 +21,37 @@ import com.velocitypowered.api.network.ProtocolVersion;
 import com.velocitypowered.proxy.connection.MinecraftSessionHandler;
 import com.velocitypowered.proxy.protocol.ProtocolUtils;
 import io.netty.buffer.ByteBuf;
+import org.jetbrains.annotations.NotNull;
 
 public class TitleClearPacket extends GenericTitlePacket {
 
+  private final ActionType action;
+
   public TitleClearPacket() {
-    setAction(ActionType.HIDE);
+    this(ActionType.HIDE);
+  }
+
+  public TitleClearPacket(ActionType action) {
+    if (action != ActionType.HIDE && action != ActionType.RESET) {
+      throw new IllegalArgumentException("TitleClearPacket only accepts the HIDE and RESET actions.");
+    }
+    this.action = action;
   }
 
   @Override
-  public void setAction(ActionType action) {
+  public @NotNull ActionType getAction() {
-    if (action != ActionType.HIDE && action != ActionType.RESET) {
+    return action;
-      throw new IllegalArgumentException("TitleClearPacket only accepts CLEAR and RESET actions");
-    }
-    super.setAction(action);
   }
 
   @Override
   public void encode(ByteBuf buf, ProtocolUtils.Direction direction, ProtocolVersion version) {
-    buf.writeBoolean(getAction() == ActionType.RESET);
+    buf.writeBoolean(this.action == ActionType.RESET);
   }
 
   @Override
   public String toString() {
     return "TitleClearPacket{"
-        + ", resetTimes=" + (getAction() == ActionType.RESET)
+        + ", resetTimes=" + (this.action == ActionType.RESET)
         + '}';
   }
 

proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/title/TitleSubtitlePacket.java

@@ -22,13 +22,15 @@ import com.velocitypowered.proxy.connection.MinecraftSessionHandler;
 import com.velocitypowered.proxy.protocol.ProtocolUtils;
 import com.velocitypowered.proxy.protocol.packet.chat.ComponentHolder;
 import io.netty.buffer.ByteBuf;
+import org.jetbrains.annotations.NotNull;
 
 public class TitleSubtitlePacket extends GenericTitlePacket {
 
   private ComponentHolder component;
 
-  public TitleSubtitlePacket() {
+  @Override
-    setAction(ActionType.SET_SUBTITLE);
+  public @NotNull ActionType getAction() {
+    return ActionType.SET_SUBTITLE;
   }
 
   @Override

proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/title/TitleTextPacket.java

@@ -22,13 +22,15 @@ import com.velocitypowered.proxy.connection.MinecraftSessionHandler;
 import com.velocitypowered.proxy.protocol.ProtocolUtils;
 import com.velocitypowered.proxy.protocol.packet.chat.ComponentHolder;
 import io.netty.buffer.ByteBuf;
+import org.jetbrains.annotations.NotNull;
 
 public class TitleTextPacket extends GenericTitlePacket {
 
   private ComponentHolder component;
 
-  public TitleTextPacket() {
+  @Override
-    setAction(ActionType.SET_TITLE);
+  public @NotNull ActionType getAction() {
+    return ActionType.SET_TITLE;
   }
 
   @Override

proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/title/TitleTimesPacket.java

@@ -21,6 +21,7 @@ import com.velocitypowered.api.network.ProtocolVersion;
 import com.velocitypowered.proxy.connection.MinecraftSessionHandler;
 import com.velocitypowered.proxy.protocol.ProtocolUtils;
 import io.netty.buffer.ByteBuf;
+import org.jetbrains.annotations.NotNull;
 
 public class TitleTimesPacket extends GenericTitlePacket {
 
@@ -28,8 +29,9 @@ public class TitleTimesPacket extends GenericTitlePacket {
   private int stay;
   private int fadeOut;
 
-  public TitleTimesPacket() {
+  @Override
-    setAction(ActionType.SET_TIMES);
+  public @NotNull ActionType getAction() {
+    return ActionType.SET_TIMES;
   }
 
   @Override