Skip to content

Expose certain scoreboard related argument types #12541

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions LICENSE.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,4 +65,5 @@ aerulion <[email protected]>
Lukas Planz <[email protected]>
granny <[email protected]>
mja00 <[email protected]>
Strokkur24 <[email protected]>
```
2 changes: 2 additions & 0 deletions build-data/paper.at
Original file line number Diff line number Diff line change
Expand Up @@ -788,3 +788,5 @@ public-f net.minecraft.world.level.saveddata.maps.MapItemSavedData locked
public-f net.minecraft.world.level.saveddata.maps.MapItemSavedData scale
public-f net.minecraft.world.level.saveddata.maps.MapItemSavedData trackingPosition
public-f net.minecraft.world.level.saveddata.maps.MapItemSavedData unlimitedTracking
public net.minecraft.commands.arguments.OperationArgument$SimpleOperation
public net.minecraft.commands.arguments.OperationArgument ERROR_INVALID_OPERATION
18 changes: 18 additions & 0 deletions paper-api/src/main/java/io/papermc/paper/InternalAPIBridge.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@
import io.papermc.paper.world.damagesource.CombatEntry;
import io.papermc.paper.world.damagesource.FallLocationType;
import net.kyori.adventure.util.Services;
import org.bukkit.OfflinePlayer;
import org.bukkit.block.Biome;
import org.bukkit.damage.DamageEffect;
import org.bukkit.damage.DamageSource;
import org.bukkit.entity.LivingEntity;
import org.bukkit.scoreboard.ScoreHolder;
import org.jetbrains.annotations.ApiStatus;
import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.Nullable;
Expand Down Expand Up @@ -73,5 +75,21 @@ class Holder {
* @return combat entry
*/
CombatEntry createCombatEntry(DamageSource damageSource, float damage, @Nullable FallLocationType fallLocationType, float fallDistance);

/**
* Creates a wrapping score holder
*
* @param entry The entry to wrap
* @return a wrapping ScoreHolder
*/
ScoreHolder scoreHolderOf(String entry);

/**
* Creates a wrapping score holder
*
* @param player The player to wrap
* @return a wrapping ScoreHolder
*/
ScoreHolder scoreHolderOf(OfflinePlayer player);
}

Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
package io.papermc.paper.command.brigadier.argument;

import com.mojang.brigadier.arguments.ArgumentType;
import io.papermc.paper.command.brigadier.argument.operation.Operation;
import io.papermc.paper.command.brigadier.argument.predicate.ItemStackPredicate;
import io.papermc.paper.command.brigadier.argument.range.DoubleRangeProvider;
import io.papermc.paper.command.brigadier.argument.range.IntegerRangeProvider;
import io.papermc.paper.command.brigadier.argument.resolvers.BlockPositionResolver;
import io.papermc.paper.command.brigadier.argument.resolvers.FinePositionResolver;
import io.papermc.paper.command.brigadier.argument.resolvers.PlayerProfileListResolver;
import io.papermc.paper.command.brigadier.argument.resolvers.RotationResolver;
import io.papermc.paper.command.brigadier.argument.resolvers.ScoreHolderResolver;
import io.papermc.paper.command.brigadier.argument.resolvers.selector.EntitySelectorArgumentResolver;
import io.papermc.paper.command.brigadier.argument.resolvers.selector.PlayerSelectorArgumentResolver;
import io.papermc.paper.entity.LookAnchor;
Expand Down Expand Up @@ -291,6 +293,35 @@ public static ArgumentType<Criteria> objectiveCriteria() {
return provider().objectiveCriteria();
}

/**
* Represents a selector that can capture any single
* score holder.
*
* @return argument
*/
public static ArgumentType<ScoreHolderResolver> scoreHolder() {
return provider().scoreHolder();
}

/**
* Represents a selector that can capture multiple
* score holders.
*
* @return argument
*/
public static ArgumentType<ScoreHolderResolver> scoreHolders() {
return provider().scoreHolders();
}

/**
* An operation argument.
*
* @return argument
*/
public static ArgumentType<Operation> operation() {
return provider().operation();
}

/**
* An entity anchor argument.
*
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
package io.papermc.paper.command.brigadier.argument;

import com.mojang.brigadier.arguments.ArgumentType;
import io.papermc.paper.command.brigadier.argument.operation.Operation;
import io.papermc.paper.command.brigadier.argument.predicate.ItemStackPredicate;
import io.papermc.paper.command.brigadier.argument.range.DoubleRangeProvider;
import io.papermc.paper.command.brigadier.argument.range.IntegerRangeProvider;
import io.papermc.paper.command.brigadier.argument.resolvers.BlockPositionResolver;
import io.papermc.paper.command.brigadier.argument.resolvers.FinePositionResolver;
import io.papermc.paper.command.brigadier.argument.resolvers.PlayerProfileListResolver;
import io.papermc.paper.command.brigadier.argument.resolvers.RotationResolver;
import io.papermc.paper.command.brigadier.argument.resolvers.ScoreHolderResolver;
import io.papermc.paper.command.brigadier.argument.resolvers.selector.EntitySelectorArgumentResolver;
import io.papermc.paper.command.brigadier.argument.resolvers.selector.PlayerSelectorArgumentResolver;
import io.papermc.paper.entity.LookAnchor;
Expand Down Expand Up @@ -75,6 +77,12 @@ static VanillaArgumentProvider provider() {
ArgumentType<SignedMessageResolver> signedMessage();

ArgumentType<DisplaySlot> scoreboardDisplaySlot();

ArgumentType<ScoreHolderResolver> scoreHolder();

ArgumentType<ScoreHolderResolver> scoreHolders();

ArgumentType<Operation> operation();

ArgumentType<NamespacedKey> namespacedKey();

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
package io.papermc.paper.command.brigadier.argument.operation;

import com.mojang.brigadier.exceptions.CommandSyntaxException;
import org.bukkit.scoreboard.Objective;
import org.bukkit.scoreboard.ScoreHolder;
import org.bukkit.scoreboard.Scoreboard;
import org.jetbrains.annotations.ApiStatus;
import org.jspecify.annotations.NullMarked;

/**
* Represents a simple arithmetic operation between two integers.
* An {@link Operation} backs an operator.
* Most arithmetic operators (like {@code +=, -=, /=, etc.}) are
* supported, alongside certain conditional operators ({@code >=, <=, ==, etc.}).
* <p>
* Note that conditional operators, instead of yielding a boolean value, return the value
* that matches the operation.
* For example, the {@code <=} operator always returns the <strong>smaller</strong> value
* of two given values.
*/
@ApiStatus.Experimental
@NullMarked
public interface Operation {
Copy link
Contributor

Choose a reason for hiding this comment

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

Javadocs, what is an operation.
What are common usage patterns etc.

Operation also is a very broad name, idk if there a is a point in finding something more specific to what this is.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is more okay in internals, because this type lives purely as an inner type to OperationArgument.
But yea, on a top level, this doesn't mean much.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also experimental annotation


/**
* Applies this operation to a pair of integers.
* <p>
* Arithmetic between two integers always follows this pattern:
* <pre>
* return left &lt;operator&gt; right
* </pre>
* On certain operators, such as division, the order matters.
* {@code 20 %= 10} yields a different result than{@code 10 %= 20}.
*
* @param left left side of the expression
* @param right right side of the expression
* @return result of the operation
*/
int apply(int left, int right) throws CommandSyntaxException;

/**
* Applies this operation to a pair of {@link ScoreHolder}s, retrieving their
* values from a {@link Scoreboard} and their respective {@link Objective}s.
* <p>
* Arithmetic between two integers always follows this pattern:
* <pre>return left &lt;operator&gt; right;</pre>
* On certain operators, such as division, the order matters.
* {@code 20 %= 10} yields a different result than{@code 10 %= 20}.
*
* @param scoreboard scoreboard to edit the score of
* @param sourceObjective objective to retrieve the score from
* @param targetObjective objective to edit the score of
* @param targetHolder target of the operation
* @param sourceHolder source of the operation
*/
void apply(Scoreboard scoreboard, Objective targetObjective, Objective sourceObjective, ScoreHolder targetHolder, ScoreHolder sourceHolder) throws CommandSyntaxException;
Copy link
Contributor

@lynxplay lynxplay May 11, 2025

Choose a reason for hiding this comment

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

In general I am not much of a fan of this split between SimpleOperation and Operation internally.
apply(int, int) is only defined on simple operations. This makes it seem like it is universally applicable (which it isn't, CommandSyntaxException is thrown for the swap operation).

Having to pass all this BS just to construct a ScoreAccess also seems overkill.
Plugins will not be using scoreboards to store/modify their data.
I think we'd be better off on implementing ScoreAccess in our own type with a noop display name / override.
That way we can a) have apply(int, int) be always functional and simply return the "target", or "left" side here.
We could also expose an apply(int,int)Int2IntPair that yields both for people caring about swap.

This scoreboard/objective based method seems just very very useless/cumbersome to work with.

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package io.papermc.paper.command.brigadier.argument.resolvers;

import io.papermc.paper.command.brigadier.CommandSourceStack;
import io.papermc.paper.command.brigadier.argument.ArgumentTypes;
import org.bukkit.scoreboard.ScoreHolder;
import org.jetbrains.annotations.ApiStatus;
import java.util.Collection;

/**
* An {@link ArgumentResolver} that's capable of resolving
* argument value using a {@link CommandSourceStack} into a
* collection of {@link ScoreHolder}s.
*
* @see ArgumentTypes#scoreHolders()
*/
@ApiStatus.Experimental
@ApiStatus.NonExtendable
public interface ScoreHolderResolver extends ArgumentResolver<Collection<ScoreHolder>> {
}
48 changes: 48 additions & 0 deletions paper-api/src/main/java/org/bukkit/scoreboard/ScoreHolder.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package org.bukkit.scoreboard;

import io.papermc.paper.InternalAPIBridge;
import net.kyori.adventure.text.Component;
import org.bukkit.OfflinePlayer;
import org.jetbrains.annotations.ApiStatus;
import org.jspecify.annotations.NullMarked;

/**
* Represents anything that can be used as an entry in a {@link Scoreboard}. Includes all entities,
* players, and text.
*/
@ApiStatus.Experimental
@NullMarked
public interface ScoreHolder {
Copy link
Contributor

Choose a reason for hiding this comment

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

Javadocs are missing on everything but also, this entire type is pretty annoying.

The fact that this always wraps means consumers cannot do anything with this beyond the exposed methods. A scoreholder is never e.g. instanceOf a Player or Entity.

I think this would be better off as a parent type to Entity and a separate implementation for those string-only implementations.


/**
* The name used to access this {@link ScoreHolder} on a scoreboard.
* @return the name
*/
String getScoreboardName();

/**
* The display name of this {@link ScoreHolder}.
* @return the display name
*/
default Component getDisplayName() {
return Component.text(getScoreboardName());
}

/**
* Wraps an {@link OfflinePlayer} into a {@link ScoreHolder}.
* @param offlinePlayer the player to wrap
* @return a {@link ScoreHolder} representing the given player
*/
static ScoreHolder of(OfflinePlayer offlinePlayer) {
return InternalAPIBridge.get().scoreHolderOf(offlinePlayer);
}

/**
* Wraps a {@link String} into a {@link ScoreHolder}.
* @param entry the text to warp
* @return a {@link ScoreHolder} representing the given String
*/
static ScoreHolder of(String entry) {
return InternalAPIBridge.get().scoreHolderOf(entry);
}
}
51 changes: 47 additions & 4 deletions paper-api/src/main/java/org/bukkit/scoreboard/Scoreboard.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.bukkit.scoreboard;

import com.google.common.collect.ImmutableSet;
import java.util.Set;
import org.bukkit.OfflinePlayer;
import org.jetbrains.annotations.NotNull;
Expand Down Expand Up @@ -231,7 +232,9 @@ default Objective registerNewObjective(@NotNull String name, @NotNull Criteria c
*/
// @Deprecated(since = "1.7.8") // Paper
@NotNull
Set<Score> getScores(@NotNull OfflinePlayer player);
default Set<Score> getScores(@NotNull OfflinePlayer player) {
return getScores(ScoreHolder.of(player));
}

/**
* Gets all scores for an entry on this Scoreboard
Expand All @@ -240,7 +243,18 @@ default Objective registerNewObjective(@NotNull String name, @NotNull Criteria c
* @return immutable set of all scores tracked for the entry
*/
@NotNull
Set<Score> getScores(@NotNull String entry);
default Set<Score> getScores(@NotNull String entry) {
return getScores(ScoreHolder.of(entry));
}

/**
* Gets all scores for a ScoreHolder on this Scoreboard
*
* @param holder the ScoreHolder whose scores are being retrieved
* @return immutable set of all scores tracked for the entry
*/
@NotNull
ImmutableSet<Score> getScores(@NotNull ScoreHolder holder);

/**
* Removes all scores for a player on this Scoreboard
Expand All @@ -249,14 +263,20 @@ default Objective registerNewObjective(@NotNull String name, @NotNull Criteria c
* @see #resetScores(String)
*/
// @Deprecated(since = "1.7.8") // Paper
void resetScores(@NotNull OfflinePlayer player);
default void resetScores(@NotNull OfflinePlayer player) {
resetScores(ScoreHolder.of(player));
}

/**
* Removes all scores for an entry on this Scoreboard
*
* @param entry the entry to drop all current scores for
*/
void resetScores(@NotNull String entry);
default void resetScores(@NotNull String entry) {
resetScores(ScoreHolder.of(entry));
}

void resetScores(@NotNull ScoreHolder holder);

/**
* Gets a player's Team on this Scoreboard
Expand Down Expand Up @@ -324,6 +344,9 @@ default Objective registerNewObjective(@NotNull String name, @NotNull Criteria c
@NotNull
Set<String> getEntries();

@NotNull
Set<ScoreHolder> getHolders();

/**
* Clears any objective in the specified slot.
*
Expand Down Expand Up @@ -361,4 +384,24 @@ default Objective registerNewObjective(@NotNull String name, @NotNull Criteria c
*/
@Nullable Team getEntityTeam(@NotNull org.bukkit.entity.Entity entity) throws IllegalArgumentException;
// Paper end - improve scoreboard entries

/**
* Gets all scores for a score holder on this Scoreboard
*
* @param holder the score holder whose scores are being retrieved
* @return immutable set of all scores tracked for the entity
* @throws IllegalArgumentException if entity is null
* @see #getScores(ScoreHolder)
*/
@NotNull Set<Score> getScoresFor(@NotNull ScoreHolder holder) throws IllegalArgumentException;
Copy link
Contributor

Choose a reason for hiding this comment

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

These methods are just copies from the ones above?
What is the point of these?


/**
* Removes all scores for a score holder on this Scoreboard
*
* @param holder the score holder to drop all current scores for
* @throws IllegalArgumentException if entity is null
* @see #resetScores(ScoreHolder)
*/
void resetScoresFor(@NotNull ScoreHolder holder) throws IllegalArgumentException;

}
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,18 @@
import io.papermc.paper.world.damagesource.PaperCombatTrackerWrapper;
import net.minecraft.Optionull;
import net.minecraft.world.damagesource.FallLocation;
import org.bukkit.OfflinePlayer;
import org.bukkit.block.Biome;
import org.bukkit.craftbukkit.block.CraftBiome;
import org.bukkit.craftbukkit.damage.CraftDamageEffect;
import org.bukkit.craftbukkit.damage.CraftDamageSource;
import org.bukkit.craftbukkit.entity.CraftLivingEntity;
import org.bukkit.craftbukkit.scoreboard.CraftScoreHolder;
import org.bukkit.craftbukkit.scoreboard.CraftScoreboard;
import org.bukkit.damage.DamageEffect;
import org.bukkit.damage.DamageSource;
import org.bukkit.entity.LivingEntity;
import org.bukkit.scoreboard.ScoreHolder;
import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.Nullable;

Expand Down Expand Up @@ -71,4 +75,14 @@ private CombatEntry createCombatEntry(
damageSource, damage, fallLocation, fallDistance
));
}

@Override
public ScoreHolder scoreHolderOf(final String entry) {
return new CraftScoreHolder(CraftScoreboard.getScoreHolder(entry));
}

@Override
public ScoreHolder scoreHolderOf(final OfflinePlayer player) {
return new CraftScoreHolder(CraftScoreboard.getScoreHolder(player));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,9 @@ public static void validatePrimitiveType(ArgumentType<?> type) {
}

converted = this.unwrapArgumentWrapper(pureArgumentNode, customArgumentType, customArgumentType.getNativeType(), suggestionProvider);
} else if (pureArgumentType instanceof final VanillaArgumentProviderImpl.ScoreHolderWrapperArgumentType scoreHolderWrapperArgumentType) {
// This is a special case, as we override the suggestions here
converted = this.unwrapArgumentWrapper(pureArgumentNode, scoreHolderWrapperArgumentType, scoreHolderWrapperArgumentType, VanillaArgumentProviderImpl.ScoreHolderWrapperArgumentType.SUGGESTIONS);
} else if (pureArgumentType instanceof final VanillaArgumentProviderImpl.NativeWrapperArgumentType<?, ?> nativeWrapperArgumentType) {
converted = this.unwrapArgumentWrapper(pureArgumentNode, nativeWrapperArgumentType, nativeWrapperArgumentType, null); // "null" for suggestion provider so it uses the argument type's suggestion provider

Expand Down
Loading
Loading