Skip to content
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

Adventure Chat Integration [EXPERIMENT] #4971

Closed
wants to merge 15 commits into from

Conversation

APickledWalrus
Copy link
Member

@APickledWalrus APickledWalrus commented Jul 26, 2022

Description

This PR is for tracking my experiment. A lot of work still needs done. Comments/tests welcome though!

Currently has some breaking changes (in that some color tags parse differently).

This PR depends on the Module API. If we don't end up doing that, I can make changes accordingly.


Target Minecraft Versions: Any
Requirements: N/A
Related Issues:

This test was designed to maintain compatibility with Spigot.
Also includes improvements and fixes to current changes
This results in much faster execution times comparable to the current system. However, it also means that Spigot is no longer supported, and versions below 1.17 won't work. (Paper 1.17+ only now)
@APickledWalrus APickledWalrus added feature Pull request adding a new feature. breaking changes Pull or feature requests that contain breaking changes (API, syntax, etc.) labels Jul 26, 2022
The goal here isn't possible of course, as we relocate the dependency.
@TheLimeGlass TheLimeGlass self-requested a review July 31, 2022 04:28

# Other API

component: component¦s @a
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should add chat component as another aliases

Copy link
Contributor

@kiip1 kiip1 left a comment

Choose a reason for hiding this comment

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

Would love to see support for sounds too, good PR and did a small review while I was at it.

Comment on lines +75 to +77
boolean format;
// Whether all formatting should be removed
boolean unformatted;
Copy link
Contributor

Choose a reason for hiding this comment

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

Could replace with a TriState.

Comment on lines +31 to +54
CODE_MAP.put('0', "black");
CODE_MAP.put('1', "dark_blue");
CODE_MAP.put('2', "dark_green");
CODE_MAP.put('3', "dark_aqua");
CODE_MAP.put('4', "dark_red");
CODE_MAP.put('5', "dark_purple");
CODE_MAP.put('6', "gold");
CODE_MAP.put('7', "gray");
CODE_MAP.put('8', "dark_gray");
CODE_MAP.put('9', "blue");
CODE_MAP.put('a', "green");
CODE_MAP.put('b', "aqua");
CODE_MAP.put('c', "red");
CODE_MAP.put('d', "light_purple");
CODE_MAP.put('e', "yellow");
CODE_MAP.put('f', "white");

CODE_MAP.put('o', "italic");
CODE_MAP.put('l', "bold");
CODE_MAP.put('m', "strikethrough");
CODE_MAP.put('n', "underlined");
CODE_MAP.put('k', "obfuscated");

CODE_MAP.put('r', "reset");
Copy link
Contributor

Choose a reason for hiding this comment

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

Pull from org.bukkit.ChatColor instead, except for underlined and obfuscated. (For those the Enum name doesn't match this string one).

Comment on lines +342 to +347
public static Audience audienceFrom(CommandSender... senders) {
List<Audience> bukkitAudiences = new ArrayList<>();
for (CommandSender sender : senders)
bukkitAudiences.add(getAdventure().sender(sender));
return Audience.audience(bukkitAudiences);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
public static Audience audienceFrom(CommandSender... senders) {
List<Audience> bukkitAudiences = new ArrayList<>();
for (CommandSender sender : senders)
bukkitAudiences.add(getAdventure().sender(sender));
return Audience.audience(bukkitAudiences);
}
public static Audience audienceFrom(CommandSender... senders) {
return audienceFrom(ImmutableList.copyOf(senders));
}

Comment on lines +331 to +334
List<Audience> bukkitAudiences = new ArrayList<>();
for (CommandSender sender : senders)
bukkitAudiences.add(getAdventure().sender(sender));
return Audience.audience(bukkitAudiences);
Copy link
Contributor

Choose a reason for hiding this comment

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

Could use Stream api but this is fine.

import java.util.List;
import java.util.Map;

public class ComponentHandler {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
public class ComponentHandler {
@ApiStatus.Internal
public final class ComponentHandler {
private ComponentHandler() {}

@APickledWalrus APickledWalrus changed the base branch from master to dev/feature September 16, 2023 18:16
@APickledWalrus
Copy link
Member Author

Closing as I do not feel these changes are well integrated. I'll re-examine how we can better integrate these libraries with a fresh start.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking changes Pull or feature requests that contain breaking changes (API, syntax, etc.) feature Pull request adding a new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants