[The E]

IIRC, the Fiction Viewer uses the same coloring code as briefing texts.

[karajorma]

It definitely does. I had to change things a while back cause the codes were borking the fiction viewer.

[Zacam]

So far, this patch does as advertised on the FS2 Campaign Missions just fine.

Have there been any changes or other needs for it?

And how well can it be applied to working with debriefing texts?

[Goober5000]

Does the patch work on "Dr. Mina Hargrove"?  Abbreviations such as Dr., Mr., Mrs., Ms., Rev., Jr., etc. should have the period in colored text.

[Zacam]

The "." in "Dr. Mina Hargrove" does not color any more than the "," at the end does. And while it might be nice to colorize a significant name like that, it's not really necessary.

Additionally, the brief text could just be made to read "Doctor" instead of "Dr." Or, some sort of an escape sequence could be placed in front of "special" characters (namely, periods) that allows them to be colored. So it would then be:

"$g Dr\. $g Mina $g Hargrove" to result in Dr. Mina Hargrove instead of the current Dr. Mina Hargrove

As well, coloration on pluralized names is still needing some work. "$g Beta's" for example results in Beta's.

[Goober5000]

Escape character (or quote character?) is probably a better idea.  Some people may not want the 's to be colorized.

[Zacam]

If I colorize "Aquitaine's" I'd expect the 's to be colored.

As well, colorizing "Terran-Vasudan" gives Terran-Vasudan instead of Terran-Vasudan

Anything in () also is not colored. Example, if I were to $g the (SOC) in "As Lightning Fall".

In related news, I have an expansion for the briefing text colors. It seems the case statements are case-sensitive, so $g and $G can be two separate colors.

Here is what I have so far, comments on additional colors would be great: (white ($w) is default text color, $b, $g, $r are default colors available already)
Bright White: $W
Standard (default) White: $w
Black: $k (Black)

Standard Blue: $b
Bright Blue: $B

Standard Green: $g
Bright Green: $G

Standard Yellow: $y
Bright Yellow: $Y

Gray: $e
Silver: $E

Violet Gray: $v
Violet: $V

Standard Red: $r
Bright Red: $R

Pink: $p
Bright Pink: $P

Additive (non pre-existent colors) are: Silver, Violet, Pink and Bright Pink. The other colors were already defined in alphacolors.cpp, with the exception of Yellow/Bright Yellow which got redefined as they looked more like Lime Green. And I realize that on a black background Grey looks really dark. In game, it's very legible and distinctive from the standard text.

[Zacam]

Okay, I made a bit of a testing Command Brief to fiddle with that contains a lot of the common usage variations that can be found in a briefing.
I performed testing on the following in windowed mode running at 1024x768 and requires either Testing Build based on 5755 or Nightly 5758 (or later).
(additionally, changing $V, $p and $y to something else will work just as well)

Code: [Select]

$Stage Text:
 XSTR("Briefing Color Test

This is a series of example $V strings, for the purpose of testing briefing colorization. This test includes $b hyphenated-words, words in $g $quotequotation$quote, as well as prefixes like $y Mr. and $y Dr. as well as pluralized forms such as $b portal's and $f Aquitane's, to name a few. Additional $quote$g quoted$quote test. Some $p (TLA) and ($p TLA) tests. A $g GTVA. Test.

Another test on $b hyphenated-words, as a secondary example.
$y Additionally: $b hyphenated-$g words as a third example.

We are also going to test $r exclusions, with $g Alpha's and $g Beta's help.
We will also be testing ending $r words.

We also need to test, when as ending $y words: quoted $g $quotewords$quote.

$b Hyphenated-Words.

And pluralized $r word's.", -1)
$end_multi_text
$Ani Filename: <NONE>
+Wave Filename: <NONE>

Here is the current form of output by the engine:

Briefing Color Test

This is a series of example strings, for the purpose of testing briefing colorization. This test includes hyphenated-words,
words in "quotation", as well as prefixes like Mr. and Dr. as well as pluralized forms such as portal's and Aquitane's, to
name a few. Additional "quoted" test. Some (TLA) and (TLA) tests. A GTVA. Test.

Another test on hyphenated-words, as a secondary example.
Additionally: hyphenated-words as a third example.

We are also going to test exclusions, with Alpha's and Beta's help.
We will also be testing ending words.

We also need to test, when as ending words: quoted "words".

Hyphenated-Words.

And pluralized word's.

Note how the first case of "hyphenated-words," is not colored? That's because it's at the end of the display line in the viewer, so it doesn't get colored like the second one does.
Interestingly enough, the third case ignores the $b and colorizes fully in green and the Additional "quoted" test is also fully in green and both of the (TLA) including the ()'s are colored.

Here is the out-put from the current proposed patch:

Briefing Color Test

This is a series of example strings, for the purpose of testing briefing colorization. This test includes hyphenated-words,
words in "quotation", as well as prefixes like Mr. and Dr. as well as pluralized forms such as portal's and Aquitane's, to
name a few. Additional "quoted" test. Some (TLA) and (TLA) tests. A GTVA. Test.

Another test on hyphenated-words, as a secondary example.
Additionally: hyphenated-words as a third example.

We are also going to test exclusions, with Alpha's and Beta's help.
We will also be testing ending words.

We also need to test, when as ending words: quoted "words".

Hyphenated-Words.

And pluralized word's.

This comes very close to almost doing it. I like that it clips the , that breaks up sentences, but it totally destroys hyphenated and quoted words and doesn't react well to Dr. or Mr. at all, even though it get's the very last word as well as words at the line wrap. Do note however that it handles the third case of hyphenated words example correctly, as well as the additional "quoted" test

Here is an example of what we are going to be aiming for:

Briefing Color Test

This is a series of example strings, for the purpose of testing briefing colorization. This test includes hyphenated-words,
words in "quotation", as well as prefixes like Mr. and Dr. as well as pluralized forms such as portal's and Aquitane's, to
name a few. Additional "quoted" test. Some (TLA) and (TLA) tests. A GTVA. Test.

Another test on hyphenated-words, as a secondary example.
Additionally: hyphenated-words as a third example.

We are also going to test exclusions, with Alpha's and Beta's help.
We will also be testing ending words.

We also need to test, when as ending words: quoted "words".

Hyphenated-Words.

And pluralized word's.

Note that while "Additionally:" is all yellow, "words:" only has "words" in yellow. And we appear to be getting schizoid with our colorization of 's.

How are we going to achieve when it does or does not manage to color a . (period) ,(comma) '(single quote)? Well, the , character could and should probably be ignored, which the current patch does. And if the .(period) is the very last item before a return it should probably be ignored, but we don't want it ignoring all other period cases that are not newlines unless we tell it to do so.

Introducing the "Non-Breaking Space" character, "$|" idea.
With which we can turn the "($p TLA)" into "($p TLA$|)" without worrying about breaking the first "$p (TLA)" case by having it exclude ( or ).
We can keep "$ Dr." and "$y Mr." coloring the period without coloring it in "$g GTVA$|. Test."
We can also use it on "$y words$|:" without breaking being able to have "$y Additionally:" colored.
It will also allow us to keep "$b portal's" and "$g Aquitaine's" but show "$Alpha$|'s" and "$g Beta$|'s" without the 's being colored.

Watch this space for either an upcoming patch, or a call for assistance.

[Zacam]

Code: [Select]

$Stage Text:
 XSTR("Briefing Color Test
This is a series of example $V strings, for the purpose of testing briefing colorization. This test includes $b hyphenated-words, words in $g $quotequotation$quote, as well as prefixes like $y Mr. and $y Dr. as well as pluralized forms such as $b portal's and $f Aquitane's, to name a few. Additional $quote$g quoted$|$quote test. Some $p (TLA) and ($p TLA$|) tests. A $g GTVA$|. Test.

Another test on $b hyphenated-words, as a secondary example.
$y Additionally: $b hyphenated$|-$g words as a third example.

We are also going to test $r exclusions, with $g Alpha$|'s and $g Beta$|'s help.
We will also be testing ending $r words$|.

We also need to test, when as ending $y words$|: quoted $g $quotewords$quote$|.

$b Hyphenated-Words$|.

And pluralized $r word's$|.
$end_multi_text
$Ani Filename: <NONE>
+Wave Filename: <NONE>

So, with the above Command Briefing, we have success. Sort of. Here are the actual results:

Briefing Color Test

This is a series of example strings, for the purpose of testing briefing colorization. This test includes hyphenated-words,
words in "quotation", as well as prefixes like Mr. and Dr. as well as pluralized forms such as portal's and Aquitane's, to
name a few. Additional "quoted" test. Some (TLA) and (TLA) tests. A GTVA. Test.

Another test on hyphenated-words, as a secondary example.
Additionally: hyphenated-words as a third example.

We are also going to test exclusions, with Alpha's and Beta's help.
We will also be testing ending words.
We also need to test, when as ending words: quoted "words".
Hyphenated-Words.
And pluralized word's.

Note how it chewed up the newline/returns, but only on "$|." where it was before a newline/return? See that "$g GTVA$|. Test." rendered just fine?

Due to the newline/return issue (and banging my head against it for 5 hours with Goober and #SCP IRC folks), it's determined that any case where you wish to non-colorize a period that occurs before a newline/return, you must place a space on either side (but NOT both) of the period.

In all other cases, "$|" does not require (but can have if you so wish) a space prior to the character being "escaped". Again, this ONLY affects when the next item after the "escaped" is a newline/return, which is most common with multi-line Briefings where a colorized word occurs before a period that you wish to not have colorized.

Here is a working Command Brief that shows the previous "Desired" result (additionally linked to as a mission file):

Code: [Select]

#Command Briefing

$Stage Text:
 XSTR("Briefing Color Test

This is a series of example $V strings, for the purpose of testing briefing colorization. This test includes $b hyphenated-words, words in $g $quotequotation$quote, as well as prefixes like $y Mr. and $y Dr. as well as pluralized forms such as $b portal's and $f Aquitane's, to name a few. Additional $quote$g quoted$|$quote test. Some $p (TLA) and ($p TLA$|) tests. A $g GTVA$|. Test.

Another test on $b hyphenated-words, as a secondary example.
$y Additionally: $b hyphenated$|-$g words as a third example.

We are also going to test $r exclusions, with $g Alpha$|'s and $g Beta$|'s help.
We will also be testing ending $r words$| .

We also need to test, when as ending $y words$|: quoted $g $quotewords$quote$| .

$b Hyphenated-Words$| .

And pluralized $r word's$|.", -1)
$end_multi_text
$Ani Filename: <NONE>
+Wave Filename: <NONE>

I also ran this patch against Retail Missions. It incurred no breaking of the presentation aside from 2 factors:
Where a "word," was colorized and the ',' was also colored, the ',' is no longer colored.
Where a word was at the end of the XSTR or edge of briefing display and did not show that it was colored, it is now colored.

I detected a minor memory access out of allocated memory, that can be fixed.

Can you check the current implementation and tell me if that is still the case or provide that fix so we can implement this patch into Trunk then?
M1992_01022010.patch
Void.fs2

Requires building against Trunk 5758 or later.

[pedro]

Zacam,

I checked your patch. Thanks a lot for your test case. I have a few questions.

Note how it chewed up the newline/returns, but only on "$|." where it was before a newline/return? See that "$g GTVA$|. Test." rendered just fine?

Due to the newline/return issue (and banging my head against it for 5 hours with Goober and #SCP IRC folks), it's determined that any case where you wish to non-colorize a period that occurs before a newline/return, you must place a space on either side (but NOT both) of the period.

The newline/return issue does not occur any more with the patch I submitted, does it ?

I noticed a mission briefing like STR(" something $") will cause a warning message because the final '\0' character is sent to brief_return_color_index function. Maybe could we check the remaining string size before to identify "$" as a text coloring character.

The out of memory read does no longer exists in your patch. It was part of a check that has been removed. By the way, it is no longer necessary to add a space after the '$'+code sequence, which is a minor behavior change. Is it intended ?

Finally I wonder why $| cannot be replaced by $w in briefing definition. Does $| introduce an new behavior ?

[Zacam]

PM quote here:

I merged your patch with my local tree, and traced through the functions.  I figured out that the problem isn't in the coloration function, but rather in split_str in parselo.  Specifically, parselo will set an ignore_until_whitespace flag when it finds an ignore character (in this case, the $), so it thinks the end-of-splitted-string marker is "$|.\n" instead of "$|".  I'm not sure what to do there, so I'm going to think about it.  You ought to do the same.

This should explain a bit better what the newline issue is. As far as I can determine from reading that, the problem is not with the briefing colorization code at all, as it also affects the original code. As for '\0', I suppose the question is, is it really necessary?

Ideally, a malformed XSTR should be fixed and never be in a final product, so trying to make an exception handling case for that would be a lot of unnecessary effort IMO. If anybody wants or needs to use '$' as a standard character in a briefing (or debrief or Fiction Viewer) we could probably come up with something that would prevent it from being an issue in terms of colorization. But in terms of checking string size, that could work for colorization, but we don't want an accidental "$Something" turning into a case of "$S omething" and we don't want it breaking "$quote" or "$semicolon" so doing case and literal matches would be best.

And yes, the space for handling '$c' was made optional for two reasons: Reduction of casual typo's causing errors and to more explicitly handle the new case "$|" and have it more easily inline with making it a more usable "Color Escape" code. And I introduced "$|" rather than re-using (or directing folks into being able to use) "$w" for the ability of future implementation: to to allow that the default Briefing Text may not actually be white. The idea is (especially for the Fiction Viewer, but also for Brief/DeBrief) is to allow for "Block Quote" colorization (probably through use of '$<' and '$>' as an example) that will still allow for '$c' to make the text in the block a different color and '$|' to indicate to return to the block quote color.

However, it is still by and large recommended that folks continue using the by-now standard practice of colorization by using "$g Alpha" but at least now if there is a stuck spacebar out there, it won't break things. And at the current moment, '$|' only requires a space when dealing with a newline/return which is currently being looked at, but frankly, while it is annoying and I'd rather not inconvenience people with too many "methods" for how to use this, I am happy with the usability of the feature.

Thanks for the testing and the questions, if I haven't answered to your satisfaction or if anything is still not clear, let me know. It's still under going review at the moment as I guess the idea is to try and resolve the newline handling issue before committing it and I'll admit that I'm at a bit of a loss on that one.

[pedro]

But in terms of checking string size, that could work for colorization, but we don't want an accidental "$Something" turning into a case of "$S omething" and we don't want it breaking "$quote" or "$semicolon" so doing case and literal matches would be best.

I think my post was not so clear about the warning. It only occurs only for a string terminated by "$" character. (To avoid the case just check the current position against the string length) . Given that you consider the string to be invalid, it is not a bug.
By the way, $quote and $semicolon are replaced far before the briefing coloring functions, so it is not possible to beak it.

It's still under going review at the moment as I guess the idea is to try and resolve the newline handling issue before committing it and I'll admit that I'm at a bit of a loss on that one.

Can you tell me more about the remaining newline issues ? I supposed it to be resolved, and I would be glad to help.

[Zacam]

See my PM quote in the above from Goober.

Essentially, the behavior is replicated in the message "Reply 28" above, where if "$|." is at the end of a line (there is a newline/return) it essentially eats the newline/return.

Reply #28 has two code snippets. The top one will example in the quote below it the truncation/ignoring/deletion of the newline/return while the code section below that shows what is to be done to prevent it. But it is not an entirely optimal solution, as it may incur some (modder) frustration of mixing how '$|' is used (and the desired method being that no spaces are necessary).

[Goober5000]

There's also an off-by-one error, which I forgot to mention in the PM.  It manifests in what pedro said in the case of a $ being at the end of the string.

Here is my local code.  It's slightly different from Zacam's patch; in addition to the off-by-one fix, it also drops the is_word_separator function:

Code: [Select]

int brief_text_colorize(char *src, int instance)
{
Assert(src);
Assert( 0<=instance && instance < (sizeof(Colored_stream)/sizeof(*Colored_stream)) );

briefing_line dest_line; //the resulting vector of colored character
ubyte active_color_index = BRIEF_TEXT_WHITE; //the current drawing color

int src_len = strlen(src);
for (int i=0; i<src_len; i++) {
// Is the character a color markup?
// text markup consists of a '$' plus a character plus an optional space
if ( (i < src_len - 1)  && (src[i] == BRIEF_META_CHAR) ) {
i++; // Consume the $ character
active_color_index = brief_return_color_index(src[i]);
i++; // Consume the color identifier and focus on the white character (if any)
 
// Skip every whitespace until the next word is reached
while ( (i < src_len) && is_white_space(src[i]) )
i++;

// The next character is not a whitespace, let's process it as usual
// (subtract 1 because the for loop will add it again)
i--;
continue;
  }

// When the word is terminated reset color to white
if (is_white_space(src[i])) {
active_color_index = BRIEF_TEXT_WHITE;
}

// Append the character to the result structure
colored_char dest;
dest.letter = src[i];
dest.color = active_color_index;
dest_line.push_back(dest);
}
Colored_stream[instance].push_back(dest_line);
return dest_line.size();
}

[Aardwolf]

Would it not be possible to have it turn on a color until it gets to something indicating it should go back to the normal color?

I don't know what notation something like that might use, but... the current system seems kind of like you're trying to predict every possible 'correct' context for colorization, instead of just letting the user specify "start color here" / "end color here" stuff.

[Zacam]

...

Hence the concept for '$<' and '$>' (or some other general equiv that can be made to work in that system). But as it currently stands, the '$c' code says "I'll color until I hit a whitespace". Then I added '$|' that says "You can stop before hitting the next character, even though it's one you would normally color, because I don't want you to color it.".

[Aardwolf]

Coooooool.