ToME: the Tales of Maj'Eyal

Everything about ToME
It is currently Sun Aug 19, 2018 7:21 pm

All times are UTC




Post new topic Reply to topic  [ 326 posts ]  Go to page Previous  1 ... 16, 17, 18, 19, 20, 21, 22  Next
Author Message
 Post subject: Re: ToME 2 maintenance
PostPosted: Fri Oct 02, 2015 5:21 am 
Offline
Spiderkin

Joined: Sat Mar 18, 2006 12:48 pm
Posts: 482
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.


Top
 Profile  
 
 Post subject: Re: ToME 2 maintenance
PostPosted: Fri Oct 02, 2015 11:55 pm 
Offline
Uruivellas

Joined: Tue Dec 13, 2005 12:35 am
Posts: 704
The automizer is quite broken. Hitting Esc or entering incorrect characters in various places will immediately crash the game.


Top
 Profile  
 
 Post subject: Re: ToME 2 maintenance
PostPosted: Sat Oct 03, 2015 9:12 am 
Offline
Spiderkin

Joined: Sat Mar 18, 2006 12:48 pm
Posts: 482
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.


Top
 Profile  
 
 Post subject: Re: ToME 2 maintenance
PostPosted: Sat Oct 03, 2015 3:34 pm 
Offline
Uruivellas

Joined: Tue Dec 13, 2005 12:35 am
Posts: 704
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.


Top
 Profile  
 
 Post subject: Re: ToME 2 maintenance
PostPosted: Sat Oct 03, 2015 4:16 pm 
Offline
Uruivellas

Joined: Tue Dec 13, 2005 12:35 am
Posts: 704
In any case, an example backtrace, from entering Esc when choosing an ASCII item symbol for a rule

Code:
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 ()


Top
 Profile  
 
 Post subject: Re: ToME 2 maintenance
PostPosted: Sat Oct 03, 2015 4:20 pm 
Offline
Uruivellas

Joined: Tue Dec 13, 2005 12:35 am
Posts: 704
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.


Top
 Profile  
 
 Post subject: Re: ToME 2 maintenance
PostPosted: Sat Oct 03, 2015 4:25 pm 
Offline
Uruivellas

Joined: Tue Dec 13, 2005 12:35 am
Posts: 704
So in this case

Code:

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...


Top
 Profile  
 
 Post subject: Re: ToME 2 maintenance
PostPosted: Sat Oct 03, 2015 4:43 pm 
Offline
Uruivellas

Joined: Tue Dec 13, 2005 12:35 am
Posts: 704
Aaand bingo.

Code:
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:

/*
 * 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:
/*
* 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.


Top
 Profile  
 
 Post subject: Re: ToME 2 maintenance
PostPosted: Sat Oct 03, 2015 4:48 pm 
Offline
Uruivellas

Joined: Tue Dec 13, 2005 12:35 am
Posts: 704
... 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.


Top
 Profile  
 
 Post subject: Re: ToME 2 maintenance
PostPosted: Sat Oct 03, 2015 4:55 pm 
Offline
Uruivellas

Joined: Tue Dec 13, 2005 12:35 am
Posts: 704
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.


Top
 Profile  
 
 Post subject: Re: ToME 2 maintenance
PostPosted: Sat Oct 03, 2015 5:04 pm 
Offline
Uruivellas

Joined: Tue Dec 13, 2005 12:35 am
Posts: 704
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.


Top
 Profile  
 
 Post subject: Re: ToME 2 maintenance
PostPosted: Sun Oct 04, 2015 5:55 pm 
Offline
Uruivellas

Joined: Tue Dec 13, 2005 12:35 am
Posts: 704
So, trying to use a NOT (negative) rule also results in an immediate crash. Same assertion failure error. Looking into that too.


Top
 Profile  
 
 Post subject: Re: ToME 2 maintenance
PostPosted: Sat Oct 24, 2015 5:44 pm 
Offline
Uruivellas

Joined: Tue Dec 13, 2005 12:35 am
Posts: 704
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...


Top
 Profile  
 
 Post subject: Re: ToME 2 maintenance
PostPosted: Sat Oct 24, 2015 5:54 pm 
Offline
Uruivellas

Joined: Tue Dec 13, 2005 12:35 am
Posts: 704
Code:
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.

Top
 Profile  
 
 Post subject: Re: ToME 2 maintenance
PostPosted: Sat Oct 24, 2015 5:56 pm 
Offline
Spiderkin

Joined: Sat Mar 18, 2006 12:48 pm
Posts: 482
Lord Estraven wrote:
Code:
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...?


Top
 Profile  
 
Display posts from previous:  Sort by  
Post new topic Reply to topic  [ 326 posts ]  Go to page Previous  1 ... 16, 17, 18, 19, 20, 21, 22  Next

All times are UTC


Who is online

Users browsing this forum: No registered users and 2 guests


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot post attachments in this forum

Search for:
Jump to:  
Powered by phpBB® Forum Software © phpBB Group