View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0003072 | FSSCP | tables | public | 2014-07-03 05:15 | 2014-07-10 00:51 |
| Reporter | Yarn | Assigned To | Goober5000 | ||
| Priority | high | Severity | block | Reproducibility | always |
| Status | resolved | Resolution | fixed | ||
| Platform | x64 | OS | Windows 7 | ||
| Product Version | 3.7.2 RC3 | ||||
| Target Version | 3.7.2 | ||||
| Summary | 0003072: German section of retail strings.tbl crashes game | ||||
| Description | Starting with revision 10753, if the German section of the retail strings.tbl is read, the game crashes with this error message: strings.tbl(line 4226): Error: Expecting int, found [; ------------------------------------------------]. ntdll.dll! ZwWaitForSingleObject + 21 bytes kernel32.dll! WaitForSingleObjectEx + 67 bytes kernel32.dll! WaitForSingleObject + 18 bytes fs2_open_3_7_1_SSE2-DEBUG.exe! SCP_DumpStack + 354 bytes fs2_open_3_7_1_SSE2-DEBUG.exe! Error + 229 bytes fs2_open_3_7_1_SSE2-DEBUG.exe! error_display + 369 bytes fs2_open_3_7_1_SSE2-DEBUG.exe! atoi2 + 109 bytes fs2_open_3_7_1_SSE2-DEBUG.exe! stuff_int + 35 bytes fs2_open_3_7_1_SSE2-DEBUG.exe! parse_stringstbl + 298 bytes fs2_open_3_7_1_SSE2-DEBUG.exe! lcl_xstr_init + 131 bytes fs2_open_3_7_1_SSE2-DEBUG.exe! game_init + 418 bytes fs2_open_3_7_1_SSE2-DEBUG.exe! game_main + 519 bytes fs2_open_3_7_1_SSE2-DEBUG.exe! WinMain + 328 bytes fs2_open_3_7_1_SSE2-DEBUG.exe! __tmainCRTStartup + 296 bytes fs2_open_3_7_1_SSE2-DEBUG.exe! WinMainCRTStartup + 13 bytes kernel32.dll! BaseThreadInitThunk + 18 bytes ntdll.dll! RtlInitializeExceptionChain + 99 bytes ntdll.dll! RtlInitializeExceptionChain + 54 bytes The problematic line in strings.tbl is 4224, which looks like this (no characters were added): 1402, "Sie haben IPX-Protokoll als Protokoll ausgewählt, aber dieses Protokoll ist auf Ihrer Maschine nicht installiert."." Notice how the period and quotation mark at the end are doubled. This is what is causing the crash in revision 10753 and later. Since this is a retail file, FSO needs to cope with this table error to avoid breaking the "don't break retail" rule. | ||||
| Steps To Reproduce | Set the language to German and run FSO with no mods. In revision 10753 and later (but not earlier revisions), the game will crash with the error message above. | ||||
| Tags | No tags attached. | ||||
| related to | 0003001 | resolved | Goober5000 | Multiline comments problems |
|
|
Ew, yuck. I think the best way to solve this is to add a "magic number" check that will treat that line as a special case and truncate it before the parser chokes on it. |
|
|
The game did not truncate that line, at least not at the stage that removes comments. I noticed that line 5929 (in the French section) also has this problem, and it causes a similar crash. I think the best way to fix this is to handle Mantis 0003001 only in real XSTR structures, not just anywhere that double quotes exist. That way, you won't need a hack for two (or perhaps more) specific lines in the game data. |
|
|
Except that if we encountered this exact problem in a mod, we would want an error to be thrown in that situation; it's a data error. Unfortunately, it's a data error present in the retail data, hence why the best way to handle it probably is to special-case these two (or more) lines. |
|
|
Now that I think about it, my suggestion is a bad idea since it wouldn't allow !* and *! to be used in translated strings. So, it looks like this does have to be treated as a special case after all. However it's handled, though, the strings in question should not be truncated; rather, if a quoted string begins on one of those lines, and there are three quotation marks on that line, then the string should end at the third quotation mark, not the second. This is important for line 5929, where the second quotation mark is placed before the actual message. I also noticed that having an odd number of double quotes in a mission note (yes, double quotes are allowed there) breaks the mission in current revisions, even though it didn't happen in retail and older revisions. Multi-line comment delimiters and quotation marks must not have any effect on the comment system if they're placed in the mission notes. (To be clear, I'm talking about the mission notes, not the description.) |
|
|
By truncation, I meant that I would treat this line... 1402, "Sie haben IPX-Protokoll als Protokoll ausgewählt, aber dieses Protokoll ist auf Ihrer Maschine nicht installiert."." ...as if it were this... 1402, "Sie haben IPX-Protokoll als Protokoll ausgewählt, aber dieses Protokoll ist auf Ihrer Maschine nicht installiert." But it wouldn't necessarily have to be truncation. Since it's a special case, I could apply whatever special processing needed to be done on a line-by-line basis. The reason the game didn't choke prior to r10753 is that the parser forgot all about quotes once it moved to a new line. If it hadn't, this bug would have been present in retail. Can you attach an example of a problematic Mission Notes section? Are there any examples in released missions? |
|
|
Yes, I know what you meant by truncation. I was saying that the strings shouldn't be truncated like that, at least not before or during comment removal. I took a look at all the missions that ever shipped with the game (as far as I know), and the only ones that use double quotes in the notes are X_One_Step_Too_Far.fs2 and X_Rites_of_Passage.fs2, but those aren't problematic since they use an even number of double quotes in their notes. Since XSTR structures actually are supported in mission notes (but aren't normally used), even in retail, I suggest the following: Don't use special treatment for mission notes during comment removal, but do have FRED save mission notes in XSTR structures in the same manner as all other mission strings (including the conversion of semicolons and double quotes to $semicolon and $quote). This way, FREDders can't "break" a mission just by adding the wrong characters to the notes. (EDIT: Actually, scratch everything in this paragraph; FRED never actually converts semicolons and double quotes to $semicolon and $quote. It's probably better to tell mission designers not to use double quotes in mission notes, even if they see them used in some existing missions.) |
|
|
Well, if you don't truncate that string, you get two full stops. FRED most certainly does convert semicolons and quotes into $semicolon and $quote. In the events editor's message editing it does, at least, and I believe the briefing editors as well. Of course, the $semicolon conversion isn't actually necessary anymore due to some of these changes in comment parsing. |
|
|
Okay, have a look at the patch I uploaded. Seems to fix the issues for me (the two you mentioned plus a third I discovered while testing). |
|
|
For the German string at least, I would have it check the whole string (including the extra period and quotation mark) rather than just the stuff before the first non-ASCII character. That way, if a mod changes that string in a way that makes it longer (unlikely, but still possible), the line will not be truncated too early. (You can use \xE4 in the place of ä.) I should also note that the way your patch fixes the German string strips out any X-offset information that might exist on that line. It's probably not a big deal, though, since that string is used only in a pop-up message, and thus the offset information is not used. |
|
|
Thanks for the character tip. I've modified the patch. If you can tell me the character codes for the French string, I can change that too. What do you mean by X-offset information? |
|
|
The codes are \xE0 for à and \xE9 for é. By X-offset information, I mean the one or two integers that can appear after the string of a strings.tbl entry. It's generally used in languages other than English where text is supposed to be right-aligned or centered on something, like a button. More information can be found in the strings.tbl page of the wiki. (That documentation was only recently added to that page despite the fact that this feature has been supported since retail.) |
|
|
Mantis3072-parselo.cpp.patch (1,142 bytes)
Index: code/parse/parselo.cpp
===================================================================
--- code/parse/parselo.cpp (revision 10907)
+++ code/parse/parselo.cpp (working copy)
@@ -2123,6 +2123,20 @@
while ( (num_chars_read = parse_get_line(outbuf, PARSE_BUF_SIZE, raw_text, raw_text_len, mp_raw)) != 0 ) {
mp_raw += num_chars_read;
+ // stupid hacks to make retail data work with fixed parser, per Mantis #3072
+ if (!strcmp(outbuf, "1402, \"Sie haben IPX-Protokoll als Protokoll ausgew\xE4hlt, aber dieses Protokoll ist auf Ihrer Maschine nicht installiert.\".\"")) {
+ outbuf[121] = ' ';
+ outbuf[122] = ' ';
+ } else if (!strcmp(outbuf, "1117, \"\\r\\n\"Aucun web browser trouva. Del\xE0 isn't on emm\xE9nagea ou if \\r\\non est emm\xE9nagea, ca isn't set pour soient la default browser.\\r\\n\\r\\n\"")) {
+ char *ch = &outbuf[11];
+ do {
+ *ch = *(ch+1);
+ ++ch;
+ } while (*ch);
+ } else if (!strcmp(outbuf, "1337, \"(fr)Loading\"")) {
+ outbuf[3] = '6';
+ }
+
strip_comments(outbuf, in_quote, in_multiline_comment_a, in_multiline_comment_b);
maybe_convert_foreign_characters(outbuf);
|
|
|
Patch revised again. Unfortunately, it looks like those characters won't work for string matching, even though they appear to match when printed to the log. I may have to go back to matching only the non-ASCII characters. |
|
|
I figured out how the problem with your patch: You aren't looking for the newline characters. You need to include them in your searches. For example, with the last string in your patch (since it's short), you need to search for this: "1337, \"(fr)Loading\"\n" This fixed it for me at least. Note that you need to use \n, not \\n. |
|
|
Derp. You are correct. Well, that's what I get for trying to code on four hours of sleep. I think this covers all the issues then. Committing. |
|
|
Fix committed to trunk@10908. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2014-07-03 05:15 | Yarn | New Issue | |
| 2014-07-03 05:15 | Yarn | Relationship added | related to 0003001 |
| 2014-07-04 00:29 | Goober5000 | Note Added: 0016016 | |
| 2014-07-04 00:29 | Goober5000 | Assigned To | => Goober5000 |
| 2014-07-04 00:29 | Goober5000 | Status | new => assigned |
| 2014-07-04 04:24 | Yarn | Note Added: 0016018 | |
| 2014-07-04 05:50 | MageKing17 | Note Added: 0016020 | |
| 2014-07-04 17:22 | Yarn | Note Added: 0016021 | |
| 2014-07-04 18:53 | Goober5000 | Note Added: 0016022 | |
| 2014-07-04 21:18 | Yarn | Note Added: 0016023 | |
| 2014-07-04 21:26 | Yarn | Note Edited: 0016023 | |
| 2014-07-04 22:01 | MageKing17 | Note Added: 0016024 | |
| 2014-07-07 08:02 | Goober5000 | File Added: Mantis3072-parselo.cpp.patch | |
| 2014-07-07 08:03 | Goober5000 | Note Added: 0016034 | |
| 2014-07-07 08:03 | Goober5000 | Status | assigned => code review |
| 2014-07-07 19:35 | Yarn | Note Added: 0016037 | |
| 2014-07-09 06:41 | Goober5000 | File Deleted: Mantis3072-parselo.cpp.patch | |
| 2014-07-09 06:42 | Goober5000 | File Added: Mantis3072-parselo.cpp.patch | |
| 2014-07-09 06:44 | Goober5000 | Note Added: 0016041 | |
| 2014-07-09 16:52 | Yarn | Note Added: 0016044 | |
| 2014-07-10 00:03 | Goober5000 | File Deleted: Mantis3072-parselo.cpp.patch | |
| 2014-07-10 00:03 | Goober5000 | File Added: Mantis3072-parselo.cpp.patch | |
| 2014-07-10 00:04 | Goober5000 | Note Added: 0016053 | |
| 2014-07-10 00:36 | Yarn | Note Added: 0016054 | |
| 2014-07-10 00:51 | Goober5000 | Note Added: 0016055 | |
| 2014-07-10 00:51 | Goober5000 | Changeset attached | => fs2open trunk r10908 |
| 2014-07-10 00:51 | Goober5000 | Note Added: 0016056 | |
| 2014-07-10 00:51 | Goober5000 | Status | code review => resolved |
| 2014-07-10 00:51 | Goober5000 | Resolution | open => fixed |
