Re: [AD] set_window_close_button_callback_hook_button()

[Eric Botcazou <ebotcazou@xxxxxxxxxx>]

> First I shall quote from api.txt:
>                                                           "But this is
>    guaranteed to never happen in a series for which major and minor version
>    numbers are fixed; in other words, two versions that differ from each
>    other only by the revision (3rd) number will be strictly API
> compatible."

Only applies to stable branches. We can't guarantee API compatibility during 
development.

> Allegro versions up to 4.1.4 had set_window_close_button() and
> set_window_close_hook(). In Allegro 4.1.5, these functions were removed and
> set_close_button_callback() was added. The rationale was that calling
> exit() from inside the handler was a bad idea.

Not a bad idea, simply an idea that doesn't work.

> On Windows, we were calling exit() from inside the handler. But there was a
> warning message in place for that exact reason! Furthermore, I don't see
> how this action is any different from the one carried out when Ctrl+Alt+End
> is pressed.

Exact. But the close button is supposed to work and not to freeze or kill half 
of the threads of the applicatin. We can tolerate that for Ctrl+Alt+End.

> As for Linux, it was originally proposed that we call exit() without
> warning the user, because window managers kill the application immediately
> when the close button is pressed anyway. This is nonsense. Now that I have
> some Linux experience, I know that Allegro wouldn't get a chance to hook
> this function; and for those window managers that don't kill the app, I
> believe the same warning as in Windows should have been implemented.

We should neither have implemented this warning in the first place, nor the 
default behaviour of killing the app behind the back of the programmer.

> Now, I'm not saying we actually want this feature (the default close button
> behaviour). Ctrl+Alt+End is considered an emergency resort, whereas
> clicking the close box is not.

I fully agree.

> However, I am pointing out that this behaviour was not fundamentally
> broken and the claims that it was were unfounded.

It was fundamentally broken: you can't expect to properly shutdown from a DLL 
the application that loaded the DLL without the consent of the application. 
Believe me, I tried various sorts of hacks to make it work, but that's simply 
undoable.

> The only remaining motivation for renaming a function would be if its
> functionality changed in such a way as to break Allegro programs. In this
> case, the only change is in the default behaviour. Let's have a look at how
> the above groups are affected by the change that was made.

The functionality has changed: set_close_button_callback() has superseded the 
two former functions. In particular, set_close_button_callback(NULL) must 
physically disable the close button.

> set_window_close_button() will enable or disable the close button, as
> before. It will default to enabled. HOWEVER, the button isn't _actually_
> enabled unless a callback has been specified; until then, it'll appear
> greyed out, or be inoperative, depending on the window manager or whatever.
>
> set_window_close_hook() will set a callback function. The close button will
> light up (or become operative), provided it is "enabled" as defined for the
> last function. If NULL is specified here, the close button will appear
> greyed out or be inoperative, even though it's "enabled" as defined for the
> last function.

These functions are not orthogonal. Only one function for one functionality.

> Amazing! We've removed the problematic feature - the default close button
> functionality - without breaking API or making anyone's programs
> malfunction.

We do break the API: the behaviour of a function has changed. Keeping the same 
name would be worse.

> <plug> This API change has affected DUMB ( http://dumb.sf.net/ ); people
> download Allegro 4.1.5 WIP, then realise to their dismay that DUMB's
> example players don't compile. Commenting set_window_close_hook() out in
> each file is tedious. They get bored and go and use one of DUMB's
> archnemeses, JGMOD or FMOD. :P </plug>

I decided almost alone (I posted the patch to the list before committing it 
though) to remove the two former functions because they can't be emulated by 
the new safe code. My rationale was: better a compilation breakage that is 
obvious and fixable within 5 seconds than a runtime breakage that can take 
hours to track down. I must admit that doing so put the burden on the add-on 
writer though.

> The damage has already been done, but it's not too late to limit it. I
> propose that the above change be made. For those who have gone on to the
> new API, we can arrange for set_close_button_callback() to be a synonym for
> set_window_close_hook().

The other way around :-) I proposed some days ago (following Evert's 
suggestion) to revive the two former symbols in the 4.1.6 WIP:

#define set_window_close_button(enable)
#define set_window_close_hook(callback) set_close_button_callback(callback)

and mark them as "deprecated". That's far from ideal, but at least we preserve 
legitimate constructs like:

   set_window_close_button(TRUE);
   set_window_close_hook(my_callback);

> A slightly less than gruntled Ben.

The list has been way too quiet lately anyway :-)

-- 
Eric Botcazou