Skip to content
This repository has been archived by the owner on May 6, 2024. It is now read-only.

Fix black color handling #225

Merged
merged 4 commits into from
Jul 22, 2021
Merged

Fix black color handling #225

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
147520f
Fix read_tty.py.
Jul 22, 2021
8d0f1ef
Handle black items correctly.
Jul 22, 2021
c7993ba
Simplify test_nethack.py::TestNethackTerminalObservation::test_crop.
Jul 22, 2021
2aef191
Simplify color logic in nle.c.
Jul 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion nle/scripts/read_tty.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def color(s, value):
channel = "->"

data = re.sub(
r"\\x1b\[([0-9];?)*.", lambda m: (color(m.group(0), 8), str(data))
r"\\x1b\[([0-9];?)*.", lambda m: color(m.group(0), 8), str(data)
)
data = re.sub(
r"(\\x1b\(0)(.*?)(\\x1b\(B)",
Expand Down
13 changes: 3 additions & 10 deletions nle/tests/test_nethack.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -436,16 +436,9 @@ def test_observations(self, game):
def test_crop(self, game):
tty_chars, tty_colors, _, chars, colors = game.reset()

g_chars = chars.reshape(-1)
g_cols = colors.reshape(-1)

# DUNGEON is [21, 79], TTY is [24, 80]. Crop as follows to get alignment.
t_chars = tty_chars[1:-2, :-1].reshape(-1)
t_cols = tty_colors[1:-2, :-1].reshape(-1)

for g_char, g_col, t_char, t_col in zip(g_chars, g_cols, t_chars, t_cols):
assert g_char == t_char
assert g_col == t_col
# DUNGEON is [21, 79], TTY is [24, 80]. Crop to get alignment.
np.testing.assert_array_equal(chars, tty_chars[1:-2, :-1])
np.testing.assert_array_equal(colors, tty_colors[1:-2, :-1])


class TestNethackMiscObservation:
Expand Down
66 changes: 24 additions & 42 deletions src/nle.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@
#include <bzlib.h>
#endif

#define STACK_SIZE (1 << 15) // 32KiB
#define STACK_SIZE (1 << 15) /* 32KiB */

#ifndef __has_feature
#define __has_feature(x) 0 // Compatibility with non-clang compilers.
#define __has_feature(x) 0 /* Compatibility with non-clang compilers. */
#endif

#if __has_feature(address_sanitizer) || defined(__SANITIZE_ADDRESS__)
Expand All @@ -36,47 +36,29 @@ vt_char_color_extract(TMTCHAR *c)
{
/* We pick out the colors in the enum tmt_color_t. These match the order
* found standard in IBM color graphics, and are the same order as those
* found in src/color.h. We take the values from color.h, and choose
* default to be bright black (NO_COLOR) as nethack does.
*
* Finally we indicate whether the color is reverse, by indicating the
* sign
* of the final integer.
*/
signed char color = 0;
switch (c->a.fg) {
case (TMT_COLOR_DEFAULT):
color =
(c->c == 32) ? CLR_BLACK : CLR_GRAY; // ' ' is BLACK else WHITE
break;
case (TMT_COLOR_BLACK):
color = (c->a.bold) ? NO_COLOR : CLR_BLACK; // c = 8:0
break;
case (TMT_COLOR_RED):
color = (c->a.bold) ? CLR_ORANGE : CLR_RED; // c = 9:1
break;
case (TMT_COLOR_GREEN):
color = (c->a.bold) ? CLR_BRIGHT_GREEN : CLR_GREEN; // c = 10:2
break;
case (TMT_COLOR_YELLOW):
color = (c->a.bold) ? CLR_YELLOW : CLR_BROWN; // c = 11:3
break;
case (TMT_COLOR_BLUE):
color = (c->a.bold) ? CLR_BRIGHT_BLUE : CLR_BLUE; // c = 12:4
break;
case (TMT_COLOR_MAGENTA):
color = (c->a.bold) ? CLR_BRIGHT_MAGENTA : CLR_MAGENTA; // c = 13:5
break;
case (TMT_COLOR_CYAN):
color = (c->a.bold) ? CLR_BRIGHT_CYAN : CLR_CYAN; // c = 14:6
break;
case (TMT_COLOR_WHITE):
color = (c->a.bold) ? CLR_WHITE : CLR_GRAY; // c = 15:7
break;
case (TMT_COLOR_MAX):
break;
* found in src/color.h. */

/* TODO: We no longer need *signed* chars. Let's change the dtype of
* tty_chars when we change the API next. */

signed char color;

if (c->a.fg == TMT_COLOR_DEFAULT) {
/* Need to make a choice for default color. To stay compatible with
NetHack, choose black for the "null glyph", gray otherwise. */
color = (c->c == ' ') ? CLR_BLACK : CLR_GRAY; /* 0 or 7 */
} else if (c->a.fg < TMT_COLOR_MAX) {
color = c->a.fg - TMT_COLOR_BLACK + CLR_BLACK; /* TMT color offset. */
if (c->a.bold) {
color |= BRIGHT;
}
} else {
fprintf(stderr, "Illegal color %d\n", (int) c->a.fg);
color = CLR_GRAY;
}

/* The above is 0..15. For "reverse" colors (bg/fg swap), let's
* use 16..31. */
if (c->a.reverse) {
color += CLR_MAX;
}
Expand Down Expand Up @@ -124,7 +106,7 @@ nle_vt_callback(tmt_msg_t m, TMT *vt, const void *a, void *p)

case TMT_MSG_MOVED:
if (nle->observation->tty_cursor) {
// cast from size_t is safe from overflow, since r,c < 256
/* cast from size_t is safe from overflow, since r,c < 256 */
nle->observation->tty_cursor[0] = (unsigned char) cur->r;
nle->observation->tty_cursor[1] = (unsigned char) cur->c;
}
Expand Down
5 changes: 5 additions & 0 deletions win/rl/winrl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -930,6 +930,11 @@ NetHackRL::rl_print_glyph(winid wid, XCHAR_P x, XCHAR_P y, int glyph,
// No win_proc_calls entry here.
if (wid == WIN_MAP) {
instance->store_glyph(x, y, glyph);
if (glyph != nul_glyph && color == CLR_BLACK) {
/* This will be 'bright black' (or blue) on tty so we change it to
* make NLE's colors and tty_colors stay compatible. */
color = iflags.wc2_darkgray ? 8 : CLR_BLUE;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or CLR_BLUE?!?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will read the termcap fix

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NetHack has a configuration option on whether it uses "bright black" (\033[1;30m) or (dark) blue to display black entries: https://github.com/facebookresearch/nle/blob/master/win/tty/termcap.c#L887

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For clarity, here's a Python version of what this code here did before this change:

CLR_BLACK = 0
CLR_RED = 1
CLR_GREEN = 2
CLR_BROWN = 3  # /* on IBM, low-intensity yellow is brown */
CLR_BLUE = 4
CLR_MAGENTA = 5
CLR_CYAN = 6
CLR_GRAY = 7  # /* low-intensity white */
NO_COLOR = 8
CLR_ORANGE = 9
CLR_BRIGHT_GREEN = 10
CLR_YELLOW = 11
CLR_BRIGHT_BLUE = 12
CLR_BRIGHT_MAGENTA = 13
CLR_BRIGHT_CYAN = 14
CLR_WHITE = 15
CLR_MAX = 16

BRIGHT = 8

hilites = [None] * 16

hilites[CLR_GRAY] = ""
hilites[NO_COLOR] = hilites[CLR_GRAY]

for c in range(CLR_MAX // 2):
    if c == CLR_BLACK:
        continue
    hilites[c | BRIGHT] = "\033[1;3%dm" % c
    if c == CLR_GRAY:
        continue
    hilites[c] = "\033[0;3%dm" % c

print(hilites)

}
instance->store_mapped_glyph(ch, color, special, x, y);
if (nle_get_obs()->screen_descriptions) {
instance->store_screen_description(x, y, glyph);
Expand Down
17 changes: 16 additions & 1 deletion win/tty/termcap.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1145,6 +1145,16 @@ init_hilite()
Sprintf(hilites[c], "\033[0;3%dm", c);
}
}

/* See TEXTCOLOR && TERMLIB && UNIX && TERMINFO code above. */
if (iflags.wc2_darkgray) {
/* Bright black is dark gray. */
hilites[CLR_BLACK] = (char *) alloc(sizeof "\033[1;30m");
Sprintf(hilites[CLR_BLACK], "\033[1;30m");
} else {
/* Use blue for black. */
hilites[CLR_BLACK] = hilites[CLR_BLUE];
}
}

static void
Expand All @@ -1166,7 +1176,12 @@ kill_hilite()
if (hilites[c | BRIGHT] && hilites[c | BRIGHT] != nh_HI)
free((genericptr_t) hilites[c | BRIGHT]), hilites[c | BRIGHT] = 0;
}
return;

if (hilites[CLR_BLACK]) {
heiner marked this conversation as resolved.
Show resolved Hide resolved
if (hilites[CLR_BLACK] != hilites[CLR_BLUE])
free(hilites[CLR_BLACK]);
hilites[CLR_BLACK] = 0;
}
}
#endif /* TEXTCOLOR && !TERMLIB && ANSI_DEFAULT */

Expand Down