Add support for more text types than string literals

[glen3b]

Currently, Fanciful only supports textual values being string literals. As mentioned by issue #9, however, there are other options available to render text, such as localized strings. In addition, the linked post mentions many other alternatives to string literals for displaying text (which are currently supported in snapshots).

This PR adds all of the text-literal alternatives documented on the above post, throwing an UnsupportedOperationException for those which are not present in the stable Minecraft client (but the code exists such that it is easy to remove the exception upon release of the applicable stable Minecraft version).

Very little API is broken, the only existing publicly-visible method signature change was then(Object) to then(String), which is a more concise definition anyway (w/o docs: what's this random Object argument?).

No documentation is added in this PR as I am waiting for a status report on PR #27. I can add further documentation to this PR upon request.

[glen3b]

If/when #23 is developed, cloning instances may be required. Therefore, I will implement clone() on TextualComponent's default implementation. It also makes sense to have that type cloneable, as it is really only a key/value pair with factory methods.

[mkremins]

The implementation looks great, although I'm a bit leery of exposing TextualComponent directly as part of the public API – it seems like adding a few new methods to FancyMessage along the lines of thenSelector(String), thenLocalizedString(String), or similar might be preferable in terms of readability. Any thoughts on that approach?

[glen3b]

I'm personally more of a fan of static factory methods, I think they're perfectly readable (it makes sense when you see TypeName.fromSomething(arg), at least to me) and they still allow method chaining, but it is possible we could use the builder pattern. I'd personally like to see it either with static methods on TextualComponent or with both builder pattern and factory methods (if nothing else for ease of maintenance in the FancyMessage class). One problem with the builder pattern in this case is there are two methods for each type of text (the then and text methods respectively), which makes maintenance slightly harder (and also slightly harder to add support for new types of text) - the static methods allow for only maintaining one copy of each text type's factory method in TextualComponent (with the exception of string literals).

I honestly think it's more of personal preference at this point, as they do pretty much the same thing, and there are advantages and disadvantages to each approach.

[MiniDigger]

I think continuing the current design with the chain builder mkremins mentioned is better. If you do some thing with the chain builder and some with static methods it will confuse people.

[glen3b]

@MiniDigger I guess it's a tradeoff. If we do it with chain methods then it's more maintenance work for API developers, as we'd have to maintain 3(!) overloads of the same method (one then, one tooltip, one constructor, and maybe more). However, it may be slightly more convenient for API users.

If I am developing (and I didn't read API docs) and I see a method overload that accepts some strange argument (TextualComponent), I would personally first check for static factory methods on that class (which exist in our case) that accept the data I have, and use those. After that, I would know they exist, and I would use them after a one-time "get familiar with the API" moment. Since we also have overloads accepting string literals, I don't think it's that much of a problem. @mkremins It's also still readable, just as thenLocalized(String) makes sense (I think "append localized string"), then(TextualComponent.fromLocalizedString(String)) makes sense (I think "append localized text component").

Regardless of what the public API does, I think that using a TextualComponent class under-the-hood is the cleanest way to implement it to avoid code duplication. If it is part of the public API, we can also avoid documentation duplication.

[MiniDigger]

@glen3b I your idea is easier for the developers I think that is what should be done. But @mkremins should make shure that everything is well documented so that users can understand this new part of the api. Most of the people developing with bukkit never did anythink with java or even programming before so there would be many questions if the documentation is not understandable. But I don't understand why it is more work to maintain the api when using a chain method, that would call TextualComponent.fromLocalizedString(String). I like it to minimize and simplify my code whenever I can.

[mkremins]

Potential compromise: continue to accept TextualComponent instances as arguments to then, text, etc., but tweak the names of the TextualComponent static factory methods a bit and encourage users to import static TextualComponent in their own code.

That way, code making use of the new API features might look something like:

new FancyMessage().text(selector("@p")).then(localized("entity.cow.name"));

...where selector and localized are the new names of TextualComponent.fromSelector and TextualComponent.fromLocalizedString respectively, statically imported for unqualified use.

I feel like this reads a bit more fluently and also allows us to avoid maintaining separate implementations of thenSelector, thenLocalized and friends. Any thoughts?

[MiniDigger]

I think this is a good compromise.

A little bit of an offtopic question: How does the @p selector works. In vanilla it chooses the nearest player but which location will it choose with Fanciful?

[glen3b]

@MiniDigger it should choose, in theory, whatever the client sees as the nearest player, usually the client themself, as in vanilla behavior.

@mkremins that's a really good compromise, keeping both readability and maintainability. I'll implement that.

[glen3b]

If this is merged, I believe PR #30 will need updates. If this is merged first, I can update the other PR to account for the final changes added by this commit to result in an easier merge. Alternatively, @mkremins can make those necessary changes for the code to compile.

[mkremins]

Would you prefer I merged this or #30 first?

[glen3b]

Merge this first please so I can update #30.