Re: [AD] set_window_close_button_callback_hook_button()

[Ben Davis <bnd25@xxxxxxxxxx>]

[No need to cc to me when replying.]

On Thursday 28 November 2002 3:12 pm, you wrote:
> > 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.

Then that should be noted in api.txt ;)

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

So, whenever the warning message / forced close was would have appeared, the 
button gets disabled instead. I fail to see how this change would break any 
programs (in particular, if they were relying on the warning message, then 
they were broken already).

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

Which is why we have draw_sprite() and masked_blit().

I don't see why having two functions is a problem. The system would work, and 
we wouldn't have broken backwards compatibility.

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

That's only a rule of thumb, and it doesn't apply here. I challenge you to 
think of a single scenario where a program would be broken by such a change.

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

I know who to flame in DUMB v0.9.1's docs then ;)

> > 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);

That would be acceptable, although not quite as robust as my proposal...

Ben