Add listing API for PlayerProfile #10392
Conversation
Flo0
force-pushed
the
player-profile-listing-api
branch
7 times, most recently
from
308af9a
to
7abad79
Compare
Flo0
force-pushed
the
player-profile-listing-api
branch
from
7abad79
to
97a499f
Compare
| + * @param gameMode The game mode of the profile. | ||
| + * @param displayName The display name of the profile. | ||
| + */ | ||
| + void listProfile(@NotNull com.destroystokyo.paper.profile.PlayerProfile profile, int latency, @NotNull GameMode gameMode, @Nullable net.kyori.adventure.text.Component displayName); |
There was a problem hiding this comment.
I'm not satisfied with the name of this method. I think it should be send something like the other "packet-only" methods we have in the API. Something like sendPlayerTabListProfile or some combinations of those words.
There was a problem hiding this comment.
Yes makes sense. Ill change those.
| + default void listProfile(@NotNull com.destroystokyo.paper.profile.PlayerProfile profile) { | ||
| + listProfile(profile, 0, GameMode.SURVIVAL, null); | ||
| + } |
There was a problem hiding this comment.
I'm not sure having this method makes much sense here. These defaults are a little arbitrary. Like 0 ping? why survival? what if its a creative server? I'm not sure we need anything except the full method with all the options.
There was a problem hiding this comment.
Those default values where taken from the internal EntryBuilder of the ClientboundPlayerInfoUpdatePacket.
Its honestly just a quol method. Would be nice to have a second opinion of this ^^
There was a problem hiding this comment.
I don't think listed is a descriptive enough word to be used alone to describe this kind of API. Along with my msg about the listProfile method name, the various update names should be changed to be like sendTabListLatency, sendTabListDisplayName, etc.
Additionally, there is no method to remove a profile, only disable it's listed status. These things are separate and should be controlled separately. Send a fake Profile but also have a separate option for listing it or not. And have methods to change the listed nature of that profile, but also have a method for removing that profile.
All of the methods except the ones that sends the initial ClientboundPlayerInfoUpdatePacket do not require the full PlayerProfile, only the UUID, although we may not care and just require that you pass the full PlayerProfile instead, unsure on that.
There was a problem hiding this comment.
Yes, separating adding and listing makes sense.
I would argue against using UUIDs as it carries some ambiguity (and possible confusion) with it.
| + | ||
| + @Override | ||
| + public void unlistProfile(@NotNull com.destroystokyo.paper.profile.PlayerProfile profile) { | ||
| + Preconditions.checkNotNull(profile, "profile cannot be null"); |
There was a problem hiding this comment.
We prefer using checkArgument(profile != null) because that will throw an IllegalArgumentException instead of an NullPointerException.
There was a problem hiding this comment.
Im also not a fan of this check. I was just copying this from another paper method.
This enables listing, unlisting and updating any of [latyncy, gamemode, displayname] for PlayerProfiles in the tablist.
Burden of tracking those listings is on the caller.
Extends the ClientboundPlayerInfoUpdatePacket class and its nested Entry class for ease of use.
Test video