Implement chat changes #1172
Implement chat changes #1172
Conversation
Signed-off-by: TheMode <themode@outlook.fr>
Signed-off-by: TheMode <themode@outlook.fr>
Signed-off-by: TheMode <themode@outlook.fr>
Signed-off-by: TheMode <themode@outlook.fr>
Signed-off-by: TheMode <themode@outlook.fr>
Signed-off-by: TheMode <themode@outlook.fr>
Signed-off-by: TheMode <themode@outlook.fr>
Signed-off-by: TheMode <themode@outlook.fr>
Signed-off-by: TheMode <themode@outlook.fr>
src/main/java/net/minestom/server/adventure/audience/PacketGroupingAudience.java
Outdated
Show resolved
Hide resolved
src/main/java/net/minestom/server/command/ArgumentsSignature.java
Outdated
Show resolved
Hide resolved
src/main/java/net/minestom/server/command/ArgumentsSignature.java
Outdated
Show resolved
Hide resolved
| private ChatTypeBuilder playerChatType = ChatTypeBuilder.builder(ChatType.CHAT.key()).chat(ChatDecoration.contentWithSender("chat.type.text")); | ||
| private ChatTypeBuilder systemChatType = ChatTypeBuilder.builder(ChatType.SYSTEM.key()).chat(); | ||
|
|
||
| public Builder setRequireValidPlayerPublicKey(boolean requireValidPlayerPublicKey) { |
There was a problem hiding this comment.
Don't most of the other builders in Minestom omit the set prefix?
There was a problem hiding this comment.
I'll change that, and this config change will be made in master first
| import java.util.function.Supplier; | ||
|
|
||
| /** | ||
| * Used to mimic the final keyword |
There was a problem hiding this comment.
Well that's not really true, it's more of a lazy initialized object than a final object, i.e. it can only be set once, and can be set lazily.
There was a problem hiding this comment.
Can I recommend a new name like LazyInitializedObject or LazyLoadedValue for this class?
There was a problem hiding this comment.
We try to avoid this object, so it's name probably won't matter, but if it ends up in the final pr i'll choose a better name.
| * | ||
| * @param <T> type of the object | ||
| */ | ||
| public final class ObjectCache<T> { |
There was a problem hiding this comment.
This isn't an object cache, it's a memoizing supplier. It calls the supplier once and caches the result. The name ObjectCache implies that it stores and tracks multiple objects, which misrepresents what this object does.
There was a problem hiding this comment.
Well i'm bad at naming things, but will change this.
There was a problem hiding this comment.
Yeah that's fair enough. I recommend something like MemoizingSupplier.
| public enum SignatureAlgorithm { | ||
| SHA256withRSA, | ||
| SHA1withRSA | ||
| } | ||
|
|
||
| public enum KeyAlgorithm { | ||
| RSA | ||
| } |
There was a problem hiding this comment.
Thinking about it, these enums don't really even need to exist. These are only used in the string constant context, i.e. by calling name() all the time, so they could just be string constants instead of enums.
There was a problem hiding this comment.
With the enum you can be sure that the method will exists, but with a string you might accidentally pass a wrong value.
There was a problem hiding this comment.
That is true, but is there even even that anyone other than us internally could pass an incorrect value? Because if there is not (and I'm pretty sure there isn't), we don't need the enum to guard against that.
|
I think this PR should wait until Adventure 4.12.0 is released, as I believe that quite a few things will be implemented in that, and so not required here. We don't want to push an API that will later be rewritten. I think this should be blocked by KyoriPowered/adventure#777 |
That was my original idea, but seems like the linked pull request barely makes any progress and if they go the route of supporting system messages only then it won't require a big change, I also had some suggestions and some of them was semi-agreed on it, yet no commits, so I'm not sure what's going on whit that pr, but anyways this pr is already blocking on the config (which will be merged today hopefully) and the registry rework (which isn't started yet). |
We now have adventure 4.12.0. When will we have this implemented? |
No description provided.