-
-
Notifications
You must be signed in to change notification settings - Fork 198
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
[1.21] Rich/Component translations #1134
[1.21] Rich/Component translations #1134
Conversation
index text components aren't added yet
|
patches are a bit cringe but meh
not really sure this is needed since they're all private, but might as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the new component pose any problem for neo<->vanilla connections?
patches/net/minecraft/client/resources/language/ClientLanguage.java.patch
Outdated
Show resolved
Hide resolved
patches/net/minecraft/network/chat/contents/TranslatableContents.java.patch
Outdated
Show resolved
Hide resolved
patches/net/minecraft/network/chat/contents/TranslatableContents.java.patch
Outdated
Show resolved
Hide resolved
src/main/java/net/neoforged/neoforge/common/util/InsertingContents.java
Outdated
Show resolved
Hide resolved
src/main/java/net/neoforged/neoforge/common/util/InsertingContents.java
Outdated
Show resolved
Hide resolved
I think it will error at runtime on the vanilla end if the component is sent over the wire, but you aren't really supposed to use it outside of translations anyways. |
Given that we forge translations will and do show up on vanilla clients, this seems like a blocker for me. I would rather this system works purely as a parser and nothing else. In other words: the translatable component is aware of it innately, but when written to the wire the component is transformed to something vanilla can understand. |
I'm saying that this will only explode if you actually instantiate an InsertingContents and send it. The wire format of translatable components isn't changed. |
Then maybe encapsulate this somehow, using for example package private access or sealed classes. |
I don't know what you mean by that (InsertingContents needs to be public since it's referenced by an MC class), but I slapped an |
this fails on dedicated servers right now as rich translations aren't fully loaded there
dedicated server tests should work now
How would this work with CrowdIn translations? Also, aside from the link in this PR there is zero documentation. Not only is the availability of this undiscoverable for modders, finding out how to use it is a chore. In my opinion, mixing the translation with the formatting is the wrong approach. Formatting needs to be applied to translations, the same way placeholders are applied. Having colours and styling isn't a property of the translation; it's a property of the usage of those translations. The translation strings need a way to markup which parts of the string are what and the code using it needs to supply the formatting needed for the place the string is used. |
I honestly don't really know, though CrowdIn would probably prefer a more string-like format like MiniMessage.
Do you think the PR itself should've added documentation anywhere? Where to? If you're talking about https://docs.neoforged.net/, I'll get to that soon™ (maybe).
How do you envision that working? I sort of agree with that (although I think that users being able to customize message formatting is nice, and I much prefer putting formatting in the language files so that it doesn't mess with code). |
My guess is that CrowdIn would either reject the whole json file or ignore those entries. A good way to find out would have been to convert at least one of the translations in neo's lang file, even if the style was just "none".
/**
* This is javadoc. It goes on methods and classes to explain what they are there for.
**/ That would be a good start, especially for the class you added. There also are lots of patches, most of them so esoteric that I don't have any clue what they do, even after reading the code at least ten times today.
Semantic markup. Just a random way to do it would be Other ways, like xml-style named markup ( I also would be concerned about mixing code an presentation---but we don't have a presentation layer in Minecraft. All the UI stuff, down to the positioning in pixels, is handled in the code. Having a proper presentation layer that is decoupled from the code would be nice, and then that part certainly would go there, but that's wishful thinking. PS: And the reason why I read that code so often is that I wanted to apply the pluralisation patches #1269 to it, too. However, I still have no clue where or how. |
PS: If I sound grumpy that's because I'm in pain. Got a nerve channel in my right shoulder that likes to flare up every couple of years. :( |
This is effectively a port of oωo-lib rich translations (which was originally contributed to oωo by me) to NeoForge patches.
Currently marked as draft to get feedback on my patches.
Support for index text components isn't added yet.