View Issue Details

IDProjectCategoryView StatusLast Update
0003001FSSCPFREDpublic2014-07-03 05:15
ReporterAxem Assigned ToGoober5000  
PrioritynormalSeverityblockReproducibilityalways
Status resolvedResolutionreopened 
Product Version3.7.1 
Summary0003001: Multiline comments problems
DescriptionIn 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?
TagsNo tags attached.

Relationships

related to 0003072 resolvedGoober5000 German section of retail strings.tbl crashes game 

Activities

Goober5000

2014-01-26 19:13

administrator   ~0015579

I'm considering {* and *}. Does anyone know of any missions which might use these character groupings?

Goober5000

2014-01-27 04:12

administrator   ~0015581

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.

Axem

2014-02-02 17:11

reporter   ~0015584

Soooo... When can this be fixed?

Goober5000

2014-02-04 02:29

administrator   ~0015585

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. :)

Goober5000

2014-03-04 06:06

administrator   ~0015636

I intend to fix this this week. Axem, feel free to pester me until I do.

Goober5000

2014-03-12 08:09

administrator  

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);
 
3001.patch (9,781 bytes)   

Goober5000

2014-03-12 08:10

administrator   ~0015649

Comment technique rewritten. Axem, can you test the attached patch?

Goober5000

2014-03-13 02:00

administrator   ~0015652

Fix committed to trunk@10492.

Axem

2014-06-05 00:40

reporter   ~0015794

This can still trigger. On trying some things out I think its the recommendations that !* can still cause the game to wig out.

Axem

2014-06-05 00:40

reporter  

CRASHESTHEGAME.fs2 (4,881 bytes)

Goober5000

2014-06-05 01:57

administrator   ~0015797

Looks like it's not the recommendations, but rather the fact that the text has embedded newlines.

Goober5000

2014-06-05 02:05

administrator   ~0015798

Fix committed to trunk@10753.

Related Changesets

fs2open: trunk r10492

2014-03-12 21:02

Goober5000


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

Goober5000


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

Issue History

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