Added a system for creating chat messages with a style. #15

killjoy1221 · 2014-09-09T04:08:02Z

Supports colors, fancy stylings, and click/hover events.

The sooner we don't need to use legacy chat the better.

Zidane · 2014-09-09T04:11:11Z

haha...you don't have the licenses in.

Make sure to run gradle :p

Zidane · 2014-09-09T04:12:27Z

src/main/java/org/spongepowered/api/chat/ChatClientEvent.java

+         */
+        @Override
+        public ChatAction getAction() {
+            // TODO Auto-generated method stub


Make sure to get rid of these

killjoy1221 · 2014-09-09T04:28:35Z

Fixed the licences and moved everything to a util package. No point in it being in the api package if it's a utility.

bensku · 2014-09-09T06:12:29Z

Looks good :) It may require some work to implement, but it'll be worth of it, in my opinion.

unenergizer · 2014-09-09T07:45:59Z

Sponge should be as lightweight as possible. No need to clutter it with extras. Plugins should be good enough to add this sort of functionality.

Think fast. Think efficient.

ryantheleach · 2014-09-09T08:58:45Z

@unenergizer what API would you expect a plugin to need in order to do this?

spaceemotion · 2014-09-09T09:36:26Z

src/main/java/org/spongepowered/util/chat/IChatMessage.java

+     */
+    void setStyle(ChatStyle style);
+
+    IChatMessage createCopy();


Missing javadoc ;)

Also this could simply be copy or implement Cloneable and call it clone()

It should also be made clear whether this is a shallow copy or a deep copy.

lenis0012 · 2014-09-09T10:56:53Z

Plugins cant do this without nms unless we add sendRawMessage

narrowtux · 2014-09-09T11:49:34Z

Sponge should be as lightweight as possible. No need to clutter it with extras. Plugins should be good enough to add this sort of functionality.

This is what I hated about bukkit. "Let plugins implement this, let plugins implement that". Led to a huge clusterduck that is permissions. Also why would you leave something as essential as this up to plugins?

DarkArc · 2014-09-09T13:20:37Z

src/main/java/org/spongepowered/util/chat/ChatAction.java

+    SHOW_TEXT("show_text", ActionType.HOVER),
+    SHOW_ACHIVEMENT("show_achivement", ActionType.HOVER),
+    SHOW_ITEM("show_item", ActionType.HOVER),
+    ;


Can we not use enums for actions? Like as a rule? They've very limiting, and while I can't think of a reason why that is bad here, for example in bukkit you could not provide custom teleport reasons because they were restricted to the enum.

In this case it makes sense. Minecraft cannot have more hover actions than already listed. If Minecraft adds more then the enum can be updated to match.

Right now yes, but if we ever also modify the client... Just... ugh enums are bad for dynamic content.

Oh, yes, I forgot Sponge is both. This should be changed to a final class with constants perhaps.

Yes, I see things like this more of a quick and easy access thing. They shouldn't be a limiting factor.

DarkArc · 2014-09-09T13:39:16Z

Perhaps the same concept should apply to the colors as well, something else might add more colors.

Mumfrey · 2014-09-09T14:15:06Z

I've seen other implementations of this idea and I'll say the same thing I've said in the past: _Why are the _ChatMessage* interface's methods not fluent?*

The underlying ChatMessageComponent API is fluent so wrapping an API around it which is not is a step backwards. appendSibling is bad and appendSiblings is frankly stupid and there should instead be simple overloads for appending components, I'd suggest following the established and well-known pattern used by StringBuilder of multiple overloads all called append which simply take different arguments and provide a fluent interface:

ChatComponent c = new ChatComponent().append("Some text").append(Colours.RED).append("more text").append(otherComponent).append(someInteger).

Using fluent interface allows use of both fluent and non-fluent usage so there's no excuse not to have appender methods return the original object reference.

tlaundal · 2014-09-09T14:34:38Z

src/main/java/org/spongepowered/util/chat/ChatClientEvent.java

+package org.spongepowered.util.chat;
+
+import org.spongepowered.util.chat.ChatAction.ActionType;
+


There should be a javadoc here.

tlaundal · 2014-09-09T14:39:16Z

What is the point of providing both an interface and an implementation in the API?
If the implementation is available in the API, all developers will use that instead of the interface, and then there will be no use for the interface.

keir-nellyer · 2014-09-09T15:49:26Z

@unenergizer Actually, for Sponge to be more efficient, it should add APIs like these. Stops 5 different plugins including their own APIs and possibly not as well optimized as the Sponge one could be.

Mumfrey · 2014-09-09T15:59:54Z

@iKeirNez he wasn't saying this API shouldn't exist, he was saying that this PR pulls implementation into the API, which is a huge no-no. Once @killjoy1221 revises this into a sensible PR containing an API and another with implementation, then it can be considered.

killjoy1221 · 2014-09-09T16:50:38Z

If someone can suggest a way for devs to create a new chat message without implementing it themselves, I'm all ears. Maybe a method somewhere else that creates a blank message?

Mumfrey · 2014-09-09T16:57:33Z

@killjoy1221 what do you mean? The implementation should be in Sponge not in SpongeAPI. You have implementation in an API project and the chances of this being pulled are pretty much zero until you fix that.

MazeXD · 2014-09-09T19:22:24Z

@Mumfrey So in general every plugin which needs to use ChatMessages has to depend on Sponge instead of SpongeAPI to avoid writing the chatmessage implementation over and over again? :/
How would a plugin otherwise create a valid ChatMessage object if it is an interface?
Except there will be some services like ChatMessageFactory.

killjoy1221 · 2014-09-09T19:44:33Z

I moved things back into api and removed the implementation. I'm expecting someone else to create a class to create new instances of a lot of this.

I also changed most enums into final classes / fields. Plugin devs can create their own now.

In the ChatMessage, there's more append methods and it behaves like StringBuilder.

I removed the change in Game in order to avoid merge conflicts.

Mumfrey · 2014-09-09T19:50:11Z

@MazeXD I'm not sure you understand the concept of an API, just because the implementation is elsewhere does not mean you can't rely on the API. This is why factory and builder pattern are important, plus provider. For example, you may have access to an IChatMessageFactory via your plugin, or via a general provider interface, you don't go new ChatMessage() instead you go WhateverProvider.getChatMessageFactory().createChatMessage() which returns the interface and then you work with that (for example). It's perfectly possible and normal to abstract this way.

It's then up to the implementation to actually flesh out the provider, the factory and the chat message instance itself.

spaceemotion · 2014-09-09T19:54:49Z

@Mumfrey This would be called a Factory or Repository pattern then I think.

~~It would also be great if the ChatMessage appending could use the a Builder-/Fluent pattern based design.~~ Forget it - just noticed that now ;)

killjoy1221 · 2014-09-09T20:00:20Z

Fixed that NPE and added a factory.

turt2live · 2014-09-14T04:33:07Z

src/main/java/org/spongepowered/api/text/TextStyle.java

+     * Gets the text to be inserted in chat when shift clicked.
+     * 
+     * @return The text to be inserted
+     * @since 1.8


Remove some methods in TextMessage in favor of MessageBuilder Signed-off-by: Matthew Messinger <mattmess1221@gmail.com>

…o chat Conflicts: src/main/java/org/spongepowered/api/Game.java

zml2008 · 2014-09-14T07:21:08Z

(going to all chat formatting PR's)
imo the legacy section-symbol formatting codes should not be exposed in the API. The system's being phased out of vanilla MC and anything that still uses it can be converted in the implementation.

turt2live · 2014-09-14T14:11:16Z

@killjoy1221 Please identify how this PR is better/worse than #47 and why it should be pulled, thanks!

killjoy1221 · 2014-09-14T20:03:26Z

@turt2live aside from the fact that that PR doesn't pass the build, I'm set for 1.8. #47 uses lots of enums and final classes. I use final classes (previously enums for extensibility) and interfaces. 47 was filled with implementation.

My messages are general purpose, so we can support most types of messages (text, translate, score, selector).

Additionally, I provide a builder. You're free to use it for your entity component system if that's how it works.

spaceemotion · 2014-09-14T20:32:03Z

src/main/java/org/spongepowered/api/text/MessageBuilder.java

+/**
+ * Builder for easily creating messages.
+ */
+public interface MessageBuilder extends Builder<MessageBuilder, TextMessage> {


Just curious; but why are you using "append" and not "add"? Because the StringBuilder names it like that as well?

I tend to try to use shorter names when I do "chainable" classes/interfaces, since I don't want to get over my 100 character limit per line ;)

ST-DDT · 2014-09-14T20:41:38Z

Can you please give me a hint what i have to add to provide 1.8 support?
I looked through most of your code but haven't seen any uncovered things, if i don't count methods you tagged as ToDo.

killjoy1221 · 2014-09-14T20:48:09Z

Insertions, score messages, and selector messages. See http://wiki.vg/Chat

ST-DDT · 2014-09-14T21:04:04Z

Thanks for pointing that out.

i will look into that, it shouldn't be that hard to implement
What does the selector do?

killjoy1221 · 2014-09-14T21:05:50Z

Usually their used in commands. They include @A, @p, @r, @e.

On Sunday, September 14, 2014, ST-DDT notifications@github.com wrote:

Thanks for pointing that out.

i will look into that, it shouldn't be that hard to implement
What does the selector do?

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

ST-DDT · 2014-09-14T21:07:27Z

I have never used them myself, will @p be replaced by the player's name for example?

killjoy1221 · 2014-09-14T21:08:38Z

Experiment with them using /tellraw.

On Sunday, September 14, 2014, ST-DDT notifications@github.com wrote:

I have never used them myself, will they replaced by the player's name for
example?

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

zml2008 · 2014-09-14T21:08:47Z

Well those selectors aren't really chat format things -- they're command
things so sorta out of the scope of this PR.

On Sun, Sep 14, 2014 at 02:07:32PM -0700, ST-DDT wrote:

I have never used them myself, will they replaced by the player's name for example?

Reply to this email directly or view it on GitHub:
#15 (comment)

ST-DDT · 2014-09-14T21:24:25Z

@zml2008 They are relevant, because you can send them as chat message.

…o chat

Added a FactoryProvider Added parameters when making a new translated message Signed-off-by: Matthew Messinger <mattmess1221@gmail.com>

turt2live · 2014-10-27T04:25:02Z

@killjoy1221 Please fix the mergability concerns for further review, thanks.

maxov · 2014-11-11T03:17:16Z

Please see my comment on the other chat PR for updates on how this API is progressing. I'm going to close this PR for now since it seems to have stagnated.

Zidane reviewed Sep 9, 2014
View reviewed changes

spaceemotion reviewed Sep 9, 2014
View reviewed changes

DarkArc reviewed Sep 9, 2014
View reviewed changes

nikosgram mentioned this pull request Sep 9, 2014

Add MC Title 1.8 interface. #26

Closed

tlaundal reviewed Sep 9, 2014
View reviewed changes

Zidane force-pushed the master branch from b441ab9 to c368a6d Compare September 10, 2014 02:10

turt2live reviewed Sep 14, 2014
View reviewed changes

killjoy1221 added 2 commits September 14, 2014 01:15

Update to new formatting and complete incomplete JavaDocs.

e5d15f5

Remove some methods in TextMessage in favor of MessageBuilder Signed-off-by: Matthew Messinger <mattmess1221@gmail.com>

Merge branch 'master' of git://github.com/SpongePowered/SpongeAPI int…

9e7b4ec

…o chat Conflicts: src/main/java/org/spongepowered/api/Game.java

simon816 mentioned this pull request Sep 14, 2014

Add Title API for Minecraft 1.8. #156

Merged

spaceemotion reviewed Sep 14, 2014
View reviewed changes

killjoy1221 added 2 commits September 14, 2014 19:05

Merge branch 'master' of git://github.com/SpongePowered/SpongeAPI int…

ea34f52

…o chat

Added many new methods to MessageBuilder

7111cb1

Added a FactoryProvider Added parameters when making a new translated message Signed-off-by: Matthew Messinger <mattmess1221@gmail.com>

turt2live self-assigned this Oct 27, 2014

Kobata unassigned turt2live Nov 2, 2014

maxov closed this Nov 11, 2014

		package org.spongepowered.util.chat;

		import org.spongepowered.util.chat.ChatAction.ActionType;

Added a system for creating chat messages with a style. #15

Added a system for creating chat messages with a style. #15

Conversation

killjoy1221 commented Sep 9, 2014

Zidane commented Sep 9, 2014

Choose a reason for hiding this comment

killjoy1221 commented Sep 9, 2014

bensku commented Sep 9, 2014

unenergizer commented Sep 9, 2014

ryantheleach commented Sep 9, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lenis0012 commented Sep 9, 2014

narrowtux commented Sep 9, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DarkArc commented Sep 9, 2014

Mumfrey commented Sep 9, 2014

Choose a reason for hiding this comment

tlaundal commented Sep 9, 2014

keir-nellyer commented Sep 9, 2014

Mumfrey commented Sep 9, 2014

killjoy1221 commented Sep 9, 2014

Mumfrey commented Sep 9, 2014

MazeXD commented Sep 9, 2014

killjoy1221 commented Sep 9, 2014

Mumfrey commented Sep 9, 2014

spaceemotion commented Sep 9, 2014

killjoy1221 commented Sep 9, 2014

Choose a reason for hiding this comment

zml2008 commented Sep 14, 2014

turt2live commented Sep 14, 2014

killjoy1221 commented Sep 14, 2014

Choose a reason for hiding this comment

ST-DDT commented Sep 14, 2014

killjoy1221 commented Sep 14, 2014

ST-DDT commented Sep 14, 2014

killjoy1221 commented Sep 14, 2014

ST-DDT commented Sep 14, 2014

killjoy1221 commented Sep 14, 2014

zml2008 commented Sep 14, 2014

ST-DDT commented Sep 14, 2014

turt2live commented Oct 27, 2014

maxov commented Nov 11, 2014