Provide access to PlayerSettings and an event when it updates #210
Conversation
|
A big +1, had wanted to use this info for long now, hadn't been able to because it wasn't saved anywhere (not even in nms (atleast chunk distance)) and i didn't want to do packet sniffing directly to find that. |
|
Is there any plans on this patch being merged? I would like to use it in another project and it would be great to see this merged. I've done some testing on my own, and have not found any issues so far. Sadly, the minecraft client has a bug where it won't send the packet to the server sometimes: #MC-108315 |
|
I'll review it |
| +/** | ||
| + * Represents the chat preferences of a {@link org.bukkit.entity.Player}. | ||
| + */ | ||
| +public enum ChatVisibility { |
There was a problem hiding this comment.
Maybe FULL, SYSTEM, HIDDEN to match the NMS enum?
| +/** | ||
| + * Various client-established settings of a {@link Player}. | ||
| + */ | ||
| +public class PlayerSettings { |
There was a problem hiding this comment.
Do any of these fields have to be nullable? There are default values for everything, right?
| + * | ||
| + * @return locale of the player. | ||
| + */ | ||
| + public String getLocale() { |
There was a problem hiding this comment.
Can we use java.util.Locale here?
| + * Get the parts of the skin the player wants to be rendered by others. | ||
| + * @return parts of the skin. | ||
| + */ | ||
| + public ImmutableSet<Skin.Part> getSkinParts() { |
There was a problem hiding this comment.
We probably don't want to introduce Guava types into the Bukkit API. I'm not sure why the dependency exists at all.
| + if(this == o) return true; | ||
| + if(o == null || getClass() != o.getClass()) return false; | ||
| + PlayerSettings other = (PlayerSettings) o; | ||
| + return new EqualsBuilder() |
There was a problem hiding this comment.
Likewise for Apache Commons types (EqualsBuilder and HashCodeBuilder)
| + /** | ||
| + * Transform one enum to another enum type based on {@link Enum#ordinal()}. | ||
| + */ | ||
| + public static <F extends Enum<F>, T extends Enum<T>> T transform(F from, Class<T> to, T def) { |
There was a problem hiding this comment.
This should probably throw an exception if the enums have different lengths. I would also keep it in CraftBukkit, doesn't need to be in the public API.
| + | ||
| +import java.io.IOException; | ||
| + | ||
| +public class PacketPlayInSettings implements Packet<PacketListenerPlayIn> { |
There was a problem hiding this comment.
Import this file in the "Import necessary NMS classes" patch
| return event; | ||
| } | ||
|
|
||
| - public static PlayerLocaleChangeEvent callPlayerLocaleChangeEvent(EntityHuman who, String oldLocale, String newLocale) { |
There was a problem hiding this comment.
If this patch is overwriting a lot of stuff from the locale change event patch, then it should just be combined with that patch. When we rebase, we don't want to merge code that gets thrown away in later patches.
| public class EntityPlayer extends EntityHuman implements ICrafting { | ||
|
|
||
| private static final Logger bS = LogManager.getLogger(); | ||
| - public String locale = null; // SportBukkit - make public and default to null |
There was a problem hiding this comment.
Could we copy to/from these NMS fields instead of replacing them? This approach could make rebasing difficult.
| + * | ||
| + * @param settings the new player settings. | ||
| + */ | ||
| + void setSettings(PlayerSettings settings); |
There was a problem hiding this comment.
Would you ever want to call this method after receiving the client settings packet? That would just desync the server and client. Maybe this method should do nothing if the packet has already been received?
No description provided.