Albeoris / Memoria

Final Fantasy IX tools
MIT License
282 stars 39 forks source link

Bug: AssetManager returns a single result instead of the accumulated one #411

Open Albeoris opened 3 months ago

Albeoris commented 3 months ago

@Tirlititi , @snouz , I found a nasty bug that at the very least makes it impossible to export texts when mods are installed, at most the method is used in many places, and I'm not sure that all of them are ready for the fact that they will receive only incremental changes instead of a complete list of strings.

Expected Behavior: The LoadString method should perform smart aggregation by overriding existing rows and adding new ones. This allow us to translate mods such as Alternative Fantasy.

        // AssetManager.cs
    public static String LoadString(String name, Boolean suppressMissingError = false)
    {
        String mainFile = LoadStringMultiple(name).FirstOrDefault(); // We get the single result instead of aggregation
        if (mainFile == null && !suppressMissingError)
            Log.Message("[AssetManager] Memoria asset not found: " + name);
        return mainFile;
    }

        // EmbadedSentenseLoader.cs
        public static String[] LoadSentense(String path)
        {
            String text = AssetManager.LoadString(path);
            return text == null ? null : ExtractSentenseEnd(text);
        }

        public static String LoadText(String path)
        {
            return AssetManager.LoadString(path);
        }

image

Tirlititi commented 3 months ago

Indeed, I didn't remember that field texts were not aggregated. I need to switch them to FF9TextTool.ImportWithCumulativeModFiles instead. The LoadString method must remain the same because it's used to load all the text assets. Things like AnimationFolderMapping.txt or SFX sequences don't have the same format as .mes or .strings files at all.

I must admit, I barely ever worked on the .strings format or the [Export] / [Import] options. I sticked with the .mes formatting of the base game and tried to work with that instead. For example, it's possible to use character name tags similar to [ZDNE] with the characters added by the Playable Character Pack mod, but only in the .mes format (there's no tag similar to {Zidane} for these characters). Also, unless I'm mistaken, there's no way to use custom RGB color tags in .strings for now.

Now, all the translation mods available use the .strings format. I don't even know if the .mes format would be suitable for translation mods. I don't see why not but since it hasn't been done yet, there must be reasons for that.

Albeoris commented 3 months ago

@Tirlititi , recently, I added .resjson support: https://github.com/Albeoris/Memoria/pull/413

After refactoring it's easy to add support of external .mes files if we want.

    public interface ITextFormatter
    {
        ITextWriter GetWriter();
        ITextReader GetReader();
    }

    public interface ITextWriter
    {
        void WriteAll(String outputPath, IList<TxtEntry> entries);
    }

    public interface ITextReader
    {
        TxtEntry[] ReadAll(String inputPath);
    }

About RGB colors, yes, I hardcoded it for better readability:

            {"[C8C8C8][HSHD]", "{White}"},
            {"[B880E0][HSHD]", "{Pink}"},
            {"[68C0D8][HSHD]", "{Cyan}"},
            {"[D06050][HSHD]", "{Brown}"},

But neither .strings nor .resjson prevent you from using native tags, so [FF0000] should work fine even in the .strings format.

RuslanMikhlevskiy commented 2 months ago

Yes, I myself encountered this bug when trying to translate beatrix.mod into russian language. If you change the strings file line by line, everything works correctly. But if you insert the text as a whole block, then the changes do not take effect and it also breaks the russian translation.

[FieldImporter] Initialization failed.

System.Exception: Cannot read 146 entry from 0754_EVT_BURMECIA_HOUSE_0.strings.
Previous: [Index: 145, Pefix: $0754_EVT_BURMECIA_HOUSE_0_, Value: {W254H2}{Beatrix}
“Но, кажется, я была недостаточно готова.”]
---> System.Exception: Cannot parse .strings entry.
[Index: -1, Prefix: , Value: ]
[Buffer:  = , Line: 0 IsKey: True, IsBlock: True, IsEscape: False]
---> System.ArgumentOutOfRangeException: Argument is out of range.
at System.Text.StringBuilder.ToString (Int32 startIndex, Int32 length) [0x00000] in <filename unknown>:0 
01.05.2024 01:52:39 |E|
at Memoria.Assets.StringsFormatter.Read (System.IO.StreamReader sr) [0x00000] in <filename unknown>:0 
                                            --- End of inner exception stack trace ---
at Memoria.Assets.StringsFormatter.Read (System.IO.StreamReader sr) [0x00000] in <filename unknown>:0 
at Memoria.Assets.TxtReader.Read (System.String& name) [0x00000] in <filename unknown>:0 
                                          --- End of inner exception stack trace ---
 at Memoria.Assets.TxtReader.Read (System.String& name) [0x00000] in <filename unknown>:0 
at Memoria.Assets.TxtReader.ReadStrings (System.String inputPath) [0x00000] in <filename unknown>:0 
at Memoria.Assets.FieldImporter.ReadExternalText (Memoria.Assets.TxtEntry[]& entries) [0x00000] in <filename unknown>:0 
at Memoria.Assets.FieldImporter.Initialize () [0x00000] in <filename unknown>:0 
[FieldImporter] Failed to find zone by ID: 754].
 [TextWatcher] Reinitialized for 00:00:00.2460550
Albeoris commented 2 months ago

@RuslanMikhlevskiy , you need to check the entry after {W254H2}{Beatrix} “Но, кажется, я была недостаточно готова.”

It looks like an empty strings at the end of file that cannot be parsed as a key.