-
Notifications
You must be signed in to change notification settings - Fork 369
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Drop 'Text' prefix in text model #174
Comments
That has always been a bit of a controversial point: some Fabric developers argue the names should be improved upon where applicable instead. I, personally, believe that while the official names are an useful hint, we should treat them as just that - a hint. Additionally, I don't believe the names in toString always reflect the actual class name. Anyhow, I'm opposed, but I'm a fan of verbosity.
|
Fabric tends to use common prefixes all over the place, which is quite useful for IDE's auto completion and recognition. Not using Text* wouldn't fit everything else. I'd rather drop the "Component" infix, it is technically wrong (the "component" usually references the whole thing, not a part ("component") of something) and doesn't add to readability. |
That's a fair point, though I don't think it'd make too big of a difference (its rarely imported) - the subclasses (Text, Score, etc) I really doubt could be apart another system so I don't think that's so much of an issue.
Well that's a different discussion, I have my preference but that can be discussed elsewhere - for the purposes of this discussion I'd be happy with either
Really? You'd certainly still get completion for
Just because in a lot of cases only a single component is used, doesn't discount that many can be used (and frequently are - e.g. This thing is, while this is certainly apart the text model (hence |
They are all representing localizable text, usually to be displayed on the client. "Text" conveys naturally that it can be embedded into itself. If you think about it from the other direction, the "component" has no real container, it is always constrained to itself until it gets consumed or exported into a string. "Component" is an unnecessary distinction towards something that doesn't exist. Stripping "Text" makes it as generic and meaningless as "Element", "Part" or similar, losing the description of its identity/purpose. |
About the package argument: I'm of the belief that you don't really look at package names when reading code... |
Sure, when reading code context is where you'd know its text related - package is only really relevant when importing. Though importing is the only case I see it being an issue anyhow... // Even when using Component in a field, context easily allows you to
// know its text-related
final Component message = new TextComponent("Hello, World!");
player.message(message);
// Each of the components are fairly easily identifiable as text
player.message(new KeybindComponent("key.jump"));
player.message(new TranslatableComponent("info.kick", "player-x"));
player.message(new ScoreComponent("name", "objective"));
player.message(new SelectorComponent("pattern")); Take a look at the examples for kyori/text, to see that even without the 'Text' prefix its still very easy to see what it means and looks a bunch cleaner too (IMO ofc). |
You might have a point with the code examples, though I'd still argue ScoreComponent/SelectorComponent/KeybindComponent can be confusing in certain contexts. And I still wouldn't be so sure on calling the base "Component". Rubs me the wrong way. |
Maybe, but it's not uncommon - take |
I wouldn't removing Text prefix as Component alone wilk cause confusion as there are many similar class with same names exists |
Okay but let's look at what happens in the reverse: // Even without Component in the name, names are still focused on the object's use
// Note that I've reversed from StringText to TextString because it flows more naturally
// in english
final Text message = new TextString("Hello, World!");
player.message(message);
// Each of these objects' purposes are easily determined without the word "Component"
player.message(new KeybindText("key.jump"));
player.message(new TranslatableText("info.kick", "player-x"));
player.message(new ScoreText("name", "objective"));
player.message(new SelectorText("pattern")); Seems legit to me. |
Well, as kashike reminded me - Mojang do not call them text components - they call them chat components (you just need to look at the changelogs to know this). They also don't refer to the text component as a string text component - text for them, is what fabric calls string. |
My research led me that are in a chat package in network for what's it's worth -> https://github.com/jamiemansfield/mcmap/tree/18w50a/workspace/net/minecraft/network/chat (Edit: Silly mobile) |
Closes FabricMC#174, Succeeds FabricMC#567 (partially). This PR brings the naming of chat components in line with Mojang - in both package and *chat* components.
Currently the
net/minecraft/text
has all of the text model components prefixed withTextComponent
, in contrast with Mojang's own naming scheme (XComponent
).Its important to note that only
TextComponent
directly represents actual text, the rest are just placeholders apart a model that is constructed into text client-side. IMO theText
prefix is verbose and unnecessary.I propose that we should use Mojang's naming (where known) and the following changes:
net/minecraft/text/ITextComponent
->net/minecraft/text/Component
(this is kind of given away in some of the serialisation code).The text was updated successfully, but these errors were encountered: