lc-sigurd / sigurd

Sigurd, the Lethal Company Library.
GNU Lesser General Public License v3.0
3 stars 1 forks source link

Localization #16

Open steven4547466 opened 7 months ago

steven4547466 commented 7 months ago

Very simple right now load localization directory and get value at key with optional default. Could be expanded in the future.

Lordfirespeed commented 7 months ago

1. There needs to be a way for a user to configure their locale, and that value should be used automatically (however overloads specifying a locale should remain public, I think)

  1. Loading all language files when we only need to use 1 set of them (the configured locale) is wasteful
Lordfirespeed commented 7 months ago

ah, system globalization current culture ...

Lordfirespeed commented 7 months ago

Also, you should use String.IsNullOrWhitespace() over equality check with String.Empty

steven4547466 commented 7 months ago

2. Loading all language files when we only need to use 1 set of them (the configured locale) is wasteful

Sure I'll make it only load the one I suppose

Lordfirespeed commented 7 months ago

It should probably try to fall back to en_us if the actual language isn't found (as a last resort)

steven4547466 commented 7 months ago

I feel like falling back like that would produce unexpected results for developers and users. As a dev, I'd much rather it threw an error on not found so it doesn't just fail silently.

Lordfirespeed commented 7 months ago

There's precedent for it - normally I would agree that errors > silent failure, but I18n is a bit of an outlier.

What you're describing would break mods on any machine whose language is not set to English, unless the developer provided translations

Lordfirespeed commented 7 months ago

But I do see what you mean. Let me go look at Forge's API again.

steven4547466 commented 7 months ago

What you're describing would break mods on any machine whose language is not set to English, unless the developer provided translations

I could add a fallback language to loading potentially?

So public static Locale RegisterLocale(string path = "", string fallbackLanguage = "en-us")

steven4547466 commented 7 months ago

Misclicked closed whoops

Lordfirespeed commented 7 months ago

Seems like a good solution to me

Lordfirespeed commented 7 months ago

The localizer Get() should also default to the key, rather than blankstring, by default

steven4547466 commented 7 months ago

Provide API for sending globalized text over the network

The Network is independent of all of our other projects, so this would be difficult to do since you can use network independently of ClientAPI, ServerAPI, and Common.

Lordfirespeed commented 7 months ago

Agreed - the important part is making an easily-de/serializable type that provides .Render() or similar, that can be instantiated on the server, serialized into a message/RPC, received by client, and .Render()ed

Rune580 commented 5 months ago

While not required, it may be beneficial to provide a MonoBehaviour that has a public field for the lang token and another field for a referenced TMP component, and auto loads the string. Mostly as a convenience for devs using this library.

public class LocalizedText : MonoBehaviour
{
    public string? langToken;
    public TextMeshProUGUI? label;

    private void OnEnable()
    {
        // add hook for when the locale gets changed
    }

    private void OnDisable()
    {
        // remove hook for when the locale gets changed
    }

    public UpdateLabel()
    {
        if (langToken is null || label is null)
            return;

        var localizedText = MethodCallForLoadingALocalizedString(); // TODO: Haven't looked at the PR too much so I don't know what this method call is supposed to look like

        label.text  = localizedText;
    }

}