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

Added Velocity support #182

Merged
merged 8 commits into from Feb 20, 2019

Conversation

Projects
None yet
6 participants
@Crypnotic
Copy link
Contributor

Crypnotic commented Dec 23, 2018

This PR adds support for the upcoming proxy Velocity.
https://www.velocitypowered.org/

A few note worthy issues:

  • Velocity does not provide a static method for its ProxyServer instance, so the instance has to be passed in by the plugin with new VelocityCommandManager(proxy, this)

  • Velocity does not provide a way to get the Plugin logger with the provided Plugin or PluginContainer, so it must be fetched using Logger logger = LoggerFactory.getLogger(plugin.getClass()); UNTESTED

  • Velocity uses an annotation to signify the main class, meaning it can't be passed around like the Bukkit and Bungeecord JavaPlugin and Plugin. An Object has to be passed in which the PluginContainer is extracted from it using the Proxy's PluginManager
    proxy.getPluginManager().getPlugin(plugin.getClass().getAnnotation(Plugin.class).id()).get();

Any and all feedback is welcomed and appreciated.

This implementation is essentially a copy/paste/rename. All code belongs to the original author, except the ACFVelocityUtil#matchPlayer which was originally written by md_5 and modified to use Velocity classes.

registerAsyncCompletion("chatcolors", c -> {
Stream<TextColor> colors = Stream.of(TextColor.values());
if (c.hasConfig("colorsonly")) {
colors = colors.filter(color -> color.ordinal() <= 0xF);

This comment has been minimized.

@kashike

kashike Dec 24, 2018

Contributor

This is useless. TextColor only has colours, no decorations - decorations are in TextDecoration.

This comment has been minimized.

@kashike

kashike Dec 24, 2018

Contributor

...and likewise, TextDecoration only has decorations, not colours.

String first = c.popFirstArg();
Stream<TextColor> colors = Stream.of(TextColor.values());
if (c.hasFlag("colorsonly")) {
colors = colors.filter(color -> color.ordinal() <= 0xF);

This comment has been minimized.

@kashike

kashike Dec 24, 2018

Contributor

This is useless. TextColor only has colours, no decorations - decorations are in TextDecoration.

This comment has been minimized.

@kashike

kashike Dec 24, 2018

Contributor

...and likewise, TextDecoration only has decorations, not colours.

This comment has been minimized.

@Crypnotic

Crypnotic Dec 24, 2018

Author Contributor

I realized that the moment I pushed it xD

registerAsyncCompletion("chatcolors", c -> {
Stream<TextColor> colors = Stream.of(TextColor.values());
if (c.hasConfig("colorsonly")) {
colors = colors.filter(color -> color.ordinal() <= 0xF);

This comment has been minimized.

@kashike

kashike Dec 24, 2018

Contributor

...and likewise, TextDecoration only has decorations, not colours.

String first = c.popFirstArg();
Stream<TextColor> colors = Stream.of(TextColor.values());
if (c.hasFlag("colorsonly")) {
colors = colors.filter(color -> color.ordinal() <= 0xF);

This comment has been minimized.

@kashike

kashike Dec 24, 2018

Contributor

...and likewise, TextDecoration only has decorations, not colours.

registerAsyncCompletion("chatcolors", c -> {
Stream<TextFormat> colors = Stream.of(TextColor.values());
if (!c.hasConfig("colorsonly")) {
Stream.concat(colors, Stream.of(TextDecoration.values()));

This comment has been minimized.

@kashike

kashike Dec 24, 2018

Contributor

Need to re-assign colors - concat doesn't mutate.

String first = c.popFirstArg();
Stream<TextFormat> colors = Stream.of(TextColor.values());
if (!c.hasFlag("colorsonly")) {
Stream.concat(colors, Stream.of(TextDecoration.values()));

This comment has been minimized.

@kashike

kashike Dec 24, 2018

Contributor

Need to re-assign colors - concat doesn't mutate.

return Collections.singleton(exactMatch.get());
}

return Sets.newHashSet(Iterables.filter(server.getAllPlayers(), new Predicate<Player>() {

This comment has been minimized.

@astei

astei Dec 24, 2018

Remove the unneeded usage of Guava. We've got Java 8 here:

return server.getAllPlayers().stream()
  .filter(player -> player.getUsername().toLowerCase(Locale.US).startsWith(partialName.toLowerCase(Locale.US)))
  .collect(Collectors.toList());

This could be even more efficient with String#regionMatches(), but I'll leave that as an exercise to you.

This comment has been minimized.

@astei

astei Dec 24, 2018

Alternatively, I could just add matchPlayer() to the Velocity API.

This comment has been minimized.

@Crypnotic

Crypnotic Dec 24, 2018

Author Contributor

I think other people could find a nice use matchPlayer()

@js6pak

This comment has been minimized.

Copy link

js6pak commented Jan 7, 2019

Defualt help/usage colors doesn't work
image

I'm temporary fixed it by doing this
image
image

@aikar

This comment has been minimized.

Copy link
Owner

aikar commented Feb 2, 2019

@kashike @js6pak @Crypnotic can someone resolve this concern with colors? otherwise looks good to me.

@Crypnotic

This comment has been minimized.

Copy link
Contributor Author

Crypnotic commented Feb 2, 2019

@aikar AFAIK, the TextColor issue is fixed. However, the fix feels rather dirty so it would be great if there was another way

@McLive

This comment has been minimized.

Copy link
Contributor

McLive commented Feb 18, 2019

So this might get merged soon? :D Really looking forward to it.

@aikar

aikar approved these changes Feb 20, 2019

@aikar aikar merged commit 9ad6497 into aikar:master Feb 20, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.