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 10 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 ""

@paulloz
Copy link
Member

paulloz commented Dec 14, 2024

Hello, and sorry for the delay.

All of this feels quite good to me. I have a small doubt about the compilation context we are building, we might be missing some translation strings because of undefined conditional compilation symbols. I noticed this because some of my test files were calling Tr(string) from inside #if TOOLS sections, and thus weren't picked up.

@molingyu
Copy link
Author

molingyu commented Dec 17, 2024

I updated the support for conditional compilation, and now we can correctly handle the acquisition of i18n resources through compilation symbols.

But the bad news is that in order to ensure that all i18n resources are collected as much as possible in the editor, I enumerated the platform information, which led to multiple code syntax tree generation work, which brought huge performance problems.

This made the work that could be completed in 1 second before take nearly 20 seconds.

The code mainly consists of two parts:

  • Reference data collection (only executed once for each generation task): used to obtain the MetadataReference data of the target project.

  • Code scanning (needs to be performed for each file): by converting the code into a syntax tree, and then scanning the entire syntax tree, the semantic model is used to obtain the functions that meet the conditions.

The test found that after the new version enumerates the platform information, the data collection part takes 6~7 seconds to complete the work. The code scanning part, using the test case above, takes 7~8 seconds to complete.

In the old version, the whole work can be completed in just over a second.

In addition, the UI interface generated by pot currently does not do any progress display, which will freeze the editor interface for a long time when the generation task takes too long. This may be a problem that needs to be fixed.

cc @paulloz @KoBeWi

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