LPMuds.net

LPMuds.net Forums => Bug Central => Topic started by: Raudhrskal on May 17, 2008, 07:27:41 am

Title: Why FluffOS implementation of terminal_colour() appends reset code to string?
Post by: Raudhrskal on May 17, 2008, 07:27:41 am
Ok, maybe it's not a bug, but a feature.

But it breaks a lot of things.

Try for example
Code: [Select]
eval return strip_colours("%^GREEN%^BOLD%^x%^RESET%^") // "x"
eval return strlen(strip_colours("%^GREEN%^BOLD%^x%^RESET%^")) //16!
eval return center("%^GREEN%^BOLD%^x%^RESET%^",20) // "  x  ", two spaces on each end
eval return strlen(center("%^GREEN%^BOLD%^x%^RESET%^",20)) //yep, 20
Results are dependent on terminal setting. In case of ansi, the difference is 15 bytes. Unknown may reduce it to 2-3.

How it can be?
Do this. Do NOT use a loop!:
Code: [Select]
eval return convert_ascii(strip_colours("%^GREEN%^BOLD%^x%^RESET%^")[0]) // "x" //ok.
eval return convert_ascii(strip_colours("%^GREEN%^BOLD%^x%^RESET%^")[1]) // " //single "? wtf?
eval return convert_ascii(strip_colours("%^GREEN%^BOLD%^x%^RESET%^")[2]) // "["
eval return convert_ascii(strip_colours("%^GREEN%^BOLD%^x%^RESET%^")[3]) // "4"
eval return convert_ascii(strip_colours("%^GREEN%^BOLD%^x%^RESET%^")[4]) // "9"
And if you continue, you'll see that your "x" has an invisible escape sequence appended to it.

In case of ansi this is <escape>[49;49m<escape>[0;10m - because of the escapes, the second doublequote is swallowed.
In case of unknown it is version-dependent - "current" FluffOS appends same sequence, older used two high-ascii characters - 255 and 239.

This obviously breaks things that use strip_colo(u)rs to obtain "visible" string length. Simplest example - center().

fluffos/packages/contrib.c , functions f_terminal_colour and f_terminal_color.
Look for places where local variables "resetstr", "resetstrname" and "resetstrlen" are used.

I know C, but I'm not familiar with driver's API. Can someone competent tell me why this code has been added? It didn't existed in MudOS' version of this function.

And, of course, how to fix (or work around) it? Currently I am using an LPC-overloaded version of terminal_colour that strips the reset sequence. But this is a major performace penalty.

DS2.7a27 + FluffOS 2.11-ds03 , and some previous versions of course.
Title: Re: Why FluffOS implementation of terminal_colour() appends reset code to string
Post by: wodan on May 17, 2008, 09:04:16 am
it's so you can put two bits of coloured text next to eachother without colours bleeding through between them, it also adds the colour you had so far to the start of the next line.
terminal_colour does line wraps based on line lengths already, if you use it to find out how long something is, chances are that you're doing something wrong!

PS definitely not a bug, but a useful feature (we use it to display ascii maps next to room descriptions)

Wodan.
Title: Re: Why FluffOS implementation of terminal_colour() appends reset code to string
Post by: Raudhrskal on May 17, 2008, 10:21:49 am
Right. So what should I use to strip ALL the color codes from a string? Standard DS strip_colours() just launches terminal_colour() , with mapping in second arg linking all codes to "". In result, the reset string is appended.

strip_colors() in http://dead-souls.net/code/alpha/ds2.7a27/lib/secure/sefun/interface.c
calls no_colour() which calls terminal_colour() - in http://dead-souls.net/code/alpha/ds2.7a27/lib/daemon/terminal.c.
Title: Re: Why FluffOS implementation of terminal_colour() appends reset code to string?
Post by: Nulvect on May 17, 2008, 02:20:16 pm
I agree that strip_colours() should return a string with _NO_ codes left in it. It's not always for line wrapping - any sort of info display (for instance, score) needs to be able to put together several pieces of text and make it fit in X characters wide, and if you can't get the visible lengths of the pieces it's impossible. Unless perhaps fluffos has a color-friendly version of sprintf, I can't think of how you'd put together a colorful info screen correctly...
Title: Re: Why FluffOS implementation of terminal_colour() appends reset code to string
Post by: wodan on May 17, 2008, 04:02:33 pm
what are you trying that would be impossible?
Title: Re: Why FluffOS implementation of terminal_colour() appends reset code to string
Post by: Tricky on May 17, 2008, 06:50:11 pm
After looking at the sefun for strip_colours() I think I see the problem. It is contained within /secure/sefun/interface.c and calls TERMINAL_D->no_colours(str). This uses terminal_colour() with the term_info mapping "unknown".

There is also another sefun called strip_colors(), note the American spelling. This iterates through the string throwing out all colour codes.

Tricky
Title: Re: Why FluffOS implementation of terminal_colour() appends reset code to string
Post by: cratylus on May 18, 2008, 09:23:11 am
Tricky suggested an interesting workaround I will share here with
y'all. It seems that the unexpected behavior is triggered when you
send a ([ "RESET" : "" ]) to terminal_colour(). Replacing "" with a
known special char that you later strip out seems to do the trick:

/secure/sefun/interface.c
Code: [Select]

mapping Uncolor = ([ "RESET": "\b", "BOLD": "", "FLASH":"", "BLACK":"", "RED":"",
  "BLUE":"", "CYAN":"", "MAGENTA":"", "ORANGE":"", "YELLOW":"",
  "GREEN":"", "WHITE":"", "BLACK":"", "B_RED":"", "B_ORANGE":"",
  "B_YELLOW":"", "B_BLACK":"", "B_CYAN":"","B_WHITE":"", "B_GREEN":"",
  "B_MAGENTA":"", "STATUS":"", "WINDOW":"", "INITTERM": "",
  "ENDTERM":""]);

#include <daemons.h>

string strip_colours(string str){
    string ret = terminal_colour(str, Uncolor);
    return replace_string(ret, "\b", "");
}

string strip_colors(string str){
    return strip_colours(str);
}

string strip_colors_old(string str){
    string output = "";
    string *input = explode(str,"%^");
    string *list = ({ "RED","YELLOW","BLUE","GREEN","MAGENTA","ORANGE","CYAN","BLACK","WHITE"});
    list += ({ "B_RED","B_YELLOW","B_BLUE","B_GREEN","B_MAGENTA","B_ORANGE","B_CYAN","B_BLACK","B_WHITE"});
    list += ({"BOLD","FLASH","RESET"});
    foreach(string color in list) input -= ({ color });
    output = implode(input,"");
    if(sizeof(output)) return output;
    else return "";
}

result:
Quote
eval return strlen(strip_colours("%^GREEN%^BOLD%^x%^RESET%^"))
Result = 1

Thanks, Tricky.

-Crat
Title: Re: Why FluffOS implementation of terminal_colour() appends reset code to string
Post by: Tricky on May 18, 2008, 11:30:38 pm
I've improved on that so you don't have to put in a special char. All you do is strip out the TELNET_GA command that the driver inserts. BTW Wodan, if I didn't strip it out this command it stops output on my client (TF v5.0 beta 6).

Edit update: Seems it is a known problem for TF.

Please note: This is only relevant to code for the FluffOS driver. MudOS doesn't have this problem/feature.

Code: (example snippet) [Select]
mapping Uncolor = ([ "RESET": "", "BOLD": "", "FLASH":"", "BLACK":"", "RED":"",
  "BLUE":"", "CYAN":"", "MAGENTA":"", "ORANGE":"", "YELLOW":"",
  "GREEN":"", "WHITE":"", "BLACK":"", "B_RED":"", "B_ORANGE":"",
  "B_YELLOW":"", "B_BLACK":"", "B_CYAN":"","B_WHITE":"", "B_GREEN":"",
  "B_MAGENTA":"", "STATUS":"", "WINDOW":"", "INITTERM": "",
  "ENDTERM":""]);

/* \xff\xf9 is the telnet code command for TELNET_GA
 * Google for it if you want to know what it does.
 */
string strip_colours(string str){
    string ret = terminal_colour(str, Uncolor);
    return replace_string(ret, "\xff\xf9", "");
}

Tricky
Title: Re: Why FluffOS implementation of terminal_colour() appends reset code to string
Post by: Raudhrskal on May 19, 2008, 12:37:34 am
Another "not a bug" ;)
This is not a problem with tf - it just interprets the GA as an end-of-prompt marker.
It's configurable somewhere (enable UNterminated prompt to make it work right).
Title: Re: Why FluffOS implementation of terminal_colour() appends reset code to string
Post by: Tricky on May 20, 2008, 08:31:01 am
Another "not a bug" ;)
This is not a problem with tf - it just interprets the GA as an end-of-prompt marker.
It's configurable somewhere (enable UNterminated prompt to make it work right).
"configurable somewhere" -- indeed.

I'm not wanting to rely on the end-user configuring their client for these type of oddities so I'd rather take them out server-side.

As to the code I proposed previously it seems that if there are more than one %^RESET%^ in the sequence then it doesn't quite work as planned. Using the code Crat proposed works everytime. Simple solutions are always the best, they always win everytime over more complex and clever solutions.

Tricky