search

mkremins / fanciful

Fancy Bukkit message formatting
MIT License
95 stars 82 forks source link

Return proper usage since users don't need to track dirty themselves whe... #37

Closed ConnorStone closed 10 years ago

ConnorStone commented 10 years ago

...n passing the message around their plugin

This will change will make it 100x easier for people like my self who pass the message around my plugin many times adding stuff to the message before sending it, and cannot keep track of if it has/hasn't been given any text yet

killje commented 10 years ago

i don't think allowing the plugin to extend itself is good behavior. as plugin creator you should be the one saying what to do. if you have problems keeping track of what happens try to do more often a then, fanciful can handle empty parts

mkremins commented 10 years ago

It does seem inconsistent that a duplicate call to .text() will throw an exception, while a duplicate call to .color() will silently update the color. On the other hand, your patch (as currently implemented) will break the semantics of .then(). Consider the following case:

new FancyMessage().color(ChatColor.RED).then("some text");

Under Fanciful's current behavior, this sequence of calls will throw an error complaining about the attempt to append a new message part while the previous one remains empty. Your patch would instead allow this sequence of calls to succeed and display "some text" in red: rather than creating a new TextualComponent with neutral styling, the call to .then() would reuse the existing (empty) TextualComponent, picking up its styling in the process.

Maybe .text() should be permitted to reassign the text of a non-empty message part without error, but .then() should be kept as is? Then again, I don't know if that behavior could be made to play well with the internal treatment of localized text, selectors and the like as distinct concrete types.