Added a text supplier for TextWidget. #69
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Caedis
reviewed
Mar 6, 2024
Caedis
reviewed
Mar 6, 2024
|
Looks good, though I think MUI2 will have something similar. |
Caedis
approved these changes
Mar 7, 2024
miozune
approved these changes
Mar 7, 2024
This was referenced Mar 7, 2024
Merged
Dream-Master
pushed a commit
to GTNewHorizons/GigaGramFab
that referenced
this pull request
Mar 9, 2024
* Replaces one instance where a dynamic text field is being communicated between server and client with a purely client-side method. In some cases this allows the client to do localization on these texts correctly. (See GTNewHorizons/ModularUI#69.) This is a part of the series of PRs implementing new ModularUI features, however this mod does not use any numeric text fields or similar, and thus no other changes are needed.
This was referenced Mar 9, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Simple change for a fairly complex reason. Let me explain:
This is a part of the localization effort for MUI. There are some text fields which use elements (strings, numbers) that need to be localized. Some of these elements can change their value dynamically, when the user is looking at the GUI. A simple example could be a readout: "EU stored: 1,500 / 20,000" (fictional, just for the sake of the example). This readout needs to be both translated (text "EU stored:") and localized (numbers with correct group separators).
Currently, readouts like this are handled by
DynamicTextWidget. However this has a major flaw: the full string is first computed on the server, and then it is sent to the client. This will not work when the server and the client use a different locale, as the server's locale is used to build the string to display.In reality, this does not need such a level of sophistication. We can simply build the string to display on the client, as long as the client has access to all the data it needs to do so (the current and maximum EU amounts in the example above). I have checked all the uses of
DynamicTextWidget(there are only a couple of them in the whole pack), and this is never an issue.I have added methods
setTextSupplierand for conveniencesetStringSupplierto TextWidget, which will allow the displayed text to be changed from the client side, without the server being involved at all. This way, the client can also properly apply localization standards as needed. I propose also deprecatingDynamicTextWidgetand moving over all uses of it to the new methods (which I will do as a part of my ongoing localization effort).For demonstration, this is a client with the French locale connecting to a server with the US locale. Top text is a static
TextWidget, middle isDynamicTextWidget, bottom isTextWidgetusing the new supplier method. (The second and third number are supposed to update every tick, it is a timestamp, hard to show in a static picture.)