2018-05-20 16:32 EDT


View Issue Details Jump to Notes ] Related Changesets ]
IDProjectCategoryView StatusLast Update
0003001FSSCPFREDpublic2014-07-03 01:15
ReporterAxem 
Assigned ToGoober5000 
PrioritynormalSeverityblockReproducibilityalways
StatusresolvedResolutionreopened 
Product Version3.7.1 
Target VersionFixed in Version 
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.
Attached Files
  • patch file icon 3001.patch (9,781 bytes) 2014-03-12 04:09 -
    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);
     
    
    patch file icon 3001.patch (9,781 bytes) 2014-03-12 04:09 +
  • ? file icon CRASHESTHEGAME.fs2 (4,881 bytes) 2014-06-04 20:40

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

-Notes

~0015579

Goober5000 (administrator)

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

~0015581

Goober5000 (administrator)

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.

~0015584

Axem (reporter)

Soooo... When can this be fixed?

~0015585

Goober5000 (administrator)

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

~0015636

Goober5000 (administrator)

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

~0015649

Goober5000 (administrator)

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

~0015652

Goober5000 (administrator)

Fix committed to trunk@10492.

~0015794

Axem (reporter)

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

~0015797

Goober5000 (administrator)

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

~0015798

Goober5000 (administrator)

Fix committed to trunk@10753.
+Notes

+Related Changesets

-Issue History
Date Modified Username Field Change
2014-01-19 11:52 Axem New Issue
2014-01-19 16:07 Goober5000 Assigned To => Goober5000
2014-01-19 16:07 Goober5000 Status new => assigned
2014-01-26 14:13 Goober5000 Note Added: 0015579
2014-01-26 23:12 Goober5000 Note Added: 0015581
2014-02-02 12:11 Axem Note Added: 0015584
2014-02-03 21:29 Goober5000 Note Added: 0015585
2014-03-04 01:06 Goober5000 Note Added: 0015636
2014-03-12 04:09 Goober5000 File Added: 3001.patch
2014-03-12 04:10 Goober5000 Note Added: 0015649
2014-03-12 22:00 Goober5000 Changeset attached => fs2open trunk r10492
2014-03-12 22:00 Goober5000 Note Added: 0015652
2014-03-12 22:00 Goober5000 Status assigned => resolved
2014-03-12 22:00 Goober5000 Resolution open => fixed
2014-06-04 20:40 Axem Note Added: 0015794
2014-06-04 20:40 Axem Status resolved => feedback
2014-06-04 20:40 Axem Resolution fixed => reopened
2014-06-04 20:40 Axem File Added: CRASHESTHEGAME.fs2
2014-06-04 21:57 Goober5000 Note Added: 0015797
2014-06-04 22:05 Goober5000 Changeset attached => fs2open trunk r10753
2014-06-04 22:05 Goober5000 Note Added: 0015798
2014-06-04 22:05 Goober5000 Status feedback => resolved
2014-07-03 01:15 Yarn Relationship added related to 0003072
+Issue History