YarnSpinner-Unity icon indicating copy to clipboard operation
YarnSpinner-Unity copied to clipboard

Rows in CSV are not sorted by the correct lineNumber when update existing files

Open houkanshan opened this issue 3 years ago • 2 comments

What is the current behavior?

When Update Existing Strings Files, the lineNumber of translated entries in won't change. As a result the lineNumber in the output is a mix of new line numbers from yarn and old line numbers from CSV. It makes the output CSV hard to read because they are not sorted following the real lineNumber & node in the current yarn file.

Please provide the steps to reproduce, and if possible a minimal demo of the problem:

title: TestCSVSort
tags:
---
A: test A #line:09d24e4
B: test B #line:05cf124
===

Click Export Strings as CSV, it will generate this CSV

language,id,text,file,node,lineNumber,lock,comment
,line:09d24e4,test A,,TestCSVSort,3,,
,line:05cf124,test B,,TestCSVSort,4,,

Swap these 2 lines

title: TestCSVSort
tags:
---
B: test B #line:05cf124
A: test A #line:09d24e4
===

Click Update Existing Strings Files, it generate:

language,id,text,file,node,lineNumber,lock,comment
,line:09d24e4,test A,,TestCSVSort,3,,
,line:05cf124,test B,,TestCSVSort,4,,

The order didn't change.

What is the expected behavior?

The order should change, and the lineNumber, node name should be updated.

language,id,text,file,node,lineNumber,lock,comment
,line:05cf124,test B,,TestCSVSort,3,,
,line:09d24e4,test A,,TestCSVSort,4,,

Please tell us about your environment:

  • Yarn Spinner Version: 2.1.0
  • Unity Version: 2021.1.26f1

Other information

Here: https://github.com/YarnSpinnerTool/YarnSpinner-Unity/blob/main/Editor/Utility/YarnProjectUtility.cs#L384-L393 Instead of only changing entry.Text. I believe we should also update LineNumber and Node.

entry.LineNumber = baseEntry.LineNumber;
entry.Node = baseEntry.Node;

or maybe update based on the baseEntry instead of the translatedEntry

var baseEntry = baseDictionary[id];
var translatedEntry = translatedDictionary[id];
var entry = new StringTableEntry(baseEntry) {
  Text = $"(NEEDS UPDATE) {translatedEntry.Text}",
  Comment = translatedEntry.Comment,
  Language = language,
}

houkanshan avatar Apr 24 '22 18:04 houkanshan

Hey thanks for the detailed report can reproduce it and you've found the spot causing the issue.

I have a question about this though, because based on some discussion internally we realised we don't use most of the fields in the CSV. Namely file, node, and lineNumber aren't used by us for localisation and as such the easiest way to fix the bug is to just remove them from the export. This won't impact actual text changes as these will still be detected by the hash changing.

But before we do that, are they actually elements of the exported CSV you use?

McJones avatar Jul 15 '22 04:07 McJones

Hi @McJones thanks for the reply! I don't use file, node or lineNumber. However, it's because my project is small, under development, currently I do the localization myself, instead of working with another team. So I can't speak for other projects.

Besides, I also added an extra originalText column to the CSV, so I can always refer to it when I update / refine the translation.

houkanshan avatar Jul 16 '22 15:07 houkanshan

Since the changes to localisation to support a better workflow in addition to unity localisation this issue is no longer happening. Thanks for the issue.

McJones avatar Sep 28 '23 05:09 McJones