NucleoidMC / Server-Translations

MIT License
42 stars 20 forks source link

TranslatableText mixin causes race conditions #17

Closed thexaero closed 2 years ago

thexaero commented 2 years ago

The fr/catcore/server/translations/api/mixin/client/TranslatableTextMixin.updateTranslations override of updateTranslations is causing the TranslatableText.translations list to constantly be cleared and reconstructed whenever net/minecraft/text/Text.getString() is called, which would only happen the very first time or when the language is changed in vanilla. I assume it is because SystemDelegatedLanguage.changed stays true forever once it's been set, but I could be wrong here.

This is a huge problem because, for entities, Text.getString() of the same object is used by both the server and the client threads in Singleplayer (which really surprised me). When a datapack repeatedly looks up entities based on their name (e.g. the Incendium datapack) or otherwise causes getString() to be called, this very often creates a race condition, if it overlaps with the client side using the getString() as well. Usually a ConcurrentModificationException is thrown on the client side (server side seems to suppress them completely). Sometimes other exceptions or issues occur. My guess is that this might also significantly affect the game's performance in some cases.

Would really appreciate if you could look into this! The best I can do from my end is suppress it.

EDIT: Oh, right, I should mention that this is about 1.17.1.

thexaero commented 2 years ago

It seems that the entity should also have a team, otherwise the game recreates the translate text component every time it's requested, which avoids the race condition. Definitely feels like the true roots of the problem are in vanilla. The team part doesn't feel intentional at all. EDIT: Fixed some stuff in the original post. Was kind of losing my mind from many hours of tracking this down so I missed some important details.

thecatcore commented 2 years ago

Should be fixed in latest version

thexaero commented 2 years ago

Awesome, thank you! @Patbox