Tidy up bits of logic, use class for Nametags (#122)

* Tidy up bits of logic, use record for Nametags

* Few more bits of cleanup

* Some feedback

* More feedback

* Fix `#prefix()` and `#suffix()` record calls

* Fixup logical error

* `nameTag` -> `nametag`

* Make TabPlayer#getNametag returns TabPlayer.Nametag

---------

Co-authored-by: AlexDev_ <alessandrodalfovo2003@gmail.com>

src/main/java/net/william278/velocitab/Velocitab.java

@@ -106,7 +106,7 @@ public class Velocitab {
         server.getScheduler().tasksByPlugin(this).forEach(ScheduledTask::cancel);
         disableScoreboardManager();
         disableTabList();
-        getLuckPermsHook().ifPresent(LuckPermsHook::close);
+        getLuckPermsHook().ifPresent(LuckPermsHook::closeEvent);
         VelocitabAPI.unregister();
         logger.info("Successfully disabled Velocitab");
     }

src/main/java/net/william278/velocitab/api/VelocitabAPI.java

@@ -31,12 +31,13 @@ import org.jetbrains.annotations.Nullable;
 
 import java.util.Optional;
 
-@SuppressWarnings("unused")
-
 /**
  * The Velocitab API class.
+ * <p>
  * Retrieve an instance of the API class via {@link #getInstance()}.
- */ public class VelocitabAPI {
+ */
+@SuppressWarnings("unused")
+public class VelocitabAPI {
 
     // Instance of the plugin
     private final Velocitab plugin;
@@ -168,7 +169,7 @@ import java.util.Optional;
      *
      * @param player the player for whom to retrieve the server group
      * @return the name of the server group that the player is connected to,
-     *         or an empty string if the player is not in a group server
+     * or an empty string if the player is not in a group server
      */
     @NotNull
     public String getServerGroup(@NotNull Player player) {

src/main/java/net/william278/velocitab/config/Formatter.java

@@ -84,6 +84,7 @@ public enum Formatter {
         return LegacyComponentSerializer.legacySection()
                 .serialize(format(text, player, plugin));
     }
+
     @NotNull
     public String escape(@NotNull String text) {
         return escaper.apply(text);

src/main/java/net/william278/velocitab/hook/LuckPermsHook.java

@@ -49,12 +49,13 @@ public class LuckPermsHook extends Hook {
     public LuckPermsHook(@NotNull Velocitab plugin) throws IllegalStateException {
         super(plugin);
         this.api = LuckPermsProvider.get();
-        lastUpdate = new HashMap<>();
-        event = api.getEventBus().subscribe(plugin, UserDataRecalculateEvent.class, this::onLuckPermsGroupUpdate);
-
+        this.lastUpdate = new HashMap<>();
+        this.event = api.getEventBus().subscribe(
+                plugin, UserDataRecalculateEvent.class, this::onLuckPermsGroupUpdate
+        );
     }
 
-    public void close() {
+    public void closeEvent() {
         event.close();
     }
 
@@ -68,6 +69,7 @@ public class LuckPermsHook extends Hook {
         if (metaData.getPrimaryGroup() == null) {
             return Role.DEFAULT_ROLE;
         }
+
         final Optional<Group> group = getGroup(metaData.getPrimaryGroup());
         return new Role(
                 group.map(this::getGroupWeight).orElse(Role.DEFAULT_WEIGHT),
@@ -129,5 +131,4 @@ public class LuckPermsHook extends Hook {
         plugin.getTabList().updatePlayer(player);
     }
 
-
 }

src/main/java/net/william278/velocitab/hook/PAPIProxyBridgeHook.java

@@ -40,5 +40,4 @@ public class PAPIProxyBridgeHook extends Hook {
         return api.formatPlaceholders(input, player.getUniqueId());
     }
 
-
 }

src/main/java/net/william278/velocitab/packet/Protocol340Adapter.java

@@ -53,7 +53,7 @@ public class Protocol340Adapter extends TeamsPacketAdapter {
             ProtocolUtils.writeString(byteBuf, packet.prefix());
             ProtocolUtils.writeString(byteBuf, packet.suffix());
             byteBuf.writeByte(UpdateTeamsPacket.FriendlyFlag.toBitMask(packet.friendlyFlags()));
-            ProtocolUtils.writeString(byteBuf, packet.nameTagVisibility().id());
+            ProtocolUtils.writeString(byteBuf, packet.nametagVisibility().id());
             ProtocolUtils.writeString(byteBuf, packet.collisionRule().id());
             byteBuf.writeByte(packet.color());
         }

src/main/java/net/william278/velocitab/packet/Protocol403Adapter.java

@@ -81,7 +81,7 @@ public class Protocol403Adapter extends TeamsPacketAdapter {
         if (mode == UpdateTeamsPacket.UpdateMode.CREATE_TEAM || mode == UpdateTeamsPacket.UpdateMode.UPDATE_INFO) {
             ProtocolUtils.writeString(byteBuf, getChatString(packet.displayName()));
             byteBuf.writeByte(UpdateTeamsPacket.FriendlyFlag.toBitMask(packet.friendlyFlags()));
-            ProtocolUtils.writeString(byteBuf, packet.nameTagVisibility().id());
+            ProtocolUtils.writeString(byteBuf, packet.nametagVisibility().id());
             ProtocolUtils.writeString(byteBuf, packet.collisionRule().id());
             byteBuf.writeByte(packet.color());
             ProtocolUtils.writeString(byteBuf, getRGBChat(packet.prefix()));

src/main/java/net/william278/velocitab/packet/Protocol48Adapter.java

@@ -53,7 +53,7 @@ public class Protocol48Adapter extends TeamsPacketAdapter {
             ProtocolUtils.writeString(byteBuf, packet.prefix());
             ProtocolUtils.writeString(byteBuf, packet.suffix());
             byteBuf.writeByte(UpdateTeamsPacket.FriendlyFlag.toBitMask(packet.friendlyFlags()));
-            ProtocolUtils.writeString(byteBuf, packet.nameTagVisibility().id());
+            ProtocolUtils.writeString(byteBuf, packet.nametagVisibility().id());
             byteBuf.writeByte(packet.color());
         }
         if (mode == UpdateTeamsPacket.UpdateMode.CREATE_TEAM || mode == UpdateTeamsPacket.UpdateMode.ADD_PLAYERS || mode == UpdateTeamsPacket.UpdateMode.REMOVE_PLAYERS) {

src/main/java/net/william278/velocitab/packet/ScoreboardManager.java

@@ -33,23 +33,21 @@ import org.slf4j.event.Level;
 
 import java.util.*;
 import java.util.concurrent.ConcurrentHashMap;
-import java.util.regex.Pattern;
 
 import static com.velocitypowered.api.network.ProtocolVersion.*;
 
 public class ScoreboardManager {
 
-    private static final String NAMETAG_DELIMITER = ":::";
     private PacketRegistration<UpdateTeamsPacket> packetRegistration;
     private final Velocitab plugin;
     private final Set<TeamsPacketAdapter> versions;
     private final Map<UUID, String> createdTeams;
-    private final Map<String, String> nameTags;
+    private final Map<String, TabPlayer.Nametag> nametags;
 
     public ScoreboardManager(@NotNull Velocitab velocitab) {
         this.plugin = velocitab;
         this.createdTeams = new ConcurrentHashMap<>();
-        this.nameTags = new ConcurrentHashMap<>();
+        this.nametags = new ConcurrentHashMap<>();
         this.versions = new HashSet<>();
         this.registerVersions();
     }
@@ -98,7 +96,6 @@ public class ScoreboardManager {
         }
 
         final UpdateTeamsPacket packet = UpdateTeamsPacket.removeTeam(plugin, teamName);
-
         siblings.forEach(server -> server.getPlayersConnected().forEach(connected -> {
             final boolean canSee = plugin.getVanishManager().canSee(connected.getUsername(), player.getUsername());
 
@@ -122,23 +119,20 @@ public class ScoreboardManager {
 
         final RegisteredServer serverInfo = optionalServerConnection.get().getServer();
         final List<RegisteredServer> siblings = plugin.getTabList().getGroupServers(serverInfo.getServerInfo().getName());
-
         final String teamName = createdTeams.get(player.getUniqueId());
         if (teamName == null) {
             return;
         }
 
-        final String nametag = nameTags.getOrDefault(teamName, "");
-        if (nametag.isEmpty()) {
-            return;
-        }
-
-        final String[] split = nametag.split(NAMETAG_DELIMITER, 2);
-        final String prefix = split[0];
-        final String suffix = split.length > 1 ? split[1] : "";
-        final UpdateTeamsPacket packet = UpdateTeamsPacket.create(plugin, createdTeams.get(player.getUniqueId()), "", prefix, suffix, player.getUsername());
-
-        siblings.forEach(server -> server.getPlayersConnected().stream().filter(p -> p != player).forEach(connected -> dispatchPacket(packet, connected)));
+        final Optional<TabPlayer.Nametag> cachedTag = Optional.ofNullable(nametags.getOrDefault(teamName, null));
+        cachedTag.ifPresent(nametag -> {
+            final UpdateTeamsPacket packet = UpdateTeamsPacket.create(
+                    plugin, createdTeams.get(player.getUniqueId()),
+                    "", nametag, player.getUsername()
+            );
+            siblings.forEach(server -> server.getPlayersConnected().stream().filter(p -> p != player)
+                    .forEach(connected -> dispatchPacket(packet, connected)));
+        });
     }
 
     public void updateRole(@NotNull Player player, @NotNull String role) {
@@ -149,23 +143,27 @@ public class ScoreboardManager {
 
         final String name = player.getUsername();
         final TabPlayer tabPlayer = plugin.getTabList().getTabPlayer(player).orElseThrow();
-
-        tabPlayer.getNametag(plugin).thenAccept(nametag -> {
-            final String[] split = nametag.split(Pattern.quote(player.getUsername()), 2);
-            final String prefix = split[0];
-            final String suffix = split.length > 1 ? split[1] : "";
-
+        tabPlayer.getNametag(plugin).thenAccept(newTag -> {
             if (!createdTeams.getOrDefault(player.getUniqueId(), "").equals(role)) {
                 if (createdTeams.containsKey(player.getUniqueId())) {
-                    dispatchGroupPacket(UpdateTeamsPacket.removeTeam(plugin, createdTeams.get(player.getUniqueId())), player);
+                    dispatchGroupPacket(
+                            UpdateTeamsPacket.removeTeam(plugin, createdTeams.get(player.getUniqueId())),
+                            player
+                    );
                 }
 
                 createdTeams.put(player.getUniqueId(), role);
-                this.nameTags.put(role, prefix + NAMETAG_DELIMITER + suffix);
-                dispatchGroupPacket(UpdateTeamsPacket.create(plugin, role, "", prefix, suffix, name), player);
-            } else if (!this.nameTags.getOrDefault(role, "").equals(prefix + NAMETAG_DELIMITER + suffix)) {
-                this.nameTags.put(role, prefix + NAMETAG_DELIMITER + suffix);
-                dispatchGroupPacket(UpdateTeamsPacket.changeNameTag(plugin, role, prefix, suffix), player);
+                this.nametags.put(role, newTag);
+                dispatchGroupPacket(
+                        UpdateTeamsPacket.create(plugin, role, "", newTag, name),
+                        player
+                );
+            } else if (this.nametags.containsKey(role) && this.nametags.get(role).equals(newTag)) {
+                this.nametags.put(role, newTag);
+                dispatchGroupPacket(
+                        UpdateTeamsPacket.changeNametag(plugin, role, newTag),
+                        player
+                );
             }
         }).exceptionally(e -> {
             plugin.log(Level.ERROR, "Failed to update role for " + player.getUsername(), e);
@@ -192,7 +190,6 @@ public class ScoreboardManager {
                 .toList();
 
         final List<String> roles = new ArrayList<>();
-
         players.forEach(p -> {
             if (p == player || !p.isActive()) {
                 return;
@@ -211,18 +208,17 @@ public class ScoreboardManager {
             if (roles.contains(role)) {
                 return;
             }
-
             roles.add(role);
 
-            final String nametag = nameTags.getOrDefault(role, "");
-            if (nametag.isEmpty()) {
-                return;
+            // Send packet
+            final TabPlayer.Nametag tag = nametags.get(role);
+            if (tag != null) {
+                final TabPlayer.Nametag nametag = nametags.get(role);
+                final UpdateTeamsPacket packet = UpdateTeamsPacket.create(
+                        plugin, role, "", nametag, p.getUsername()
+                );
+                dispatchPacket(packet, player);
             }
-
-            final String[] split = nametag.split(NAMETAG_DELIMITER, 2);
-            final String prefix = split[0];
-            final String suffix = split.length > 1 ? split[1] : "";
-            dispatchPacket(UpdateTeamsPacket.create(plugin, role, "", prefix, suffix, p.getUsername()), player);
         });
     }
 
@@ -301,14 +297,12 @@ public class ScoreboardManager {
      * This method updates the player's scoreboard to reflect the vanish status of another player.
      *
      * @param tabPlayer The TabPlayer object representing the player whose scoreboard will be updated.
-     * @param target The TabPlayer object representing the player whose vanish status will be reflected.
-     * @param canSee A boolean indicating whether the player can see the target player.
+     * @param target    The TabPlayer object representing the player whose vanish status will be reflected.
+     * @param canSee    A boolean indicating whether the player can see the target player.
      */
     public void recalculateVanishForPlayer(TabPlayer tabPlayer, TabPlayer target, boolean canSee) {
         final Player player = tabPlayer.getPlayer();
-
         final String team = createdTeams.get(target.getPlayer().getUniqueId());
-
         if (team == null) {
             return;
         }
@@ -317,18 +311,14 @@ public class ScoreboardManager {
         dispatchPacket(removeTeam, player);
 
         if (canSee) {
-            final String nametag = nameTags.getOrDefault(team, "");
-            if (nametag.isEmpty()) {
-                return;
+            final TabPlayer.Nametag tag = nametags.get(team);
+            if (tag != null) {
+                final UpdateTeamsPacket addTeam = UpdateTeamsPacket.create(
+                        plugin, team, "", tag, target.getPlayer().getUsername()
+                );
+                dispatchPacket(addTeam, player);
             }
-
-            final String[] split = nametag.split(NAMETAG_DELIMITER, 2);
-            final String prefix = split[0];
-            final String suffix = split.length > 1 ? split[1] : "";
-            final UpdateTeamsPacket addTeam = UpdateTeamsPacket.create(plugin, team, "", prefix, suffix, target.getPlayer().getUsername());
-            dispatchPacket(addTeam, player);
         }
     }
 
-
 }

src/main/java/net/william278/velocitab/packet/UpdateTeamsPacket.java

@@ -29,6 +29,7 @@ import lombok.experimental.Accessors;
 import net.kyori.adventure.text.Component;
 import net.kyori.adventure.text.serializer.legacy.LegacyComponentSerializer;
 import net.william278.velocitab.Velocitab;
+import net.william278.velocitab.player.TabPlayer;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
@@ -51,7 +52,7 @@ public class UpdateTeamsPacket implements MinecraftPacket {
     private UpdateMode mode;
     private String displayName;
     private List<FriendlyFlag> friendlyFlags;
-    private NameTagVisibility nameTagVisibility;
+    private NametagVisibility nametagVisibility;
     private CollisionRule collisionRule;
     private int color;
     private String prefix;
@@ -64,40 +65,43 @@ public class UpdateTeamsPacket implements MinecraftPacket {
 
     @NotNull
     protected static UpdateTeamsPacket create(@NotNull Velocitab plugin, @NotNull String teamName,
-                                              @NotNull String displayName, @Nullable String prefix,
-                                              @Nullable String suffix, @NotNull String... teamMembers) {
+                                              @NotNull String displayName, @NotNull TabPlayer.Nametag nametag,
+                                              @NotNull String... teamMembers) {
         return new UpdateTeamsPacket(plugin)
                 .teamName(teamName.length() > 16 ? teamName.substring(0, 16) : teamName)
                 .mode(UpdateMode.CREATE_TEAM)
                 .displayName(displayName)
                 .friendlyFlags(List.of(FriendlyFlag.CAN_HURT_FRIENDLY))
-                .nameTagVisibility(isNametagPresent(prefix, suffix, plugin) ? NameTagVisibility.ALWAYS : NameTagVisibility.NEVER)
+                .nametagVisibility(isNametagPresent(nametag, plugin) ? NametagVisibility.ALWAYS : NametagVisibility.NEVER)
                 .collisionRule(CollisionRule.ALWAYS)
-                .color(getLastColor(prefix))
-                .prefix(prefix == null ? "" : prefix)
-                .suffix(suffix == null ? "" : suffix)
+                .color(getLastColor(nametag.getPrefix()))
+                .prefix(nametag.getPrefix() == null ? "" : nametag.getPrefix())
+                .suffix(nametag.getSuffix() == null ? "" : nametag.getSuffix())
                 .entities(Arrays.asList(teamMembers));
     }
 
-    private static boolean isNametagPresent(@Nullable String prefix, @Nullable String suffix, @NotNull Velocitab plugin) {
-        if (!plugin.getSettings().isRemoveNametags()) return true;
+    private static boolean isNametagPresent(@NotNull TabPlayer.Nametag nametag, @NotNull Velocitab plugin) {
+        if (!plugin.getSettings().isRemoveNametags()) {
+            return true;
+        }
 
-        return prefix != null && !prefix.isEmpty() || suffix != null && !suffix.isEmpty();
+        return nametag.getPrefix() != null && !nametag.getPrefix().isEmpty()
+                || nametag.getSuffix() != null && !nametag.getSuffix().isEmpty();
     }
 
     @NotNull
-    protected static UpdateTeamsPacket changeNameTag(@NotNull Velocitab plugin, @NotNull String teamName,
-                                                     @Nullable String prefix, @Nullable String suffix) {
+    protected static UpdateTeamsPacket changeNametag(@NotNull Velocitab plugin, @NotNull String teamName,
+                                                     @NotNull TabPlayer.Nametag nametag) {
         return new UpdateTeamsPacket(plugin)
                 .teamName(teamName.length() > 16 ? teamName.substring(0, 16) : teamName)
                 .mode(UpdateMode.UPDATE_INFO)
                 .displayName(teamName)
                 .friendlyFlags(List.of(FriendlyFlag.CAN_HURT_FRIENDLY))
-                .nameTagVisibility(isNametagPresent(prefix, suffix, plugin) ? NameTagVisibility.ALWAYS : NameTagVisibility.NEVER)
+                .nametagVisibility(isNametagPresent(nametag, plugin) ? NametagVisibility.ALWAYS : NametagVisibility.NEVER)
                 .collisionRule(CollisionRule.ALWAYS)
-                .color(getLastColor(prefix))
-                .prefix(prefix == null ? "" : prefix)
-                .suffix(suffix == null ? "" : suffix);
+                .color(getLastColor(nametag.getPrefix()))
+                .prefix(nametag.getPrefix() == null ? "" : nametag.getPrefix())
+                .suffix(nametag.getSuffix() == null ? "" : nametag.getSuffix());
     }
 
     @NotNull
@@ -259,7 +263,7 @@ public class UpdateTeamsPacket implements MinecraftPacket {
         }
     }
 
-    public enum NameTagVisibility {
+    public enum NametagVisibility {
         ALWAYS("always"),
         NEVER("never"),
         HIDE_FOR_OTHER_TEAMS("hideForOtherTeams"),
@@ -267,7 +271,7 @@ public class UpdateTeamsPacket implements MinecraftPacket {
 
         private final String id;
 
-        NameTagVisibility(@NotNull String id) {
+        NametagVisibility(@NotNull String id) {
             this.id = id;
         }
 
@@ -277,7 +281,7 @@ public class UpdateTeamsPacket implements MinecraftPacket {
         }
 
         @NotNull
-        public static NameTagVisibility byId(@Nullable String id) {
+        public static NametagVisibility byId(@Nullable String id) {
             return id == null ? ALWAYS : Arrays.stream(values())
                     .filter(visibility -> visibility.id.equals(id))
                     .findFirst()

src/main/java/net/william278/velocitab/player/TabPlayer.java

@@ -20,6 +20,7 @@
 package net.william278.velocitab.player;
 
 import com.velocitypowered.api.proxy.Player;
+import lombok.AllArgsConstructor;
 import lombok.Getter;
 import lombok.Setter;
 import net.kyori.adventure.text.Component;
@@ -32,6 +33,7 @@ import org.jetbrains.annotations.Nullable;
 
 import java.util.Optional;
 import java.util.concurrent.CompletableFuture;
+import java.util.regex.Pattern;
 
 public final class TabPlayer implements Comparable<TabPlayer> {
     private final Player player;
@@ -125,9 +127,10 @@ public final class TabPlayer implements Comparable<TabPlayer> {
     }
 
     @NotNull
-    public CompletableFuture<String> getNametag(@NotNull Velocitab plugin) {
+    public CompletableFuture<Nametag> getNametag(@NotNull Velocitab plugin) {
         final String serverGroup = plugin.getSettings().getServerGroup(getServerName());
-        return Placeholder.replace(plugin.getSettings().getNametag(serverGroup), plugin, this);
+        return Placeholder.replace(plugin.getSettings().getNametag(serverGroup), plugin, this)
+                .thenApply(n -> new Nametag(n, player));
     }
 
     @NotNull
@@ -192,4 +195,29 @@ public final class TabPlayer implements Comparable<TabPlayer> {
         return obj instanceof TabPlayer other && player.getUniqueId().equals(other.player.getUniqueId());
     }
 
+    /**
+     * Represents a nametag to be displayed above a player, with prefix & suffix
+     */
+    @Getter
+    @AllArgsConstructor
+    public static class Nametag {
+        private final String prefix;
+        private final String suffix;
+
+        private Nametag(@NotNull String tag, @NotNull Player player) {
+            final String[] split = tag.split(Pattern.quote(player.getUsername()), 2);
+            this.prefix = split[0];
+            this.suffix = split.length > 1 ? split[1] : "";
+        }
+
+        @Override
+        public boolean equals(Object obj) {
+            if (!(obj instanceof Nametag other)) {
+                return false;
+            }
+            return (prefix != null && prefix.equals(other.prefix)) &&
+                    (suffix != null && suffix.equals(other.suffix));
+        }
+
+    }
 }

src/main/java/net/william278/velocitab/sorting/SortingManager.java

@@ -97,7 +97,6 @@ public class SortingManager {
         if (charList.isEmpty()) {
             charList.add((char) 0);
         }
-
         return charList.stream().map(String::valueOf).collect(Collectors.joining()) + decimalChar;
     }
 }

src/main/java/net/william278/velocitab/vanish/VanishManager.java

@@ -67,7 +67,7 @@ public class VanishManager {
     public void unVanishPlayer(@NotNull Player player) {
         final Optional<TabPlayer> tabPlayer = plugin.getTabList().getTabPlayer(player);
         if (tabPlayer.isEmpty()) {
-            plugin.log("Failed to unVanish player " + player.getUsername() + " as they are not in the tab list");
+            plugin.log("Failed to un-vanish player " + player.getUsername() + " as they are not in the tab list");
             return;
         }