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

Add c# translation parser support #99195

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

molingyu
Copy link

For a long time, the Godot engine lacks support for i18n string collection for C# scripts.
This PR implements CsharpTranslationParserPlugin by analyzing the AST returned by Roslyn and adds it to GodotTools.

@molingyu
Copy link
Author

I've observed that dalexeev is adding support for comments to this system.

But the corresponding PR has not been merged. In theory, I should follow up and let C# adopt the same approach to support comments. But obviously I need his changes to EditorTranslationParserPlugin. If dalexeev's PR is merged, I will follow up and add support for comments soon.

#98099

@paulloz
Copy link
Member

paulloz commented Nov 13, 2024

Hi, and thank you for your contribution 🙂

This is something I wanted to tackle for quite some time, but never got to, so I'm excited to see this PR! This is a quick first review, I haven't tested thoroughly.

Did you by any chance forget to commit your editor_internal_calls.cpp file? Since you are shifting the unmanaged callbacks array in Internal.cs, not having the native change that goes with it breaks everything. Although, we don't need to define an unmanaged callback here to register the parser. We can directly call AddTranslationParserPlugin(EditorTranslationParserPlugin) from GodotSharpEditor. E.g. look at how we instantiate, and dispose of, the export or inspector plugins.

I'm pretty sure the check for a MemberAccessExpressionSyntax prevents simple Tr(string)/TrN(string) from being picked up (while e.g. this.Tr(string)/this.TrN(string) would be fine). But from a broader perspective, I don't know if the implementation isn't too brittle as is. For instance, if I have something cursed like this in my codebase, all the calls to Foo.Tr(string) are going to be picked up as if they were calls to the translation server.

public static class Foo
{
    public static void Tr(string myString)
    {
    }
}

I'm wondering if we could easily make this more resilient. Otherwise, I think this would be a very good addition 🥳

@molingyu
Copy link
Author

molingyu commented Nov 14, 2024

To avoid recompiling godot completely on my poor computer, I did the actual coding and testing in another godot source code directory where I was working. It seems that I forgot to move editor_internal_calls.cpp when I moved to the clean directory.

Regarding the selection problem you mentioned. It seems that simple ast parsing cannot fully handle these situations well, and more semantic analysis may be needed to more accurately identify the functions that need to be captured.

I will try to use a new method to select.

@AThousandShips AThousandShips changed the title Feat: Add c# translation parser support Add c# translation parser support Nov 14, 2024
@molingyu
Copy link
Author

I followed up on dalexeev's PR. Added support for translation comments.

In addition to supporting single-line comments, there are also multi-line single-line comments and C# multi-line comments.

Example
using Godot;
using System;

public partial class Test : Node
{
    public override void _Ready()
    {
            GD.Print(Tr("Tr1"));
            GD.Print(Tr("Tr2", "ctx_a"));
            GD.Print(Tr("Tr3")); // NO_TRANSLATE
            GD.Print(Tr("Tr4")); // TRANSLATE: Message 4
            // TRANSLATE: Message 5
            GD.Print(Tr("Tr5"));
            // Above comment.
            // TRANSLATE: Message 6 Line 1
            // Message 6 Line 2
            // Message 6 Line 3
            GD.Print(Tr("Tr6"));
            // Above comment.
            /* TRANSLATE: Message 7
             * Message 7 Line 2
             * Message 7 Line 3
             * Message 7 Line 4
             */
            GD.Print(TrN("Tr_n7", "Tr_n7_plural", 0, "ctx_b"));
        
            GD.Print(Tr("Tr8", "ctx_c")); // TRANSLATE: Message 8.1
            GD.Print(Tr("Tr8", "ctx_c")); // TRANSLATE: Message 8.1
            GD.Print(Tr("Tr8", "ctx_c")); // TRANSLATE: Message 8.2
            GD.Print(Tr("Tr8", "ctx_d")); // TRANSLATE: Message 8.3
        
            // TRANSLATE: Message 9
        
            // Empty line ^^
            GD.Print(Tr("Tr9"));
        
            GD.Print(Tr("Tr10")); // TRANSLATE: Message 10
            GD.Print(Tr("Tr11"));
    }
}
# LANGUAGE translation for i18n_test for the following files:
# res://test.cs
#
# FIRST AUTHOR <EMAIL@ADDRESS>, YEAR.
#
#, fuzzy
msgid ""
msgstr ""
"Project-Id-Version: i18n_test\n"
"MIME-Version: 1.0\n"
"Content-Type: text/plain; charset=UTF-8\n"
"Content-Transfer-Encoding: 8-bit\n"

#: test.cs
msgctxt "ctx_a"
msgid "Tr2"
msgstr ""

#. TRANSLATORS: TRANSLATE: Message 7
#. * Message 7 Line 2
#. * Message 7 Line 3
#. * Message 7 Line 4
#: test.cs
msgctxt "ctx_b"
msgid "Tr_n7"
msgid_plural "Tr_n7_plural"
msgstr[0] ""
msgstr[1] ""

#. TRANSLATORS: TRANSLATE: Message 8.1
#. TRANSLATE: Message 8.2
#: test.cs
msgctxt "ctx_c"
msgid "Tr8"
msgstr ""

#. TRANSLATORS: TRANSLATE: Message 8.3
#: test.cs
msgctxt "ctx_d"
msgid "Tr8"
msgstr ""

#: test.cs
msgid "Tr1"
msgstr ""

#. TRANSLATORS: TRANSLATE: Message 4
#: test.cs
msgid "Tr4"
msgstr ""

#: test.cs
msgid "Tr5"
msgstr ""

#: test.cs
msgid "Tr6"
msgstr ""

#: test.cs
msgid "Tr9"
msgstr ""

#. TRANSLATORS: TRANSLATE: Message 10
#: test.cs
msgid "Tr10"
msgstr ""

#: test.cs
msgid "Tr11"
msgstr ""

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

Successfully merging this pull request may close these issues.

2 participants