Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separate text and tr_text properties to control auto-translation #2783

Closed
dalexeev opened this issue May 26, 2021 · 6 comments
Closed

Separate text and tr_text properties to control auto-translation #2783

dalexeev opened this issue May 26, 2021 · 6 comments

Comments

@dalexeev
Copy link
Member

Describe the project you are working on

No matter.

Describe the problem or limitation you are having in your project

godotengine/godot#43151
godotengine/godot#48662
godotengine/godot#43159

People complain that once a string is added as a translation key, it becomes impossible to display it without tweaks (adding a space or invisible character, or using set_message_translation(false)).

set_message_translation is not a good enough solution because:

  • one control can have several auto-translatable properties, and they can only be disabled all at once;
  • this changes the behavior of the tr function for this object, which can be inconvenient if a script is attached to it;
  • this is not enough for complex nodes;
  • NOTIFICATION_TRANSLATION_CHANGED is not automatically sent.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Add paired properties for auto-translated properties. In the editor, you need to set text to display the value as is or tr_text to auto-translate.

text = "MSG_ID"     # MSG_ID
text = tr("MSG_ID") # Message
tr_text = "MSG_ID"  # Message

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

var _text = ""
var _tr_text = ""

var text:
    get:
        return _text
    set(value):
        _text = value
        _tr_text = ""
        update()

var tr_text:
    get:
        return _tr_text
    set(value):
        _text = tr(value)
        _tr_text = value
        update()

func _notification(what):
    match what:
        NOTIFICATION_TRANSLATION_CHANGED:
            if _tr_text:
                _text = tr(_tr_text)
                update()

func _draw():
    draw_text(_text)

There is no need for set_message_translation and can_translate_messages, tr behaves the same regardless of the object (we can move it to @GDScript or @GlobalScope).

If this enhancement will not be used often, can it be worked around with a few lines of script?

No.

Is there a reason why this should be core and not an add-on in the asset library?

This is core related and is a common problem.

@Calinou
Copy link
Member

Calinou commented May 26, 2021

I think adding a property to Control to disable automatic translation is a better way to go about it, as you won't have to edit two properties every time you wish a label to be translated automatically. (This new property should not affect manual tr() behavior.)

NOTIFICATION_TRANSLATION_CHANGED is not automatically sent.

We can probably make set_object_translation send this notification if its value changes.

@dalexeev
Copy link
Member Author

as you won't have to edit two properties every time you wish a label to be translated automatically

Not. These properties work in pairs, similar to how the text and bbcode_text properties work. Just take a closer look at the logic in the gdscript example. You only change one of the properties. If you need to change the text itself, you only change the text property. If you need to set a key for auto-translation, you only change the tr_text property. In most cases, tr_text will be set in the editor.

@Calinou
Copy link
Member

Calinou commented May 26, 2021

Just take a closer look at the logic in the gdscript example. You only change one of the properties. If you need to change the text itself, you only change the text property.

I consider properties that change behind your back to be an anti-pattern, if my experience with bbcode_text is anything to go by 🙂

We've had similar issues with the Weight property in RigidBody, which was removed in master already.

@dalexeev
Copy link
Member Author

Like this:

var _text = ""
var _text_autotranslate = true

var text:
    get:
        return _text
    set(value):
        _text = value
        update()

var text_autotranslate:
    get:
        return _text_autotranslate
    set(value):
        _text_autotranslate = value
        update()

func _notification(what):
    match what:
        NOTIFICATION_TRANSLATION_CHANGED:
            if _text_autotranslate:
                update()

func _draw():
    if _text_autotranslate:
        draw_text(tr(_text))
    else:
        draw_text(_text)

?

Because this point is still important:

set_message_translation is not a good enough solution because:

  • this changes the behavior of the tr function for this object, which can be inconvenient if a script is attached to it;

There is no need for set_message_translation and can_translate_messages, tr behaves the same regardless of the object (we can move it to @GDScript or @GlobalScope).

@dalexeev
Copy link
Member Author

dalexeev commented May 26, 2021

I consider properties that change behind your back to be an anti-pattern

Again, it depends on which side. I see some problem that the text getter may not return what is visible on the screen. And in my proposal, the text getter always returns what is displayed, regardless of auto-translation.

Then we need to add the get_actual_text function:

func get_actual_text():
    if _text_autotranslate:
        return tr(_text)
    else:
        return _text

func _draw():
    draw_text(get_actual_text())

@dalexeev
Copy link
Member Author

Closed because godotengine/godot#49149 is merged.

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

No branches or pull requests

2 participants