feat: add dynamic localisation and EDMC version compatibility

[GLWine]

Description:

This pull request introduces a comprehensive refactor of the localisation utilities in utils.py to ensure robust compatibility with both legacy and future versions of EDMC, while improving code clarity, maintainability, and type safety.

---

Key Changes:

• Dynamic EDMC Version Handling:
  Introduced the _get_core_version function to reliably determine the EDMC core version, supporting both string and callable formats for appversion. This ensures the plugin can adapt to changes in how EDMC exposes its version information across releases.

• Unified Translation Interface:
  Added _get_tl_func, which abstracts the selection of the correct translation function and translations object based on the detected EDMC version. This function returns both the appropriate translation callable and the correct translations object, allowing the rest of the codebase to interact with localisation in a consistent way, regardless of the underlying EDMC API.

• Future-Proof Localisation Logic:
  All direct references to l10n.Translations and related constants have been replaced with the dynamic translations_obj returned by _get_tl_func. This approach ensures the code will continue to work seamlessly when the legacy API is removed or made private in future EDMC versions (such as 6.0+).

• Refactored Localisation Functions:
  The __ and available_langs functions have been updated to use translations_obj and the new version logic, ensuring correct behaviour and compatibility across all supported EDMC versions.

• Improved Type Annotations:
  Type annotations have been updated throughout the module, replacing the use of the built-in any with the more appropriate Any from the typing module. This enhances static analysis and helps prevent subtle bugs.

• Enhanced Documentation and Readability:
  Docstrings have been revised for clarity and consistency, making the codebase easier to understand and maintain for current and future contributors.

• General Code Cleanup:
  Additional imports for Callable, Any, and Tuple from typing have been added where needed, and the code has been refactored for improved maintainability and clarity.

---

Rationale:

These changes were made to address the evolving localisation API in EDMC, which is moving away from the legacy l10n.Translations interface in favour of a new, more flexible approach. By centralising version detection and translation object selection, the codebase is now more resilient to upstream changes and easier to maintain. The improved type annotations and documentation further enhance code quality and developer experience.

This refactor ensures that the plugin will continue to function correctly with both current and future versions of EDMC, while also making it easier to remove legacy compatibility code when support for older versions is eventually dropped.

---

How to Test:

• Run the plugin with both older and newer versions of EDMC to verify that localisation works as expected in all scenarios.
• Check that language selection, translation overrides, and language listing features behave correctly.
• Review the code for clarity and maintainability.

---

Summary:

This pull request modernises the localisation utilities, making them robust, maintainable, and compatible with the full range of EDMC versions. It lays a solid foundation for future development and simplifies the eventual removal of legacy code.

---

Note: The intent was not to revolutionize the original code, but to make it as robust as possible against future changes in EDMC’s localisation API. Even if this PR is not accepted as-is, I hope the underlying concept—abstracting and future-proofing the localisation logic—can be useful for the project going forward. The codebase has grown in volume and complexity solely to introduce this abstraction and to ensure that it is not easily breakable by upstream changes.

[GLWine]

Possible solution to issues #249

[aussig]

This is great, thank you, particularly the well-written PR description.

I would like to suggest a further improvement though - since EDMC v5.11, the translate function has taken a further optional lang argument as its last parameter. As seen in the comments in utils.py on lines 91 and 92, I was intending to eliminate our custom __(string: str, lang: str) function altogether, replacing it with a simple call to translate but passing in the language.

Would you be able to modify your PR to incorporate this, I suggest returning three values from your _get_tl_func() instead of two: _, __, translations_obj.

[GLWine]

But I have a question, if I have to manage what happens with lines https://github.com/GLWine/BGS-Tally-Fork/blob/86019ad0351dff915517f8742e35d1b73d078e86/bgstally/utils.py#L91-L92 now I wonder all the management from line https://github.com/GLWine/BGS-Tally-Fork/blob/86019ad0351dff915517f8742e35d1b73d078e86/bgstally/utils.py#L95-L123 should be removed?

could you ask me the thing?

[aussig]

now I wonder all the management from line https://github.com/GLWine/BGS-Tally-Fork/blob/86019ad0351dff915517f8742e35d1b73d078e86/bgstally/utils.py#L95-L123 should be removed?

Yes, that was the plan, but of course it will need testing to make sure the plan actually works :)

[GLWine]

I thought to solve the problem like this:

# Wrapper for dynamic language translation; cannot use functools.partial for lang.
def __(string: str, lang: str) -> str:
    """
    Translate a string using the specified language, compatible with all EDMC versions.

    Args:
        string (str): The string to translate.
        lang (str): The language code to use for translation.

    Returns:
        str: The translated string.
    """
    return translations_obj.translate(string, context=__file__, lang=lang)

I cannot add this to the tuple because the language parameter must be handled dynamically. When using functools.partial, the arguments are fixed at the moment of creation, so I cannot provide the language later at call time. For this reason, I need to manage translation with a dedicated function that accepts the language as an argument each time it is called. This approach keeps the implementation simple and flexible.

[aussig]

I thought that when using functools.partial, although default args can be specified (e.g. lang=None) you can still override those defaulted named arguments when calling the partial (e.g. lang="en").

[GLWine]

I re-edited the message above, now it should work, without making it too complex

[GLWine]

In the documentation provided (https://github.com/EDCD/EDMarketConnector/blob/main/PLUGINS.md#localisation) the "forced" translation system is managed statically and not dynamically, so my choice is quite good... at least I think so

[aussig]

That documentation is just meant to be an example. I know this because I wrote it :D

My addition of the lang parameter to the EDMC translate function a year ago was specifically to support this 'alternate language' feature in BGS-Tally, but of course it takes time for new additions like that to percolate through the EDMC release cycle, hence the addition of __() as a function in our code which replicated the EDMC code (and which was always meant to be temporary until we could rely on the feature being present in core EDMC).

I'm pretty sure (but not certain) that I tested the __ = functools.partial(l10n.Translations.translate, context=__file__, lang=lang) approach back then, but am very willing to accept that I didn't and that it therefore doesn't work! Have you tried extending the partial to include a lang parameter?

[GLWine]

we can use this structure

__ = functools.partial(l10n.Translations.translate, context=__file__, lang=lang)

but lang must be defined or taken from somewhere, this means that it must update with the config update for example

[GLWine]

I should study better how languages ​​are managed in the config for example, provided that you only use it in webooks... otherwise I would leave the very simple def __(...)

[GLWine]

During my research on functools.partial, I understood that this function allows you to preset some parameters of an original function. The parameters not specified when creating the partial function can still be provided later, but it is important to know that, in some cases, they must be passed by keyword (i.e., specifying the parameter name) rather than positionally.

For example, if I write:

import functools

__ = functools.partial(translations_obj.translate, context=__file__)

to call this function later, I must provide the lang parameter by keyword:

__("text to translate", lang="it")

I cannot simply write:

__("text to translate", "it")

because, from a positional point of view, between the text to translate and the language there is the context parameter, which was already set via functools.partial. To avoid ambiguity or errors, it is therefore necessary to explicitly specify lang with its keyword.

---

Going back to the idea of ​​assigning the translation function dynamically via a structure like

_, __, translations_obj = _get_tl_func(core_version)

and therefore not using an explicit wrapper but passing the function thus obtained directly, is a possible solution. However, this method does not allow to correctly handle the exception that is generated when the lang parameter is assigned "en" and the related translation file is not present.

Currently, EDMC looks for language files in the dedicated folder; if it does not find the requested language, it does not use an automatic fallback but raises an exception that interrupts the operation. This choice is understandable to handle languages that are truly unavailable, but it would be appropriate to include a specific exception for the "en" language or require the presence of an en.strings file to be left in the l10n folder.

Until EDMC integrates a more robust handling of this aspect, I believe the solution I devised is potentially optimal:

def __(string: str, lang: str) -> str:
    """
    Translate a string using the specified language, compatible with all EDMC versions.

    This wrapper handles the KeyError that occurs if the requested language
    or translation file is missing, ensuring the original string is returned
    as a fallback instead of raising an exception.

    Args:
        string (str): The string to translate.
        lang (str): The language code to use for translation.

    Returns:
        str: The translated string. If the language is empty, None, the fallback language,
             or if translation fails, returns the original string.
    """
    # Return the original string if the language is empty, None, or the fallback language.
    if lang == "":
        Debug.logger.warning("Translation requested with empty language, returning original string.")
        return string
    elif lang is None:
        Debug.logger.warning("Translation requested with None language, returning original string.")
        return string
    elif lang == translations_obj.FALLBACK:
        # If the requested language is the fallback (usually 'en'), return the original string.
        return string

    # Attempt to translate the string using the translations object.
    try:
        return translations_obj.translate(string, context=__file__, lang=lang)
    except KeyError as e:
        # If the translation file or language is not found, log the error and return the original string.
        Debug.logger.error(
            f"Translation file missing or language not available for '{string}' in '{lang}': {e}"
        )
        return string

[GLWine]

To keep this in mind, I also submitted a possible solution to the problem encountered in EDMC

• https://github.com/EDCD/EDMarketConnector/issues/2437#issuecomment-2914564558

[aussig]

Yes I'm happy with that - as you say we need to catch that exception for the moment, until the issue is resolved in EDMC core, so an updated __() wrapper function on our side makes sense for now.

[GLWine]

@aussig what do you think about just putting the English language as a file? Simply rename the en.template to en.strings, then we might not have to handle the exception!

What do you think?

[aussig]

Yes that could work (would need testing of course).

There is already en.template.strings which could "simply" be renamed, but note that this file is used by Crowdin as the master source file for English strings, so we would have to synchronise a change to crowdin.yaml (line 2) to change this in the same commit. I hope Crowdin wouldn't get confused by that...

[GLWine]

@aussig Could we have both the tamplete file and the English one so as not to bother crowdin, what do you think?

But in general I don't think that crowdin cares much about the file you pass them if it is formatted in the same way.

[aussig]

I think it should be fine if it's renamed, so go ahead and test with a renamed file to check the English still works.

[GLWine]

To properly rename the source file en.template.strings to en.strings in Crowdin without losing existing translations, it's essential to follow a specific sequence of steps. Below is a clear and linear breakdown of the process:

1. Preparation

If the project is connected to GitHub/GitLab, it’s advisable to temporarily pause the integration to prevent unintended syncs during the process.

2. Manual Rename on Crowdin

Log into the Crowdin web interface and go to the Sources > Files section. Locate the file en.template.strings and rename it manually to en.strings. This ensures that Crowdin retains the existing translation memory and doesn’t treat the renamed file as a brand-new resource.

Sources > Files	Rename	enstrings

3. Rename the File in the Git Repository

In your repository, rename L10n/en.template.strings to L10n/en.strings locally. Commit the change and push it to GitHub/GitLab, preferably to the branch that Crowdin syncs with.

4. Update the crowdin.yml Configuration

In your crowdin.yml file, update the source path to reflect the new filename and add the update_option: update_without_changes directive to ensure that Crowdin retains previous translations even if the source content changes slightly:

files: 
  - source: /L10n/en.strings
    translation: /L10n/%two_letters_code%.strings
    update_option: update_without_changes

Commit and push this change as well.

5. Reactivate and Verify

Once all changes are in place, re-enable the GitHub/GitLab integration. Make sure that en.strings is being properly handled by Crowdin and that all previously translated content is still available. Finally, confirm that EDMC no longer raises errors due to the missing en.strings file.

---

For this reason, I wouldn’t include this change in this PR, as it involves actions that are only accessible to you, @aussig. Proceeding partially or without the proper setup could compromise the existing translations or the Crowdin sync, making it hard for me to manage on my own.

[GLWine]

I tested the direct approach using a tuple, but it turned out to be significantly more complex to manage: the KeyError is raised again, this time by the available_langs() function. The issue stems from the fact that this function retrieves language codes from the plugin’s L10n folder and then attempts to match them with the available contents in EDMC via l10n.Translation.contents(). However, since the en.strings file is not present in EDMC, the error is triggered.

Personally, I believe that—for now—the most practical solution is to implement a wrapper function that explicitly handles the special case of the en language. Once EDMC provides a structural fix for this issue, we can consider replacing the wrapper with a more integrated tuple-based approach.

[GLWine]

Detailed description (https://github.com/aussig/BGS-Tally/pull/324/commits/cd30bd24b9a9298cece199762658c6a3c4616447)

• Refactor of the __ translation function:

  • The function now handles more robustly the cases where the requested language is empty (""), None or matches the fallback language (usually 'en'), returning the original string directly without attempting translation.
  • A warning is added to the log if translation is requested with empty language or None, making it easier to debug incorrect calls.
  • If a KeyError is raised during translation (e.g. missing translation file or unavailable language), the error is logged in detail and the function returns the original string as a fallback, avoiding crashes or unhandled exceptions.

• Docstring improvements:

  • All utility functions have been updated with docstrings, improving the readability and maintainability of the code.

• Overall impact:

  • The code is now more robust, clear, and easily extendable.
  • Error handling in localization is safer and more informative, reducing the possibility of silent errors or unexpected crashes.
  • Inline documentation is more complete, making it easier for future developers or maintainers of the project to understand.

---

In summary: This commit improves the quality and robustness of the translation and language management utilities, making the code more reliable and documented.

[GLWine]

Before closing the PR I would like to discuss the control of the pre 5.0.0 version.

_get_edmc_version()

def _get_edmc_version(appversion) -> semantic_version.Version:
    """
    Return the EDNC version as a semantic_version.Version object.

    Determines the EDMC version based on the type of `appversion`:
    - For versions up to 5.0.0-beta1, `appversion` is expected to be a string.
    - From 5.0.0-beta1 onwards, `appversion` is expected to be a callable that returns a semantic_version.Version object.

    Args:
        appversion (str | Callable[[], semantic_version.Version]): The application version, either as a string or a callable.

    Returns:
        semantic_version.Version: The parsed EDMC version.

    Raises:
        TypeError: If the callable does not return a semantic_version.Version object.
        ValueError: If `appversion` is neither a string nor a callable.
    """
    # Up until version 5.0.0-beta1, appversion is a string.
    if isinstance(appversion, str):
        return semantic_version.Version(appversion)
    # From 5.0.0-beta1 onwards, appversion is a function returning semantic_version.Version.
    elif callable(appversion):
        version = appversion()
        if not isinstance(version, semantic_version.Version):
            Debug.logger.error(
                "The appversion function must return a semantic_version.Version object, got %r.",
                type(version)
            )
            raise TypeError("The appversion function must return a semantic_version.Version object.")
        return version
    Debug.logger.error(
        "appversion must be a string or a callable returning semantic_version.Version, got %r.",
        type(appversion)
    )
    raise ValueError("appversion must be a string or a callable returning semantic_version.Version.")


# Parse and store the current EDMC version.
edmc_version: semantic_version.Version = _get_edmc_version(appversion)

I tried to start the BGS-Tally in version 4.2.7, the latter did not start due to other incompatibilities that are not managed.

Log error

2025-05-30 10:02:09.305 - ERROR - 18208:18696:18696 plug.Plugin.__init__:64: : Failed for Plugin "BGS-Tally"
Traceback (most recent call last):
  File "plug.pyc", line 51, in __init__
  File "<frozen importlib._bootstrap_external>", line 529, in _check_name_wrapper
  File "<frozen importlib._bootstrap_external>", line 1034, in load_module
  File "<frozen importlib._bootstrap_external>", line 859, in load_module
  File "<frozen importlib._bootstrap>", line 274, in _load_module_shim
  File "<frozen importlib._bootstrap>", line 711, in _load
  File "<frozen importlib._bootstrap>", line 680, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 855, in exec_module
  File "<frozen importlib._bootstrap>", line 228, in _call_with_frames_removed
  File "C:\Users\Giampiero\AppData\Local\EDMarketConnector\plugins\BGS-Tally\load.py", line 7, in <module>
    from bgstally.bgstally import BGSTally
  File "C:\Users\Giampiero\AppData\Local\EDMarketConnector\plugins\BGS-Tally\bgstally\bgstally.py", line 127
    match entry.get('event'):
          ^
SyntaxError: invalid syntax

At this point I could lighten the code by directly excluding its management, what do you think @aussig?

Removing that feature would boil down to:

# Assign the current EDMC version to the variable.
edmc_version: semantic_version.Version = appversion()

[aussig]

We're absolutely not supporting EDMC versions that old, 4.2.7 was released over 4 years ago, there are quite a lot of BGS-Tally features that will break on that - for example this already requires v5.6.0.

It might be worth an explicit check for an EDMC version at plugin launch though, so we can ensure a minimum core, perhaps back one year which would take us to 5.11.0? Using 5.11.0 as the cutoff would also mean you can simplify further, not needing the semantic version vs string check...

[GLWine]

At this point, the edmc_version variable is assigned by the appversion() function (https://github.com/aussig/BGS-Tally/pull/324/commits/03152614842aa050b24a29b5967b240b6c8058f1).

• Simplifying the structure and logic.

If for you @aussig there are no further points to validate, you could also call this PR concluded.