[B+C] Add API to use 1.7 chat features. Adds BUKKIT-5245

[Ribesg]

The Issue:

There are currently no easy way to use the new chat features introduced in 1.7.

Justification for this PR:

As said before, there are currently no way (except from directly using the /tellraw command and passing your own generated json to it) to use the 1.7 chat features.

PR Breakdown:

This PR adds a builder interface based on a RichMessage to which you append RichMessageParts to build a rich chat message containing (or not):

• a multi-color text, an LocalizedText, an ItemStack name or an Achievement name ;
• an optional hoverable (showing a multi-color multi-line text, an ItemStack or an Achievement) ;
• an optional clickable (executing commands / sending chat message, proposing commands / proposing chat message, or opening urls (actually opens the prompt)).

These messages handle colors and formatting using ChatColor just like standard messages, internally using the existing algorithm built by the great @mbax.

Example LocalizedText:

new LocalizedText("commands.scoreboard.players.reset.success", "Notch")

Many (many) shortcuts have been added to RichMessage to make it easier to append specific elements, see RichMessage constructors, append methods and insert methods.

RichMessage and RichMessagePart are ConfigurationSerializable making it easy to save and load them from a config file.

This PR also adds methods to Player to send a RichMessage and two methods to Server and Bukkit to broadcast a RichMessage. Those methods are mapped on the standards String methods.

Usage examples can be found in the TestPlugin after this line. You can get builds of all of this and test it yourself using the link below.

Testing Results and Materials:

Test Plugin Source
Build everything yourself to test it as we're not allowed to link CB builds here.

Relevant PR(s):

CB-1378 - Bukkit/CraftBukkit#1378 - Associated CraftBukkit PR

JIRA Ticket:

BUKKIT-5245 - https://bukkit.atlassian.net/browse/BUKKIT-5245

Authors

• @bendem did a lot of things (Mostly Bukkit part)
• @Ribesg did a lot of other things (CB part and Bukkit part)
• @ST-DDT did the Javadocs for multiple Bukkit files

[turt2live]

Is there a reason why the 'fix imports' commit is included?

Also, links to unofficial builds need to be torn down ASAP.

[bendem]

I think I read somewhere about not using import stuff.*... Maybe I'm wrong.

[Ribesg]

@turt2live @bendem I think he means that everything else is stashed, but that's because I cleaned the repo and rebased it correctly on upstream. The stash is just a consequence and wasn't intended.

The repo was dirty because I updated it to upstream with Github PRs in the past.

[Ribesg]

@mbax suggested that it would be nice to have the ability to modify every single already-modifiable-String-message as a RichMessage, for example in AsyncChat, Join, Quit, Kick events.

Those events currently have some kind of String getMessage() and setMessage(String) methods. We would need to deprecate the String getMessage() because it would be better for the API user to use the RichMessage. The String getMessage() method would return a String version of the RichMessage, cleaned from any hoverable and/or clickable. We would add RichMessage getMessage() and setMessage(RichMessage) methods to those events.

[Wolvereness]

This is a large change. I warn that there are many options we are considering, but for any of them, I want to see tutorials included in the javadocs so that immediately after being pushed, it would be easy to understand and there would be sufficient documentation to refer to for new users.

[bendem]

@Wolvereness Would the jd of RichMessage class be a correct place to put it?

Also, here is the yaml generated by the 21st test case.

[SpaceManiac]

Regarding RichMessage getMessage() in various events: it's probably a good idea to have these (esp. on chat), but would it be preferable to overmap (as for health and projectile shooters) or use a getRichMessage()?

[nightpool]

@Wolvereness When you said "there are many options we are considering" what exactly do you mean? We've been discussing this issue on leaky ([BUKKIT-5245]) but I've yet to see any input from a Bukkit team member.

[riking]

@Ribesg the gist for the test 21 output is wrong - a line was cut off.

Fixed it: https://gist.github.com/riking/9e3cb96ebb1c07dbd884

[bendem]

Thanks, @riking I don't know how I missed it.

[bendem]

@Wolvereness I added the requested documentation. I know you are pretty strict on the formatting, so let me know if you see anything I can improve.

[riking]

You can break @link tags over multiple lines.

[bendem]

Well, this is actually exactly 72 chars long... Isn't it the recommended width?

[Wolvereness]

The line before it is wrapped too early.

[mickare]

Just for imaginations there could be an easy ChatBuilder like the java.lang.StringBuilder or Google's Guava Builders.

I wrote an example here: https://bukkit.atlassian.net/browse/BUKKIT-5245?focusedCommentId=30724&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-30724

Currently I am working on a plugin API, but it would be way better implemented directly into Bukkit.
Greetings from Germany at 3 AM... ^^'

[Ribesg]

@mickare We already discussed that and it's not really sorter than what we got here. We should stop trying to make something short to use: a complex RichMessage is complex, the building process to create it has to be, too.

I'm still really busy but I hope that I'll be able to work more on this (especially integrating it with all events containing messages, etc) later. Maybe I'll have time in 2 weeks, if not it will be after the Ten.Java.

[rmsy]

Is there no way to set the color of a RichMessage other than instantiating one from legacy text? How would I color a LocalizedText?

[Ribesg]

Iirc you can do that:

new RichMessage(ChatColor.RED.toString()).append(new LocalizedText("localized.text.key", foo, bar));
new RichMessage("localized.text.key", foo, bar).insert(0, ChatColor.RED.toString());

We could add a setColor(ChatColor) which would basically do the exact same thing than append(String)... but I don't think that it's necessary.

[literalplus]

I think a setColor(ChatColor) would especially be helpful for new
developers and would also allow code to be shorter and easier to read. It
would also prevent the need of changes on existing code if Mojang decided
to change their chat format.

-- Above message was sent from my mobile phone. There may and will be all
kinds of errors included. For free!
On 13 Jun 2014 09:38, "Gael Ribes" notifications@github.com wrote:

Iirc you can do that:

new RichMessage(ChatColor.RED.toString()).append(new LocalizedText("localized.text.key", foo, bar));new RichMessage("localized.text.key", foo, bar).insert(0, ChatColor.RED.toString());

We could add a setColor(ChatColor color) which would basically do the
exact same thing than append(String)... but I don't think that it's
necessary.

—
Reply to this email directly or view it on GitHub
#1065 (comment).

[riking]

@Ribesg Isn't there a color attribute for the NMS TextComponents? There definitely is in the MoJson used in commands.

[rmsy]

Yes, that is what I was referring to. Doesn't your suggestion do the equivalent of:

{
color: null
components: [
{RichText
color: RED
message: null
},
{LocalisedText
color: null
key: "foo.bar"
args: ["foo", "bar"]
}
]
}

That is, if I'm understanding correctly. If I am, I'd prefer for all component types to inherit from a common interface, so that I can directly set the color of each without relying on the current color parsing behavior (left to right, persisting across multiple components), because that may change.

On Jun 13, 2014, at 10:32, Kane York notifications@github.com wrote:

@Ribesg Isn't there a color attribute for the NMS TextComponents?

—
Reply to this email directly or view it on GitHub.

[rmsy]

In fact, it feels weird for each type of text component to stand alone without any sort of inheritance when they share so many common characteristics.

[bendem]

@rmsy You mean like the fact that all the parts inherit from RichMessagePart?

[rmsy]

@bendem LocalizedText and RichMessage don't

[ST-DDT]

@rmsy There is also support for inline color codes (See Javadocs)
"&5I guess i don't know &LANY &1color codes!"

[nightpool]

@Ribesg It's weird to have to insert a text color code when what you really want to do is change the color of a RichMessagePart.

[Ribesg]

The API does not have to be based on implementation details. A RichMessage is not structured like the Mojangson / NMS code, it's converted to it. Exactly like text with color codes is currently converted to Mojangson.

A RichMessage is not a tree, it's a sequence, a list of parts. It's simpler to understand for user than trees.

@rmsy LocalizedText isn't a RichMessagePart because it's the equivalent of the String element composing a RichMessagePart (Maybe we should name it LocalizedString?). RichMessage doesn't have to extend RichMessagePart, but I agree that being able to append/insert a RichMessage to/into another could be a good thing to have.

@nightpool I understand your point, but the idea was to stay close to normal messages. It's weird to insert color codes into Strings while in fact they are colors of tree nodes in the implementation.

Adding a setColor would break the sequence/list thing for me. I would expect it to insert a color code (like a appendColor or insertColor) and not to change the color of some obscure tree node that RichMessage doesn't even represent. Also what should happen if I change the color of a RichMessagePart then of the whole RichMessage?

[ST-DDT]

Looks like they are going to use (some of) these chat features for books and co (in a future version) as well:
http://minecraft.gamepedia.com/Upcoming_features#Gameplay_3
See NBT Tags

[bendem]

I guess we will deal with that when 1.8 is out :)

[ST-DDT]

i don't know whether this is integrated in 1.8, thats why i called it future version.
But the current design looks also good for the new usecase.

[narrowtux]

What is still left on the TODO for this PR? I need some features from this ASAP, so I'd like to help accelerate the development so it can be in the next Bukkit snapshot.

[bendem]

This is a large change. I warn that there are many options we are considering, but for any of them, I want to see tutorials included in the javadocs so that immediately after being pushed, it would be easy to understand and there would be sufficient documentation to refer to for new users.

I'd guess the team still doesn't know the choice they will make concerning this api. I'd like to use these changes as soon as possible as well and I'm ready to continue write code and doc for this pr. Just waiting for some news. ;)

[Ribesg]

We also still have to add the ability to set RichMessages everywhere you can currently set String messages in the API. And fix conflicts.

This will be done.

[ST-DDT]

I sent you a PR to update your PR to latest Bukkit version.

[Ribesg]

Merged, thanks @ST-DDT.

[ST-DDT]

Any progress here?

[Ribesg]

@ST-DDT https://github.com/Ribesg/Bukkit/tree/BUKKIT-5245-v2/src/main/java/org/bukkit/chat

"Working" on a new version, inspired by this one and @feildmaster's one

[Ribesg]

Closed in favor of #1111