Open GoogleCodeExporter opened 8 years ago
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
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
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
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
[deleted comment]
[deleted comment]
Original comment by pulkoma...@gmail.com
on 15 Jan 2010 at 7:34
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
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
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
Original issue reported on code.google.com by
pulkoma...@gmail.com
on 17 Jun 2009 at 3:35