Text these days is really rather complicated, supporting numerous options and features. However, this also means that the JSON text can get really large really fast, even if the visible/interactable content is actually rather minimal.
This became really apparent in #1309 which has a bunch of seemingly excessive information.
While working on a plugin that generated a list, I have managed to break the 32k byte limit and disconnect clients with messages that are just too long. Pagination relieves the problem, but only by so much.
My idea was that it could be possible to take a complicated Text object, and simplify it without removing any of its content. This would be done by combining like but separate elements, removing empty ones, and collapsing any unnecessary hierarchy. Ideally we could evaluate a Text object and compress it down to the smallest size possible that still represents the same thing.
While this isn't a large or important optimization, it could be helpful to those plugins that just want to dump a lump of text into chat without disconnecting anyone.
Thoughts anyone?
would this require the CLIENT to have mods to deconvolute the modified datastream though?
No. Here's an example of what he's talking about:
{text: "Spo", children=[{text: "ngePo"}, {text: "wered"}]}
can be simplified to:
{text: "SpongePowered"}
without breaking protocol compatibility.
Ah, sweet example
Here an example implemententation:
https://gist.github.com/ST-DDT/b296398917891345e8d4c160916bed48
Output
From @JBYoshi 's example
test(Text.builder("Spon")
.append(Text.of("gePo"))
.append(Text.of("wered"))
.build());
Original: Text{children=[Text{gePo}, Text{wered}], Spon} (Length: 46)
Simplfied: Text{SpongePowered} (Length: 19)
more complex example
test(Text.of(FakeColor.C1,
Text.of("Spon"),
"ge",
Text.of(FakeColor.C2),
Text.of(FakeColor.C1, "Pow",
Text.of("ered"))));
Original: Text{children=[Text{format=TextFormat{color=C1, style=TextStyle{}}, Spon}, Text{format=TextFormat{color=C1, style=TextStyle{}}, ge}, Text{format=TextFormat{color=C1, style=TextStyle{}}, }, Text{format=TextFormat{color=C1, style=TextStyle{}}, children=[Text{format=TextFormat{color=C1, style=TextStyle{}}, Pow}, Text{format=TextFormat{color=C1, style=TextStyle{}}, ered}], }], } (Length: 377)
Simplfied: Text{format=TextFormat{color=C1, style=TextStyle{}}, SpongePowered} (Length: 67)
Most devs are not too stupid to write these lines, iterating over texts to simplify them every time we send a message or something that contains a text to the client would cause more performance issues than benefits.
We can provide the simplifier as an optional tool, useful for some devs but not for all.
I think that was the idea. You would have a method call to do this.
The reason this is useful is that i have a lot of code that dynamically constructs text objects, but because of the dynamic nature, there are a lot of extra elements that can be trimmed out in many cases.
It could be like Text#compact() or Text#minify() or something.
@gravityfox If you dynamically construct Text objects, you should try to build them as compact as possible. Compacting a Text object is a potentially quite expensive operation since you need to iterate through the whole tree and re-create all needed Text objects.
It will be always much more efficient if you modify your method to create efficient texts.
I mean, that's the goal, but with dynamically generated text, there's always a myriad of bizzare conditions that make it so that certain elements aren't needed.
So as someone that recently implemented a Text system inspired by Sponge's system where I completely bypassed this problem, wouldn't a better and simpler solution just be to make Text.Builder#append less stupid? Now yes, that is just one set of methods, but from what I've seen, literally everything is done using append. If it knew when to combine to simple texts into one simple text I think that would make a huge difference alone.
Two things that I can see helping here. First, check if the text to append is empty, and don't do anything if it is. Second, check if the last child appended was a plain text (contains nothing other than a string) and if the text to append is a plain text. If they both are, merge them together.
I can see if I can get a pull request up to add these if anyone is interested.
First, check if the text to append is empty, and don't do anything if it is.
I don't think that should be done.
Second, check if the last child appended was a plain text (contains nothing other than a string) and if the text to append is a plain text. If they both are, merge them together.
:+1:
Feel free to submit a PR, I'd be interested, and I'm sure others would be too.
The problem with doing this is that it would be a breaking change in some ways.
That's why i say make it a specialized function.
The problem that my suggestion is trying to solve is oversized text packets, otherwise it wouldn't matter much at all.
First, check if the text to append is empty, and don't do anything if it is.
I don't think that should be done.
Second, check if the last child appended was a plain text (contains nothing other than a string) and if the text to append is a plain text. If they both are, merge them together.
馃憤
IMO everything should either become simplified when appended or nothing at all, thus both should be done or none. @kashike Mind sharing the reason for only doing one of both?
If the empty text is intended as structural thing that gets extended later on then the same will be possible for the none-empty text. If it is not intended as structural thing then there is no point adding an empty text at all.
The problem with doing this is that it would be a breaking change in some ways.
That's why i say make it a specialized function.
The problem that my suggestion is trying to solve is oversized text packets, otherwise it wouldn't matter much at all.
I agree with that. Suggestions for names for such a method?
appendSimplified()?simplifedAppend()?mergeOrAppend()?Sending empty text is one case - special casing for empty shouldn't really happen, as chances are when empty is appended it was done intentionally.
Well the same goes for any (Literal-)Text. It could have been split in two pieces for a special reason... (e.g. a placeholder for an infix.)
That's why I would prefer a new method.
I prefer a method for Text: trim()
It would effectively traverse the text tree, and start merging leaves as it sees fit.
That already exists @gravityfox. Currently it just removes empty children from the beginning and end. Feel free to make adjustments in a PR if you think you can improve it.
Ok so a appendSimplified methods sounds like a good, idea, but how would you specify if you wanted it the Text simplified or not when using Text.of(stuff)?
@gravityfox: IMO trim() is a prejudiced term (throw away whitespace at the end and beginning. Such a method should never do something to the inner parts). I would go for simplify() or alike.
@Katrix- Maybe using a new method called: Text.simplifiedOf(stuff)? (That better than adding a boolean param in a var arg Object method) as an alternative we could go for Text.of(stuff).simplifiy().
Then the methods would be
Text#simplify()Text#simplifiedOf(Object...)TextBuilder#appendSimplified(Text ...)that way it should be obvious that all three methods do something similar aka simplifying the text.
I don't really like an append simplified method, because to doesn't solve the problem of just using Text.of().
Trimming should really be a post processing function, as it would be a lot more effective.
PR merged (https://github.com/SpongePowered/SpongeAPI/pull/2054)
Most helpful comment
Here an example implemententation:
https://gist.github.com/ST-DDT/b296398917891345e8d4c160916bed48
Output
From @JBYoshi 's example
more complex example