From e00715ceabf9246cbfc63e72a559a1953f5fc156 Mon Sep 17 00:00:00 2001 From: Tim203 Date: Mon, 9 Nov 2020 10:34:27 +0100 Subject: [PATCH] Fixed some issues related to Scoreboards (#1446) * Fixes some issues related to Scoreboard Teams * The cached Score info should update no matter what kind of update the Team got. * Team entities specified at the create Team packet should also be checked if they exist as Score in the registered Objectives * Rewrote some Scoreboard code and fixed various issues * Minor formatting changes --- .../connector/entity/PlayerEntity.java | 17 +- .../JavaDisplayScoreboardTranslator.java | 5 +- .../JavaScoreboardObjectiveTranslator.java | 17 +- .../connector/scoreboard/Objective.java | 62 ++++--- .../geysermc/connector/scoreboard/Score.java | 99 +++++++++--- .../connector/scoreboard/Scoreboard.java | 135 +++++++++------- .../geysermc/connector/scoreboard/Team.java | 152 +++++++++++++++--- 7 files changed, 333 insertions(+), 154 deletions(-) diff --git a/connector/src/main/java/org/geysermc/connector/entity/PlayerEntity.java b/connector/src/main/java/org/geysermc/connector/entity/PlayerEntity.java index 390110d18..bfadd83cd 100644 --- a/connector/src/main/java/org/geysermc/connector/entity/PlayerEntity.java +++ b/connector/src/main/java/org/geysermc/connector/entity/PlayerEntity.java @@ -27,7 +27,6 @@ package org.geysermc.connector.entity; import com.github.steveice10.mc.auth.data.GameProfile; import com.github.steveice10.mc.protocol.data.game.entity.metadata.EntityMetadata; -import com.github.steveice10.mc.protocol.data.game.scoreboard.NameTagVisibility; import com.github.steveice10.mc.protocol.data.message.TextMessage; import com.github.steveice10.opennbt.tag.builtin.CompoundTag; import com.nukkitx.math.vector.Vector3f; @@ -235,18 +234,12 @@ public class PlayerEntity extends LivingEntity { } Team team = session.getWorldCache().getScoreboard().getTeamFor(username); if (team != null) { - // Cover different visibility settings - if (team.getNameTagVisibility() == NameTagVisibility.NEVER) { - metadata.put(EntityData.NAMETAG, ""); - } else if (team.getNameTagVisibility() == NameTagVisibility.HIDE_FOR_OTHER_TEAMS && - !team.getEntities().contains(session.getPlayerEntity().getUsername())) { - metadata.put(EntityData.NAMETAG, ""); - } else if (team.getNameTagVisibility() == NameTagVisibility.HIDE_FOR_OWN_TEAM && - team.getEntities().contains(session.getPlayerEntity().getUsername())) { - metadata.put(EntityData.NAMETAG, ""); - } else { - metadata.put(EntityData.NAMETAG, team.getPrefix() + MessageUtils.toChatColor(team.getColor()) + username + team.getSuffix()); + String displayName = ""; + if (team.isVisibleFor(session.getPlayerEntity().getUsername())) { + displayName = MessageUtils.toChatColor(team.getColor()) + username; + displayName = team.getCurrentData().getDisplayName(displayName); } + metadata.put(EntityData.NAMETAG, displayName); } } diff --git a/connector/src/main/java/org/geysermc/connector/network/translators/java/scoreboard/JavaDisplayScoreboardTranslator.java b/connector/src/main/java/org/geysermc/connector/network/translators/java/scoreboard/JavaDisplayScoreboardTranslator.java index 3ee174d7a..b42ac78e4 100644 --- a/connector/src/main/java/org/geysermc/connector/network/translators/java/scoreboard/JavaDisplayScoreboardTranslator.java +++ b/connector/src/main/java/org/geysermc/connector/network/translators/java/scoreboard/JavaDisplayScoreboardTranslator.java @@ -36,8 +36,7 @@ public class JavaDisplayScoreboardTranslator extends PacketTranslator cachedData.updateTime || + (currentData.team != null && currentData.team.shouldUpdate()); + } + + public void update(String objectiveName) { + if (cachedData == null) { + cachedData = new ScoreData(); + cachedData.updateType = UpdateType.ADD; + if (currentData.updateType == UpdateType.REMOVE) { + cachedData.updateType = UpdateType.REMOVE; + } + } else { + cachedData.updateType = currentData.updateType; + } + + cachedData.updateTime = currentData.updateTime; + cachedData.team = currentData.team; + cachedData.score = currentData.score; + + String name = this.name; + if (cachedData.team != null) { + cachedData.team.prepareUpdate(); + name = cachedData.team.getDisplayName(name); + } + cachedInfo = new ScoreInfo(id, objectiveName, cachedData.score, name); + } + + @Getter + public static final class ScoreData { + protected UpdateType updateType; + protected long updateTime; + + private Team team; + private int score; + + protected ScoreData() { + updateType = UpdateType.ADD; + } } } diff --git a/connector/src/main/java/org/geysermc/connector/scoreboard/Scoreboard.java b/connector/src/main/java/org/geysermc/connector/scoreboard/Scoreboard.java index 732a056eb..8eaa2e277 100644 --- a/connector/src/main/java/org/geysermc/connector/scoreboard/Scoreboard.java +++ b/connector/src/main/java/org/geysermc/connector/scoreboard/Scoreboard.java @@ -43,7 +43,7 @@ import java.util.concurrent.atomic.AtomicLong; import static org.geysermc.connector.scoreboard.UpdateType.*; @Getter -public class Scoreboard { +public final class Scoreboard { private final GeyserSession session; private final GeyserLogger logger; private final AtomicLong nextId = new AtomicLong(0); @@ -51,7 +51,8 @@ public class Scoreboard { private final Map objectives = new ConcurrentHashMap<>(); private final Map teams = new HashMap<>(); - private int lastScoreCount = 0; + private int lastAddScoreCount = 0; + private int lastRemoveScoreCount = 0; public Scoreboard(GeyserSession session) { this.session = session; @@ -59,19 +60,21 @@ public class Scoreboard { } public Objective registerNewObjective(String objectiveId, boolean active) { - if (active || objectives.containsKey(objectiveId)) { - return objectives.get(objectiveId); + Objective objective = objectives.get(objectiveId); + if (active || objective != null) { + return objective; } - Objective objective = new Objective(this, objectiveId); + objective = new Objective(this, objectiveId); objectives.put(objectiveId, objective); return objective; } - public Objective registerNewObjective(String objectiveId, ScoreboardPosition displaySlot) { + public Objective displayObjective(String objectiveId, ScoreboardPosition displaySlot) { Objective objective = objectives.get(objectiveId); if (objective != null) { if (!objective.isActive()) { objective.setActive(displaySlot); + removeOldObjectives(objective); return objective; } despawnObjective(objective); @@ -79,9 +82,21 @@ public class Scoreboard { objective = new Objective(this, objectiveId, displaySlot, "unknown", 0); objectives.put(objectiveId, objective); + removeOldObjectives(objective); return objective; } + private void removeOldObjectives(Objective newObjective) { + for (Objective next : objectives.values()) { + if (next.getId() == newObjective.getId()) { + continue; + } + if (next.getDisplaySlot() == newObjective.getDisplaySlot()) { + next.setUpdateType(REMOVE); + } + } + } + public Team registerNewTeam(String teamName, Set players) { Team team = teams.get(teamName); if (team != null) { @@ -89,7 +104,7 @@ public class Scoreboard { return team; } - team = new Team(this, teamName).setEntities(players); + team = new Team(this, teamName).addEntities(players); teams.put(teamName, team); return team; } @@ -117,8 +132,9 @@ public class Scoreboard { } public void onUpdate() { - List addScores = new ArrayList<>(getLastScoreCount()); - List removeScores = new ArrayList<>(getLastScoreCount()); + List addScores = new ArrayList<>(getLastAddScoreCount()); + List removeScores = new ArrayList<>(getLastRemoveScoreCount()); + List removedObjectives = new ArrayList<>(); for (Objective objective : objectives.values()) { if (!objective.isActive()) { @@ -129,65 +145,58 @@ public class Scoreboard { // hearts can't hold teams, so we treat them differently if (objective.getType() == 1) { for (Score score : objective.getScores().values()) { - if (score.getUpdateType() == NOTHING) { - continue; - } + boolean update = score.shouldUpdate(); - boolean update = score.getUpdateType() == UPDATE; if (update) { - score.update(); + score.update(objective.getObjectiveName()); } - if (score.getUpdateType() == ADD || update) { + if (score.getUpdateType() != REMOVE && update) { addScores.add(score.getCachedInfo()); } - if (score.getUpdateType() == REMOVE || update) { + if (score.getUpdateType() != ADD && update) { removeScores.add(score.getCachedInfo()); } } continue; } - boolean globalUpdate = objective.getUpdateType() == UPDATE; - boolean globalAdd = objective.getUpdateType() == ADD; - boolean globalRemove = objective.getUpdateType() == REMOVE; + boolean objectiveUpdate = objective.getUpdateType() == UPDATE; + boolean objectiveAdd = objective.getUpdateType() == ADD; + boolean objectiveRemove = objective.getUpdateType() == REMOVE; for (Score score : objective.getScores().values()) { Team team = score.getTeam(); - boolean add = globalAdd || globalUpdate; - boolean remove = globalRemove; - boolean teamChanged = false; + boolean add = objectiveAdd || objectiveUpdate; + boolean remove = false; if (team != null) { if (team.getUpdateType() == REMOVE || !team.hasEntity(score.getName())) { score.setTeam(null); - teamChanged = true; + add = true; + remove = true; } - - teamChanged |= team.getUpdateType() == UPDATE; - - add |= team.getUpdateType() == ADD || team.getUpdateType() == UPDATE; - remove |= team.getUpdateType() != NOTHING; } - add |= score.getUpdateType() == ADD || score.getUpdateType() == UPDATE; - remove |= score.getUpdateType() == REMOVE || score.getUpdateType() == UPDATE; + add |= score.shouldUpdate(); + remove |= score.shouldUpdate(); - if (score.getUpdateType() == REMOVE || globalRemove) { + if (score.getUpdateType() == REMOVE || objectiveRemove) { add = false; } - if (score.getUpdateType() == ADD) { + if (score.getUpdateType() == ADD || objectiveRemove) { remove = false; } - if (score.getUpdateType() == ADD || score.getUpdateType() == UPDATE || teamChanged) { - score.update(); + if (score.shouldUpdate()) { + score.update(objective.getObjectiveName()); } if (add) { addScores.add(score.getCachedInfo()); } + if (remove) { removeScores.add(score.getCachedInfo()); } @@ -200,17 +209,17 @@ public class Scoreboard { score.setUpdateType(NOTHING); } - if (globalRemove || globalUpdate) { + if (objectiveRemove) { + removedObjectives.add(objective); + } + + if (objectiveUpdate) { RemoveObjectivePacket removeObjectivePacket = new RemoveObjectivePacket(); removeObjectivePacket.setObjectiveId(objective.getObjectiveName()); session.sendUpstreamPacket(removeObjectivePacket); - if (globalRemove) { - objectives.remove(objective.getObjectiveName()); // now we can deregister - objective.removed(); - } } - if ((globalAdd || globalUpdate) && !globalRemove) { + if ((objectiveAdd || objectiveUpdate) && !objectiveRemove) { SetDisplayObjectivePacket displayObjectivePacket = new SetDisplayObjectivePacket(); displayObjectivePacket.setObjectiveId(objective.getObjectiveName()); displayObjectivePacket.setDisplayName(objective.getDisplayName()); @@ -223,6 +232,20 @@ public class Scoreboard { objective.setUpdateType(NOTHING); } + Iterator teamIterator = teams.values().iterator(); + while (teamIterator.hasNext()) { + Team current = teamIterator.next(); + + switch (current.getUpdateType()) { + case ADD: + case UPDATE: + current.markUpdated(); + break; + case REMOVE: + teamIterator.remove(); + } + } + if (!removeScores.isEmpty()) { SetScorePacket setScorePacket = new SetScorePacket(); setScorePacket.setAction(SetScorePacket.Action.REMOVE); @@ -237,37 +260,27 @@ public class Scoreboard { session.sendUpstreamPacket(setScorePacket); } - lastScoreCount = addScores.size(); + // prevents crashes in some cases + for (Objective objective : removedObjectives) { + despawnObjective(objective); + } + + lastAddScoreCount = addScores.size(); + lastRemoveScoreCount = removeScores.size(); } public void despawnObjective(Objective objective) { + objectives.remove(objective.getObjectiveName()); + objective.removed(); + RemoveObjectivePacket removeObjectivePacket = new RemoveObjectivePacket(); removeObjectivePacket.setObjectiveId(objective.getObjectiveName()); session.sendUpstreamPacket(removeObjectivePacket); - objectives.remove(objective.getDisplayName()); - - List toRemove = new ArrayList<>(); - for (String identifier : objective.getScores().keySet()) { - Score score = objective.getScores().get(identifier); - toRemove.add(new ScoreInfo( - score.getId(), score.getObjective().getObjectiveName(), - 0, "" - )); - } - - objective.removed(); - - if (!toRemove.isEmpty()) { - SetScorePacket setScorePacket = new SetScorePacket(); - setScorePacket.setAction(SetScorePacket.Action.REMOVE); - setScorePacket.setInfos(toRemove); - session.sendUpstreamPacket(setScorePacket); - } } public Team getTeamFor(String entity) { for (Team team : teams.values()) { - if (team.getEntities().contains(entity)) { + if (team.hasEntity(entity)) { return team; } } diff --git a/connector/src/main/java/org/geysermc/connector/scoreboard/Team.java b/connector/src/main/java/org/geysermc/connector/scoreboard/Team.java index a073e2e99..377a15f1e 100644 --- a/connector/src/main/java/org/geysermc/connector/scoreboard/Team.java +++ b/connector/src/main/java/org/geysermc/connector/scoreboard/Team.java @@ -28,6 +28,7 @@ package org.geysermc.connector.scoreboard; import com.github.steveice10.mc.protocol.data.game.scoreboard.NameTagVisibility; import com.github.steveice10.mc.protocol.data.game.scoreboard.TeamColor; import it.unimi.dsi.fastutil.objects.ObjectOpenHashSet; +import lombok.AccessLevel; import lombok.Getter; import lombok.Setter; import lombok.experimental.Accessors; @@ -36,62 +37,90 @@ import java.util.ArrayList; import java.util.List; import java.util.Set; -@Getter @Setter +@Getter @Accessors(chain = true) -public class Team { +public final class Team { private final Scoreboard scoreboard; private final String id; - private UpdateType updateType = UpdateType.ADD; - private String name; + @Getter(AccessLevel.NONE) + private final Set entities; + @Setter private NameTagVisibility nameTagVisibility; + @Setter private TeamColor color; - private NameTagVisibility nameTagVisibility; - private String prefix; - private TeamColor color; - private String suffix; - private Set entities = new ObjectOpenHashSet<>(); + private TeamData currentData; + private TeamData cachedData; + + private boolean updating; public Team(Scoreboard scoreboard, String id) { this.scoreboard = scoreboard; this.id = id; + currentData = new TeamData(); + entities = new ObjectOpenHashSet<>(); } - public void addEntities(String... names) { - List added = new ArrayList<>(); - for (String name : names) { - if (entities.add(name)) { - added.add(name); - } + private void checkAddedEntities(List added) { + if (added.size() == 0) { + return; } - setUpdateType(UpdateType.UPDATE); + // we don't have to change the updateType, + // because the scores itself need updating, not the team for (Objective objective : scoreboard.getObjectives().values()) { - for (Score score : objective.getScores().values()) { - if (added.contains(score.getName())) { + for (String addedEntity : added) { + Score score = objective.getScores().get(addedEntity); + if (score != null) { score.setTeam(this); } } } } + public Team addEntities(String... names) { + List added = new ArrayList<>(); + for (String name : names) { + if (entities.add(name)) { + added.add(name); + } + } + checkAddedEntities(added); + return this; + } + + public Team addEntities(Set names) { + List added = new ArrayList<>(); + for (String name : names) { + if (entities.add(name)) { + added.add(name); + } + } + checkAddedEntities(added); + return this; + } + public void removeEntities(String... names) { for (String name : names) { entities.remove(name); } - setUpdateType(UpdateType.UPDATE); } public boolean hasEntity(String name) { return entities.contains(name); } + public Team setName(String name) { + currentData.name = name; + return this; + } + public Team setPrefix(String prefix) { // replace "null" to an empty string, // we do this here to improve the performance of Score#getDisplayName if (prefix.length() == 4 && "null".equals(prefix)) { - this.prefix = ""; + currentData.prefix = ""; return this; } - this.prefix = prefix; + currentData.prefix = prefix; return this; } @@ -99,15 +128,92 @@ public class Team { // replace "null" to an empty string, // we do this here to improve the performance of Score#getDisplayName if (suffix.length() == 4 && "null".equals(suffix)) { - this.suffix = ""; + currentData.suffix = ""; return this; } - this.suffix = suffix; + currentData.suffix = suffix; return this; } + public String getDisplayName(String score) { + return cachedData != null ? + cachedData.getDisplayName(score) : + currentData.getDisplayName(score); + } + + public void markUpdated() { + updating = false; + } + + public boolean shouldUpdate() { + return updating || cachedData == null || currentData.updateTime > cachedData.updateTime; + } + + public void prepareUpdate() { + if (updating) { + return; + } + updating = true; + + if (cachedData == null) { + cachedData = new TeamData(); + cachedData.updateType = currentData.updateType != UpdateType.REMOVE ? UpdateType.ADD : UpdateType.REMOVE; + } else { + cachedData.updateType = currentData.updateType; + } + + cachedData.updateTime = currentData.updateTime; + cachedData.name = currentData.name; + cachedData.prefix = currentData.prefix; + cachedData.suffix = currentData.suffix; + } + + public UpdateType getUpdateType() { + return cachedData != null ? cachedData.updateType : currentData.updateType; + } + + public Team setUpdateType(UpdateType updateType) { + if (updateType != UpdateType.NOTHING) { + currentData.updateTime = System.currentTimeMillis(); + } + currentData.updateType = updateType; + return this; + } + + public boolean isVisibleFor(String entity) { + switch (nameTagVisibility) { + case HIDE_FOR_OTHER_TEAMS: + return hasEntity(entity); + case HIDE_FOR_OWN_TEAM: + return !hasEntity(entity); + case ALWAYS: + return true; + case NEVER: + return false; + } + return true; + } + @Override public int hashCode() { return id.hashCode(); } + + @Getter + public static final class TeamData { + protected UpdateType updateType; + protected long updateTime; + + protected String name; + protected String prefix; + protected String suffix; + + protected TeamData() { + updateType = UpdateType.ADD; + } + + public String getDisplayName(String score) { + return prefix + score + suffix; + } + } }