Login
User Name:

Password:



Register
Forgot your password?
Vote for Us!
Bug in disarm( )
Nov 12, 2017, 6:54 pm
By GatewaySysop
Bug in will_fall( )
Oct 23, 2017, 1:35 am
By GatewaySysop
Bug in do_zap( ), do_brandish( )
Oct 18, 2017, 1:52 pm
By GatewaySysop
Bug in get_exp_worth( )
Oct 10, 2017, 1:26 am
By GatewaySysop
Bug in do_drag( )
Oct 8, 2017, 12:40 am
By GatewaySysop
LOP Heroes Edition
Author: Vladaar
Submitted by: Vladaar
Heroes sound extras
Author: Vladaar
Submitted by: Vladaar
6Dragons 4.3
Author: Vladaar
Submitted by: Vladaar
Memwatch
Author: Johan Lindh
Submitted by: Vladaar
Beastmaster 6D sound files
Author: Vladaar
Submitted by: Vladaar
Users Online
CommonCrawl, DotBot, Sogou, Yahoo!

Members: 0
Guests: 12
Stats
Files
Topics
Posts
Members
Newest Member
476
3,704
19,231
608
LAntorcha
Today's Birthdays
There are no member birthdays today.
Related Links
» SmaugMuds.org » Codebases » AFKMud Support & Development » Bug Fix: color_align
Forum Rules | Mark all | Recent Posts

Bug Fix: color_align
< Newer Topic :: Older Topic >

Pages:<< prev 1 next >>
Post is unread #1 Apr 4, 2010, 1:10 pm
Go to the top of the page
Go to the bottom of the page

Atrox
Fledgling
GroupMembers
Posts19
JoinedFeb 14, 2005

color_align doesn't work properly when there are color codes involved. Also, color_strlen doesn't always return the correct length of a string. This problem is related to the problem I'm having with get_line, so I'm fairly certain it's the colorcode function. Anyway, here's a fix to get your color_align working properly with ALIGN_RIGHT.

You'll need these two functions:
// This will give the proper length of a string while ignoring color codes - Atrox
unsigned int stripped_length( const char *string )
{
	unsigned int count = 0, i = 0;

	for( i = 0; i < strlen( string ); i++ )
	{
		if( string[i] == '}' || string[i] == '{' || string[i] == '&' )
		{
			if( string[i + 1] == '}' || string[i + 1] == '{' || string[i + 1] == '&' )
			{
				count += 2;
				i++;
				continue;
			}
			i++;
			continue;
		}
		count++;
	}

	return count;
}

// Go through a string and count the number of color codes (needed for some formatting issues) - Atrox
unsigned int num_colors( const char *string )
{
	unsigned int count = 0, i = 0;

	for( i = 0; i < strlen( string ); i++ )
	{
		if( string[i] == '}' || string[i] == '{' || string[i] == '&' )
		{
			if( string[i + 1] == '}' || string[i + 1] == '{' || string[i + 1] == '&' )
			{
				i++;
				continue;
			}
			count += 2;
			i++;
			continue;
		}
	}

	return count;
}


Now replace your color_align function with this:
char *color_align( const char *argument, int size, int align )
{
	int space = 0;
	int len = 0;
	int clrlen = num_colors( argument ) + size; // This increases the size to account for color codes
	static char buf[MSL];

	len = stripped_length( argument );
	space = ( size - len );
	if( align == ALIGN_RIGHT )
		snprintf( buf, MSL, "%*.*s", clrlen, clrlen, argument ); // Use the new color length
	else if( align == ALIGN_CENTER )
		snprintf( buf, MSL, "%*s%s%*s", ( space / 2 ), "", argument, ( ( space / 2 ) * 2 ) == space ? ( space / 2 ) : ( ( space / 2 ) + 1 ), "" );
	else if( align == ALIGN_LEFT )
		snprintf( buf, MSL, "%s%*s", argument, space, "" );

	return buf;
}


If anyone wants to go back and optimize this code, feel free. For now, however, your ALIGN_RIGHT case with color_align will work properly.
       
Post is unread #2 Apr 4, 2010, 9:09 pm
Go to the top of the page
Go to the bottom of the page

David Haley
Sorcerer
GroupMembers
Posts903
JoinedJan 29, 2007

In your comments, it would help to say what exactly you mean by "proper" length. Does this mean the number of characters in the string? Does it mean the number of characters after removing color codes -- i.e., the number of displayed characters?

I'm a little confused about why you seem to count {{ as two characters; isn't {{ meant to mean just {, and so would be just one displayable character?

And in num_colors, why is a single color code counted as two in the returned results?
       
Post is unread #3 Apr 5, 2010, 6:34 pm
Go to the top of the page
Go to the bottom of the page

Atrox
Fledgling
GroupMembers
Posts19
JoinedFeb 14, 2005

By proper length, I mean the actual visual length of the string, not counting color codes. What you would see on the screen. As for why I'm counting {{ as 2, I really hate to say that I actually don't know. I coded it so that if [i] == { and i+1 != {, then count it as 2 cause it's a color code, but it did the opposite of what I was intending. However, the code I posted here does exactly what it should do, in every case that I've tested. If you can break it, by all means bring it to my attention. For num_colors, I thought it would be obvious why a single code is counted as 2, it has 2 characters; & and R are one color code, but 2 characters. If you don't count both of them, you'll have to double the number somewhere else, so I figured I might as well do it once and forget about it.
       
Post is unread #4 Apr 6, 2010, 6:58 am
Go to the top of the page
Go to the bottom of the page

David Haley
Sorcerer
GroupMembers
Posts903
JoinedJan 29, 2007

Then num_colors isn't counting the number of color codes: it's counting the number of characters involved in color codes. It's important to be precise with this sort of stuff given how easy it is to mix up what is meant (e.g., "proper" length).

There should be a guarantee that strlen(str) - num_colors(str) == stripped_length(str). If this does not hold, one of the two functions is broken. Perhaps you had to count it as two in stripped_length, because you don't count it at all in num_colors, even though in a sense you should be treating {{ as a single visual character, and not two, because it is indeed rendered just once.

You do have at least one bug though: if the last character is a {, } or &, you will incorrectly try to read past the end of the string.
       
Post is unread #5 Apr 6, 2010, 10:52 am
Go to the top of the page
Go to the bottom of the page

Atrox
Fledgling
GroupMembers
Posts19
JoinedFeb 14, 2005

The functions work the same separately or combined. And yes, I know that {{, && and }} are all 1 single character, but that doesn't change the fact that it's not producing the result that I intended. However, the way the code is here, it works the way it should work. Num_colors does count the number of color codes, it just returns it in double so that you don't have to account for them elsewhere in another function.
       
Post is unread #6 Apr 6, 2010, 2:32 pm
Go to the top of the page
Go to the bottom of the page

David Haley
Sorcerer
GroupMembers
Posts903
JoinedJan 29, 2007

Code working the way it should work does not mean that it is written in a sensible way. ;)

For example, should strlen return twice the length of a string just because some other function depends on that behavior? Yeah yeah, it might "work", but only with a complicated set of assumptions that won't make sense the next time you look at the function.

num_colors is not counting color codes: it's counting non-printed characters. Similarly, stripped_length is returning the length of the string after non-printed characters have been removed. The name num_colors therefore is confusing and not descriptive of what is actually happening.

This is why there is a necessary duality between the two functions. This does not explain why {{, && and }} need to break the rules.

I would suggest changing both to use only one, and see what happens.
       
Post is unread #7 Apr 8, 2010, 2:03 pm   Last edited Apr 8, 2010, 2:03 pm by Atrox
Go to the top of the page
Go to the bottom of the page

Atrox
Fledgling
GroupMembers
Posts19
JoinedFeb 14, 2005

Maybe I should have been a little more explicit with the function names and comments. Num_colors is meant to count the number of colors, and return a number representing the number of characters the color codes in a string occupy. I named it num_colors for myself, if you use it, feel free to rename it to however you feel comfortable having it. I told you in an earlier post that I had ...
if( string[i + 1] == '}' || string[i + 1] == '{' || string[i + 1] == '&' )

set to ...
if( string[i + 1] != '}' || string[i + 1] != '{' || string[i + 1] != '&' )

like I would think it SHOULD be, and it worked the opposite of how I intended. This is really old code that I wrote ages ago, so I don't question it when it works, all I care is that it does.
       
Post is unread #8 Apr 9, 2010, 12:27 pm
Go to the top of the page
Go to the bottom of the page

David Haley
Sorcerer
GroupMembers
Posts903
JoinedJan 29, 2007

If I'm saying all this, it's only because I believe that bug fixes should be done properly, and not really be the equivalent of throwing darts at a board until it "just works" without questioning it. If it works for you that's great and I guess you don't need to fix it, but the fix should be made more properly before being included into the main release.
       
Post is unread #9 Apr 9, 2010, 2:06 pm   Last edited Apr 9, 2010, 2:10 pm by Kayle
Go to the top of the page
Go to the bottom of the page

Kayle
Off the Edge of the Map
GroupAdministrators
Posts1,195
JoinedMar 21, 2006

I still haven't figured out what this is trying to fix. As far as I know my color_align work has always come out properly. And I use it extensively. Ranks are centered at a specific length, part of my who display is centered between a pair of ( ), and a couple more places I can't recall. I've never really found a use for align_right that I know of.
       
Post is unread #10 Apr 9, 2010, 4:50 pm
Go to the top of the page
Go to the bottom of the page

Samson
Black Hand
GroupAdministrators
Posts3,639
JoinedJan 1, 2002

If it's only the align_right part that's broken it wouldn't come as any great surprise. That's the only part I never found a use for. Center and left alignment both work though.
       
Post is unread #11 Apr 12, 2010, 2:48 pm
Go to the top of the page
Go to the bottom of the page

Atrox
Fledgling
GroupMembers
Posts19
JoinedFeb 14, 2005

ALIGN_LEFT and ALIGN_CENTER work fine, but ALIGN_RIGHT didn't. Works for me now, so if you guys wanna fix it for your next release, you can go and do that.
       
Post is unread #12 Apr 13, 2010, 6:38 am
Go to the top of the page
Go to the bottom of the page

David Haley
Sorcerer
GroupMembers
Posts903
JoinedJan 29, 2007

Samson & Kayle, do you know off-hand if this is an AFKMud-specific thing, or if the code is shared meaning that right alignment is likely to be a problem for FUSS as well?
       
Post is unread #13 Apr 13, 2010, 12:01 pm
Go to the top of the page
Go to the bottom of the page

Samson
Black Hand
GroupAdministrators
Posts3,639
JoinedJan 1, 2002

/*
 * Quixadhal - And this one needs to use the new version too.
 */
char *color_align( const char *argument, int size, int align )
{
   int space = 0;
   int len = 0;
   static char buf[MAX_STRING_LENGTH];

   len = color_strlen( argument );
   space = ( size - len );
   if( align == ALIGN_RIGHT || len >= size )
      snprintf( buf, MAX_STRING_LENGTH, "%*.*s", len, len, argument );
   else if( align == ALIGN_CENTER )
      snprintf( buf, MAX_STRING_LENGTH, "%*s%s%*s", ( space / 2 ), "", argument,
                ( ( space / 2 ) * 2 ) == space ? ( space / 2 ) : ( ( space / 2 ) + 1 ), "" );
   else if( align == ALIGN_LEFT )
      snprintf( buf, MAX_STRING_LENGTH, "%s%*s", argument, space, "" );

   return buf;
}


That's the function from color.c in SmaugFUSS. So yes, it will at some point have an impact. I guess it's just been a matter of hardly anyone actually using ALIGN_RIGHT.
       
Post is unread #14 Apr 13, 2010, 12:09 pm
Go to the top of the page
Go to the bottom of the page

Atrox
Fledgling
GroupMembers
Posts19
JoinedFeb 14, 2005

I also had a problem with color_strlen not always returning the correct visible length of a string, if you guys wanna look into that.
       
Pages:<< prev 1 next >>