From 42ebec1f71bebdd8768e6024828a964e48154588 Mon Sep 17 00:00:00 2001 From: md_5 Date: Sat, 17 Jan 2015 09:41:52 +1100 Subject: [PATCH] SPIGOT-242: Fix scoreboard API. In particular don't maintain an internal list of state, as this gets out of whack when Minecraft adds or removes scoreboards / teams. --- .../scoreboard/CraftObjective.java | 13 ++-- .../craftbukkit/scoreboard/CraftScore.java | 3 +- .../scoreboard/CraftScoreboard.java | 60 +++++++++---------- .../scoreboard/CraftScoreboardComponent.java | 12 +--- .../craftbukkit/scoreboard/CraftTeam.java | 12 +++- 5 files changed, 50 insertions(+), 50 deletions(-) diff --git a/src/main/java/org/bukkit/craftbukkit/scoreboard/CraftObjective.java b/src/main/java/org/bukkit/craftbukkit/scoreboard/CraftObjective.java index 9eaec710b7..1171335703 100644 --- a/src/main/java/org/bukkit/craftbukkit/scoreboard/CraftObjective.java +++ b/src/main/java/org/bukkit/craftbukkit/scoreboard/CraftObjective.java @@ -17,8 +17,6 @@ final class CraftObjective extends CraftScoreboardComponent implements Objective super(scoreboard); this.objective = objective; this.criteria = CraftCriteria.getFromNMS(objective); - - scoreboard.objectives.put(objective.getName(), this); } ScoreboardObjective getHandle() { @@ -104,8 +102,15 @@ final class CraftObjective extends CraftScoreboardComponent implements Objective public void unregister() throws IllegalStateException { CraftScoreboard scoreboard = checkState(); - scoreboard.objectives.remove(this.getName()); scoreboard.board.unregisterObjective(objective); - setUnregistered(); + } + + @Override + CraftScoreboard checkState() throws IllegalStateException { + if (getScoreboard().board.getObjective(objective.getName()) == null) { + throw new IllegalStateException("Unregistered scoreboard component"); + } + + return getScoreboard(); } } diff --git a/src/main/java/org/bukkit/craftbukkit/scoreboard/CraftScore.java b/src/main/java/org/bukkit/craftbukkit/scoreboard/CraftScore.java index 7095f6d83a..d3ae91bc03 100644 --- a/src/main/java/org/bukkit/craftbukkit/scoreboard/CraftScore.java +++ b/src/main/java/org/bukkit/craftbukkit/scoreboard/CraftScore.java @@ -3,6 +3,7 @@ package org.bukkit.craftbukkit.scoreboard; import java.util.Map; import net.minecraft.server.Scoreboard; +import net.minecraft.server.ScoreboardObjective; import net.minecraft.server.ScoreboardScore; import org.bukkit.Bukkit; @@ -41,7 +42,7 @@ final class CraftScore implements Score { Scoreboard board = objective.checkState().board; if (board.getPlayers().contains(entry)) { // Lazy - Map scores = board.getPlayerObjectives(entry); + Map scores = board.getPlayerObjectives(entry); ScoreboardScore score = scores.get(objective.getHandle()); if (score != null) { // Lazy return score.getScore(); diff --git a/src/main/java/org/bukkit/craftbukkit/scoreboard/CraftScoreboard.java b/src/main/java/org/bukkit/craftbukkit/scoreboard/CraftScoreboard.java index 6fdad6e617..8d0e9503f4 100644 --- a/src/main/java/org/bukkit/craftbukkit/scoreboard/CraftScoreboard.java +++ b/src/main/java/org/bukkit/craftbukkit/scoreboard/CraftScoreboard.java @@ -1,8 +1,6 @@ package org.bukkit.craftbukkit.scoreboard; import java.util.Collection; -import java.util.HashMap; -import java.util.Map; import net.minecraft.server.Scoreboard; import net.minecraft.server.ScoreboardObjective; import net.minecraft.server.ScoreboardTeam; @@ -15,22 +13,15 @@ import org.bukkit.scoreboard.Objective; import org.bukkit.scoreboard.Score; import org.bukkit.scoreboard.Team; +import com.google.common.base.Function; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; public final class CraftScoreboard implements org.bukkit.scoreboard.Scoreboard { final Scoreboard board; - final Map objectives = new HashMap(); - final Map teams = new HashMap(); CraftScoreboard(Scoreboard board) { this.board = board; - - for (ScoreboardObjective objective : (Iterable) board.getObjectives()) { - new CraftObjective(this, objective); // It adds itself to map - } - for (ScoreboardTeam team : (Iterable) board.getTeams()) { - new CraftTeam(this, team); // It adds itself to map - } } public CraftObjective registerNewObjective(String name, String criteria) throws IllegalArgumentException { @@ -46,14 +37,15 @@ public final class CraftScoreboard implements org.bukkit.scoreboard.Scoreboard { public Objective getObjective(String name) throws IllegalArgumentException { Validate.notNull(name, "Name cannot be null"); - return objectives.get(name); + return new CraftObjective(this, board.getObjective(name)); } public ImmutableSet getObjectivesByCriteria(String criteria) throws IllegalArgumentException { Validate.notNull(criteria, "Criteria cannot be null"); ImmutableSet.Builder objectives = ImmutableSet.builder(); - for (CraftObjective objective : this.objectives.values()) { + for (ScoreboardObjective netObjective : (Collection) this.board.getObjectives()) { + CraftObjective objective = new CraftObjective(this, netObjective); if (objective.getCriteria().equals(criteria)) { objectives.add(objective); } @@ -62,7 +54,13 @@ public final class CraftScoreboard implements org.bukkit.scoreboard.Scoreboard { } public ImmutableSet getObjectives() { - return ImmutableSet.copyOf((Collection) objectives.values()); + return ImmutableSet.copyOf(Iterables.transform((Collection) this.board.getObjectives(), new Function() { + + @Override + public Objective apply(ScoreboardObjective input) { + return new CraftObjective(CraftScoreboard.this, input); + } + })); } public Objective getObjective(DisplaySlot slot) throws IllegalArgumentException { @@ -71,25 +69,21 @@ public final class CraftScoreboard implements org.bukkit.scoreboard.Scoreboard { if (objective == null) { return null; } - return this.objectives.get(objective.getName()); + return new CraftObjective(this, objective); } public ImmutableSet getScores(OfflinePlayer player) throws IllegalArgumentException { Validate.notNull(player, "OfflinePlayer cannot be null"); - ImmutableSet.Builder scores = ImmutableSet.builder(); - for (CraftObjective objective : objectives.values()) { - scores.add(objective.getScore(player)); - } - return scores.build(); + return getScores(player.getName()); } public ImmutableSet getScores(String entry) throws IllegalArgumentException { Validate.notNull(entry, "Entry cannot be null"); ImmutableSet.Builder scores = ImmutableSet.builder(); - for (CraftObjective objective : objectives.values()) { - scores.add(objective.getScore(entry)); + for (ScoreboardObjective objective : (Collection) this.board.getObjectives()) { + scores.add(new CraftScore(new CraftObjective(this, objective), entry)); } return scores.build(); } @@ -97,34 +91,38 @@ public final class CraftScoreboard implements org.bukkit.scoreboard.Scoreboard { public void resetScores(OfflinePlayer player) throws IllegalArgumentException { Validate.notNull(player, "OfflinePlayer cannot be null"); - for (CraftObjective objective : objectives.values()) { - board.resetPlayerScores(player.getName(), objective.getHandle()); - } + resetScores(player.getName()); } public void resetScores(String entry) throws IllegalArgumentException { Validate.notNull(entry, "Entry cannot be null"); - for (CraftObjective objective : objectives.values()) { - board.resetPlayerScores(entry, objective.getHandle()); + for (ScoreboardObjective objective : (Collection) this.board.getObjectives()) { + board.resetPlayerScores(entry, objective); } } public Team getPlayerTeam(OfflinePlayer player) throws IllegalArgumentException { Validate.notNull(player, "OfflinePlayer cannot be null"); - ScoreboardTeam team = board.getPlayerTeam(player.getName()); - return team == null ? null : teams.get(team.getName()); + return getTeam(player.getName()); } public Team getTeam(String teamName) throws IllegalArgumentException { Validate.notNull(teamName, "Team name cannot be null"); - return teams.get(teamName); + ScoreboardTeam team = board.getPlayerTeam(teamName); + return team == null ? null : new CraftTeam(this, team); } public ImmutableSet getTeams() { - return ImmutableSet.copyOf((Collection) teams.values()); + return ImmutableSet.copyOf(Iterables.transform((Collection) this.board.getTeams(), new Function() { + + @Override + public Team apply(ScoreboardTeam input) { + return new CraftTeam(CraftScoreboard.this, input); + } + })); } public Team registerNewTeam(String name) throws IllegalArgumentException { diff --git a/src/main/java/org/bukkit/craftbukkit/scoreboard/CraftScoreboardComponent.java b/src/main/java/org/bukkit/craftbukkit/scoreboard/CraftScoreboardComponent.java index 3855a2b79b..d26d09d411 100644 --- a/src/main/java/org/bukkit/craftbukkit/scoreboard/CraftScoreboardComponent.java +++ b/src/main/java/org/bukkit/craftbukkit/scoreboard/CraftScoreboardComponent.java @@ -7,21 +7,11 @@ abstract class CraftScoreboardComponent { this.scoreboard = scoreboard; } - CraftScoreboard checkState() throws IllegalStateException { - CraftScoreboard scoreboard = this.scoreboard; - if (scoreboard == null) { - throw new IllegalStateException("Unregistered scoreboard component"); - } - return scoreboard; - } + abstract CraftScoreboard checkState() throws IllegalStateException; public CraftScoreboard getScoreboard() { return scoreboard; } abstract void unregister() throws IllegalStateException; - - final void setUnregistered() { - scoreboard = null; - } } diff --git a/src/main/java/org/bukkit/craftbukkit/scoreboard/CraftTeam.java b/src/main/java/org/bukkit/craftbukkit/scoreboard/CraftTeam.java index e8d1c081e5..5584353bf2 100644 --- a/src/main/java/org/bukkit/craftbukkit/scoreboard/CraftTeam.java +++ b/src/main/java/org/bukkit/craftbukkit/scoreboard/CraftTeam.java @@ -19,7 +19,6 @@ final class CraftTeam extends CraftScoreboardComponent implements Team { CraftTeam(CraftScoreboard scoreboard, ScoreboardTeam team) { super(scoreboard); this.team = team; - scoreboard.teams.put(team.getName(), this); } public String getName() throws IllegalStateException { @@ -155,8 +154,6 @@ final class CraftTeam extends CraftScoreboardComponent implements Team { CraftScoreboard scoreboard = checkState(); scoreboard.board.removeTeam(team); - scoreboard.teams.remove(team.getName()); - setUnregistered(); } public static EnumNameTagVisibility bukkitToNotch(NameTagVisibility visibility) { @@ -188,4 +185,13 @@ final class CraftTeam extends CraftScoreboardComponent implements Team { throw new IllegalArgumentException("Unknown visibility level " + visibility); } } + + @Override + CraftScoreboard checkState() throws IllegalStateException { + if (getScoreboard().board.getTeam(team.getName()) == null) { + throw new IllegalStateException("Unregistered scoreboard component"); + } + + return getScoreboard(); + } }