COM_Parse and multiline comments/DP save compatibility

Post tutorials on how to do certain tasks within game or engine code here.
Post Reply
mh
Posts: 2292
Joined: Sat Jan 12, 2008 1:38 am

COM_Parse and multiline comments/DP save compatibility

Post by mh »

Inspired by a thread on Func (clicky), here's a handler for multi-line comments in save games and other files that are parsed by COM_Parse. Pop this after the "skip // comments" section and you can load DarkPlaces save games now.

Code: Select all

	// skip /*..*/ comments
	if (c == '/' && data[1] == '*')
	{
		// comment
		data++;

		while (data[0] && (data[0] != '*' || data[1] != '/'))
			data++;

		if (data[0]) data++;
		if (data[0]) data++;

		goto skipwhite;
	}
We had the power, we had the space, we had a sense of time and place
We knew the words, we knew the score, we knew what we were fighting for
taniwha
Posts: 401
Joined: Thu Jan 14, 2010 7:11 am
Contact:

Re: COM_Parse and multiline comments/DP save compatibility

Post by taniwha »

Hmm, I find it hard to imagine that this would cause any compatibility issues, so it's probably a good thing to spread through the various engines.

However, I see a minor bug: "/*/" is treated as a complete comment because you skip over only the opening "/" but not the opening "*".

The double increment after the check for "*/" seemed suspicious, but then I realized it's skipping the closing "*/". I'm not 100% certain (~98%), but I think you can get away with "if (data[0]) data += 2;" because the only way to get there is for data[0] to be 0, or for data[0..1] to be "*/".
Leave others their otherness.
http://quakeforge.net/
mh
Posts: 2292
Joined: Sat Jan 12, 2008 1:38 am

Re: COM_Parse and multiline comments/DP save compatibility

Post by mh »

I was a bit suspicious of the double data++ myself (it's been a coupla years after all) but I've double-checked against the way DarkPlaces handles this and can see that my code was almost a direct copy & paste from it (which I obviously did and then completely forgot about) so I'll leave everything stand.
We had the power, we had the space, we had a sense of time and place
We knew the words, we knew the score, we knew what we were fighting for
taniwha
Posts: 401
Joined: Thu Jan 14, 2010 7:11 am
Contact:

Re: COM_Parse and multiline comments/DP save compatibility

Post by taniwha »

Oh, it's correct, but the second "if (data[0])" is redundant (the double increment should be merged).

However, the first single data++ is not correct (doesn't matter what darkplaces does). I've tested :)
Leave others their otherness.
http://quakeforge.net/
mh
Posts: 2292
Joined: Sat Jan 12, 2008 1:38 am

Re: COM_Parse and multiline comments/DP save compatibility

Post by mh »

I don't disagree. I'm just being wary of the fact that LordHavoc never added a comment explaining his reasons for doing it this way, and am wondering if there was some obscure bug it was intended to fix, or was he just having a brainfart that day? Since the double "if (data[0])" does no harm I'm more inclined to leave it be on account of that.
We had the power, we had the space, we had a sense of time and place
We knew the words, we knew the score, we knew what we were fighting for
taniwha
Posts: 401
Joined: Thu Jan 14, 2010 7:11 am
Contact:

Re: COM_Parse and multiline comments/DP save compatibility

Post by taniwha »

I'd say the biggest problem with the double "if (data[0])" is it is confusing.

Any comments on that single "data++" before the while loop?
Leave others their otherness.
http://quakeforge.net/
mh
Posts: 2292
Joined: Sat Jan 12, 2008 1:38 am

Re: COM_Parse and multiline comments/DP save compatibility

Post by mh »

It definitely seems wrong and unnecessary to me too. I can only guess that LH did it because the "handle quoted strings specially" part also does it - but that's all it is, a guess.

Just for reference, here's the relevant section from Q3A:

Code: Select all

		else if (c == '/' && data[1] == '*')
		{
			data += 2;

			while (*data && (*data != '*' || data[1] != '/'))
			{
				data++;
			}

			if (*data)
			{
				data += 2;
			}
		}
I note the use of "data += 2" at the end instead of the double "if (data[0])", but I also note a "data += 2" at the beginning.
We had the power, we had the space, we had a sense of time and place
We knew the words, we knew the score, we knew what we were fighting for
taniwha
Posts: 401
Joined: Thu Jan 14, 2010 7:11 am
Contact:

Re: COM_Parse and multiline comments/DP save compatibility

Post by taniwha »

That's very similar to how I implemented it in QF. Differences caused by copying your code and different coding style :)

Code: Select all

    // skip /*..*/ comments
    if (data[0] == '/' && data[1] == '*') {
        data += 2;      // skip over the leading /*
        while (data[0] && (data[0] != '*' && data[1] != '/'))
            data++;
        if (data[0])
            data +=2;   // skip over the trailing */
        goto skipwhite;
    }
Leave others their otherness.
http://quakeforge.net/
szo
Posts: 132
Joined: Mon Dec 06, 2010 4:42 pm

Re: COM_Parse and multiline comments/DP save compatibility

Post by szo »

The problem with the QF code posted here is that they would stop at the first '*' within the comment, e.g. something like:

Code: Select all

/*
 * $Header$
 */
... will result in '$' as the first available character, an issue which I hit myself. Correct one would be like the following:

Code: Select all

// skip /*..*/ comments
	if (c == '/' && data[1] == '*')
	{
		data += 2;
		while (*data && !(*data == '*' && data[1] == '/'))
			data++;
		if (*data)
			data += 2;
		goto skipwhite;
	}
taniwha
Posts: 401
Joined: Thu Jan 14, 2010 7:11 am
Contact:

Re: COM_Parse and multiline comments/DP save compatibility

Post by taniwha »

Yeah, I got the email, too. I'm considering how to write a test case for "make check".
Leave others their otherness.
http://quakeforge.net/
Post Reply