ToME 2 maintenance

Everything about ToME 2.x.x. No spoilers, please

Moderator: Moderator

Message
Author
AnonymousHero
Spiderkin
Posts: 482
Joined: Sat Mar 18, 2006 12:48 pm

Re: ToME 2 maintenance

#271 Post by AnonymousHero »

Lord Estraven wrote:Hmm... Not at the moment; I don't have a dedicated OpenBSD machine, and it can't run on VirtualBox. I'll see when I can get around to it though.

Although - this was in master, with the broken automizer and all... I thought you weren't working on master any more?
How broken is the automatizer? There are a couple of semi-recent fixes to it in 'master'. (I don't mind applying patches to master, FWIW. I'm just not going to do any releases.)

Anyway, I'm pretty sure the problem would still exist in the "cpp" branch.

Lord Estraven
Uruivellas
Posts: 718
Joined: Tue Dec 13, 2005 12:35 am

Re: ToME 2 maintenance

#272 Post by Lord Estraven »

The automizer is quite broken. Hitting Esc or entering incorrect characters in various places will immediately crash the game.

AnonymousHero
Spiderkin
Posts: 482
Joined: Sat Mar 18, 2006 12:48 pm

Re: ToME 2 maintenance

#273 Post by AnonymousHero »

Oh, dear :(. I'll certainly accept patches that'll make it... not do that kind of thing.

(I think I'm on record as stating that such a thing as the Automatizer shouldn't be needed in an ideal situation, but clearly it is in practice. :/) FWIW, I think "squelch" in Vanilla works much better UX-wise.

Lord Estraven
Uruivellas
Posts: 718
Joined: Tue Dec 13, 2005 12:35 am

Re: ToME 2 maintenance

#274 Post by Lord Estraven »

Re patches: wish I knew what the deal was. All I've seen is that assert() statements are failing, and I don't know why. OTOH it is Saturday so I might as well take a look. :)

As far as as squelch, yeah, something like V's system would be ideal. Heck, even a regex-based system would be preferable, IMO.

Edit: actually, I think I've realized one of the implicit problems with the automizer. I'm going to start another thread about that.

Edit 2: the automizer wants to be a scripting language.

Actually this might be something I could try to implement, using embedded Python or such.

Lord Estraven
Uruivellas
Posts: 718
Joined: Tue Dec 13, 2005 12:35 am

Re: ToME 2 maintenance

#275 Post by Lord Estraven »

In any case, an example backtrace, from entering Esc when choosing an ASCII item symbol for a rule

Code: Select all

tome: /home/proteus/Projects/tome2/src/squeltch.c:582: condition_and_add: Assertion `c != ((void *)0)' failed.

Program received signal SIGABRT, Aborted.
0xf70cc9dc in raise () from /lib/libc.so.6
(gdb) bt
#0  0xf70cc9dc in raise () from /lib/libc.so.6
#1  0xf70ce1f3 in abort () from /lib/libc.so.6
#2  0xf70c5ac6 in __assert_fail_base () from /lib/libc.so.6
#3  0xf70c5b77 in __assert_fail () from /lib/libc.so.6
#4  0x080d3064 in condition_and_add ()
#5  0x080d30eb in condition_or_add ()
#6  0x080d529f in add_child ()
#7  0x080d6630 in add_new_condition ()
#8  0x080d6ee9 in do_cmd_automatizer ()
#9  0x0816fd06 in do_cmd_options ()
#10 0x0809d8f2 in process_command ()
#11 0x0809e15e in process_player ()
#12 0x0809ecd5 in dungeon ()
#13 0x0809f6f2 in play_game ()
#14 0x081df6a0 in main ()

Lord Estraven
Uruivellas
Posts: 718
Joined: Tue Dec 13, 2005 12:35 am

Re: ToME 2 maintenance

#276 Post by Lord Estraven »

Ah... I think I got it. The create_condition_* functions, for the metadata->create_condition() function pointer, all return null for a zero length string. Pressing Esc for the symbol probably generates that, passing null to the next function.

Lord Estraven
Uruivellas
Posts: 718
Joined: Tue Dec 13, 2005 12:35 am

Re: ToME 2 maintenance

#277 Post by Lord Estraven »

So in this case

Code: Select all


static condition_type *create_condition_symbol()
{
	char c;
	cptr s = lua_input_box("Symbol to match?", 1);
	if (sscanf(s, "%c", &c) < 1)
	{
		return NULL;
	}

	return condition_new_symbol(c);
}
sscanf is unsuccessful, so this function returns null. The null gets passed to condition_and_add, which promptly chokes on it.

Edit: what's weird, is that hitting Esc once seems to cause sscanf to fail ad infinitum. No idea why that is.

Edit 2: maybe the problem is how lua_input_box parses Esc...

Lord Estraven
Uruivellas
Posts: 718
Joined: Tue Dec 13, 2005 12:35 am

Re: ToME 2 maintenance

#278 Post by Lord Estraven »

Aaand bingo.

Code: Select all

char *lua_input_box(cptr title, int max)
{
	static char buf[80];
	int wid, hgt;

	strcpy(buf, "");
	Term_get_size(&wid, &hgt);
	if (!input_box(title, hgt / 2, wid / 2, buf, (max > 79) ? 79 : max))
		return "";
	return buf;
}

Code: Select all


/*
 * Creates an input box
 */
bool_ input_box(cptr text, int y, int x, char *buf, int max)
{
	int smax = strlen(text);

	if (max > smax) smax = max;
	smax++;

	draw_box(y - 1, x - (smax / 2), 3, smax);
	c_put_str(TERM_WHITE, text, y, x - (strlen(text) / 2));

	Term_gotoxy(x - (smax / 2) + 1, y + 1);
	return askfor_aux(buf, max);
}

Code: Select all

/*
* Get some input at the cursor location.
* Assume the buffer is initialized to a default string.
* Note that this string is often "empty" (see below).
* The default buffer is displayed in yellow until cleared.
* Pressing RETURN right away accepts the default entry.
* Normal chars clear the default and append the char.
* Backspace clears the default or deletes the final char.
* ESCAPE clears the buffer and the window and returns FALSE.
* RETURN accepts the current buffer contents and returns TRUE.
*/
bool_ askfor_aux_complete = FALSE;
bool_ askfor_aux(char *buf, int len)
{
	int y, x;

	int i = 0;

	int k = 0;

        int wid, hgt;

	bool_ done = FALSE;


	/* Locate the cursor */
	Term_locate(&x, &y);

        /* Get terminal size */
        Term_get_size(&wid, &hgt);

	/* Paranoia -- check column */
	if ((x < 0) || (x >= wid)) x = 0;

	/* Restrict the length */
	if (x + len > wid) len = wid - x;


	/* Paranoia -- Clip the default entry */
	buf[len] = '\0';


	/* Display the default answer */
	Term_erase(x, y, len);
	Term_putstr(x, y, -1, TERM_YELLOW, buf);

	if (askfor_aux_complete)
	{
		screen_save();
		complete_where = 0;
		strncpy(complete_buf, buf, 100);
	}

	/* Process input */
	while (!done)
	{
		/* Place cursor */
		Term_gotoxy(x + k, y);

		/* Get a key */
		i = inkey();

		/* Analyze the key */
		switch (i)
		{
		case ESCAPE:
			k = 0;
			done = TRUE;
			break;

		case '\n':
		case '\r':
			k = strlen(buf);
			done = TRUE;
			break;

		case '\t':
			if (askfor_aux_complete && k)
			{
				screen_load();
				screen_save();
				k = complete_command(buf, k, len);
			}
			else
			{
				bell();
			}

		case 0x7F:
		case '\010':
			if (k > 0) k--;
			strncpy(complete_buf, buf, k);
			break;

		default:
			if ((k < len) && (isprint(i)))
			{
				buf[k++] = i;
				strncpy(complete_buf, buf, k);
			}
			else
			{
				bell();
			}
			break;
		}

		/* Terminate */
		buf[k] = '\0';

		/* Update the entry */
		Term_erase(x, y, len);
		Term_putstr(x, y, -1, TERM_WHITE, buf);
	}

	if (askfor_aux_complete)
	{
		screen_load();
	}

	/* Aborted */
	if (i == ESCAPE) return (FALSE);

	/* Success */
	return (TRUE);
}
Esc makes askfor_aux() return FALSE.
That makes input_box() fail.
And that makes lua_input_box() return an empty string, which causes the chain of events leading to the crash.

Hurray.

Lord Estraven
Uruivellas
Posts: 718
Joined: Tue Dec 13, 2005 12:35 am

Re: ToME 2 maintenance

#279 Post by Lord Estraven »

... Actually, looking at the automizer in the cpp branch, the difference is that nothing there returns NULL. Ever. At all. Which makes a lot more sense, because doing so at this point amounts to deliberately crashing the game on failure. The correct behavior should really be to abort the input box, not crash the game.

Edit: not that it's so easy to do so. This game's code is all over the place.

Lord Estraven
Uruivellas
Posts: 718
Joined: Tue Dec 13, 2005 12:35 am

Re: ToME 2 maintenance

#280 Post by Lord Estraven »

Wait, I might actually have the above completely wrong. Somehow, getting a NULL doesn't crash things with e.g. item status...

Edit: nope, N/M. I didn't see the crash because I wasn't embedding in an AND/OR rule. Returning NULL from within anything nested results in an immediate crash, as expected.

Lord Estraven
Uruivellas
Posts: 718
Joined: Tue Dec 13, 2005 12:35 am

Re: ToME 2 maintenance

#281 Post by Lord Estraven »

Okay - mitigated for now by allowing an empty or default rule to be stuck in, instead of returning NULL. This is ugly, but IMO preferable to crashing the game with an assert() failure.

Edit: I'll create a merge request for this on gitlab.

Lord Estraven
Uruivellas
Posts: 718
Joined: Tue Dec 13, 2005 12:35 am

Re: ToME 2 maintenance

#282 Post by Lord Estraven »

So, trying to use a NOT (negative) rule also results in an immediate crash. Same assertion failure error. Looking into that too.

Lord Estraven
Uruivellas
Posts: 718
Joined: Tue Dec 13, 2005 12:35 am

Re: ToME 2 maintenance

#283 Post by Lord Estraven »

Wow, sorry it took me 20 days to get back to this...

Anyway I've found another nice issue, where Return at the automizer's "add a rule" prompt gets caught by Linux's stack-smashing protection. (That would be the create_new_rule() function.)

I'm thinking maybe I should go over the squelch code with a static analyzer, or something...

Lord Estraven
Uruivellas
Posts: 718
Joined: Tue Dec 13, 2005 12:35 am

Re: ToME 2 maintenance

#284 Post by Lord Estraven »

Code: Select all

int compare_condition_list(condition_list *a, condition_list *b)
{
        assert(FALSE);
}
Huh? I don't know yet what that function is supposed to do, but "deliberately crash the program no matter what" probably isn't it. :)

Edit: actually N/M, looks like it's something deprecated.
Last edited by Lord Estraven on Sat Oct 24, 2015 6:16 pm, edited 1 time in total.

AnonymousHero
Spiderkin
Posts: 482
Joined: Sat Mar 18, 2006 12:48 pm

Re: ToME 2 maintenance

#285 Post by AnonymousHero »

Lord Estraven wrote:

Code: Select all

int compare_condition_list(condition_list *a, condition_list *b)
{
        assert(FALSE);
}
Huh? I don't know yet what that function is supposed to do, but "deliberately crash the program no matter what" probably isn't it. :)
Hmm... that name doesn't show up at all in my source tree...?

Post Reply