Window system : use callbacks

[GoogleCodeExporter]

While working on the new skin window, i got fed up of renumbering the 
switch cases in the event loops each time I removed a button.
It's unclean, it's not readable, it's annoying to maintain, it allow easy 
mistakes...

I tried to do a system with macros for autonumbering, but the problem is 
the button number is not known at compile time, only at runtime. So we'll 
have to change the way we keep track of them...
I would have liked to do something like Window_add_normal_button
(x,y,w,h,"text",active,B_TEXT) and then use 'case B_TEXT:' in the event 
loop.
This would be defined as a macro that actually would look like :

#define Window_add_normal_button(x,y,w,h,"text",active,__NAME) \
const short __NAME = NB_BUTTONS +1 ; \
#define NB_BUTTONS NB_BUTTONS + 1 \
fWindow_add_normal_button((x),(y),(w),(h),("text"),(active));

Or something similar (did not try if that does actually work).

What do you think about it ? It would allow us to insert and remove 
buttons where we want, and avoid the need of keeping the cases order in 
sync with the button declaration, and make them have more clear names.

Maybe we could also use a table of buttons and add them all at once. To me 
it seems easier to build the window as a table than a bunch of function 
calls. See what MUI do on amiga for example, it looks quite clean, but 
heavily uses macro to do the real work.
Well, I don't want a full MUI-over-SDL implementation, but I think there 
are some good ideas we could borrow there...

Original issue reported on code.google.com by pulkoma...@gmail.com on 17 Jun 2009 at 3:35

[GoogleCodeExporter]

You can use an enum (a local one), but then these IDs will be at 3 places: enum 
declaration (top of function), control creation, switch case.

If you need to, you CAN replace the switches by a series of if else if... I'm 
pretty 
sure you won't be fried by the lightning bolt of an angry god. 

In any case, I'm not sure if modifying ALL the control creations and tests now 
is 
going to be less work than current maintenance... It's not so easy to name all 
controls.

Original comment by yrizoud on 18 Jun 2009 at 11:41

[GoogleCodeExporter]

Well, the idea was to create new functions (or macros) to handle this 
autonumbering 
properly, but actually doing only that and calling the old ones. So we could 
migrate 
slowly to the new system as we change dialogs.

Original comment by pulkoma...@gmail.com on 18 Jun 2009 at 11:55

[GoogleCodeExporter]

If we allowed mix of declarations and code (breaks on gcc2), we could do 
directly:

const int B_OK = Window_set_normal_button
(91,129,78,14,"OK"    ,0,1,SDLK_RETURN).Number;
const int B_CANCEL = Window_set_normal_button
(7,129,78,14,"Cancel",0,1,KEY_ESC).Number; 
(....)
if (Key == B_OK)
...
else if (Key == B_CANCEL)
...

Original comment by yrizoud on 18 Jun 2009 at 12:05

[GoogleCodeExporter]

Yes, but that would be ->Number as the function return a pointer.
And it don't allow us to use switch/case anymore, as the value is not known at 
compile time. I'd like to do the numbering in a macro system, that would be 
more 
easily manageable. And a #define can be done anywhere in the code.

Original comment by pulkoma...@gmail.com on 18 Jun 2009 at 12:12

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

Original comment by pulkoma...@gmail.com on 15 Jan 2010 at 7:34

• Added labels: Milestone-NotPlanned

[GoogleCodeExporter]

I spent some more time thinking about it.
Here's another solution :

 * when creating a button, we would add one more parameter that would be a callback function. Likely something like int callback(void*)
 * the callbacks would be put in an array of functions, so the event loop switch/case would be replaced with callbacks[clicked_button](param)
 * The parameter would be a struct holding all the window state. This would replace all the variables declared in the function currently. The main advantage is that it is easy to copy the window state, so you can play in the window, press undo, and restore the saved state easily.
 * Each callback would return 0 (continue looping), or another value. This value would then be returned to the function that opened the window so it knows what to do. Did the user press cancel, ok, ... ?

Original comment by pulkoma...@gmail.com on 5 Aug 2010 at 5:35

• Changed title: Window system : use callbacks

[GoogleCodeExporter]

It would work, I can confirm it. The biggest problem is not very big, it's that 
for all the "context" data, it requires prefixing by "state->", which is as 
user-friendly as if C++ imposed an explicit "this->" before any member :/
(It's funny because for years I found C++ totally useless. The first case where 
I found C++ more suitable than C was... for coding a GUI! So I have several)

In my opinion, if you ever have to work on a dialog where you feel you'll spend 
less time or efforts by coding it event-driven, go for it.

I'm specifically thinking of Palette window : it's now 1700 lines, including 
several blocks that are mostly repeated 3 times for the R G and B sliders: I 
coulnd't refactor these into functions, because these functions would need like 
20 parameters each in order to "reach" all the controls of the window (an issue 
solved by the idea of passing a pointer to the state, and doing it for all 
handlers, for consistency).

We can prepare some naming conventions for when event-driven system is used, so 
the specific elements (state structs, handler functions) can be easily 
understood as such.

This said, I think it's a bad idea to even *think* of rewriting all window 
handlers accordingly. I see it as a big work for no immediate user gain with 
high risk of error, and remember that we get a result that is not as pretty as 
any C++ system - it can feel very disappointing.

One specific warning: Since working in Grafx2, I've moved a lot of behavioral 
code from screens to the widgets themselves, defining for example an 
auto-repeat button and a list selector linked with its vertical slider. When 
you look at the code of a window that uses these, it looks very clean and 
trivial to make an event handler.
But all other controls are more tricky, many force a specific UI behavior for 
each screen, waiting for mouse release sometimes, handling dragging, handling 
the entry in a text field etc. The keywords that can indicate a specific window 
behavior are: Window_set_special_button(), SDL_Delay() when used with 
Get_input(), Wait_click_in_palette(), Wait_end_of_click(), all variants of 
Readline(), Get_color_behind_window. If there's more than one call to 
Window_clicked_button() in a window, it can also indicate a second loop of 
event handling.

So, be aware that some controls will need a bit of analysis , coding and 
testing, before you can use them as standard-behaving widgets in a window.

Original comment by yrizoud on 5 Aug 2010 at 8:19

[GoogleCodeExporter]

Yes, I'm not going to rewrite all the dialogs at once just for having a little 
cleaner code. The main problem I see is that C doesn't allow us to have 
multiple functions with the same name (for example a create_button with 
callback and one without), so we'll have to have new names for the 
callback-based ones.

= Having to prefix everything =
Ok, but if we call the state variable just 's', it keeps short enough.
Also, to some extent, macro can be helpful.
For example lets say we have :
struct {
int window_attribute2,
(other internal data),
void* customData
} callback_param;

of course doing (mydatatype*)(state->customData)->myItem would be annoying.
It can be macroized as:
#define M(x,y) (x*)(state->customData)->y, so that in the code it would look 
like :
M(mydatatype,myItem)

Or for a more concrete exemple :

red_value = M(wPalette,redSlider);

The macro can have a better name such as wa 'window attributes'. Not sure it 
helps with reading the code, however...

= Special controls =
those may be a bit more tricky, but I think it's worth getting rid of hacks 
like Window_attribute2, as it would now be part of the context.

When there is a second loop of event handling, it can be ok sometimes as it is 
handled as a different window (dropdown menus for example). I guess it can use 
the same context, or have another one, depending on the needs.

While we are at gui refactoring, I'd also like to standardize widgets like the 
checkbox, that we use a lot.

Original comment by pulkoma...@gmail.com on 5 Aug 2010 at 8:38