View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0003001 | FSSCP | FRED | public | 2014-01-19 16:52 | 2014-07-03 05:15 |
Reporter | Axem | Assigned To | Goober5000 | ||
Priority | normal | Severity | block | Reproducibility | always |
Status | resolved | Resolution | reopened | ||
Product Version | 3.7.1 | ||||
Summary | 0003001: Multiline comments problems | ||||
Description | In rev 9945 Goober added !* *! as a new way to do multi-line comments. But in JAD2.21, Mission 5a (the race mission), there is a message... $Name: Everyone - GASP! $Team: -1 $MessageNew: XSTR("*GASP!*", -1) $end_multi_text Starting a giant 450 line comment that never ends. Can we pick a better token for multi line comments? | ||||
Tags | No tags attached. | ||||
related to | 0003072 | resolved | Goober5000 | German section of retail strings.tbl crashes game |
|
I'm considering {* and *}. Does anyone know of any missions which might use these character groupings? |
|
Yarn suggested disabling multiline-comment delimiters in XSTR strings, which seems like the better approach, given that this happens in every other programming language which uses both string literals and comments. So I'll look into that. |
|
Soooo... When can this be fixed? |
|
Well, a lot of people have their hackles raised about the installer situation, so that's my first priority. After that, I have a patch by m!m for the SEXP system that needs integrating. But I think this can be next on the list. Of course, if anyone else feels like disabling comments within quoted strings, they are welcome to take a stab at it. :) |
|
I intend to fix this this week. Axem, feel free to pester me until I do. |
|
3001.patch (9,781 bytes)
Index: code/parse/parselo.cpp =================================================================== --- code/parse/parselo.cpp (revision 10491) +++ code/parse/parselo.cpp (working copy) @@ -1642,240 +1642,225 @@ } // Goober5000 -int get_number_before_separator(char *text, char separator) +bool get_number_before_separator(int &number, int &number_chars, const char *text, char separator) { - char buf[10]; - char *ch; + char buf[8]; + const char *ch = text; + int len = 0; - memset(buf, 0, 10); - strncpy(buf, text, 9); + while (true) + { + // didn't find separator + if (*ch == '\0' || len == 8) + return false; - ch = strchr(buf, separator); - if (ch == NULL) - return 0; - *ch = '\0'; + // found separator + if (*ch == separator) + break; - return atoi(buf); -} + // found nondigit + if (!isdigit(*ch)) + return false; -// Goober5000 -int get_number_before_separator(SCP_string &text, char separator) -{ - char buf[10]; - char *ch; + // copying in progress + buf[len] = *ch; + len++; + ch++; + } - memset(buf, 0, 10); - text.copy(buf, 9); - - ch = strchr(buf, separator); - if (ch == NULL) - return 0; - *ch = '\0'; - - return atoi(buf); + // got an integer + buf[len] = '\0'; + number = atoi(buf); + number_chars = len; + return true; } -// Strip comments from a line of input. -// Goober5000 - rewritten to make a lot more sense and to be extensible -int strip_comments(char *line, int in_multiline_comment) +// Goober5000 +bool get_number_before_separator(int &number, int &number_chars, const SCP_string &text, SCP_string::iterator text_pos, char separator) { - char *ch; + char buf[8]; + SCP_string::iterator ch = text_pos; + int len = 0; - // if we're in a comment, see if we can close it - if (in_multiline_comment) + while (true) { - ch = strstr(line, "*/"); - if (ch == NULL) - ch = strstr(line, "*!"); - if (ch != NULL) - { - char *writep = line; - char *readp = ch + 2; + // didn't find separator + if (ch == text.end() || len == 8) + return false; - // copy all characters past the close of the comment - while (*readp != '\0') - { - *writep = *readp; + // found separator + if (*ch == separator) + break; - writep++; - readp++; - } + // found nondigit + if (!isdigit(*ch)) + return false; - *writep = '\0'; - - // recurse with the other characters - return strip_comments(line, 0); - } - - // can't close it, so drop the whole line - ch = line; - goto done_with_line; + // copying in progress + buf[len] = *ch; + len++; + ch++; } + // got an integer + buf[len] = '\0'; + number = atoi(buf); + number_chars = len; + return true; +} - /* Goober5000 - this interferes with hyperlinks, heh - // search for // - ch = strstr(line, "//"); - if (ch != NULL) - goto done_with_line; - */ - - +bool matches_version_specific_tag(const char *line_start, bool &compatible_version, int &tag_len) +{ // special version-specific comment // formatted like e.g. ;;FSO 3.7.0;; - ch = stristr(line, ";;FSO "); - if (ch != NULL) - { - int major, minor, build; - char *numch, *sep, *linech; + if (strnicmp(line_start, ";;FSO ", 6)) + return false; - numch = ch + 6; - sep = strchr(numch, '.'); - if (sep == NULL) - goto done_with_line; + int major, minor, build, num_len; + const char *ch; - major = get_number_before_separator(numch, '.'); + ch = line_start + 6; + if (!get_number_before_separator(major, num_len, ch, '.')) + return false; - numch = sep + 1; - sep = strchr(numch, '.'); - if (sep == NULL) - goto done_with_line; + ch += (num_len + 1); + if (!get_number_before_separator(minor, num_len, ch, '.')) + return false; - minor = get_number_before_separator(numch, '.'); + ch += (num_len + 1); + if (!get_number_before_separator(build, num_len, ch, ';')) + return false; - numch = sep + 1; - sep = strchr(numch, ';'); - if (sep == NULL) - goto done_with_line; + ch += (num_len + 1); + if (*ch != ';') + return false; - build = get_number_before_separator(numch, ';'); + ch++; - if (*(sep + 1) != ';') - goto done_with_line; + // tag is a match! + tag_len = ch - line_start; + compatible_version = true; - linech = sep + 2; - - - // check whether major, minor, and build line up with this version - if (major > FS_VERSION_MAJOR) + // check whether major, minor, and build line up with this version + if (major > FS_VERSION_MAJOR) + { + compatible_version = false; + } + else if (major == FS_VERSION_MAJOR) + { + if (minor > FS_VERSION_MINOR) { - goto done_with_line; + compatible_version = false; } - else if (major == FS_VERSION_MAJOR) + else if (minor == FS_VERSION_MINOR) { - if (minor > FS_VERSION_MINOR) + if (build > FS_VERSION_BUILD) { - goto done_with_line; + compatible_version = false; } - else if (minor == FS_VERSION_MINOR) + } + } + + // true for tag match + return true; +} + +// Strip comments from a line of input. +// Goober5000 - rewritten for the second time +void strip_comments(char *line, bool &in_multiline_comment_a, bool &in_multiline_comment_b) +{ + bool in_quote = false; + char *writep = line; + char *readp = line; + + // copy all characters from read to write, unless they're commented + while (*readp != '\r' && *readp != '\n' && *readp != '\0') + { + // only check for comments if not quoting + if (!in_quote) + { + bool compatible_version; + int tag_len; + + // see what sort of comment characters we recognize + if (!strncmp(readp, "/*", 2)) { - if (build > FS_VERSION_BUILD) + // comment styles are mutually exclusive + if (!in_multiline_comment_b) + in_multiline_comment_a = true; + } + else if (!strncmp(readp, "!*", 2)) + { + // comment styles are mutually exclusive + if (!in_multiline_comment_a) + in_multiline_comment_b = true; + } + else if (!strncmp(readp, "*/", 2)) + { + if (in_multiline_comment_a) { - goto done_with_line; + in_multiline_comment_a = false; + readp += 2; + continue; } } + else if (!strncmp(readp, "*!", 2)) + { + if (in_multiline_comment_b) + { + in_multiline_comment_b = false; + readp += 2; + continue; + } + } + // special version-specific comment + // formatted like e.g. ;;FSO 3.7.0;; + else if (matches_version_specific_tag(readp, compatible_version, tag_len)) + { + // comment passes, so advance pass the tag and keep reading + if (compatible_version) + { + readp += tag_len; + continue; + } + // comment does not pass, so ignore the line + else + { + break; + } + } + // standard comment + else if (*readp == ';') + { + break; + } } + // maybe toggle quoting + if (*readp == '\"') + in_quote = !in_quote; - // this version is compatible, so copy the line past the tag + // if not inside a comment, copy the characters + if (!in_multiline_comment_a && !in_multiline_comment_b) { - char *writep = ch; - char *readp = linech; - - // copy all characters past the close of the comment - while (*readp != '\0') - { + if (writep != readp) *writep = *readp; - writep++; - readp++; - } + writep++; + } - *writep = '\0'; - - // recurse with the other characters - return strip_comments(ch, 0); - } + // read the next character + readp++; } - - // search for ; - ch = strchr(line, ';'); - if (ch != NULL) - goto done_with_line; - - - // start of a multi-line comment? - // (You can now use !* *! in addition to /* */ because prior to 3.7.1, a /* would be flagged - // even if it appeared after an initial ; such as in a version-specific comment. - ch = strstr(line, "/*"); - if (ch == NULL) - ch = strstr(line, "!*"); - if (ch != NULL) + // if we moved any characters, or if we haven't reached the end of the string, then mark end-of-line and terminate string + if (writep != readp || *readp != '\0') { - // treat it as the beginning of a new line and recurse - return strip_comments(ch, 1); + writep[0] = EOLN; + writep[1] = '\0'; } - - - // no comments found... try to find the newline - ch = strchr(line, '\n'); - if (ch != NULL) - goto done_with_line; - - - // just skip to the end of the line - ch = line + strlen(line); - - -done_with_line: - ch[0] = EOLN; - ch[1] = 0; - - return in_multiline_comment; } -/*#if 0 -void strip_all_comments( char *readp, char *writep ) -{ - int ch; - //char *writep = readp; - - while ( *readp != EOF_CHAR ) { - ch = *readp; - if ( ch == COMMENT_CHAR ) { - while ( *readp != EOLN ) - readp++; - - *writep = EOLN; - writep++; - // get to next character after EOLN - readp++; - } else if ( (ch == '/') && (readp[1] == '*')) { // Start of multi-line comment - int done; - - done = 0; - while ( !done ) { - while ( *readp != '*' ) - readp++; - if ( readp[1] == '/' ) { - readp += 2; - done = 1; - } else { - readp++; - } - } - } else { - *writep = (char)ch; - *writep++; - readp++; - } - } - - *writep = EOF_CHAR; -} -#endif*/ - int parse_get_line(char *lineout, int max_line_len, char *start, int max_size, char *cur) { char * t = lineout; @@ -2112,7 +2097,8 @@ char *mp; char *mp_raw; char outbuf[PARSE_BUF_SIZE], *str; - int in_multiline_comment = 0; + bool in_multiline_comment_a = false; + bool in_multiline_comment_b = false; int raw_text_len = strlen(raw_text); if (processed_text == NULL) @@ -2132,7 +2118,7 @@ while ( (num_chars_read = parse_get_line(outbuf, PARSE_BUF_SIZE, raw_text, raw_text_len, mp_raw)) != 0 ) { mp_raw += num_chars_read; - in_multiline_comment = strip_comments(outbuf, in_multiline_comment); + strip_comments(outbuf, in_multiline_comment_a, in_multiline_comment_b); maybe_convert_foreign_characters(outbuf); |
|
Comment technique rewritten. Axem, can you test the attached patch? |
|
Fix committed to trunk@10492. |
|
This can still trigger. On trying some things out I think its the recommendations that !* can still cause the game to wig out. |
|
|
|
Looks like it's not the recommendations, but rather the fact that the text has embedded newlines. |
|
Fix committed to trunk@10753. |
fs2open: trunk r10492 2014-03-12 21:02 Ported: N/A Details Diff |
New method of handling quotes and comments that examines lines character by character like most languages do, instead of just searching for the start of a comment. If a comment character is inside a quoted string, it will not kick off a new comment. (This actually fixes the core design flaw that caused semicolons to truncate messages, so the $semicolon tag is technically no longer necessary. It will still be kept, of course.) Fixes Mantis 0003001. |
Affected Issues 0003001 |
|
mod - /trunk/fs2_open/code/parse/parselo.cpp | Diff File | ||
fs2open: trunk r10753 2014-06-04 21:21 Ported: N/A Details Diff |
quotes can span multiple lines as well, so let's keep track of that flag (fixes Mantis 0003001) |
Affected Issues 0003001 |
|
mod - /trunk/fs2_open/code/parse/parselo.cpp | Diff File |
Date Modified | Username | Field | Change |
---|---|---|---|
2014-01-19 16:52 | Axem | New Issue | |
2014-01-19 21:07 | Goober5000 | Assigned To | => Goober5000 |
2014-01-19 21:07 | Goober5000 | Status | new => assigned |
2014-01-26 19:13 | Goober5000 | Note Added: 0015579 | |
2014-01-27 04:12 | Goober5000 | Note Added: 0015581 | |
2014-02-02 17:11 | Axem | Note Added: 0015584 | |
2014-02-04 02:29 | Goober5000 | Note Added: 0015585 | |
2014-03-04 06:06 | Goober5000 | Note Added: 0015636 | |
2014-03-12 08:09 | Goober5000 | File Added: 3001.patch | |
2014-03-12 08:10 | Goober5000 | Note Added: 0015649 | |
2014-03-13 02:00 | Goober5000 | Changeset attached | => fs2open trunk r10492 |
2014-03-13 02:00 | Goober5000 | Note Added: 0015652 | |
2014-03-13 02:00 | Goober5000 | Status | assigned => resolved |
2014-03-13 02:00 | Goober5000 | Resolution | open => fixed |
2014-06-05 00:40 | Axem | Note Added: 0015794 | |
2014-06-05 00:40 | Axem | Status | resolved => feedback |
2014-06-05 00:40 | Axem | Resolution | fixed => reopened |
2014-06-05 00:40 | Axem | File Added: CRASHESTHEGAME.fs2 | |
2014-06-05 01:57 | Goober5000 | Note Added: 0015797 | |
2014-06-05 02:05 | Goober5000 | Changeset attached | => fs2open trunk r10753 |
2014-06-05 02:05 | Goober5000 | Note Added: 0015798 | |
2014-06-05 02:05 | Goober5000 | Status | feedback => resolved |
2014-07-03 05:15 | Yarn | Relationship added | related to 0003072 |