Adding Rich chat elements. Adds BUKKIT-5245

[bendem]

Base API entry points:

• [x] Add a sendRichMessage method to Player
• [x] Add a broadcastRichMessage method to Server / Bukkit
• [x] Implement RichMessage
• [x] REALLY implement RichMessage
• [x] Implement RichMessage conversion to NMS in CraftChatMessage
• [x] All the JavaDocs in the world!
• [x] Make all the thingz implement ConfigurationSerializable

  Current state

• Bukkit diff
• CraftBukkit diff

  Documents

• Example of what a /tellraw command can take as argument.
• Related ticket : BUKKIT-5245 Note that everything is subject to changes / improvements.
• A thread about /tellraw
• The specification of the Json object on the wiki

[Ribesg]

Currently blocking: choose API design.

Should we actually expose the recursive design entirely, or make something that is REALLY like a StringBuilder, taking "RichMessagePart" as argument?

What do you think of this example usage?

// Green and underlined
final RichMessage mes = new RichMessage("Beginning ", ChatColor.GREEN, ChatColor.UNDERLINE);

// Green and underlined too
mes.append(Tooltip.withItem("StringWithItemTooltip", myItemStack));

 // Still green and underlined
mes.append(" some more text ");

 // Still green and underlined
mes.append(Clickable.url("clicking this opens Google", "http://www.google.com"));

// Red, bold and underlined
mes.append(" some final text", ChatColor.BOLD, ChatColor.RED);

somePlayer.sendRichMessage(mes);

// Or why not
somePlayer.sendMessage(mes);

Comments? @bendem @ST-DDT

I'll try to make a full example later, covering every use cases (Clickable + tooltip, etc)

[ST-DDT]

Mhh, this example is good. But i guess we should drop the extra color part and handle that internaly.

final RichMessage mes = new RichMessage(ChatColor.GREEN.toString() + ChatColor.UNDERLINE + "Beginning " + ChatColor.RED.toString() + ChatColor.BOLD+ChatColor.UNDERLINE + " some final text.");

Which would actualy result in the above.

We should use the short one, there is no more information contained in the long version than there is in the short version.

somePlayer.sendMessage(mes);

This will also help new users who will probably use some kind of IDE which orders the methods based on their name displaying both methods next to each others. But there are already methods which use the long version, but i guess it is required in those cases.

IDE view:

saveData();
sendMap(MapView map);
sendMessage(String message);
sendMessage(String[] messages);
sendMessage(RichMessage message); // short version
sendPluginMessage(Plugin plugin, String channel, byte[] data);
sendRawMessage(String rawMessage);
sendRichMessage(RichMessage message); // long version
serialize(): Map<String,Object>;

Will it then work somehow this way?

public void sendMessage(RichMessage message) {
    sendRawMessage(message.magic().toString());
}

Or do i just miss the point with sendRawMessage(String message)?

[Ribesg]

@ST-DDT Where are the tooltip and the clickable in your onliner?

Also, the only difference between sendMessage(String) and sendRawMessage(String) is that it bypasses Conversations. Also, we will directly convert RichMessage object to net.minecraft.server.IChatBaseComponent object (see CB diff https://github.com/Ribesg/CraftBukkit/compare/BUKKIT-5245 )

[ST-DDT]

@Ribesg Got me:

Now here is the long version:

// Green and underlined
final RichMessage mes = new RichMessage(ChatColor.GREEN.toString() + ChatColor.UNDERLINE.toString() + "Beginning ");

// Green and underlined too
mes.append(Tooltip.withItem("StringWithItemTooltip", myItemStack));

 // Still green and underlined
mes.append(" some more text ");

 // Still green and underlined
mes.append(Clickable.url("clicking this opens Google", "http://www.google.com"));

// Red, bold and underlined
mes.append(ChatColor.RED.toString() + ChatColor.BOLD.toString() + ChatColor.UNDERLINE.toString() + " some final text.");
// Same behaviour as sendMessage(String) reset all formating codes when adding a new color.

mes.append(ChatColor.GREEN.toString() + " This should " + ChatColor.BOLD.toString() + "WORK" + ChatColor.RESET.toString() + ", too!");

// somePlayer.sendRichMessage(mes);

// Or why not
somePlayer.sendMessage(mes);

[Ribesg]

@ST-DDT ok, I see. I think that this breaks the Appendable paradigm.

For example if you use a StringBuilder and use append(ChatColor.RED), the next append will be red too.

[TomyLobo]

camel, for instance, has ".end()" to end a block

[ST-DDT]

Why? What exactly does break that paradigmn?

// Same behaviour as sendMessage(String) reset all formating codes when adding a new color.

That one? In my opinion it does not break anything.

For example if you use a StringBuilder and use append(ChatColor.RED), the next append will be red too.

I know and that is exactly as i want, but:

player.sendMessage(ChatColor.BOLD + "Bold and Default " + ChatColor.RED + "Thin and Red");

is the same as

player.sendMessage(ChatColor.BOLD + "Bold and Default " + ChatColor.RESET + ChatColor.RED + "Thin and Red");

and i think our builder should reflect that behaviour.

Here is the kind of procedure i thought that could run in the background with append(String)

while (text.startsWith(ColorCode.XYZ)) {
    activeCodes.add(text.substring(0,2);
    text = text.substring(2);
}

int index = text.find(ColorCode.XYZ);
if (index == -1) {
    return append(text, activeCodes.getColor(), activeCodes.getFormats());
} else {
    return append(text.substring(0, index - 1), activeCodes.getColor(), activeCodes.getFormats())
        .append(text.substring(index));
}

[TomyLobo]

But why aren't you making a fluent api? Why this MessagePart stuff?

final RichMessage mes =
    color(ChatColor.GREEN,
        underline(
            text("Beginning ")
            .tooltip(item("StringWithItemTooltip", myItemStack),
                text("some more text")
            )
            .clickable(url("http://www.google.com"),
                text("clicking this opens Google")
            )
        )
    )
    .color(ChatColor.RED,
        bold(
            underline(
                text(" some final text")
            )
        )
    )
    .text(ChatColor.GREEN.toString() + " This should " + ChatColor.BOLD.toString() + "WORK" + ChatColor.RESET.toString() + ", too!");

somePlayer.sendMessage(mes);

See how this doesn't require comments around it and still you see precisely where each format starts and ends? It should also autoindent well in any java/C/perl-aware text editor.

EDIT: the text() wrapper could be made optional as well in many instances. Makes it less consistent though, which is why it's not in the example.

[Ribesg]

@TomyLobo This is horrible, seriously. are those bold, underline, text methods static in this example? Static import is not what people usually do.

[bendem]

@TomyLobo You know Bukkit is written with Java right?

[ST-DDT]

It should also autoindent well in any java/C/perl-aware text editor.

My IDE (Eclipse) thinks it should push everything in a single line or just content un-aware in a few lines. Which is horrible to read. In addition to that why reinvent the wheel with bold(???) and color(ColorCode, ???) when there is there alread the ColorCode.BOLD... etc part in use? I know Fluent-API is the new buzz word out there but you don't have to push it everywhere. In my opinion the Builder chaining (which is also considered being part of Fluent-API) new RichMessage("Text").append("More Text").append(stuff) like suggested above works pretty good.

Camels RouteBuilder is nice clean, readable even with bad linebreaks.

from("cxf:webserviceEndpoint").to("securityBean").doTry().beanRef("validator", "validate").doCatch(Exception.class).to("errorProcessor").endTry().to("processor");

Groovy/grails is also nice and clean.

def someSet = []
someSet << "a" << "b" << "c"

[TomyLobo]

@Ribesg the idea was to use encourage using a static .* import, yes. And what exactly is horrible about it? It's quite the opposite. It makes code readable, maintainable and @bendem yes. That was Java.

@ST-DDT Eclipse: It is known to do that, yeah... one reason I switched to IDEA :)

Reinventing the wheel: If you ask me whether I want to see .color(ChatColor.BOLD, ???) or .bold(???), I'm going to pick the former. Why have a .color method at all? Because then it can be cascaded.

Camel: to is for endpoints, process is for processors :)

Groovy: Let's not require a specific language just to write text. Might as well go back to my XML proposal then :)

[TomyLobo]

Here's an optimized version of the same example:

final RichMessage mes =
    underline(
        color(ChatColor.GREEN,
            "Beginning ",
            tooltip(item("StringWithItemTooltip", myItemStack),
                "some more text"
            )
            .clickable(url("http://www.google.com"),
                "clicking this opens Google"
            )
        )
        .color(ChatColor.RED,
            bold(
                " some final text"
            )
        )
    )
    .text(ChatColor.GREEN.toString() + " This should " + ChatColor.BOLD.toString() + "WORK" + ChatColor.RESET.toString() + ", too!");

somePlayer.sendMessage(mes);

[Ribesg]

@TomyLobo Will definitely not do that. Looks too much JavaScript-ish, that's not something you do in Java.

And for your .color(...) and .bold(...) thing... Why not just a method that takes (String, ChatColor...)?

And the String could also contain ChatColor char's. Anybody could use it however he wants then.

[ST-DDT]

@Ribesg Why did you add the append(String, ChatCholor...) method, if you have to ChatColor aware append(String) method? Performance or readability reasons?

@TomyLobo I don't like static imports. I don't like this kind of cascading. Both are not used in Bukkit or CraftBukkit.

You can use your format also in Eclipse, but it does not match Bukkit's requirements. https://github.com/Bukkit/Bukkit/blob/master/CONTRIBUTING.md#code-requirements

Camel with Spring (uses to() instead of process())

As @Ribesg is the author of this PR i guess you have to create your own PR if you want to see your version added to Bukkit.

[TomyLobo]

@Ribesg Are you familiar with Apache Camel? It uses a similar DSL for its routes: http://camel.apache.org/routes.html What I don't like about Camel's Java DSL is that it has no compile-time checks for structural soundness and it doesn't indent well at all in any editor (both for much the same reason) That's why in my mock-up, I chose to employ Java's expression structure in order to convey the cascading structure.

Also did you take a look at my XML proposal? You could use minecraft color codes there, too. This mock-up is essentially a different way to write the same thing, with all the advantages still in place, except you require a separate serialization format.

@ST-DDT oh, so you don't like cascading formatting? I see. How do you propose to remove part of the format? Whether or not this is already used in Bukkit is kinda irrelevant, tbh. whether it matches the requirements is also irrelevant if the requirement in question is arbitrary. If the requirements said "Write JDK 1.3-compatible code", would you make the same argument? Just out of curiosity, what part of the requirements did my mock-up not fulfil?

corresponds to .to() What PR? This is an issue, not a PR.

[TomyLobo]

I don't know why I bother with you guys anyway. You don't seem to be that keen on crafting the best proposal possible. Instead you're more interested in asides like whether it looks like Java or whether it fulfils the contribution guidelines...

Personally, all I really need is a stable backend for my XML parser so I don't have to port that every update. And arguing with you over this is only gonna get you done slower.

So, see ya

[ST-DDT]

This is an issue to create a place to make the discussions. Everyone is free to join the discussion and propose new things. Things that have been declined, should not been repeated over and over. If you want to have your way added to Bukkit feel free to create your own PR.

Once this discussion and the code is done the PR will be created.

This discussion is not about Camel and Bukkit does not use Camel at all so we should drop it. I only brought this up to show some (from my point of view) more readable Fluent-API example.

You don't seem to be that keen on crafting the best proposal possible.

I guess the best is somehow based on the point of view.

Instead you're more interested in asides like whether it looks like Java or whether it fulfils the contribution guidelines...

If it does not look like expected or does not fullfills the contribution guidelines (arbitrary or not): It won't be pulled. (Whether the contribution guidelines are good is not part of the discussion)

oh, so you don't like cascading formatting?

I'm not part of Bukkit's code team, nor do i have any other function for Bukkit except as a contributor. So others can prefer your way i don't care. I simply wanted to point out that i don't like it myself and my point of view is almost the same as the author's point of view. If there is something wrong with my translation i'm sorry. (i don't hate the cascading/static import style, i just try to avoid it).

Personally, all I really need is a stable backend for my XML parser so I don't have to port that every update.

XML? What has XML to do with minecraft's messages? Does your plugin some conversion between XML->Minecraft messages? EDIT: Forget this question as it is not topic related. Once this got pulled, however it might look like in the end, it can be considered stable. Because Bukkit only change things if they really don't work anymore in minecraft.

[Ribesg]

@ST-DDT TomyLobo made a PR about this, he was using XML. So there was basically Player.sendXmlMessage and stuff like that. I don't really tried to read all comments about it, but I think that forcing everybody to use XML is not a good thing.

On the other hand, it would allow to directly read messages from some xml files, which could be nice. But I think that this should not be something in the API, but as TomyLobo said, something using the API.

To get back on what we're doing, I think we will first implement a simple, base concept:

• A RichMessage is basically a list of RichMessagePart, which can be from a simple String to a clickable bold underlined red link with a tooltip. You would basically have RichMessage.append(RichMessagePart) and RichMessage.insert(int, RichMessage) for that. TomyLobo's XML thing, for example would just rely on that.
• Then, on top of that, we add optional shortcuts, for example RichMessage.append(String).

@ST-DDT My idea behind RichMessage.append(String, ChatColor...) was to reflect the internal representation a bit, but I think that your idea to just consider the String as a ChatColor-ed String is good. This will add complexity to the CB conversion process to IChatBaseComponents, but at least it would work like people expect it to work.

[TomyLobo]

you could make RichMessage implement/extend RichMessagePart and just append it wherever you want.

(And then your API is also cascading, tehehe)

[bendem]

I just added the last jd's, so I think this ends this discussion :smile: This is kinda the biggest copy paste in my life so it can contains some incoherences...