Skip to content

Conversation

@davight
Copy link
Contributor

@davight davight commented Nov 22, 2025

First part of updating sidebar command. Related discussion - https://discord.com/channels/315163488085475337/1439931889405395027

Debug.report(scriptEntry, getName(), action, elTitle, elScores, elValue, elIncrement, elStart, db("players", players));
}
if (players == null) {
players = Utilities.entryHasPlayer(scriptEntry) ? Collections.singletonList(Utilities.getEntryPlayer(scriptEntry)) : Collections.emptyList();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

List.of methods are cleaner usually

&& !scriptEntry.hasObject("increment") && !scriptEntry.hasObject("start")) {
throw new InvalidArgumentsException("Must specify at least one of: value(s), title, increment, or start for that action!");
if (action == Action.SET && stringValues == null && stringTitle == null) {
Debug.echoError("Must specify at least one of: value(s), title, increment, or start for that action!");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

increment has a default value now and isn't checked here anymore, either remove the default value and add back the check so that it's like before (is -1 a correct default value for this?) or remove it from the error.

Comment on lines +266 to +273
public static BukkitTagContext getContext(PlayerTag player, BukkitTagContext global, boolean perPlayer) {
if (perPlayer) {
BukkitTagContext context = new BukkitTagContext(global);
context.player = player;
return context;
}
return global;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes a new context every time, but you only really need the global context or a single copy to change the player on for per_player.

else {
current.add(new Sidebar.SidebarLine(value.get(i), score));
}
} else {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor formatting error

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants