Skip to content

Sidebar update#2795

Draft
davight wants to merge 10 commits into
DenizenScript:devfrom
davight:sidebar_update
Draft

Sidebar update#2795
davight wants to merge 10 commits into
DenizenScript:devfrom
davight:sidebar_update

Conversation

@davight
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member

Choose a reason for hiding this comment

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

Minor formatting error

@tal5 tal5 marked this pull request as draft April 27, 2026 13:34
@tal5
Copy link
Copy Markdown
Member

tal5 commented Apr 27, 2026

Marking as draft for now until comments can be addressed

@davight
Copy link
Copy Markdown
Contributor Author

davight commented Jun 4, 2026

guess what I did some changes, removed the whole ass class and just replaced it with few variables (in my dream world I would probably create some highly abstracted LazyParser for all of these things but whatever lol)
also fixed some other issues, so probably again ready to review (dunno if I can click it myself or you want to do it)

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