Bug 3318 - renew Q_IsColorString()
Status: RESOLVED FIXED
Alias: None
Product: ioquake3
Classification: Unclassified
Component: Misc
Version: unspecified
Hardware: All All
: P3 enhancement
Assignee: Zachary J. Slater
QA Contact: ioquake3 bugzilla mailing list
URL:
Depends on:
Blocks:
 
Reported: 2007-08-17 09:53 EDT by /dev/humancontroller
Modified: 2007-09-21 08:34:06 EDT
0 users

See Also:



Description /dev/humancontroller 2007-08-17 09:53:13 EDT
Currently, Q3 defines a color escape codes as a '^' followed by anything except '\0':
#define Q_IsColorString(p) ( p && *(p) == Q_COLOR_ESCAPE && *((p)+1) && *((p)+1) != Q_COLOR_ESCAPE )

I see no reason why ^a, ^b, etc. should be recongized as color escapes (It's not even consistent throughout platforms). Also, in a message ending with ^, after which a newline should have been outputted (like any server message, such as a chat), ^\n is a color code, and not a ^ and a new line (bug!). I think ^1 ^2 ^3 ^4 ^5 ^6 ^7 ^8 ^9 and ^0 should be the only color escape codes.

We also shouldn't check wether p is a valid pointer, because we usually guarantee it, check it and whatsoever, and if we do check p, and Q_IsColorString doesn't result in crash, then the code after it will.

So my recommended method is:
#define Q_IsColorString(p) ( *(p) == Q_COLOR_ESCAPE && isdigit( *((p)+1) ) )
Comment 1 /dev/humancontroller 2007-08-19 14:04:18 EDT
Or even better, only use ^A to ^Z !!!

Allow new colors: all RGB color combinations of 0, 128 and 255, such as RGB(0,255,128), RGB(128,128,255), etc., which makes a total of 27 colors. The english alphabet has only 26 characters, leaving out the one annoying color: black. All other colors can be easily read on black background.

#define Q_IsColorString(p) ( *(p) == Q_COLOR_ESCAPE && isalpha( *((p)+1) ) )

void Index2Color( char index, vec4_t color ) {
	int num, tmp;
	int col[3];

	color[3] = 1.0f;

	if( !isalpha( index ) ) {
		color[0] = 1.0f;
		color[1] = 1.0f;
		color[2] = 1.0f;
		return;
	}

	num = ( tolower( index ) - 'a' ) % 26 + 1;

	tmp = num / 3;
	col[2] = num - tmp * 3;
	num = tmp;
	tmp = num / 3;
	col[1] = num - tmp * 3;
	num = tmp;
	tmp = num / 3;
	col[0] = num - tmp * 3;

	color[0] = col[0] / 2.0f;
	color[1] = col[1] / 2.0f;
	color[2] = col[2] / 2.0f;
}
Comment 2 /dev/humancontroller 2007-08-21 12:51:35 EDT
Or fill up g_color_table[] with 26 colors and use
    ( tolower( index ) - 'a' ) % 26
as an index ;)
Comment 3 /dev/humancontroller 2007-08-23 10:40:22 EDT
Invalid already? Slow down a bit! I will contact the OSP/CPMA developers and ask them about this. They may release a quick 1.3c version, or not.

But either way, using
    #define Q_IsColorString(p) ( *(p) == Q_COLOR_ESCAPE && ( isdigit( *((p)+1) ) || isalpha( *((p)+1) ) ) // ^[0-9a-zA-Z]
will not cause any trouble, and will standardize a bit, while fixing stuff like the the bug where ^ characters are eating newlines.

How's that?
Comment 4 Tim Angus 2007-08-23 11:41:43 EDT
There are mods that use ^[a-zA-Z] (OSP/CPMA, I believe). Any of the proposed changes here would interfere with those mods.
Comment 5 Tim Angus 2007-09-21 08:34:06 EDT
r1185.