Skip to content

fix: Load default english translations as fallback#1814

Open
Emilxyz wants to merge 1 commit into
PaperMC:dev/3.0.0from
Emilxyz:fix/load-fallback-translations
Open

fix: Load default english translations as fallback#1814
Emilxyz wants to merge 1 commit into
PaperMC:dev/3.0.0from
Emilxyz:fix/load-fallback-translations

Conversation

@Emilxyz

@Emilxyz Emilxyz commented May 29, 2026

Copy link
Copy Markdown
Contributor

As pointed out in #1775 (comment), when adding new translation strings to messages.properties, these strings are not populated into the translation files of existing setups. I think a full migration system, as suggested by the comment, for this would be overkill, we're rarely adding translations anyways, so I went with a different approach.

This pr loads the default messages.properties translations that are bundled with velocity as a "patch" on top of any existing translation files. This will prevent any missing translations from showing up as their key since the default english translations will always be available.

@WouterGritter

Copy link
Copy Markdown
Contributor

For a fork of Velocity, we have added a basic migration system for messages.properties.

See: https://github.com/GemstoneGG/Velocity-CTD/blob/libdeflate/proxy/src/main/java/com/velocitypowered/proxy/TranslationRegistryManager.java

This would take it one step further than this PR.

The tail of a messages.properties file that has been migrated by the above class looks like:

# Messages below have been added by a migration of this file at 2026-05-09 12:27:44 UTC.
velocity.command.heapdump-created=<green>Heap dump saved to <arg:0>
velocity.command.heapdump-failed=<red>Failed to write heap dump, see server log for details.
# Messages below have been added by a migration of this file at 2026-05-22 14:26:58 UTC.
velocity.command.version-offer-copy-version=<white>Click to copy version to clipboard
velocity.error.plugin-message-overflow=<red>You sent too many plugin messages before completing the connection.

Feel free to take inspiration or copy (it's licensed under Velocity's license as it was building ontop of existing code).

Silently adding the English translation as a fallback (this PR's current state) might be confusing for people as it's not immediately apparent how they can modify the strings. Velocity doesn't share its translation key, and it also can't be found in their messages file.

@Emilxyz

Emilxyz commented May 29, 2026

Copy link
Copy Markdown
Contributor Author

I understand the concern that this might cause confusion for people, but I'm also not entirely happy with your approach. In the past, if users wanted to force velocity to only use a specific language, we would recommend them to delete all other translation files and rename the target translation file to messages.properties. This is something that's lost with your linked approach. So I think at most those migrations should only apply to the default messages.properties and ignore any missing keys in other translation files (or even entirely missing translation files). Curious what maintainers think about this though, before I make any changes.

@Timongcraft

Copy link
Copy Markdown
Contributor

IMO the default locale should try to use the system default similar to how I did it here: https://github.com/Timongcraft/TgcTranslations/blob/master/src/main/java/de/timongcraft/tgctranslations/TranslationManager.java#L240-L256
Then I don't really see the need to force a specific language as the default can be overridden via system property and thus we could migrate translations.

@WouterGritter

Copy link
Copy Markdown
Contributor

I agree on both comments, just for clarification: the referenced fork did away with all language .properties files and only kept the one messages.properties with english messages (in favor of maintainability of the fork itself). Ideally, if we go for an actual migration approach, it would take all of this into account of course.

For the migration we can try to keep the ordering of the properties keys (which is hard because java's properties implementation is backed by an unordered map) or just add the missing keys to the bottom (possibly with a comment like my example)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants