Re: [AD] GUI Proc Added: d_titlebox_proc

[Evert Glebbeek <eglebbk@xxxxxxxxxx>]

On Thursday 05 August 2004 18:26, Bobby Ferris wrote:
> I made a new gui proc, d_titlebox_proc, that draws the dialog box with a 
> titlebar.

Basically, if I understand correctly, this draws the text you pass in dp 
with a coloured background? Isn't this something that could be done with 
d_text_proc() and its relatives already?
I agree that having a titlebar object is useful, although I'd personally 
like it to have something like a close button that cancels the dialog box 
(if the D_EXIT flag in the titlebar properties is set, for instance). In 
other words, something more than a coloured bar of text at the top of the 
dialog box ;)

> I added docs for it in allegro._tx too. Attached is the patch 
> for guiproc.c, gui.h, and allegro._tx to add the new proc.

Ok, some remarks:

> if(msg == MSG_DRAW && d->dp)

I'd expect to get an empty title bar if I pass NULL as the titlebar text. 
In other words, I'd move the if (d->dp) inward before the textprintf_ex 
call.

>  titlebar color
> +   can be specified in d1 (or set it to 0 for the default), and title
> +   text color can be set in d2 (or set it to 0 for the default).

The dialog struct has fg and bg fields that should be used for passing 
dialog colours, and the default setting should be taken from the global 
gui_fg_color/gui_bg_color, I think. 
Using 0 as a flag to indicate the default colour is problematic too, 
because 0 is either a valid colour in the palette, or black - and you may 
actually want to use those. Allegro's convention is to pass -1 to indicate 
that the default colour (gui_fg_color/gui_bg_color) should be used, though 
this can again be problematic for 32 bit RGBA drawing (not an issue for 
the GUI, I think).

There are also some formatting issues with the code. This isn't a big 
problem as they can be dealt with when a patch is applied, but it saves 
time if you can (try to) follow them. In short,  use 3 space indents and 8 
space tabs (yes, this is horrible, I know). If blocks look like
   if (foo) {
      /* stuff */
   }
   else {
      /* stuff */
   }
For a list of details, refer to ahack._tx/ahack.html file in the Allegro 
documentation. It explains Allegro's sometimes weird (IMHO) source code 
layout and style. You can also use the indent.pro and the indent program 
to fix the formatting if you don't want to constantly bother with it.

Hope you're not getting demotivated by all these remarks, by the way :)

Evert