-
-
Notifications
You must be signed in to change notification settings - Fork 112
Sidebar update #2795
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Sidebar update #2795
Conversation
plugin/src/main/java/com/denizenscript/denizen/scripts/commands/player/SidebarCommand.java
Show resolved
Hide resolved
| 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(); |
There was a problem hiding this comment.
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!"); |
There was a problem hiding this comment.
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.
| public static BukkitTagContext getContext(PlayerTag player, BukkitTagContext global, boolean perPlayer) { | ||
| if (perPlayer) { | ||
| BukkitTagContext context = new BukkitTagContext(global); | ||
| context.player = player; | ||
| return context; | ||
| } | ||
| return global; | ||
| } |
There was a problem hiding this comment.
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 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor formatting error
First part of updating
sidebarcommand. Related discussion - https://discord.com/channels/315163488085475337/1439931889405395027